From b6c21de0bbdb5a7e94430c20933526a96a595618 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Fri, 23 Oct 2026 20:30:08 -0400 Subject: [PATCH 1/5] feat(frontend): persist the query cache and restore the signed-in session offline Allowlisted query data is persisted to localStorage for 24h, scoped to the origin and user and cleared on sign-out. The last validated user is remembered so a cold start opens with cached lists when the session check fails on the network, while a 401 or signed-out response still signs out. Lists keep cached data when a refetch fails. --- frontend/package.json | 4 +- frontend/src/App.tsx | 18 +- .../navigation/RemoteProjectNav.test.tsx | 71 +++-- .../navigation/RemoteProjectNav.tsx | 9 +- .../src/components/repo/RepoList.test.tsx | 30 ++- frontend/src/components/repo/RepoList.tsx | 6 +- .../components/servers/RemoteProjectList.tsx | 18 +- .../__tests__/RemoteProjectList.test.tsx | 26 +- .../components/session/SessionList.test.tsx | 20 +- .../src/components/session/SessionList.tsx | 7 +- frontend/src/contexts/AuthContext.test.tsx | 141 ++++++++-- frontend/src/contexts/AuthContext.tsx | 47 +++- .../QueryCachePersistenceContext.test.tsx | 228 ++++++++++++++++ .../contexts/QueryCachePersistenceContext.tsx | 93 +++++++ frontend/src/hooks/useOpenCode.ts | 1 + frontend/src/hooks/useOpenCodeServers.ts | 5 + frontend/src/hooks/useRemoteProjects.test.tsx | 31 ++- frontend/src/hooks/useRemoteProjects.ts | 20 +- frontend/src/hooks/useSessionSearch.ts | 2 + frontend/src/lib/auth-loaders.test.ts | 62 ++++- frontend/src/lib/auth-loaders.ts | 35 ++- frontend/src/lib/authSession.ts | 61 +++++ frontend/src/lib/persistentStorage.ts | 8 + frontend/src/lib/queryPersistence.test.ts | 250 ++++++++++++++++++ frontend/src/lib/queryPersistence.ts | 111 ++++++++ frontend/src/lib/queryState.test.ts | 43 +++ frontend/src/lib/queryState.ts | 27 ++ pnpm-lock.yaml | 51 +++- 28 files changed, 1324 insertions(+), 101 deletions(-) create mode 100644 frontend/src/contexts/QueryCachePersistenceContext.test.tsx create mode 100644 frontend/src/contexts/QueryCachePersistenceContext.tsx create mode 100644 frontend/src/lib/authSession.ts create mode 100644 frontend/src/lib/queryPersistence.test.ts create mode 100644 frontend/src/lib/queryPersistence.ts create mode 100644 frontend/src/lib/queryState.test.ts create mode 100644 frontend/src/lib/queryState.ts diff --git a/frontend/package.json b/frontend/package.json index b7b4dfd48..33d670686 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -28,7 +28,9 @@ "@radix-ui/react-switch": "^1.2.6", "@radix-ui/react-tabs": "^1.1.13", "@tailwindcss/vite": "^4.1.14", - "@tanstack/react-query": "^5.90.5", + "@tanstack/query-sync-storage-persister": "5.101.2", + "@tanstack/react-query": "^5.101.2", + "@tanstack/react-query-persist-client": "5.101.2", "@xterm/addon-fit": "^0.11.0", "@xterm/addon-web-links": "^0.12.0", "@xterm/xterm": "^6.0.0", diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 6453076d7..ddb4e713c 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -28,6 +28,7 @@ import { useMobileTabBar } from '@/hooks/useMobileTabBar' import { TTSProvider } from './contexts/TTSContext' import { ThemeProvider, useThemeMode } from './contexts/ThemeContext' import { AuthProvider } from './contexts/AuthContext' +import { QueryCachePersistenceProvider } from './contexts/QueryCachePersistenceContext' import { KeyboardShortcutsProvider } from './contexts/KeyboardShortcutsContext' import { EventProvider, usePermissions, useEventContext } from '@/contexts/EventContext' import { OpenCodeServerProvider, isLocalOpenCodeServer } from '@/contexts/OpenCodeServerContext' @@ -38,6 +39,7 @@ import { SSHHostKeyDialog } from './components/ssh/SSHHostKeyDialog' import { loginLoader, setupLoader, registerLoader, protectedLoader } from './lib/auth-loaders' import { getSwipeBackTarget, isSessionDetailRoute } from '@/lib/navigation' import { onNotificationClick } from '@/lib/serviceWorker' +import { applyPersistedQueryDefaults } from '@/lib/queryPersistence' import { useAuth } from '@/hooks/useAuth' import { useOpenCodeFailureToast } from '@/hooks/useOpenCodeFailureToast' import { useOpenCodeServerActions } from '@/hooks/useOpenCodeServerActions' @@ -55,6 +57,8 @@ const queryClient = new QueryClient({ }, }) +applyPersistedQueryDefaults(queryClient) + function SSHHostKeyDialogWrapper() { const { sshHostKey } = useEventContext() return ( @@ -168,12 +172,14 @@ function ServerScope({ children }: { children: ReactNode }) { function RootLayout() { return ( - - - - - - + + + + + + + + ) } diff --git a/frontend/src/components/navigation/RemoteProjectNav.test.tsx b/frontend/src/components/navigation/RemoteProjectNav.test.tsx index 1e12c8d68..0567be327 100644 --- a/frontend/src/components/navigation/RemoteProjectNav.test.tsx +++ b/frontend/src/components/navigation/RemoteProjectNav.test.tsx @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' import { MemoryRouter, Route, Routes, useLocation, useNavigate } from 'react-router-dom' -import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { IsRestoringProvider, QueryClient, QueryClientProvider } from '@tanstack/react-query' import type { OpenCodeProject } from '@/api/opencode' import { RemoteProjectNavList } from './RemoteProjectNav' @@ -132,22 +132,25 @@ function createWrapper( queryClient = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, }), + isRestoring = false, ) { return ({ children }: { children: React.ReactNode }) => ( - - - - {children} - - > - } - /> - - + + + + + {children} + + > + } + /> + + + ) } @@ -174,7 +177,11 @@ describe('RemoteProjectNavList', () => { mocks.sessions = [createSession('a1', '/projects/alpha', Date.now())] mocks.hasNextPage.current = false mocks.listProjects.mockResolvedValue([alpha, beta]) - mocks.useOpenCodeServers.mockReturnValue({ servers: [kitchen], isLoading: false }) + mocks.useOpenCodeServers.mockReturnValue({ + servers: [kitchen], + isLoading: false, + showsInitialLoading: false, + }) mocks.useOpenCodeServerStatuses.mockReturnValue({ statuses: [], statusById: { srv_kitchen: { serverId: 'srv_kitchen', connected: true, version: null, error: null } }, @@ -289,6 +296,7 @@ describe('RemoteProjectNavList', () => { mocks.useOpenCodeServers.mockReturnValue({ servers: [{ ...kitchen, enabled: false }], isLoading: false, + showsInitialLoading: false, }) render( @@ -343,6 +351,39 @@ describe('RemoteProjectNavList', () => { expect(screen.queryByText('Could not load projects')).not.toBeInTheDocument() }) + it('keeps the project list loading while the cache restore holds the query', () => { + mocks.listProjects.mockReturnValue(new Promise(() => {})) + + render(, { + wrapper: createWrapper('/', undefined, true), + }) + + expect(screen.getByText('Loading projects...')).toBeInTheDocument() + expect(screen.queryByText('No projects')).not.toBeInTheDocument() + expect(mocks.listProjects).not.toHaveBeenCalled() + }) + + it('keeps the sidebar loading while the server list is restoring', () => { + mocks.useOpenCodeServers.mockReturnValue({ + servers: [], + isLoading: false, + showsInitialLoading: true, + }) + + render( + , + { wrapper: createWrapper('/') }, + ) + + expect(screen.getByText('Loading projects...')).toBeInTheDocument() + expect(screen.queryByText('Server not found')).not.toBeInTheDocument() + }) + it('reports an unreachable server on a failed project load', async () => { mocks.listProjects.mockRejectedValue(new Error('offline')) mocks.useOpenCodeServerStatuses.mockReturnValue({ diff --git a/frontend/src/components/navigation/RemoteProjectNav.tsx b/frontend/src/components/navigation/RemoteProjectNav.tsx index e1c8f764e..6bb5c068b 100644 --- a/frontend/src/components/navigation/RemoteProjectNav.tsx +++ b/frontend/src/components/navigation/RemoteProjectNav.tsx @@ -360,20 +360,19 @@ export function RemoteProjectNavList({ onOpenProject, onSelectSession, }: RemoteProjectNavListProps) { - const { servers, isLoading: isServersLoading } = useOpenCodeServers() + const { servers, showsInitialLoading: isServersLoading } = useOpenCodeServers() const { statusById } = useOpenCodeServerStatuses() const server = servers.find((candidate) => candidate.id === serverId) const { directory: activeDirectory } = useActiveRemoteDirectory(serverId) - const { projects, isLoading: projectsLoading, isError: projectsError, hasData: hasProjects } = - useRemoteProjectsByActivity(serverId) + const { projects, showsInitialLoading, showsError } = useRemoteProjectsByActivity(serverId) const trimmedSearch = search.trim() if (isServersLoading) return Loading projects... if (!server) return Server not found if (!server.enabled) return Server is disabled - if (projectsLoading) return Loading projects... - if (projectsError && !hasProjects) { + if (showsInitialLoading) return Loading projects... + if (showsError) { return ( {statusById[serverId]?.connected === false ? 'Server unreachable' : 'Could not load projects'} diff --git a/frontend/src/components/repo/RepoList.test.tsx b/frontend/src/components/repo/RepoList.test.tsx index 3c0966784..8946c1655 100644 --- a/frontend/src/components/repo/RepoList.test.tsx +++ b/frontend/src/components/repo/RepoList.test.tsx @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { render, screen, within } from '@testing-library/react' +import { render, screen, waitFor, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { BrowserRouter } from 'react-router-dom' @@ -64,8 +64,12 @@ function createRepo(overrides: Partial = {}): Repo { } } -function renderRepoList(onAddRepo: () => void = vi.fn()) { - const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }) +function renderRepoList( + onAddRepo: () => void = vi.fn(), + options: { queryClient?: QueryClient } = {}, +) { + const queryClient = options.queryClient + ?? new QueryClient({ defaultOptions: { queries: { retry: false } } }) return render( @@ -121,6 +125,26 @@ describe('RepoList', () => { expect(await screen.findByText('No recently accessed repositories')).toBeInTheDocument() }) + it('shows the error state when the first load fails', async () => { + mocks.listRepos.mockRejectedValue(new Error('offline')) + + renderRepoList() + + expect(await screen.findByText(/Failed to load repositories/)).toBeInTheDocument() + }) + + it('keeps cached repositories when the refetch fails', async () => { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + queryClient.setQueryData(['repos'], [createRepo()]) + mocks.listRepos.mockRejectedValue(new Error('offline')) + + renderRepoList(vi.fn(), { queryClient }) + + expect(await screen.findByText('demo-app')).toBeInTheDocument() + await waitFor(() => expect(queryClient.getQueryState(['repos'])?.status).toBe('error')) + expect(screen.queryByText(/Failed to load repositories/)).not.toBeInTheDocument() + }) + it('labels the confirm button "Unlink" for a local repository', async () => { mocks.listRepos.mockResolvedValue([createRepo()]) const user = userEvent.setup() diff --git a/frontend/src/components/repo/RepoList.tsx b/frontend/src/components/repo/RepoList.tsx index 28175a3dd..68f744b2c 100644 --- a/frontend/src/components/repo/RepoList.tsx +++ b/frontend/src/components/repo/RepoList.tsx @@ -162,7 +162,7 @@ export function RepoList({ onAddRepo }: { onAddRepo: () => void }) { const { data: repos, - isLoading, + isPending, error, } = useQuery({ queryKey: ["repos"], @@ -312,7 +312,7 @@ export function RepoList({ onAddRepo }: { onAddRepo: () => void }) { const renderContent = () => { switch (true) { - case isLoading && !repos: + case isPending && !repos: return ( @@ -332,7 +332,7 @@ export function RepoList({ onAddRepo }: { onAddRepo: () => void }) { ) - case !!error: + case !!error && !repos: return ( Failed to load repositories:{" "} diff --git a/frontend/src/components/servers/RemoteProjectList.tsx b/frontend/src/components/servers/RemoteProjectList.tsx index c0f8f14b7..89cdd9db4 100644 --- a/frontend/src/components/servers/RemoteProjectList.tsx +++ b/frontend/src/components/servers/RemoteProjectList.tsx @@ -86,7 +86,7 @@ function RemoteProjectListLoading() { } function RemoteProjectListContent({ serverId }: { serverId: string }) { - const { servers, isLoading: serversLoading } = useOpenCodeServers() + const { servers, showsInitialLoading: serversLoading } = useOpenCodeServers() const { openTab } = useSettingsDialog() const [openDirectoryOpen, setOpenDirectoryOpen] = useState(false) const [searchQuery, setSearchQuery] = useState('') @@ -94,8 +94,8 @@ function RemoteProjectListContent({ serverId }: { serverId: string }) { const server = servers.find((candidate) => candidate.id === serverId) const isDisabled = server !== undefined && !server.enabled - const { projects, activityById, isLoading, isError, hasData, error, refetch } = useRemoteProjectsByActivity(serverId) - const showsProjectError = isError && !hasData + const { projects, activityById, showsInitialLoading, showsError, error, refetch } = + useRemoteProjectsByActivity(serverId) const filteredProjects = useMemo(() => { const query = searchQuery.trim().toLowerCase() @@ -121,6 +121,8 @@ function RemoteProjectListContent({ serverId }: { serverId: string }) { /> ) : null} + {serversLoading ? : null} + {isDisabled ? ( ) : null} - {server && !isDisabled && showsProjectError ? ( + {server && !isDisabled && showsError ? ( Could not load projects - {error instanceof Error - ? error.message - : 'The server did not respond.'} + {error instanceof Error ? error.message : 'The server did not respond.'} refetch()}> @@ -148,8 +148,8 @@ function RemoteProjectListContent({ serverId }: { serverId: string }) { ) : null} - {server && !isDisabled && !showsProjectError ? ( - isLoading ? ( + {server && !isDisabled && !showsError ? ( + showsInitialLoading ? ( ) : projects.length === 0 ? ( diff --git a/frontend/src/components/servers/__tests__/RemoteProjectList.test.tsx b/frontend/src/components/servers/__tests__/RemoteProjectList.test.tsx index 9492e0aa0..fd709c3f3 100644 --- a/frontend/src/components/servers/__tests__/RemoteProjectList.test.tsx +++ b/frontend/src/components/servers/__tests__/RemoteProjectList.test.tsx @@ -148,7 +148,11 @@ describe('RemoteProjectList', () => { mocks.listProjects.mockResolvedValue([project]) mocks.listSessionPage.mockResolvedValue({ items: [] }) mocks.useCreateSession.mockReturnValue({ mutateAsync: mocks.createSession, isPending: false }) - mocks.useOpenCodeServers.mockReturnValue({ servers: [server], isLoading: false }) + mocks.useOpenCodeServers.mockReturnValue({ + servers: [server], + isLoading: false, + showsInitialLoading: false, + }) mocks.useSettingsDialog.mockReturnValue({ openTab: mocks.openTab }) }) @@ -396,6 +400,7 @@ describe('RemoteProjectList', () => { mocks.useOpenCodeServers.mockReturnValue({ servers: [{ ...server, enabled: false }], isLoading: false, + showsInitialLoading: false, }) renderProjectList() @@ -405,7 +410,11 @@ describe('RemoteProjectList', () => { }) it('shows a missing server state when the server is no longer configured', async () => { - mocks.useOpenCodeServers.mockReturnValue({ servers: [], isLoading: false }) + mocks.useOpenCodeServers.mockReturnValue({ + servers: [], + isLoading: false, + showsInitialLoading: false, + }) renderProjectList('srv_gone') @@ -413,6 +422,19 @@ describe('RemoteProjectList', () => { expect(mocks.listProjects).not.toHaveBeenCalled() }) + it('loads instead of reporting a missing server while the server list is restoring', () => { + mocks.useOpenCodeServers.mockReturnValue({ + servers: [], + isLoading: false, + showsInitialLoading: true, + }) + + const { container } = renderProjectList('srv_gone') + + expect(screen.queryByText('Server not found')).not.toBeInTheDocument() + expect(container.querySelectorAll('.animate-pulse').length).toBeGreaterThan(0) + }) + it('shows an empty state when the server has no projects', async () => { mocks.listProjects.mockResolvedValue([]) diff --git a/frontend/src/components/session/SessionList.test.tsx b/frontend/src/components/session/SessionList.test.tsx index 34be5ca72..15b0fd180 100644 --- a/frontend/src/components/session/SessionList.test.tsx +++ b/frontend/src/components/session/SessionList.test.tsx @@ -5,7 +5,7 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import type { ReactNode } from 'react' import { SessionList } from './SessionList' -const { createSessionMock, deleteSessionMock, sessionsData, createSessionState, fetchNextPageMock, hasNextPageRef, isFetchingNextPageRef, isFetchNextPageErrorRef, isLoadingRef, isPlaceholderDataRef, useRealSessionsHookRef, lastSessionsHookArgs, sessionPinsData, togglePinMock } = vi.hoisted(() => ({ +const { createSessionMock, deleteSessionMock, sessionsData, createSessionState, fetchNextPageMock, hasNextPageRef, isFetchingNextPageRef, isFetchNextPageErrorRef, isLoadingRef, isPendingRef, isPlaceholderDataRef, useRealSessionsHookRef, lastSessionsHookArgs, sessionPinsData, togglePinMock } = vi.hoisted(() => ({ createSessionMock: vi.fn(), deleteSessionMock: vi.fn(), sessionsData: [] as Array<{ id: string; title: string; location: { directory: string }; parentID?: string; time: { updated: number } }>, @@ -15,6 +15,7 @@ const { createSessionMock, deleteSessionMock, sessionsData, createSessionState, isFetchingNextPageRef: { current: false }, isFetchNextPageErrorRef: { current: false }, isLoadingRef: { current: false }, + isPendingRef: { current: false }, isPlaceholderDataRef: { current: false }, useRealSessionsHookRef: { current: false }, lastSessionsHookArgs: { current: undefined as { directories: string[]; options?: { search?: string; limit?: number; keepPreviousResults?: boolean } } | undefined }, @@ -38,6 +39,7 @@ vi.mock('@/hooks/useOpenCode', async (importOriginal) => { return { data, isLoading: isLoadingRef.current, + isPending: isPendingRef.current || isLoadingRef.current, isPlaceholderData: isPlaceholderDataRef.current, fetchNextPage: fetchNextPageMock, hasNextPage: hasNextPageRef.current, @@ -78,6 +80,7 @@ describe('SessionList', () => { isFetchingNextPageRef.current = false isFetchNextPageErrorRef.current = false isLoadingRef.current = false + isPendingRef.current = false isPlaceholderDataRef.current = false useRealSessionsHookRef.current = false lastSessionsHookArgs.current = undefined @@ -589,6 +592,21 @@ describe('SessionList', () => { it('keeps the search input rendered while the first page is loading', () => { isLoadingRef.current = true + isPendingRef.current = true + + render( + , + ) + + expect(screen.getByPlaceholderText('Search sessions...')).toBeTruthy() + expect(screen.getByText('Loading sessions...')).toBeTruthy() + }) + + it('keeps the search input rendered while the cache is restoring', () => { + isPendingRef.current = true render( @@ -286,7 +289,7 @@ export const SessionList = ({ onScroll={handleSessionsScroll} > - {isLoading && !renderSessions ? ( + {isPending && !renderSessions ? ( Loading sessions... ) : renderSessions ? ( renderSessions({ diff --git a/frontend/src/contexts/AuthContext.test.tsx b/frontend/src/contexts/AuthContext.test.tsx index a61f5672c..da9151af7 100644 --- a/frontend/src/contexts/AuthContext.test.tsx +++ b/frontend/src/contexts/AuthContext.test.tsx @@ -1,9 +1,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { render, waitFor } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { MemoryRouter, Route, Routes } from 'react-router-dom' import { LocationCatcher } from '@/test/test-utils' import { AuthProvider } from './AuthContext' import { useAuth } from '@/hooks/useAuth' +import { QUERY_CACHE_STORAGE_KEY } from '@/lib/queryPersistence' +import { STORED_AUTH_USER_KEY } from '@/lib/authSession' const mocks = vi.hoisted(() => ({ useSession: vi.fn(), @@ -25,41 +28,47 @@ vi.mock('@/lib/auth-client', () => ({ type AuthValue = ReturnType -function AuthProbe({ captured }: { captured: { current: AuthValue | null } }) { - captured.current = useAuth() - return null -} +const STORED_USER = { id: 'user-1', email: 'user@example.com', name: 'Ada' } function renderAuth(initialEntry: string) { const capturedPathname: { current: string } = { current: '' } const capturedSearch: { current: string } = { current: '' } const captured: { current: AuthValue | null } = { current: null } + const queryClient = new QueryClient() render( - - - - - - - )} - /> - - - - , + + + + + + + + )} + /> + + + + + , ) - return { capturedPathname, capturedSearch, captured } + return { capturedPathname, capturedSearch, captured, queryClient } +} + +function AuthProbe({ captured }: { captured: { current: AuthValue | null } }) { + captured.current = useAuth() + return null } describe('AuthProvider post-login navigation', () => { beforeEach(() => { vi.clearAllMocks() - mocks.useSession.mockReturnValue({ data: null, isPending: false, refetch: mocks.refetch }) + localStorage.clear() + mocks.useSession.mockReturnValue({ data: null, error: null, isPending: false, refetch: mocks.refetch }) mocks.refetch.mockResolvedValue(undefined) mocks.signInEmail.mockResolvedValue({ data: { user: { id: 'user-1' } } }) mocks.signInPasskey.mockResolvedValue({ data: { user: { id: 'user-1' } } }) @@ -124,3 +133,93 @@ describe('AuthProvider post-login navigation', () => { }) }) }) + +describe('AuthProvider session restore', () => { + beforeEach(() => { + vi.clearAllMocks() + localStorage.clear() + mocks.useSession.mockReturnValue({ data: null, error: null, isPending: false, refetch: mocks.refetch }) + mocks.refetch.mockResolvedValue(undefined) + mocks.signOut.mockResolvedValue({ data: null }) + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ enabledProviders: ['credentials'], registrationEnabled: true, isFirstUser: false, adminConfigured: true }), + })) + }) + + it('stores the validated user for the next cold start', async () => { + mocks.useSession.mockReturnValue({ + data: { user: STORED_USER }, + error: null, + isPending: false, + refetch: mocks.refetch, + }) + + const { captured } = renderAuth('/login') + + await waitFor(() => expect(captured.current?.isAuthenticated).toBe(true)) + expect(JSON.parse(localStorage.getItem(STORED_AUTH_USER_KEY) ?? '')).toEqual(STORED_USER) + }) + + it('treats a transient session failure as signed in with the stored user', async () => { + localStorage.setItem(STORED_AUTH_USER_KEY, JSON.stringify(STORED_USER)) + mocks.useSession.mockReturnValue({ + data: null, + error: new TypeError('Failed to fetch'), + isPending: false, + refetch: mocks.refetch, + }) + + const { captured } = renderAuth('/login') + + await waitFor(() => expect(captured.current?.isAuthenticated).toBe(true)) + expect(captured.current?.user).toEqual(STORED_USER) + expect(localStorage.getItem(STORED_AUTH_USER_KEY)).not.toBeNull() + }) + + it('keeps the stored user while the session is still loading', async () => { + localStorage.setItem(STORED_AUTH_USER_KEY, JSON.stringify(STORED_USER)) + mocks.useSession.mockReturnValue({ data: null, error: null, isPending: true, refetch: mocks.refetch }) + + const { captured } = renderAuth('/login') + + await waitFor(() => expect(captured.current?.isAuthenticated).toBe(true)) + expect(captured.current?.isLoading).toBe(false) + }) + + it.each([ + ['a 401 response', { status: 401, statusText: 'Unauthorized' }], + ['a successful response without a user', null], + ])('clears the stored user on %s', async (_label, error) => { + localStorage.setItem(STORED_AUTH_USER_KEY, JSON.stringify(STORED_USER)) + mocks.useSession.mockReturnValue({ + data: null, + error, + isPending: false, + refetch: mocks.refetch, + }) + + const { captured } = renderAuth('/login') + + await waitFor(() => expect(captured.current?.isAuthenticated).toBe(false)) + expect(localStorage.getItem(STORED_AUTH_USER_KEY)).toBeNull() + }) + + it('clears the stored user, the persisted cache and the query cache on logout', async () => { + localStorage.setItem(STORED_AUTH_USER_KEY, JSON.stringify(STORED_USER)) + localStorage.setItem(QUERY_CACHE_STORAGE_KEY, '{}') + const { captured, queryClient } = renderAuth('/login') + const clearSpy = vi.spyOn(queryClient, 'clear') + queryClient.setQueryData(['repos'], [{ id: 1 }]) + mocks.signOut.mockImplementation(async (options: { fetchOptions?: { onSuccess?: () => void } }) => { + options.fetchOptions?.onSuccess?.() + return { data: null } + }) + + await captured.current?.logout() + + expect(localStorage.getItem(STORED_AUTH_USER_KEY)).toBeNull() + expect(localStorage.getItem(QUERY_CACHE_STORAGE_KEY)).toBeNull() + expect(clearSpy).toHaveBeenCalled() + }) +}) diff --git a/frontend/src/contexts/AuthContext.tsx b/frontend/src/contexts/AuthContext.tsx index cfc8e696a..90d151af6 100644 --- a/frontend/src/contexts/AuthContext.tsx +++ b/frontend/src/contexts/AuthContext.tsx @@ -1,8 +1,18 @@ /* eslint-disable react-refresh/only-export-components */ import { createContext, useEffect, useMemo, useState, useCallback, type ReactNode } from 'react' +import { useQueryClient } from '@tanstack/react-query' import { useSession, signIn, signUp, signOut, authClient, type AuthUser } from '@/lib/auth-client' import { useNavigate, useLocation } from 'react-router-dom' import { getLoginRedirectTarget } from '@/lib/auth-loaders' +import { + clearStoredAuthUser, + isTransientSessionError, + readStoredAuthUser, + toStoredAuthUser, + writeStoredAuthUser, + type StoredAuthUser, +} from '@/lib/authSession' +import { clearPersistedQueryCache } from '@/lib/queryPersistence' interface AuthConfig { enabledProviders: string[] @@ -11,8 +21,10 @@ interface AuthConfig { adminConfigured: boolean } +export type AuthContextUser = Pick + interface AuthContextValue { - user: AuthUser | null + user: AuthContextUser | null isAuthenticated: boolean isLoading: boolean config: AuthConfig | null @@ -34,7 +46,9 @@ interface AuthProviderProps { } export function AuthProvider({ children }: AuthProviderProps) { - const { data: session, isPending, refetch } = useSession() + const { data: session, error: sessionError, isPending, refetch } = useSession() + const [storedUser, setStoredUser] = useState(() => readStoredAuthUser()) + const queryClient = useQueryClient() const [config, setConfig] = useState(null) const navigate = useNavigate() const location = useLocation() @@ -43,6 +57,19 @@ export function AuthProvider({ children }: AuthProviderProps) { [location.search], ) + useEffect(() => { + if (session?.user) { + const validatedUser = toStoredAuthUser(session.user) + writeStoredAuthUser(validatedUser) + setStoredUser(validatedUser) + return + } + if (isPending) return + if (isTransientSessionError(sessionError)) return + clearStoredAuthUser() + setStoredUser(null) + }, [session, sessionError, isPending]) + useEffect(() => { const fetchConfig = async () => { try { @@ -115,6 +142,10 @@ export function AuthProvider({ children }: AuthProviderProps) { }, []) const logout = useCallback(async () => { + clearStoredAuthUser() + setStoredUser(null) + clearPersistedQueryCache() + queryClient.clear() await signOut({ fetchOptions: { onSuccess: () => { @@ -122,16 +153,18 @@ export function AuthProvider({ children }: AuthProviderProps) { }, }, }) - }, []) + }, [queryClient]) const refreshSession = useCallback(async () => { await refetch() }, [refetch]) + const user = session?.user ?? storedUser + const value = useMemo(() => ({ - user: session?.user ?? null, - isAuthenticated: !!session?.user, - isLoading: isPending, + user, + isAuthenticated: !!user, + isLoading: isPending && !user, config, signInWithEmail, signInWithProvider, @@ -141,7 +174,7 @@ export function AuthProvider({ children }: AuthProviderProps) { logout, refreshSession, }), [ - session, + user, isPending, config, signInWithEmail, diff --git a/frontend/src/contexts/QueryCachePersistenceContext.test.tsx b/frontend/src/contexts/QueryCachePersistenceContext.test.tsx new file mode 100644 index 000000000..4e3782332 --- /dev/null +++ b/frontend/src/contexts/QueryCachePersistenceContext.test.tsx @@ -0,0 +1,228 @@ +import { act, render, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { QueryClient, QueryClientProvider, dehydrate, useIsRestoring } from '@tanstack/react-query' +import { createSyncStoragePersister } from '@tanstack/query-sync-storage-persister' +import type { Persister } from '@tanstack/react-query-persist-client' +import { QueryCachePersistenceProvider } from './QueryCachePersistenceContext' +import { QUERY_CACHE_STORAGE_KEY, queryCacheBuster } from '@/lib/queryPersistence' + +const mocks = vi.hoisted(() => ({ + useAuth: vi.fn(), +})) + +vi.mock('@/hooks/useAuth', () => ({ useAuth: mocks.useAuth })) + +interface AuthStub { + user: { id: string; email: string; name: string } | null + isAuthenticated: boolean + isLoading: boolean +} + +function signedIn(userId = 'user-1'): AuthStub { + return { + user: { id: userId, email: `${userId}@example.com`, name: userId }, + isAuthenticated: true, + isLoading: false, + } +} + +function signedOut(): AuthStub { + return { user: null, isAuthenticated: false, isLoading: false } +} + +function loading(): AuthStub { + return { user: null, isAuthenticated: false, isLoading: true } +} + +function createStorage() { + const store = new Map() + return { + store, + storage: { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => { + store.set(key, value) + }, + removeItem: (key: string) => { + store.delete(key) + }, + }, + } +} + +type TestStorage = ReturnType['storage'] + +function createTestPersister(storage: TestStorage): Persister { + return createSyncStoragePersister({ storage, key: QUERY_CACHE_STORAGE_KEY, throttleTime: 0 }) +} + +function seed( + storage: TestStorage, + buster: string, + queryKey: readonly unknown[] = ['repos'], + data: unknown = [{ id: 1 }], +): void { + const seedClient = new QueryClient() + seedClient.setQueryData(queryKey, data) + storage.setItem( + QUERY_CACHE_STORAGE_KEY, + JSON.stringify({ buster, timestamp: Date.now(), clientState: dehydrate(seedClient) }), + ) + seedClient.clear() +} + +let restoringValue: boolean | undefined + +function RestoringProbe() { + restoringValue = useIsRestoring() + return null +} + +const clients: QueryClient[] = [] + +function createClient(): QueryClient { + const client = new QueryClient() + clients.push(client) + return client +} + +function tree(auth: AuthStub, persister: Persister, client: QueryClient) { + mocks.useAuth.mockReturnValue(auth) + return ( + + + + + + ) +} + +async function settle(): Promise { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 10)) + }) +} + +beforeEach(() => { + restoringValue = undefined + mocks.useAuth.mockReset() + localStorage.clear() +}) + +afterEach(() => { + for (const client of clients.splice(0)) client.clear() +}) + +describe('QueryCachePersistenceProvider', () => { + it('restores the cache written for the current user', async () => { + const { store, storage } = createStorage() + seed(storage, queryCacheBuster('user-1')) + const client = createClient() + + render(tree(signedIn(), createTestPersister(storage), client)) + + await waitFor(() => expect(client.getQueryData(['repos'])).toEqual([{ id: 1 }])) + await waitFor(() => expect(restoringValue).toBe(false)) + expect(store.size).toBe(1) + }) + + it('discards a cache written for another user', async () => { + const { store, storage } = createStorage() + seed(storage, queryCacheBuster('user-2')) + const client = createClient() + + render(tree(signedIn('user-1'), createTestPersister(storage), client)) + + await waitFor(() => expect(restoringValue).toBe(false)) + expect(client.getQueryData(['repos'])).toBeUndefined() + expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(false) + }) + + it('keeps the stored cache while the session is still loading', async () => { + const { store, storage } = createStorage() + seed(storage, queryCacheBuster('user-1')) + const client = createClient() + + render(tree(loading(), createTestPersister(storage), client)) + await settle() + + expect(client.getQueryData(['repos'])).toBeUndefined() + expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(true) + expect(restoringValue).toBe(false) + }) + + it('persists allowlisted successful queries only', async () => { + const { store, storage } = createStorage() + const client = createClient() + + render(tree(signedIn(), createTestPersister(storage), client)) + await waitFor(() => expect(restoringValue).toBe(false)) + + act(() => { + client.setQueryData(['repos'], [{ id: 1 }]) + client.setQueryData(['opencode', 'sessions', '/work/repo'], [{ id: 'ses_1' }]) + client.setQueryData(['settings', 'user-1'], { opencodeServerAuth: 'secret' }) + client.setQueryData(['opencode', 'transcript', 'ses_1'], [{ text: 'streamed' }]) + client.setQueryData(['provider-credentials'], [{ apiKey: 'secret' }]) + }) + await settle() + + const persisted = JSON.parse(store.get(QUERY_CACHE_STORAGE_KEY) ?? '{}') + const roots = persisted.clientState.queries.map((query: { queryKey: unknown[] }) => query.queryKey[0]) + + expect(persisted.buster).toBe(queryCacheBuster('user-1')) + expect(roots.sort()).toEqual(['opencode', 'repos']) + expect(store.get(QUERY_CACHE_STORAGE_KEY)).not.toContain('secret') + expect(store.get(QUERY_CACHE_STORAGE_KEY)).not.toContain('streamed') + }) + + it('is restoring until the restore resolves', async () => { + let resolveRestore: () => void = () => {} + const persister: Persister = { + persistClient: vi.fn(), + restoreClient: vi.fn( + () => new Promise((resolve) => { + resolveRestore = () => resolve(undefined) + }), + ), + removeClient: vi.fn(), + } + + render(tree(signedIn(), persister, createClient())) + expect(restoringValue).toBe(true) + + act(() => resolveRestore()) + await waitFor(() => expect(restoringValue).toBe(false)) + }) + + it('clears the persisted cache and the in-memory cache on sign-out', async () => { + const { store, storage } = createStorage() + seed(storage, queryCacheBuster('user-1')) + const persister = createTestPersister(storage) + const client = createClient() + + const view = render(tree(signedIn(), persister, client)) + await waitFor(() => expect(client.getQueryData(['repos'])).toEqual([{ id: 1 }])) + + view.rerender(tree(signedOut(), persister, client)) + + await waitFor(() => expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(false)) + expect(client.getQueryData(['repos'])).toBeUndefined() + }) + + it('clears the in-memory cache when the validated user changes', async () => { + const { store, storage } = createStorage() + seed(storage, queryCacheBuster('user-1')) + const persister = createTestPersister(storage) + const client = createClient() + + const view = render(tree(signedIn('user-1'), persister, client)) + await waitFor(() => expect(client.getQueryData(['repos'])).toEqual([{ id: 1 }])) + + view.rerender(tree(signedIn('user-2'), persister, client)) + + await waitFor(() => expect(client.getQueryData(['repos'])).toBeUndefined()) + await waitFor(() => expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(false)) + await waitFor(() => expect(restoringValue).toBe(false)) + }) +}) diff --git a/frontend/src/contexts/QueryCachePersistenceContext.tsx b/frontend/src/contexts/QueryCachePersistenceContext.tsx new file mode 100644 index 000000000..5e126c573 --- /dev/null +++ b/frontend/src/contexts/QueryCachePersistenceContext.tsx @@ -0,0 +1,93 @@ +import { useEffect, useMemo, useRef, useState, type ReactNode } from 'react' +import { IsRestoringProvider, useQueryClient } from '@tanstack/react-query' +import { + persistQueryClientRestore, + persistQueryClientSubscribe, + type Persister, +} from '@tanstack/react-query-persist-client' +import { useAuth } from '@/hooks/useAuth' +import { + PERSISTED_CACHE_MAX_AGE_MS, + createQueryCachePersister, + queryCacheBuster, + shouldPersistQuery, +} from '@/lib/queryPersistence' + +export interface QueryCachePersistenceProviderProps { + children?: ReactNode + persister?: Persister +} + +/** + * Restores the allowlisted part of the React Query cache for the signed-in account and keeps it + * persisted while that account stays signed in. + * + * The restore starts only once the session resolves to a signed-in user and uses + * `"|"` as the persister buster, so a cache written for another origin or account + * is discarded instead of hydrating this one. While the restore is in flight the subtree runs under + * `IsRestoringProvider`, which keeps queries from fetching before the persisted data is available. + * + * A cache stored for a different user id is dropped from memory as well, and leaving the signed-in + * state clears both the in-memory client and the persisted cache. + */ +export function QueryCachePersistenceProvider({ + children, + persister, +}: QueryCachePersistenceProviderProps) { + const queryClient = useQueryClient() + const { user, isAuthenticated, isLoading } = useAuth() + const defaultPersister = useMemo(() => createQueryCachePersister(), []) + const activePersister = persister ?? defaultPersister + const userId = isAuthenticated ? user?.id ?? null : null + const buster = userId === null ? null : queryCacheBuster(userId) + const [restoredBuster, setRestoredBuster] = useState(null) + const previousUserIdRef = useRef(null) + const restoring = buster !== null && restoredBuster !== buster + + useEffect(() => { + const previousUserId = previousUserIdRef.current + previousUserIdRef.current = userId + if (previousUserId !== null && userId !== null && previousUserId !== userId) { + queryClient.clear() + } + }, [userId, queryClient]) + + useEffect(() => { + if (isLoading) return + + if (buster === null) { + setRestoredBuster(null) + void Promise.resolve(activePersister.removeClient()).catch(() => {}) + queryClient.clear() + return + } + + let cancelled = false + let unsubscribe: (() => void) | undefined + + void (async () => { + await persistQueryClientRestore({ + queryClient, + persister: activePersister, + maxAge: PERSISTED_CACHE_MAX_AGE_MS, + buster, + }).catch(() => {}) + if (cancelled) return + + setRestoredBuster(buster) + unsubscribe = persistQueryClientSubscribe({ + queryClient, + persister: activePersister, + buster, + dehydrateOptions: { shouldDehydrateQuery: shouldPersistQuery }, + }) + })() + + return () => { + cancelled = true + unsubscribe?.() + } + }, [buster, isLoading, activePersister, queryClient]) + + return {children} +} diff --git a/frontend/src/hooks/useOpenCode.ts b/frontend/src/hooks/useOpenCode.ts index dc2db00a2..15a1e1d3a 100644 --- a/frontend/src/hooks/useOpenCode.ts +++ b/frontend/src/hooks/useOpenCode.ts @@ -191,6 +191,7 @@ export const useSessionsAcrossDirectories = ( return { data, isLoading: query.isLoading, + isPending: query.isPending && uniqueDirectories.length > 0, isPlaceholderData: query.isPlaceholderData, isError: query.isError, fetchNextPage: query.fetchNextPage, diff --git a/frontend/src/hooks/useOpenCodeServers.ts b/frontend/src/hooks/useOpenCodeServers.ts index 750101083..2a4904a6c 100644 --- a/frontend/src/hooks/useOpenCodeServers.ts +++ b/frontend/src/hooks/useOpenCodeServers.ts @@ -14,6 +14,7 @@ import { testOpenCodeServerById, updateOpenCodeServer, } from '@/api/opencodeServers' +import { cachedListStates } from '@/lib/queryState' const OPENCODE_SERVERS_QUERY_KEY = ['opencode-servers'] const OPENCODE_SERVER_STATUSES_QUERY_KEY = ['opencode-server-statuses'] @@ -55,9 +56,13 @@ export function useOpenCodeServers(options: { enabled?: boolean } = {}) { mutationFn: (id: string) => testOpenCodeServerById(id), }) + const serversListState = cachedListStates(serversQuery.data, serversQuery.isError) + return { servers: serversQuery.data ?? [], isLoading: serversQuery.isLoading, + /** True only while no server list is cached and no error settled, so a restore is not read as a missing server. */ + showsInitialLoading: serversListState.showsInitialLoading, isSuccess: serversQuery.isSuccess, error: serversQuery.error, refetch: serversQuery.refetch, diff --git a/frontend/src/hooks/useRemoteProjects.test.tsx b/frontend/src/hooks/useRemoteProjects.test.tsx index 741e61a7a..28a230dfb 100644 --- a/frontend/src/hooks/useRemoteProjects.test.tsx +++ b/frontend/src/hooks/useRemoteProjects.test.tsx @@ -99,7 +99,7 @@ describe('useRemoteProjects', () => { wrapper: createWrapper(), }) - expect(result.current.isLoading).toBe(false) + expect(result.current.showsInitialLoading).toBe(false) expect(result.current.projects).toEqual([]) expect(mocks.listProjects).not.toHaveBeenCalled() }) @@ -112,6 +112,30 @@ describe('useRemoteProjects', () => { expect(mocks.listProjects).not.toHaveBeenCalled() }) + it('reports the initial loading state while nothing is cached, including a restored pending query', () => { + mocks.listProjects.mockReturnValue(new Promise(() => {})) + + const { result } = renderHook(() => useRemoteProjects('srv_kitchen'), { + wrapper: createWrapper(), + }) + + expect(result.current.showsInitialLoading).toBe(true) + expect(result.current.showsError).toBe(false) + expect(result.current.projects).toEqual([]) + }) + + it('reports the error state only when the load failed with nothing cached', async () => { + mocks.listProjects.mockRejectedValue(new Error('offline')) + + const { result } = renderHook(() => useRemoteProjects('srv_kitchen'), { + wrapper: createWrapper(), + }) + + await waitFor(() => expect(result.current.showsError).toBe(true)) + expect(result.current.showsInitialLoading).toBe(false) + expect(result.current.error?.message).toBe('offline') + }) + it('keeps the cached projects and reports the failure when a refetch fails', async () => { mocks.listProjects .mockResolvedValueOnce([createProject('app', { created: 1, updated: 2 })]) @@ -125,8 +149,9 @@ describe('useRemoteProjects', () => { result.current.refetch() - await waitFor(() => expect(result.current.isError).toBe(true)) - expect(result.current.hasData).toBe(true) + await waitFor(() => expect(result.current.error?.message).toBe('offline')) + expect(result.current.showsError).toBe(false) + expect(result.current.showsInitialLoading).toBe(false) expect(result.current.projects.map((project) => project.id)).toEqual(['app']) expect(result.current.error?.message).toBe('offline') }) diff --git a/frontend/src/hooks/useRemoteProjects.ts b/frontend/src/hooks/useRemoteProjects.ts index fc81bb542..df1c6099d 100644 --- a/frontend/src/hooks/useRemoteProjects.ts +++ b/frontend/src/hooks/useRemoteProjects.ts @@ -4,6 +4,7 @@ import { listProjects, rankProjectsBySessionActivity, sortProjectsByUpdatedAt, t import { useOpenCodeServers } from '@/hooks/useOpenCodeServers' import { useSessionsAcrossDirectories } from '@/hooks/useOpenCode' import { serverScopedKey } from '@/lib/serverScope' +import { cachedListStates } from '@/lib/queryState' const REMOTE_PROJECT_ACTIVITY_SESSION_LIMIT = 100 @@ -14,9 +15,10 @@ export interface UseRemoteProjectsOptions { export interface UseRemoteProjectsResult { projects: OpenCodeProject[] - isLoading: boolean - isError: boolean - hasData: boolean + /** True only while no project list is cached and no error settled, so a restore does not flash a skeleton. */ + showsInitialLoading: boolean + /** True only when the query failed and no cached list remains to render. */ + showsError: boolean error: Error | null refetch: () => void } @@ -67,12 +69,16 @@ export function useRemoteProjects( () => sortProjectsByUpdatedAt(projectsQuery.data ?? []), [projectsQuery.data], ) + const { showsInitialLoading, showsError } = cachedListStates( + projectsQuery.data, + projectsQuery.isError, + projectsQuery.isEnabled, + ) return { projects, - isLoading: projectsQuery.isLoading, - isError: projectsQuery.isError, - hasData: projectsQuery.data !== undefined, + showsInitialLoading, + showsError, error: projectsQuery.error, refetch: projectsQuery.refetch, } @@ -96,7 +102,7 @@ export function useRemoteProjectsByActivity( const projectsResult = useRemoteProjects(serverId, options) const { data: sessions } = useSessionsAcrossDirectories([], { serverId, - allDirectories: projectsResult.hasData, + allDirectories: projectsResult.projects.length > 0, limit: REMOTE_PROJECT_ACTIVITY_SESSION_LIMIT, }) diff --git a/frontend/src/hooks/useSessionSearch.ts b/frontend/src/hooks/useSessionSearch.ts index 03ba5e46c..1298b94b9 100644 --- a/frontend/src/hooks/useSessionSearch.ts +++ b/frontend/src/hooks/useSessionSearch.ts @@ -33,6 +33,7 @@ export function useSessionSearch(directories: string[], options: UseSessionSearc const { data: sessions, isLoading, + isPending, isError, isPlaceholderData, fetchNextPage, @@ -79,6 +80,7 @@ export function useSessionSearch(directories: string[], options: UseSessionSearc filteredSessions, isSearchPending, isLoading, + isPending, isError, isPlaceholderData, fetchNextPage, diff --git a/frontend/src/lib/auth-loaders.test.ts b/frontend/src/lib/auth-loaders.test.ts index 0c4268f45..8df337a9e 100644 --- a/frontend/src/lib/auth-loaders.test.ts +++ b/frontend/src/lib/auth-loaders.test.ts @@ -1,6 +1,9 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { LoaderFunctionArgs } from 'react-router-dom' import { getLoginRedirectTarget, loginLoader, loginRedirectPath, protectedLoader, resolveRedirectTarget } from './auth-loaders' +import { STORED_AUTH_USER_KEY } from './authSession' + +const STORED_USER_KEY_VALUE = { id: 'user-1', email: 'user@example.com', name: 'Ada' } const mocks = vi.hoisted(() => ({ getSession: vi.fn(), @@ -52,10 +55,11 @@ describe('auth redirect targets', () => { describe('protectedLoader', () => { beforeEach(() => { vi.clearAllMocks() + localStorage.clear() }) it('redirects an anonymous visitor to login carrying the requested location', async () => { - mocks.getSession.mockResolvedValue({ data: null }) + mocks.getSession.mockResolvedValue({ data: null, error: null }) const result = await protectedLoader(loaderArgs('http://localhost/repos/2?tab=workspaces#session')) @@ -63,9 +67,63 @@ describe('protectedLoader', () => { }) it('lets an authenticated visitor through', async () => { - mocks.getSession.mockResolvedValue({ data: { user: { id: 'user-1' } } }) + mocks.getSession.mockResolvedValue({ data: { user: { id: 'user-1' } }, error: null }) + + await expect(protectedLoader(loaderArgs('http://localhost/repos/2'))).resolves.toBeNull() + }) + + it('remembers the validated user for an offline cold start', async () => { + mocks.getSession.mockResolvedValue({ + data: { user: { id: 'user-1', email: 'user@example.com', name: 'Ada' } }, + error: null, + }) + + await protectedLoader(loaderArgs('http://localhost/repos/2')) + + expect(JSON.parse(localStorage.getItem(STORED_AUTH_USER_KEY) ?? '')).toEqual({ + id: 'user-1', + email: 'user@example.com', + name: 'Ada', + }) + }) + + it('lets a stored user through when the session read fails transiently', async () => { + localStorage.setItem(STORED_AUTH_USER_KEY, JSON.stringify(STORED_USER_KEY_VALUE)) + mocks.getSession.mockRejectedValue(new TypeError('Failed to fetch')) await expect(protectedLoader(loaderArgs('http://localhost/repos/2'))).resolves.toBeNull() + expect(localStorage.getItem(STORED_AUTH_USER_KEY)).not.toBeNull() + }) + + it('lets a stored user through on a 5xx response', async () => { + localStorage.setItem(STORED_AUTH_USER_KEY, JSON.stringify(STORED_USER_KEY_VALUE)) + mocks.getSession.mockResolvedValue({ + data: null, + error: { status: 503, statusText: 'Service Unavailable' }, + }) + + await expect(protectedLoader(loaderArgs('http://localhost/repos/2'))).resolves.toBeNull() + }) + + it('redirects when a transient failure has no stored user', async () => { + mocks.getSession.mockRejectedValue(new TypeError('Failed to fetch')) + + const result = await protectedLoader(loaderArgs('http://localhost/repos/2')) + + expect(redirectLocation(result)).toBe('/login?redirect=%2Frepos%2F2') + }) + + it.each([ + ['a 401 response', { status: 401, statusText: 'Unauthorized' }], + ['a successful response without a user', null], + ])('forgets the stored user and redirects on %s', async (_label, error) => { + localStorage.setItem(STORED_AUTH_USER_KEY, JSON.stringify(STORED_USER_KEY_VALUE)) + mocks.getSession.mockResolvedValue({ data: null, error }) + + const result = await protectedLoader(loaderArgs('http://localhost/repos/2')) + + expect(redirectLocation(result)).toBe('/login?redirect=%2Frepos%2F2') + expect(localStorage.getItem(STORED_AUTH_USER_KEY)).toBeNull() }) }) diff --git a/frontend/src/lib/auth-loaders.ts b/frontend/src/lib/auth-loaders.ts index 1c16c7533..91bed82ed 100644 --- a/frontend/src/lib/auth-loaders.ts +++ b/frontend/src/lib/auth-loaders.ts @@ -1,5 +1,12 @@ import { redirect, type LoaderFunctionArgs } from 'react-router-dom' import { getSession } from './auth-client' +import { + clearStoredAuthUser, + isTransientSessionError, + readStoredAuthUser, + toStoredAuthUser, + writeStoredAuthUser, +} from './authSession' export interface AuthConfig { enabledProviders: string[] @@ -110,13 +117,31 @@ export async function registerLoader() { return { config } } +/** + * Guards a signed-in route. A validated user is remembered so the next cold start can open the app + * before the network answers; when the session read fails transiently (network, 5xx, timeout) the + * remembered user stands in for it. A successful read with no user, or a 401, is authoritative and + * deletes the remembered profile. + */ export async function protectedLoader({ request }: LoaderFunctionArgs) { - const session = await getSession() + const url = new URL(request.url) + let session: Awaited> | null = null + try { + session = await getSession() + } catch { + session = null + } + + if (session?.data?.user) { + writeStoredAuthUser(toStoredAuthUser(session.data.user)) + return null + } - if (!session.data?.user) { - const url = new URL(request.url) - return redirect(loginRedirectPath(`${url.pathname}${url.search}${url.hash}`)) + if (session === null || isTransientSessionError(session.error)) { + if (readStoredAuthUser() !== null) return null + } else { + clearStoredAuthUser() } - return null + return redirect(loginRedirectPath(`${url.pathname}${url.search}${url.hash}`)) } diff --git a/frontend/src/lib/authSession.ts b/frontend/src/lib/authSession.ts new file mode 100644 index 000000000..f04a84c20 --- /dev/null +++ b/frontend/src/lib/authSession.ts @@ -0,0 +1,61 @@ +import { readStoredValue, removeStoredValue, writeStoredValue } from './persistentStorage' +import type { AuthUser } from './auth-client' + +export type StoredAuthUser = Pick + +export const STORED_AUTH_USER_KEY = 'ocm.auth.user' + +function isStoredAuthUser(value: unknown): value is StoredAuthUser { + if (value === null || typeof value !== 'object') return false + const candidate = value as Record + return ( + typeof candidate.id === 'string' && + candidate.id.length > 0 && + typeof candidate.email === 'string' && + typeof candidate.name === 'string' + ) +} + +/** + * Reads the profile saved by the last successful session validation. A missing, corrupt or partial + * record is treated as absent so a failed write cannot wedge the offline restore path. + */ +export function readStoredAuthUser(): StoredAuthUser | null { + const raw = readStoredValue(STORED_AUTH_USER_KEY) + if (raw === null) return null + try { + const parsed: unknown = JSON.parse(raw) + return isStoredAuthUser(parsed) ? toStoredAuthUser(parsed) : null + } catch { + return null + } +} + +/** Keeps only the fields the offline restore needs, so no session tokens reach storage. */ +export function toStoredAuthUser(user: StoredAuthUser): StoredAuthUser { + return { id: user.id, email: user.email, name: user.name } +} + +/** Saves the validated profile so a later cold start can open the app before the network answers. */ +export function writeStoredAuthUser(user: StoredAuthUser): void { + writeStoredValue(STORED_AUTH_USER_KEY, toStoredAuthUser(user)) +} + +/** Deletes the saved profile after logout or after the server rejected the session. */ +export function clearStoredAuthUser(): void { + removeStoredValue(STORED_AUTH_USER_KEY) +} + +/** + * Whether a failed session read is transient (network error, 5xx, timeout, abort) rather than the + * server rejecting the session. better-auth reports HTTP failures as an `error` carrying `status` + * and a rejected fetch as a thrown error with no status, so only an explicit 401 counts as + * authoritative. + */ +export function isTransientSessionError(error: unknown): boolean { + if (error === null || error === undefined) return false + if (typeof error === 'object' && 'status' in error) { + return (error as { status?: unknown }).status !== 401 + } + return true +} diff --git a/frontend/src/lib/persistentStorage.ts b/frontend/src/lib/persistentStorage.ts index 65c610f68..6546d0579 100644 --- a/frontend/src/lib/persistentStorage.ts +++ b/frontend/src/lib/persistentStorage.ts @@ -13,3 +13,11 @@ export function writeStoredValue(key: string, value: unknown): void { return } } + +export function removeStoredValue(key: string): void { + try { + localStorage.removeItem(key) + } catch { + return + } +} diff --git a/frontend/src/lib/queryPersistence.test.ts b/frontend/src/lib/queryPersistence.test.ts new file mode 100644 index 000000000..f2e464307 --- /dev/null +++ b/frontend/src/lib/queryPersistence.test.ts @@ -0,0 +1,250 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { QueryClient, type Query } from '@tanstack/react-query' +import { + PERSISTED_CACHE_MAX_AGE_MS, + PERSISTED_QUERY_RULES, + QUERY_CACHE_STORAGE_KEY, + applyPersistedQueryDefaults, + clearPersistedQueryCache, + createQueryCachePersister, + isPersistedQueryKey, + queryCacheBuster, + shouldPersistQuery, +} from './queryPersistence' + +function buildQuery( + queryClient: QueryClient, + queryKey: readonly unknown[], + status: 'pending' | 'success' | 'error' = 'success', +): Query { + const query = queryClient + .getQueryCache() + .build(queryClient, { + queryKey, + queryFn: async () => null, + }) + + if (status === 'success') { + query.setState({ status: 'success', data: { ok: true }, fetchStatus: 'idle' }) + } else if (status === 'error') { + query.setState({ status: 'error', error: new Error('boom'), fetchStatus: 'idle' }) + } + + return query +} + +function createStorage() { + const store = new Map() + return { + store, + storage: { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => { + store.set(key, value) + }, + removeItem: (key: string) => { + store.delete(key) + }, + }, + } +} + +const clients: QueryClient[] = [] + +function createClient(): QueryClient { + const client = new QueryClient() + clients.push(client) + return client +} + +afterEach(() => { + vi.useRealTimers() + for (const client of clients.splice(0)) client.clear() +}) + +describe('isPersistedQueryKey', () => { + it.each([ + ['repo list', ['repos']], + ['repo git status', ['reposGitStatus', [1, 2]]], + ['session pins', ['session-pins']], + ['linked servers', ['opencode-servers']], + ['session list', ['opencode', 'sessions', '/work/repo', { search: '' }]], + ['single session', ['opencode', 'session', 'ses_1', '/work/repo']], + ['active work', ['opencode', 'active-work-sessions', 'srv_remote']], + ['remote projects', ['opencode', 'projects', 'srv_remote']], + ['vcs info', ['opencode', 'vcs', '/work/repo', 'srv_remote', 'info']], + ])('allows the %s key', (_label, queryKey) => { + expect(isPersistedQueryKey(queryKey)).toBe(true) + }) + + it.each([ + ['settings', ['settings', 'user-1']], + ['provider credentials', ['provider-credentials']], + ['provider auth methods', ['provider-auth-methods']], + ['passkeys', ['passkeys']], + ['schedules', ['schedules', '/work/repo']], + ['all schedules', ['all-schedules']], + ['repo schedules', ['repo-schedules', 1]], + ['transcript', ['opencode', 'transcript', 'ses_1']], + ['providers', ['opencode', 'providers', '/work/repo']], + ['commands', ['opencode', 'commands', '/work/repo']], + ['agents', ['opencode', 'agents', '/work/repo']], + ['config', ['opencode', 'config', 'model', '/work/repo']], + ['pending actions', ['opencode', 'pending-actions', 'ses_1', '/work/repo']], + ['shells', ['opencode', 'shells', '/work/repo']], + ['session reconciliation', ['opencode', 'session-reconcile', 'ses_1']], + ['vcs status', ['opencode', 'vcs', '/work/repo', 'status']], + ['vcs diff', ['opencode', 'vcs', '/work/repo', 'diff', 'branch']], + ['repo detail', ['repo', 1]], + ['server health', ['health']], + ])('rejects the %s key', (_label, queryKey) => { + expect(isPersistedQueryKey(queryKey)).toBe(false) + }) + + it('rejects an empty or non-string key', () => { + expect(isPersistedQueryKey([])).toBe(false) + expect(isPersistedQueryKey([42])).toBe(false) + }) +}) + +describe('shouldPersistQuery', () => { + it('persists a successful query under an allowlisted rule', () => { + const queryClient = createClient() + + expect(shouldPersistQuery(buildQuery(queryClient, ['repos']))).toBe(true) + expect(shouldPersistQuery(buildQuery(queryClient, ['opencode', 'sessions', '/work/repo']))).toBe(true) + }) + + it('rejects a non-success query even under an allowlisted rule', () => { + const queryClient = createClient() + + expect(shouldPersistQuery(buildQuery(queryClient, ['repos'], 'pending'))).toBe(false) + expect(shouldPersistQuery(buildQuery(queryClient, ['repos'], 'error'))).toBe(false) + }) + + it('rejects a successful query whose data root carries secrets', () => { + const queryClient = createClient() + + expect(shouldPersistQuery(buildQuery(queryClient, ['settings', 'user-1']))).toBe(false) + expect(shouldPersistQuery(buildQuery(queryClient, ['provider-credentials']))).toBe(false) + expect(shouldPersistQuery(buildQuery(queryClient, ['passkeys']))).toBe(false) + }) +}) + +describe('PERSISTED_QUERY_RULES', () => { + it('covers exactly the list and roster roots', () => { + expect(PERSISTED_QUERY_RULES.map((rule) => rule.prefix)).toEqual([ + ['repos'], + ['reposGitStatus'], + ['session-pins'], + ['opencode-servers'], + ['opencode', 'sessions'], + ['opencode', 'session'], + ['opencode', 'active-work-sessions'], + ['opencode', 'projects'], + ['opencode', 'vcs'], + ]) + }) + + it('never names a secret-bearing or transcript root', () => { + const roots = PERSISTED_QUERY_RULES.map((rule) => rule.prefix.join('.')) + const excluded = [ + 'settings', + 'provider-credentials', + 'provider-auth-methods', + 'passkeys', + 'schedules', + 'all-schedules', + 'opencode.transcript', + 'opencode.providers', + 'opencode.config', + ] + + for (const root of excluded) expect(roots).not.toContain(root) + }) +}) + +describe('queryCacheBuster', () => { + it('scopes the buster to the origin and the user id', () => { + expect(queryCacheBuster('user-1')).toBe('http://localhost|user-1') + expect(queryCacheBuster('user-2')).not.toBe(queryCacheBuster('user-1')) + }) +}) + +describe('createQueryCachePersister', () => { + it('writes the dehydrated cache under the fixed storage key after the throttle', async () => { + vi.useFakeTimers() + const { store, storage } = createStorage() + const persister = createQueryCachePersister(storage) + + void persister.persistClient({ + buster: 'http://localhost|user-1', + timestamp: Date.now(), + clientState: { mutations: [], queries: [] }, + }) + await vi.advanceTimersByTimeAsync(1000) + + expect([...store.keys()]).toEqual([QUERY_CACHE_STORAGE_KEY]) + expect(JSON.parse(store.get(QUERY_CACHE_STORAGE_KEY) ?? '')).toMatchObject({ + buster: 'http://localhost|user-1', + clientState: { queries: [] }, + }) + }) + + it('still writes after dropping the oldest query when the quota rejects a save', async () => { + vi.useFakeTimers() + const { store, storage } = createStorage() + let rejections = 1 + const persister = createQueryCachePersister({ + ...storage, + setItem: (key: string, value: string) => { + if (rejections > 0) { + rejections -= 1 + throw new Error('QuotaExceededError') + } + store.set(key, value) + }, + }) + + void persister.persistClient({ + buster: 'http://localhost|user-1', + timestamp: Date.now(), + clientState: { + mutations: [], + queries: [{ queryKey: ['repos'], state: { data: [], dataUpdatedAt: 1 } } as never], + }, + }) + await vi.advanceTimersByTimeAsync(1000) + + expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(true) + }) +}) + +describe('applyPersistedQueryDefaults', () => { + it('raises gcTime for every allowlisted rule only', () => { + const queryClient = createClient() + applyPersistedQueryDefaults(queryClient) + + for (const rule of PERSISTED_QUERY_RULES) { + expect(queryClient.getQueryDefaults(rule.prefix).gcTime).toBe(PERSISTED_CACHE_MAX_AGE_MS) + } + + expect(queryClient.getQueryDefaults(['settings', 'user-1']).gcTime).toBeUndefined() + expect(queryClient.getQueryDefaults(['opencode', 'transcript', 'ses_1']).gcTime).toBeUndefined() + expect(queryClient.getDefaultOptions().queries?.gcTime).toBeUndefined() + expect(PERSISTED_CACHE_MAX_AGE_MS).toBe(24 * 60 * 60 * 1000) + }) +}) + +describe('clearPersistedQueryCache', () => { + it('removes only the persisted cache key', () => { + const { store, storage } = createStorage() + store.set(QUERY_CACHE_STORAGE_KEY, '{}') + store.set('ocm.auth.user', '{}') + + clearPersistedQueryCache(storage) + + expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(false) + expect(store.has('ocm.auth.user')).toBe(true) + }) +}) diff --git a/frontend/src/lib/queryPersistence.ts b/frontend/src/lib/queryPersistence.ts new file mode 100644 index 000000000..3308f4bc0 --- /dev/null +++ b/frontend/src/lib/queryPersistence.ts @@ -0,0 +1,111 @@ +import type { Query, QueryClient } from '@tanstack/react-query' +import { createSyncStoragePersister } from '@tanstack/query-sync-storage-persister' +import { removeOldestQuery, type Persister } from '@tanstack/react-query-persist-client' + +export interface PersistedQueryRule { + readonly prefix: readonly string[] + readonly lastSegment?: string +} + +export interface QueryCacheStorage { + getItem: (key: string) => string | null + setItem: (key: string, value: string) => void + removeItem: (key: string) => void +} + +/** + * Query-key prefixes whose cached data is written to disk so a previously loaded list can paint + * before the network answers. + * + * Every rule is a list or roster key whose shape is stable across reloads of the same account. + * Excluded on purpose: `settings` (contains the OpenCode server auth token and API keys), + * `provider-credentials`, `passkeys`, `provider-auth-methods`, the `opencode` transcript kinds + * (large, and rewritten on every streamed token), `schedules` and `all-schedules` (a schedule or + * MCP config can carry `client_secret`), and every root not listed here. + * + * `opencode` shares one root with transcript, provider and config keys, so those rules match the + * second segment too and the `vcs` rule additionally requires `info` as the last segment. + */ +export const PERSISTED_QUERY_RULES: readonly PersistedQueryRule[] = [ + { prefix: ['repos'] }, + { prefix: ['reposGitStatus'] }, + { prefix: ['session-pins'] }, + { prefix: ['opencode-servers'] }, + { prefix: ['opencode', 'sessions'] }, + { prefix: ['opencode', 'session'] }, + { prefix: ['opencode', 'active-work-sessions'] }, + { prefix: ['opencode', 'projects'] }, + { prefix: ['opencode', 'vcs'], lastSegment: 'info' }, +] + +export const PERSISTED_CACHE_MAX_AGE_MS = 24 * 60 * 60 * 1000 + +export const QUERY_CACHE_STORAGE_KEY = 'ocm.query-cache.v1' + +function matchesRule(rule: PersistedQueryRule, queryKey: readonly unknown[]): boolean { + if (queryKey.length < rule.prefix.length) return false + if (!rule.prefix.every((segment, index) => queryKey[index] === segment)) return false + return rule.lastSegment === undefined || queryKey[queryKey.length - 1] === rule.lastSegment +} + +/** + * Whether a query key belongs to one of the allowlisted roots. Only the segments named by a rule + * are inspected, so the directory, server id or search options appended to a list key stay free. + */ +export function isPersistedQueryKey(queryKey: readonly unknown[]): boolean { + return PERSISTED_QUERY_RULES.some((rule) => matchesRule(rule, queryKey)) +} + +/** + * Decides whether one query is allowed on disk: its key must match an allowlisted rule and it must + * hold successfully fetched data. Pending and error states carry nothing worth restoring. + */ +export function shouldPersistQuery(query: Query): boolean { + return query.state.status === 'success' && isPersistedQueryKey(query.queryKey) +} + +/** + * Cache owner key used as the persister `buster`. A persisted cache written for another origin or + * account is discarded on restore instead of hydrating this one. + */ +export function queryCacheBuster(userId: string): string { + return `${window.location.origin}|${userId}` +} + +/** + * Creates the localStorage persister that stores the dehydrated cache under + * {@link QUERY_CACHE_STORAGE_KEY}, saving at most once per second. `removeOldestQuery` keeps a full + * quota from silencing persistence: the oldest entry is dropped and the write retried. + */ +export function createQueryCachePersister( + storage: QueryCacheStorage | null = typeof window === 'undefined' ? null : window.localStorage, +): Persister { + return createSyncStoragePersister({ + storage, + key: QUERY_CACHE_STORAGE_KEY, + throttleTime: 1000, + retry: removeOldestQuery, + }) +} + +/** + * Raises `gcTime` to {@link PERSISTED_CACHE_MAX_AGE_MS} for the allowlisted rules only, so a + * restored inactive query survives long enough to be rendered while the global `gcTime` keeps its + * normal value. + */ +export function applyPersistedQueryDefaults(queryClient: QueryClient): void { + for (const rule of PERSISTED_QUERY_RULES) { + queryClient.setQueryDefaults(rule.prefix, { gcTime: PERSISTED_CACHE_MAX_AGE_MS }) + } +} + +/** Removes the persisted cache from storage, e.g. on logout. */ +export function clearPersistedQueryCache( + storage: QueryCacheStorage | null = typeof window === 'undefined' ? null : window.localStorage, +): void { + try { + storage?.removeItem(QUERY_CACHE_STORAGE_KEY) + } catch { + return + } +} diff --git a/frontend/src/lib/queryState.test.ts b/frontend/src/lib/queryState.test.ts new file mode 100644 index 000000000..1ead7b273 --- /dev/null +++ b/frontend/src/lib/queryState.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest' +import { cachedListStates } from './queryState' + +describe('cachedListStates', () => { + it('shows the skeleton only while there is no data and no error', () => { + expect(cachedListStates(undefined, false)).toEqual({ + showsInitialLoading: true, + showsError: false, + }) + }) + + it('shows the error only when the query failed without cached data', () => { + expect(cachedListStates(undefined, true)).toEqual({ + showsInitialLoading: false, + showsError: true, + }) + }) + + it('keeps cached data on screen after a failed refetch', () => { + expect(cachedListStates([], true)).toEqual({ + showsInitialLoading: false, + showsError: false, + }) + expect(cachedListStates([{ id: 1 }], true)).toEqual({ + showsInitialLoading: false, + showsError: false, + }) + }) + + it('keeps loaded data on screen while a refetch runs', () => { + expect(cachedListStates([], false)).toEqual({ + showsInitialLoading: false, + showsError: false, + }) + }) + + it('does not report a load for a query that is not allowed to run', () => { + expect(cachedListStates(undefined, false, false)).toEqual({ + showsInitialLoading: false, + showsError: false, + }) + }) +}) diff --git a/frontend/src/lib/queryState.ts b/frontend/src/lib/queryState.ts new file mode 100644 index 000000000..4b056d697 --- /dev/null +++ b/frontend/src/lib/queryState.ts @@ -0,0 +1,27 @@ +export interface CachedListStates { + showsInitialLoading: boolean + showsError: boolean +} + +/** + * Derives the two states a list screen renders from a query that may already hold cached data. + * + * A restored or offline query runs pending without fetching, so `isLoading` stays false while no + * data is available yet, and a failed refetch keeps the succeeded data. Keying both states on + * `data === undefined` keeps cached lists on screen instead of flashing a skeleton or an empty + * state during a restore. + * + * @param data Query data, `undefined` until the first successful load or restore. + * @param isError Whether the query settled with an error. + * @param enabled Whether the query is allowed to run, so an intentionally idle query is not a load. + */ +export function cachedListStates( + data: T | undefined, + isError: boolean, + enabled = true, +): CachedListStates { + return { + showsInitialLoading: enabled && data === undefined && !isError, + showsError: isError && data === undefined, + } +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 22ed7a3c5..358210671 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -153,9 +153,15 @@ importers: '@tailwindcss/vite': specifier: ^4.1.14 version: 4.1.18(vite@7.3.0(@types/node@24.10.4)(jiti@2.6.1)(lightningcss@1.30.2)(terser@5.46.0)(yaml@2.9.0)) + '@tanstack/query-sync-storage-persister': + specifier: 5.101.2 + version: 5.101.2 '@tanstack/react-query': - specifier: ^5.90.5 - version: 5.90.16(react@19.2.3) + specifier: ^5.101.2 + version: 5.101.2(react@19.2.3) + '@tanstack/react-query-persist-client': + specifier: 5.101.2 + version: 5.101.2(@tanstack/react-query@5.101.2(react@19.2.3))(react@19.2.3) '@xterm/addon-fit': specifier: ^0.11.0 version: 0.11.0 @@ -2256,11 +2262,23 @@ packages: peerDependencies: vite: ^5.2.0 || ^6 || ^7 - '@tanstack/query-core@5.90.16': - resolution: {integrity: sha512-MvtWckSVufs/ja463/K4PyJeqT+HMlJWtw6PrCpywznd2NSgO3m4KwO9RqbFqGg6iDE8vVMFWMeQI4Io3eEYww==} + '@tanstack/query-core@5.101.2': + resolution: {integrity: sha512-hH5MLoJhF7KaIGd7q3xTXGXvslI+GYlM1Z/35aSHHWaCJWB7XvTSHYuV3eM7tw+aE0mT/xMro4M4Q9rCGHT0lw==} - '@tanstack/react-query@5.90.16': - resolution: {integrity: sha512-bpMGOmV4OPmif7TNMteU/Ehf/hoC0Kf98PDc0F4BZkFrEapRMEqI/V6YS0lyzwSV6PQpY1y4xxArUIfBW5LVxQ==} + '@tanstack/query-persist-client-core@5.101.2': + resolution: {integrity: sha512-rgMsbvBVpSPDUsprC9FYxojdG0Q1LH6yq1i3DXz2aps4VEqv2l4Msj3YDqIifU3xkl/DrYe9YWntZAJLrM6xTg==} + + '@tanstack/query-sync-storage-persister@5.101.2': + resolution: {integrity: sha512-hRyLEJvkUWtfCUA3Lf5vD1CrXbHHK4pZNM/iNd7JM5GgC4CWzGIHkVkeg8zC2LV11KceKObKgjNuMSg3b6NOjA==} + + '@tanstack/react-query-persist-client@5.101.2': + resolution: {integrity: sha512-19UpaRtf0lvB2QAJ2MoixxtGf3osMOd508g99EsttUj4+3eLs+ulnNgYjB7AkQMHrRlcbVXqpVNqWkB4a7g8Zw==} + peerDependencies: + '@tanstack/react-query': ^5.101.2 + react: ^18 || ^19 + + '@tanstack/react-query@5.101.2': + resolution: {integrity: sha512-seDkr6kzGzX1okaaTtZPtgA688CDPlXUz1C6xSg0ESqn04Vuc8tlrYms1s3de+znBqhPVxFRfpAfUf+6XvfPWg==} peerDependencies: react: ^18 || ^19 @@ -7618,11 +7636,26 @@ snapshots: tailwindcss: 4.1.18 vite: 7.3.0(@types/node@24.10.4)(jiti@2.6.1)(lightningcss@1.30.2)(terser@5.46.0)(yaml@2.9.0) - '@tanstack/query-core@5.90.16': {} + '@tanstack/query-core@5.101.2': {} + + '@tanstack/query-persist-client-core@5.101.2': + dependencies: + '@tanstack/query-core': 5.101.2 + + '@tanstack/query-sync-storage-persister@5.101.2': + dependencies: + '@tanstack/query-core': 5.101.2 + '@tanstack/query-persist-client-core': 5.101.2 + + '@tanstack/react-query-persist-client@5.101.2(@tanstack/react-query@5.101.2(react@19.2.3))(react@19.2.3)': + dependencies: + '@tanstack/query-persist-client-core': 5.101.2 + '@tanstack/react-query': 5.101.2(react@19.2.3) + react: 19.2.3 - '@tanstack/react-query@5.90.16(react@19.2.3)': + '@tanstack/react-query@5.101.2(react@19.2.3)': dependencies: - '@tanstack/query-core': 5.90.16 + '@tanstack/query-core': 5.101.2 react: 19.2.3 '@testing-library/dom@10.4.1': From b0a1640565698d52126b19592e278e5d6e93ab1c Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Fri, 23 Oct 2026 23:10:09 -0400 Subject: [PATCH 2/5] fix(frontend): harden the persisted query cache Persist only data that survives a JSON round trip, bound both the age and the size of what is written, and stop a restore from being read as a missing or empty list. - Drop the repository git status rule: its `Map` data serializes to `{}`, so a restored cache made `gitStatuses.get` throw. Round-trip every remaining rule through the real persister to catch class-backed data. - Persist only the unfiltered session list and give just that list the 24h gcTime; a search key keeps the default and is collected when unused. - Set the 24h gcTime in the vcs info query options instead of a prefix default, so vcs status and diff keep their normal gcTime. - Drop the oldest half of the persisted queries per retry pass instead of one per pass, sorting once per save, and cap infinite queries to their first page before the write. - Build the session-pin and active-work rules from the shared keys. --- .../navigation/DesktopSessionTree.test.tsx | 1 + .../components/session/SessionList.test.tsx | 1 + .../session/SessionPickerDialog.test.tsx | 1 + .../session/SessionPickerSheet.test.tsx | 1 + frontend/src/hooks/useOpenCode.ts | 7 + frontend/src/hooks/useVcsInfo.ts | 3 + frontend/src/lib/queryInvalidation.ts | 10 +- frontend/src/lib/queryPersistence.test.ts | 336 +++++++++++++++++- frontend/src/lib/queryPersistence.ts | 134 ++++++- 9 files changed, 471 insertions(+), 23 deletions(-) diff --git a/frontend/src/components/navigation/DesktopSessionTree.test.tsx b/frontend/src/components/navigation/DesktopSessionTree.test.tsx index 186e4eba6..864c4a8a3 100644 --- a/frontend/src/components/navigation/DesktopSessionTree.test.tsx +++ b/frontend/src/components/navigation/DesktopSessionTree.test.tsx @@ -89,6 +89,7 @@ vi.mock('@/hooks/useOpenCode', async (importOriginal) => { }) vi.mock('@/hooks/useSessionPins', () => ({ + SESSION_PINS_QUERY_KEY: ['session-pins'], useSessionPins: () => ({ data: sessionPinsData.current }), })) diff --git a/frontend/src/components/session/SessionList.test.tsx b/frontend/src/components/session/SessionList.test.tsx index 15b0fd180..e3b75e420 100644 --- a/frontend/src/components/session/SessionList.test.tsx +++ b/frontend/src/components/session/SessionList.test.tsx @@ -56,6 +56,7 @@ vi.mock('@/hooks/useOpenCode', async (importOriginal) => { }) vi.mock('@/hooks/useSessionPins', () => ({ + SESSION_PINS_QUERY_KEY: ['session-pins'], useSessionPins: () => ({ data: sessionPinsData }), useToggleSessionPin: () => ({ mutate: togglePinMock }), })) diff --git a/frontend/src/components/session/SessionPickerDialog.test.tsx b/frontend/src/components/session/SessionPickerDialog.test.tsx index eedba48f8..02f76dc47 100644 --- a/frontend/src/components/session/SessionPickerDialog.test.tsx +++ b/frontend/src/components/session/SessionPickerDialog.test.tsx @@ -108,6 +108,7 @@ vi.mock('@/hooks/useOpenCode', async (importOriginal) => { }) vi.mock('@/hooks/useSessionPins', () => ({ + SESSION_PINS_QUERY_KEY: ['session-pins'], useSessionPins: () => ({ data: mocks.sessionPins }), useToggleSessionPin: () => ({ mutate: mocks.togglePinMock }), })) diff --git a/frontend/src/components/session/SessionPickerSheet.test.tsx b/frontend/src/components/session/SessionPickerSheet.test.tsx index 33ec181d9..87357acb4 100644 --- a/frontend/src/components/session/SessionPickerSheet.test.tsx +++ b/frontend/src/components/session/SessionPickerSheet.test.tsx @@ -88,6 +88,7 @@ vi.mock('@/hooks/useOpenCode', async (importOriginal) => { }) vi.mock('@/hooks/useSessionPins', () => ({ + SESSION_PINS_QUERY_KEY: ['session-pins'], useSessionPins: () => ({ data: [] }), useToggleSessionPin: () => ({ mutate: vi.fn() }), })) diff --git a/frontend/src/hooks/useOpenCode.ts b/frontend/src/hooks/useOpenCode.ts index 15a1e1d3a..e774e8669 100644 --- a/frontend/src/hooks/useOpenCode.ts +++ b/frontend/src/hooks/useOpenCode.ts @@ -31,6 +31,7 @@ import { useSendErrorStore } from "../stores/sendErrorStore"; import { useSessionStatus, useChildLifecycleForSession } from "../stores/sessionStatusStore"; import { isRunningLifecycle } from "../lib/backgroundWork"; import { childSessionReconciliationQueryKey, invalidateSessionListCaches, sessionTranscriptQueryKey } from "../lib/queryInvalidation"; +import { PERSISTED_CACHE_MAX_AGE_MS } from "../lib/queryPersistence"; import { buildPinnedSessionKeys, buildSessionKey } from "../lib/sessionKey"; import { toggleSessionPin } from "../api/sessionPins"; import { SESSION_PINS_QUERY_KEY } from "./useSessionPins"; @@ -82,6 +83,11 @@ export const useSessionsAcrossDirectories = ( const projectScoped = !allDirectories && projectId !== undefined; const singleCursor = allDirectories || projectScoped; const includeChildren = options?.includeChildren ?? false; + /** + * Only the unfiltered list is restored from disk, so only it keeps the longer `gcTime`; a search + * result stays at the default and is collected as soon as it goes unused. + */ + const persistedGcTime = normalizedSearch === undefined ? PERSISTED_CACHE_MAX_AGE_MS : undefined; const directoryKey = uniqueDirectories.join('|'); const childSessionsParam = includeChildren ? { includeChildren: true } : {}; @@ -179,6 +185,7 @@ export const useSessionsAcrossDirectories = ( getNextPageParam: (lastPage) => lastPage.nextParam, enabled: uniqueDirectories.length > 0 || allDirectories, staleTime: 10000, + gcTime: persistedGcTime, refetchOnWindowFocus: true, refetchOnReconnect: true, }); diff --git a/frontend/src/hooks/useVcsInfo.ts b/frontend/src/hooks/useVcsInfo.ts index b1d27ce74..e1446b633 100644 --- a/frontend/src/hooks/useVcsInfo.ts +++ b/frontend/src/hooks/useVcsInfo.ts @@ -1,6 +1,7 @@ import { useQuery } from '@tanstack/react-query' import { getVcsInfo } from '@/api/opencode' import { vcsInfoQueryKey } from '@/lib/queryInvalidation' +import { PERSISTED_CACHE_MAX_AGE_MS } from '@/lib/queryPersistence' export interface UseVcsInfoOptions { enabled?: boolean @@ -18,6 +19,8 @@ export function useVcsInfo( queryFn: () => getVcsInfo(directory ?? '', serverId), enabled: enabled && Boolean(directory), staleTime: 30_000, + /** The key is only persisted when it ends in `info`, which a prefix default cannot express. */ + gcTime: PERSISTED_CACHE_MAX_AGE_MS, retry: false, }) } diff --git a/frontend/src/lib/queryInvalidation.ts b/frontend/src/lib/queryInvalidation.ts index 764bb3232..dbdc51468 100644 --- a/frontend/src/lib/queryInvalidation.ts +++ b/frontend/src/lib/queryInvalidation.ts @@ -214,14 +214,16 @@ export function invalidateSessionCaches(queryClient: QueryClient) { }) } -const SESSION_LIST_QUERY_GROUPS = new Set(['sessions', 'active-work-sessions']) +export const ACTIVE_WORK_SESSIONS_QUERY_ROOT = 'active-work-sessions' + +const SESSION_LIST_QUERY_GROUPS = new Set(['sessions', ACTIVE_WORK_SESSIONS_QUERY_ROOT]) /** * Query key of one server's active-work session list: flat, newest-first, across all directories. * It is distinct from the paged session-list keys so the two data shapes never share a cache entry. */ export function activeWorkSessionsQueryKey(serverId: string): readonly unknown[] { - return ['opencode', 'active-work-sessions', serverId] + return ['opencode', ACTIVE_WORK_SESSIONS_QUERY_ROOT, serverId] } export function invalidateSessionListCaches(queryClient: QueryClient) { @@ -236,7 +238,7 @@ export function invalidateSessionListCaches(queryClient: QueryClient) { * session-list key appends it after its options, and an unscoped key is a local-server key. */ function sessionListQueryServerId(queryKey: readonly unknown[]): string { - if (queryKey[1] === 'active-work-sessions') return String(queryKey[2]) + if (queryKey[1] === ACTIVE_WORK_SESSIONS_QUERY_ROOT) return String(queryKey[2]) return typeof queryKey[4] === 'string' ? queryKey[4] : LOCAL_OPENCODE_SERVER_ID } @@ -256,7 +258,7 @@ function sessionListQueryMatchesDirectories( if (queryKey[0] !== 'opencode') return false if (!SESSION_LIST_QUERY_GROUPS.has(String(queryKey[1]))) return false if (sessionListQueryServerId(queryKey) !== serverId) return false - if (queryKey[1] === 'active-work-sessions') return true + if (queryKey[1] === ACTIVE_WORK_SESSIONS_QUERY_ROOT) return true const directoryKey = queryKey[2] if (typeof directoryKey !== 'string') return false return directoryKey.split('|').some((directory) => directories.has(directory)) diff --git a/frontend/src/lib/queryPersistence.test.ts b/frontend/src/lib/queryPersistence.test.ts index f2e464307..8a4574e2a 100644 --- a/frontend/src/lib/queryPersistence.test.ts +++ b/frontend/src/lib/queryPersistence.test.ts @@ -1,17 +1,24 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import { QueryClient, type Query } from '@tanstack/react-query' +import { QueryClient, dehydrate, hydrate, type Query } from '@tanstack/react-query' +import type { PersistedClient } from '@tanstack/react-query-persist-client' +import { SESSION_PINS_QUERY_KEY } from '@/hooks/useSessionPins' import { PERSISTED_CACHE_MAX_AGE_MS, PERSISTED_QUERY_RULES, QUERY_CACHE_STORAGE_KEY, applyPersistedQueryDefaults, + capInfiniteQueryPages, clearPersistedQueryCache, + createDropOldestHalfRetryer, createQueryCachePersister, isPersistedQueryKey, queryCacheBuster, + serializePersistedClient, shouldPersistQuery, } from './queryPersistence' +type DehydratedQuery = PersistedClient['clientState']['queries'][number] + function buildQuery( queryClient: QueryClient, queryKey: readonly unknown[], @@ -49,6 +56,21 @@ function createStorage() { } } +function buildPersistedClient(clientState: PersistedClient['clientState']): PersistedClient { + return { buster: 'http://localhost|user-1', timestamp: Date.now(), clientState } +} + +function buildDehydratedQuery( + queryKey: readonly unknown[], + dataUpdatedAt: number, +): DehydratedQuery { + return { + queryHash: JSON.stringify(queryKey), + queryKey, + state: { status: 'success', data: { id: queryKey[0] }, dataUpdatedAt } as DehydratedQuery['state'], + } +} + const clients: QueryClient[] = [] function createClient(): QueryClient { @@ -57,6 +79,19 @@ function createClient(): QueryClient { return client } +async function persistAndRestore(client: QueryClient, storage: ReturnType['storage']) { + vi.useFakeTimers() + const persister = createQueryCachePersister(storage) + void persister.persistClient(buildPersistedClient(dehydrate(client))) + await vi.advanceTimersByTimeAsync(1000) + vi.useRealTimers() + + const restoredClient = createClient() + const persisted = await persister.restoreClient() + if (persisted) hydrate(restoredClient, persisted.clientState) + return restoredClient +} + afterEach(() => { vi.useRealTimers() for (const client of clients.splice(0)) client.clear() @@ -65,10 +100,10 @@ afterEach(() => { describe('isPersistedQueryKey', () => { it.each([ ['repo list', ['repos']], - ['repo git status', ['reposGitStatus', [1, 2]]], ['session pins', ['session-pins']], ['linked servers', ['opencode-servers']], ['session list', ['opencode', 'sessions', '/work/repo', { search: '' }]], + ['session search', ['opencode', 'sessions', '/work/repo', { search: 'fix login' }]], ['single session', ['opencode', 'session', 'ses_1', '/work/repo']], ['active work', ['opencode', 'active-work-sessions', 'srv_remote']], ['remote projects', ['opencode', 'projects', 'srv_remote']], @@ -78,6 +113,7 @@ describe('isPersistedQueryKey', () => { }) it.each([ + ['repo git status', ['reposGitStatus', [1, 2]]], ['settings', ['settings', 'user-1']], ['provider credentials', ['provider-credentials']], ['provider auth methods', ['provider-auth-methods']], @@ -129,13 +165,25 @@ describe('shouldPersistQuery', () => { expect(shouldPersistQuery(buildQuery(queryClient, ['provider-credentials']))).toBe(false) expect(shouldPersistQuery(buildQuery(queryClient, ['passkeys']))).toBe(false) }) + + it('persists only the unfiltered session list', () => { + const queryClient = createClient() + const listKey = ['opencode', 'sessions', '/work/repo', { search: '', limit: 25 }] + + expect(shouldPersistQuery(buildQuery(queryClient, listKey))).toBe(true) + expect( + shouldPersistQuery(buildQuery(queryClient, ['opencode', 'sessions', '/work/repo', { search: 'login' }])), + ).toBe(false) + expect( + shouldPersistQuery(buildQuery(queryClient, ['opencode', 'sessions', '/work/repo', { search: ' ' }])), + ).toBe(true) + }) }) describe('PERSISTED_QUERY_RULES', () => { it('covers exactly the list and roster roots', () => { expect(PERSISTED_QUERY_RULES.map((rule) => rule.prefix)).toEqual([ ['repos'], - ['reposGitStatus'], ['session-pins'], ['opencode-servers'], ['opencode', 'sessions'], @@ -155,6 +203,7 @@ describe('PERSISTED_QUERY_RULES', () => { 'passkeys', 'schedules', 'all-schedules', + 'reposGitStatus', 'opencode.transcript', 'opencode.providers', 'opencode.config', @@ -162,6 +211,25 @@ describe('PERSISTED_QUERY_RULES', () => { for (const root of excluded) expect(roots).not.toContain(root) }) + + it('builds the session-pin and active-work rules from the shared keys', () => { + expect(PERSISTED_QUERY_RULES.map((rule) => rule.prefix)).toContainEqual([...SESSION_PINS_QUERY_KEY]) + expect(PERSISTED_QUERY_RULES.map((rule) => rule.prefix)).toContainEqual([ + 'opencode', + 'active-work-sessions', + ]) + }) + + it('marks the rules that cannot use a prefix gcTime default', () => { + const rulesWithoutPrefixDefault = PERSISTED_QUERY_RULES.filter( + (rule) => rule.lastSegment !== undefined || rule.gcTimeInQueryOptions, + ) + + expect(rulesWithoutPrefixDefault.map((rule) => rule.prefix)).toEqual([ + ['opencode', 'sessions'], + ['opencode', 'vcs'], + ]) + }) }) describe('queryCacheBuster', () => { @@ -171,6 +239,250 @@ describe('queryCacheBuster', () => { }) }) +const allowlistedFixtures = [ + { + label: 'repo list', + queryKey: ['repos'], + data: [ + { + id: 1, + name: 'app', + localPath: '/work/app', + fullPath: '/work/app', + defaultBranch: 'main', + cloneStatus: 'ready', + clonedAt: 1_700_000_000_000, + lastAccessedAt: 1_700_000_100_000, + isLocal: true, + }, + ], + }, + { + label: 'session pins', + queryKey: [...SESSION_PINS_QUERY_KEY], + data: [{ sessionId: 'ses_1', directory: '/work/repo', pinnedAt: 1_700_000_000_000 }], + }, + { + label: 'linked servers', + queryKey: ['opencode-servers'], + data: [ + { + id: 'srv_kitchen', + name: 'Kitchen', + baseUrl: 'https://kitchen.example.com', + username: 'opencode', + enabled: true, + hasPassword: true, + createdAt: 1, + updatedAt: 2, + }, + ], + }, + { + label: 'session list', + queryKey: ['opencode', 'sessions', '/work/repo', { search: '', limit: 25 }], + data: { + pages: [ + { + items: [ + { + id: 'ses_1', + projectID: 'prj_1', + title: 'Fix login', + time: { created: 1_700_000_000_000, updated: 1_700_000_100_000 }, + location: { directory: '/work/repo' }, + cost: 0.02, + tokens: { input: 10, output: 20, reasoning: 0, cache: { read: 0, write: 0 } }, + }, + ], + nextParam: { mode: 'all', cursor: 'cursor-1' }, + }, + ], + pageParams: [null], + }, + }, + { + label: 'single session', + queryKey: ['opencode', 'session', 'ses_1', '/work/repo'], + data: { + id: 'ses_1', + projectID: 'prj_1', + title: 'Fix login', + time: { created: 1_700_000_000_000, updated: 1_700_000_100_000, archived: 1_700_000_200_000 }, + location: { directory: '/work/repo' }, + cost: 0.02, + tokens: { input: 10, output: 20, reasoning: 0, cache: { read: 0, write: 0 } }, + }, + }, + { + label: 'active work', + queryKey: ['opencode', 'active-work-sessions', 'srv_kitchen'], + data: [ + { + id: 'ses_2', + projectID: 'prj_1', + title: 'Review diff', + time: { created: 1_700_000_000_000, updated: 1_700_000_100_000 }, + location: { directory: '/work/repo' }, + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + }, + ], + }, + { + label: 'remote projects', + queryKey: ['opencode', 'projects', 'srv_kitchen'], + data: [ + { + id: 'prj_1', + name: 'app', + canonical: '/work/app', + vcs: 'git', + time: { created: 1_700_000_000_000, updated: 1_700_000_100_000, active: 1_700_000_200_000 }, + sandboxes: [{ id: 'sbx_1', directory: '/work/app', created: 1_700_000_000_000 }], + }, + ], + }, + { + label: 'vcs info', + queryKey: ['opencode', 'vcs', '/work/app', 'srv_kitchen', 'info'], + data: { branch: 'main', default_branch: 'main', root: '/work/app' }, + }, +] as const + +describe('persisted query round trip', () => { + it.each(allowlistedFixtures)('restores the $label data unchanged', async ({ queryKey, data }) => { + const sourceClient = createClient() + sourceClient.setQueryData(queryKey, data) + const { storage } = createStorage() + + const restoredClient = await persistAndRestore(sourceClient, storage) + + expect(restoredClient.getQueryData(queryKey)).toEqual(data) + }) + + it('loses repository git status data, which is why it is not allowlisted', async () => { + const sourceClient = createClient() + sourceClient.setQueryData(['reposGitStatus', [1]], new Map([[1, { hasChanges: true }]])) + const { storage } = createStorage() + + const restoredClient = await persistAndRestore(sourceClient, storage) + + expect(restoredClient.getQueryData(['reposGitStatus', [1]])).toEqual({}) + }) +}) + +describe('capInfiniteQueryPages', () => { + it('keeps only the first page and its page param of every infinite query', () => { + const clientState = { + mutations: [], + queries: [ + buildDehydratedQuery(['repos'], 1), + { + ...buildDehydratedQuery(['opencode', 'sessions', '/work/repo'], 2), + state: { + status: 'success', + dataUpdatedAt: 2, + data: { pages: [{ items: ['a'] }, { items: ['b'] }, { items: ['c'] }], pageParams: [undefined, 'c2', 'c3'] }, + } as DehydratedQuery['state'], + }, + ], + } + + const capped = capInfiniteQueryPages(clientState) + + expect(capped.queries[0].state.data).toEqual({ id: 'repos' }) + expect(capped.queries[1].state.data).toEqual({ + pages: [{ items: ['a'] }], + pageParams: [undefined], + }) + expect(clientState.queries[1].state.data).toMatchObject({ + pages: [{ items: ['a'] }, { items: ['b'] }, { items: ['c'] }], + }) + }) + + it('caps the infinite query before the persister writes it', async () => { + const sourceClient = createClient() + sourceClient.setQueryData(['opencode', 'sessions', '/work/repo', { search: '' }], { + pages: [{ items: ['a'] }, { items: ['b'] }], + pageParams: [undefined, 'c2'], + }) + const { storage, store } = createStorage() + + const restoredClient = await persistAndRestore(sourceClient, storage) + + const stored = JSON.parse(store.get(QUERY_CACHE_STORAGE_KEY) ?? '{}') + expect(stored.clientState.queries[0].state.data).toEqual({ + pages: [{ items: ['a'] }], + pageParams: [null], + }) + expect(restoredClient.getQueryData(['opencode', 'sessions', '/work/repo', { search: '' }])).toEqual({ + pages: [{ items: ['a'] }], + pageParams: [null], + }) + }) + + it('leaves plain list data alone', () => { + const clientState = { mutations: [], queries: [buildDehydratedQuery(['repos'], 1)] } + + expect(serializePersistedClient(buildPersistedClient(clientState))).toBe( + JSON.stringify(buildPersistedClient(clientState)), + ) + }) +}) + +describe('createDropOldestHalfRetryer', () => { + it('drops the oldest half of the queries on the first pass', () => { + const retryer = createDropOldestHalfRetryer() + const queries = [4, 1, 3, 2].map((age) => buildDehydratedQuery([`query-${age}`], age)) + + const retried = retryer({ + persistedClient: buildPersistedClient({ mutations: [], queries }), + error: new Error('QuotaExceededError'), + errorCount: 1, + }) + + expect(retried?.clientState.queries.map((query) => query.state.dataUpdatedAt)).toEqual([4, 3]) + }) + + it('keeps halving the remaining queries on later passes of the same save', () => { + const retryer = createDropOldestHalfRetryer() + const queries = [1, 2, 3, 4].map((age) => buildDehydratedQuery([`query-${age}`], age)) + const props = { + persistedClient: buildPersistedClient({ mutations: [], queries }), + error: new Error('QuotaExceededError'), + errorCount: 1, + } + + const first = retryer(props) + const second = retryer({ ...props, persistedClient: first!, errorCount: 2 }) + const third = retryer({ ...props, persistedClient: second!, errorCount: 3 }) + + expect(first?.clientState.queries.map((query) => query.state.dataUpdatedAt)).toEqual([3, 4]) + expect(second?.clientState.queries.map((query) => query.state.dataUpdatedAt)).toEqual([4]) + expect(third?.clientState.queries).toEqual([]) + }) + + it('sorts once per save and starts over for the next save', () => { + const retryer = createDropOldestHalfRetryer() + retryer({ + persistedClient: buildPersistedClient({ mutations: [], queries: [buildDehydratedQuery(['a'], 5)] }), + error: new Error('QuotaExceededError'), + errorCount: 1, + }) + + const nextSave = [buildDehydratedQuery(['b'], 9), buildDehydratedQuery(['c'], 1)] + + const retried = retryer({ + persistedClient: buildPersistedClient({ mutations: [], queries: nextSave }), + error: new Error('QuotaExceededError'), + errorCount: 1, + }) + + expect(retried?.clientState.queries.map((query) => query.queryKey)).toEqual([['b']]) + }) +}) + describe('createQueryCachePersister', () => { it('writes the dehydrated cache under the fixed storage key after the throttle', async () => { vi.useFakeTimers() @@ -191,7 +503,7 @@ describe('createQueryCachePersister', () => { }) }) - it('still writes after dropping the oldest query when the quota rejects a save', async () => { + it('still writes after dropping the oldest half when the quota rejects a save', async () => { vi.useFakeTimers() const { store, storage } = createStorage() let rejections = 1 @@ -221,12 +533,14 @@ describe('createQueryCachePersister', () => { }) describe('applyPersistedQueryDefaults', () => { - it('raises gcTime for every allowlisted rule only', () => { + it('raises gcTime only for the allowlisted rules that use a prefix default', () => { const queryClient = createClient() applyPersistedQueryDefaults(queryClient) for (const rule of PERSISTED_QUERY_RULES) { - expect(queryClient.getQueryDefaults(rule.prefix).gcTime).toBe(PERSISTED_CACHE_MAX_AGE_MS) + const gcTime = queryClient.getQueryDefaults(rule.prefix).gcTime + const usesPrefixDefault = rule.lastSegment === undefined && !rule.gcTimeInQueryOptions + expect(gcTime).toBe(usesPrefixDefault ? PERSISTED_CACHE_MAX_AGE_MS : undefined) } expect(queryClient.getQueryDefaults(['settings', 'user-1']).gcTime).toBeUndefined() @@ -234,6 +548,16 @@ describe('applyPersistedQueryDefaults', () => { expect(queryClient.getDefaultOptions().queries?.gcTime).toBeUndefined() expect(PERSISTED_CACHE_MAX_AGE_MS).toBe(24 * 60 * 60 * 1000) }) + + it('leaves the paged session list and vcs info to their own query options', () => { + const queryClient = createClient() + applyPersistedQueryDefaults(queryClient) + + expect(queryClient.getQueryDefaults(['opencode', 'sessions']).gcTime).toBeUndefined() + expect(queryClient.getQueryDefaults(['opencode', 'vcs']).gcTime).toBeUndefined() + expect(queryClient.defaultQueryOptions({ queryKey: ['opencode', 'sessions', '/work/repo'] }).gcTime) + .toBeUndefined() + }) }) describe('clearPersistedQueryCache', () => { diff --git a/frontend/src/lib/queryPersistence.ts b/frontend/src/lib/queryPersistence.ts index 3308f4bc0..82f93c4ad 100644 --- a/frontend/src/lib/queryPersistence.ts +++ b/frontend/src/lib/queryPersistence.ts @@ -1,10 +1,20 @@ -import type { Query, QueryClient } from '@tanstack/react-query' +import type { DehydratedState, Query, QueryClient } from '@tanstack/react-query' import { createSyncStoragePersister } from '@tanstack/query-sync-storage-persister' -import { removeOldestQuery, type Persister } from '@tanstack/react-query-persist-client' +import type { + PersistedClient, + Persister, + PersistRetryer, +} from '@tanstack/react-query-persist-client' +import { SESSION_PINS_QUERY_KEY } from '@/hooks/useSessionPins' +import { ACTIVE_WORK_SESSIONS_QUERY_ROOT } from '@/lib/queryInvalidation' + +type DehydratedQuery = DehydratedState['queries'][number] export interface PersistedQueryRule { readonly prefix: readonly string[] readonly lastSegment?: string + /** Set `gcTime` in the query options instead of a prefix default, because keys under one prefix need different retention. */ + readonly gcTimeInQueryOptions?: boolean } export interface QueryCacheStorage { @@ -21,19 +31,19 @@ export interface QueryCacheStorage { * Excluded on purpose: `settings` (contains the OpenCode server auth token and API keys), * `provider-credentials`, `passkeys`, `provider-auth-methods`, the `opencode` transcript kinds * (large, and rewritten on every streamed token), `schedules` and `all-schedules` (a schedule or - * MCP config can carry `client_secret`), and every root not listed here. + * MCP config can carry `client_secret`), `reposGitStatus` (a `Map` that JSON turns into `{}`), and + * every root not listed here. * * `opencode` shares one root with transcript, provider and config keys, so those rules match the * second segment too and the `vcs` rule additionally requires `info` as the last segment. */ export const PERSISTED_QUERY_RULES: readonly PersistedQueryRule[] = [ { prefix: ['repos'] }, - { prefix: ['reposGitStatus'] }, - { prefix: ['session-pins'] }, + { prefix: SESSION_PINS_QUERY_KEY }, { prefix: ['opencode-servers'] }, - { prefix: ['opencode', 'sessions'] }, + { prefix: ['opencode', 'sessions'], gcTimeInQueryOptions: true }, { prefix: ['opencode', 'session'] }, - { prefix: ['opencode', 'active-work-sessions'] }, + { prefix: ['opencode', ACTIVE_WORK_SESSIONS_QUERY_ROOT] }, { prefix: ['opencode', 'projects'] }, { prefix: ['opencode', 'vcs'], lastSegment: 'info' }, ] @@ -42,6 +52,8 @@ export const PERSISTED_CACHE_MAX_AGE_MS = 24 * 60 * 60 * 1000 export const QUERY_CACHE_STORAGE_KEY = 'ocm.query-cache.v1' +const SESSION_LIST_OPTIONS_INDEX = 3 + function matchesRule(rule: PersistedQueryRule, queryKey: readonly unknown[]): boolean { if (queryKey.length < rule.prefix.length) return false if (!rule.prefix.every((segment, index) => queryKey[index] === segment)) return false @@ -56,12 +68,29 @@ export function isPersistedQueryKey(queryKey: readonly unknown[]): boolean { return PERSISTED_QUERY_RULES.some((rule) => matchesRule(rule, queryKey)) } +function isSessionListQueryKey(queryKey: readonly unknown[]): boolean { + return queryKey[0] === 'opencode' && queryKey[1] === 'sessions' +} + +function sessionListHasSearch(queryKey: readonly unknown[]): boolean { + const options = queryKey[SESSION_LIST_OPTIONS_INDEX] + if (typeof options !== 'object' || options === null) return false + const search = (options as { search?: unknown }).search + return typeof search === 'string' && search.trim() !== '' +} + /** - * Decides whether one query is allowed on disk: its key must match an allowlisted rule and it must - * hold successfully fetched data. Pending and error states carry nothing worth restoring. + * Decides whether one query is allowed on disk: its key must match an allowlisted rule, it must + * hold successfully fetched data, and a paged session list must be the unfiltered one. + * + * A search result depends on the query the user typed, so persisting it would spend the storage + * budget on keys that are rarely read again while evicting the lists that are. */ export function shouldPersistQuery(query: Query): boolean { - return query.state.status === 'success' && isPersistedQueryKey(query.queryKey) + if (query.state.status !== 'success') return false + if (!isPersistedQueryKey(query.queryKey)) return false + if (isSessionListQueryKey(query.queryKey) && sessionListHasSearch(query.queryKey)) return false + return true } /** @@ -72,10 +101,84 @@ export function queryCacheBuster(userId: string): string { return `${window.location.origin}|${userId}` } +interface InfiniteQueryData { + pages: unknown[] + pageParams: unknown[] +} + +function isInfiniteQueryData(data: unknown): data is InfiniteQueryData { + if (typeof data !== 'object' || data === null) return false + const candidate = data as Partial + return Array.isArray(candidate.pages) && Array.isArray(candidate.pageParams) +} + +function capInfinitePages(data: unknown): unknown { + if (!isInfiniteQueryData(data) || data.pages.length <= 1) return data + return { pages: data.pages.slice(0, 1), pageParams: data.pageParams.slice(0, 1) } +} + +function capQueryPages(query: DehydratedQuery): DehydratedQuery { + if (query.state.data === undefined) return query + return { + ...query, + state: { ...query.state, data: capInfinitePages(query.state.data) }, + } +} + +/** + * Keeps only the first page of every infinite query in a dehydrated state. + * + * A restored list is refetched page by page anyway, so the extra pages only cost storage, and the + * first page keeps `hasNextPage` derivable from its own next-page parameter. + */ +export function capInfiniteQueryPages(clientState: DehydratedState): DehydratedState { + return { ...clientState, queries: clientState.queries.map(capQueryPages) } +} + +/** Serializes the persisted client with {@link capInfiniteQueryPages} applied before the write. */ +export function serializePersistedClient(client: PersistedClient): string { + return JSON.stringify({ ...client, clientState: capInfiniteQueryPages(client.clientState) }) +} + +/** + * Drops the oldest half of the persisted queries on every failed save. + * + * The sync persister re-serializes after each retry, so removing one query at a time burns a whole + * write per query on a large cache. Halving the remaining set converges in a few passes, and the + * age order is sorted once per save so the retries only filter it. + */ +export function createDropOldestHalfRetryer(): PersistRetryer { + let ageOrderedQueries: readonly DehydratedQuery[] = [] + + return ({ persistedClient }) => { + const queries = persistedClient.clientState.queries + const isRetryOfSameSave = queries.length < ageOrderedQueries.length + && queries.every((query) => ageOrderedQueries.includes(query)) + if (!isRetryOfSameSave) { + ageOrderedQueries = [...queries].sort( + (left, right) => left.state.dataUpdatedAt - right.state.dataUpdatedAt, + ) + } + + const remaining = new Set(queries) + const oldestFirst = ageOrderedQueries.filter((query) => remaining.has(query)) + const keptQueries = new Set(oldestFirst.slice(Math.ceil(oldestFirst.length / 2))) + + return { + ...persistedClient, + clientState: { + ...persistedClient.clientState, + queries: queries.filter((query) => keptQueries.has(query)), + }, + } + } +} + /** * Creates the localStorage persister that stores the dehydrated cache under - * {@link QUERY_CACHE_STORAGE_KEY}, saving at most once per second. `removeOldestQuery` keeps a full - * quota from silencing persistence: the oldest entry is dropped and the write retried. + * {@link QUERY_CACHE_STORAGE_KEY}, saving at most once per second. Infinite queries are capped to + * their first page before the write, and {@link createDropOldestHalfRetryer} keeps a full quota + * from silencing persistence. */ export function createQueryCachePersister( storage: QueryCacheStorage | null = typeof window === 'undefined' ? null : window.localStorage, @@ -84,7 +187,8 @@ export function createQueryCachePersister( storage, key: QUERY_CACHE_STORAGE_KEY, throttleTime: 1000, - retry: removeOldestQuery, + serialize: serializePersistedClient, + retry: createDropOldestHalfRetryer(), }) } @@ -92,9 +196,13 @@ export function createQueryCachePersister( * Raises `gcTime` to {@link PERSISTED_CACHE_MAX_AGE_MS} for the allowlisted rules only, so a * restored inactive query survives long enough to be rendered while the global `gcTime` keeps its * normal value. + * + * `setQueryDefaults` matches by key prefix, so a rule whose keys need different retention, or + * whose key only ends in a specific segment, sets `gcTime` in its own query options instead. */ export function applyPersistedQueryDefaults(queryClient: QueryClient): void { for (const rule of PERSISTED_QUERY_RULES) { + if (rule.lastSegment !== undefined || rule.gcTimeInQueryOptions) continue queryClient.setQueryDefaults(rule.prefix, { gcTime: PERSISTED_CACHE_MAX_AGE_MS }) } } From ae461a7b16a6f45062d210a9271558b9c87fef02 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Fri, 23 Oct 2026 23:39:12 -0400 Subject: [PATCH 3/5] fix(frontend): persist only allowlisted changes and make sign-out teardown strict --- frontend/src/contexts/AuthContext.test.tsx | 88 ++++++-- frontend/src/contexts/AuthContext.tsx | 66 +++--- .../QueryCachePersistenceContext.test.tsx | 105 ++++++++- .../contexts/QueryCachePersistenceContext.tsx | 106 ++++++--- frontend/src/lib/auth-loaders.ts | 36 ++-- frontend/src/lib/authSession.test.ts | 75 +++++++ frontend/src/lib/authSession.ts | 26 +++ frontend/src/lib/queryPersistence.test.ts | 201 +++++++++++++++++- frontend/src/lib/queryPersistence.ts | 124 +++++++++-- frontend/src/pages/RemoteProject.tsx | 2 +- .../pages/__tests__/RemoteProject.test.tsx | 17 +- 11 files changed, 711 insertions(+), 135 deletions(-) create mode 100644 frontend/src/lib/authSession.test.ts diff --git a/frontend/src/contexts/AuthContext.test.tsx b/frontend/src/contexts/AuthContext.test.tsx index da9151af7..d25fc2766 100644 --- a/frontend/src/contexts/AuthContext.test.tsx +++ b/frontend/src/contexts/AuthContext.test.tsx @@ -1,11 +1,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { render, waitFor } from '@testing-library/react' -import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { act, render, waitFor } from '@testing-library/react' +import { QueryClient, QueryClientProvider, dehydrate } from '@tanstack/react-query' import { MemoryRouter, Route, Routes } from 'react-router-dom' import { LocationCatcher } from '@/test/test-utils' import { AuthProvider } from './AuthContext' +import { QueryCachePersistenceProvider } from './QueryCachePersistenceContext' import { useAuth } from '@/hooks/useAuth' -import { QUERY_CACHE_STORAGE_KEY } from '@/lib/queryPersistence' +import { QUERY_CACHE_STORAGE_KEY, queryCacheBuster } from '@/lib/queryPersistence' import { STORED_AUTH_USER_KEY } from '@/lib/authSession' const mocks = vi.hoisted(() => ({ @@ -30,7 +31,7 @@ type AuthValue = ReturnType const STORED_USER = { id: 'user-1', email: 'user@example.com', name: 'Ada' } -function renderAuth(initialEntry: string) { +function renderAuth(initialEntry: string, options: { persistence?: boolean } = {}) { const capturedPathname: { current: string } = { current: '' } const capturedSearch: { current: string } = { current: '' } const captured: { current: AuthValue | null } = { current: null } @@ -45,7 +46,13 @@ function renderAuth(initialEntry: string) { path="/login" element={( - + {options.persistence ? ( + + + + ) : ( + + )} )} /> @@ -205,21 +212,76 @@ describe('AuthProvider session restore', () => { expect(localStorage.getItem(STORED_AUTH_USER_KEY)).toBeNull() }) - it('clears the stored user, the persisted cache and the query cache on logout', async () => { + it('clears the stored user on logout and leaves the cache teardown to its owner', async () => { localStorage.setItem(STORED_AUTH_USER_KEY, JSON.stringify(STORED_USER)) - localStorage.setItem(QUERY_CACHE_STORAGE_KEY, '{}') - const { captured, queryClient } = renderAuth('/login') - const clearSpy = vi.spyOn(queryClient, 'clear') - queryClient.setQueryData(['repos'], [{ id: 1 }]) + const { captured } = renderAuth('/login') mocks.signOut.mockImplementation(async (options: { fetchOptions?: { onSuccess?: () => void } }) => { options.fetchOptions?.onSuccess?.() return { data: null } }) - await captured.current?.logout() + await act(async () => { + await captured.current?.logout() + }) + + expect(localStorage.getItem(STORED_AUTH_USER_KEY)).toBeNull() + expect(captured.current?.isAuthenticated).toBe(false) + expect(mocks.signOut).toHaveBeenCalled() + }) + + it('signs out and clears the persisted cache when sign-out fails offline', async () => { + localStorage.setItem(STORED_AUTH_USER_KEY, JSON.stringify(STORED_USER)) + mocks.useSession.mockReturnValue({ + data: { user: STORED_USER }, + error: null, + isPending: false, + refetch: mocks.refetch, + }) + const seededClient = new QueryClient() + seededClient.setQueryData(['repos'], [{ id: 1 }]) + localStorage.setItem(QUERY_CACHE_STORAGE_KEY, JSON.stringify({ + buster: queryCacheBuster(STORED_USER.id), + timestamp: Date.now(), + clientState: dehydrate(seededClient), + })) + mocks.signOut.mockRejectedValue(new TypeError('Failed to fetch')) + + const { captured } = renderAuth('/login', { persistence: true }) + await waitFor(() => expect(localStorage.getItem(QUERY_CACHE_STORAGE_KEY)).not.toBeNull()) + + await act(async () => { + await captured.current?.logout() + }) expect(localStorage.getItem(STORED_AUTH_USER_KEY)).toBeNull() - expect(localStorage.getItem(QUERY_CACHE_STORAGE_KEY)).toBeNull() - expect(clearSpy).toHaveBeenCalled() + await waitFor(() => expect(captured.current?.isAuthenticated).toBe(false)) + await waitFor(() => expect(localStorage.getItem(QUERY_CACHE_STORAGE_KEY)).toBeNull()) + }) + + it.each([ + ['removes the stored user', () => localStorage.removeItem(STORED_AUTH_USER_KEY)], + ['switches the stored user', () => localStorage.setItem( + STORED_AUTH_USER_KEY, + JSON.stringify({ id: 'user-2', email: 'two@example.com', name: 'Grace' }), + )], + ])('signs out when another tab %s', async (_label, changeStoredUser) => { + localStorage.setItem(STORED_AUTH_USER_KEY, JSON.stringify(STORED_USER)) + mocks.useSession.mockReturnValue({ + data: { user: STORED_USER }, + error: null, + isPending: false, + refetch: mocks.refetch, + }) + + const { captured } = renderAuth('/login') + await waitFor(() => expect(captured.current?.isAuthenticated).toBe(true)) + + act(() => { + changeStoredUser() + window.dispatchEvent(new StorageEvent('storage', { key: STORED_AUTH_USER_KEY })) + }) + + await waitFor(() => expect(captured.current?.isAuthenticated).toBe(false)) + expect(mocks.signOut).toHaveBeenCalled() }) }) diff --git a/frontend/src/contexts/AuthContext.tsx b/frontend/src/contexts/AuthContext.tsx index 90d151af6..0d9df3c64 100644 --- a/frontend/src/contexts/AuthContext.tsx +++ b/frontend/src/contexts/AuthContext.tsx @@ -1,18 +1,15 @@ /* eslint-disable react-refresh/only-export-components */ import { createContext, useEffect, useMemo, useState, useCallback, type ReactNode } from 'react' -import { useQueryClient } from '@tanstack/react-query' -import { useSession, signIn, signUp, signOut, authClient, type AuthUser } from '@/lib/auth-client' +import { useSession, signIn, signUp, signOut, authClient } from '@/lib/auth-client' import { useNavigate, useLocation } from 'react-router-dom' import { getLoginRedirectTarget } from '@/lib/auth-loaders' import { + STORED_AUTH_USER_KEY, clearStoredAuthUser, - isTransientSessionError, readStoredAuthUser, - toStoredAuthUser, - writeStoredAuthUser, + reconcileStoredAuthUser, type StoredAuthUser, } from '@/lib/authSession' -import { clearPersistedQueryCache } from '@/lib/queryPersistence' interface AuthConfig { enabledProviders: string[] @@ -21,10 +18,8 @@ interface AuthConfig { adminConfigured: boolean } -export type AuthContextUser = Pick - interface AuthContextValue { - user: AuthContextUser | null + user: StoredAuthUser | null isAuthenticated: boolean isLoading: boolean config: AuthConfig | null @@ -48,7 +43,7 @@ interface AuthProviderProps { export function AuthProvider({ children }: AuthProviderProps) { const { data: session, error: sessionError, isPending, refetch } = useSession() const [storedUser, setStoredUser] = useState(() => readStoredAuthUser()) - const queryClient = useQueryClient() + const [signedOut, setSignedOut] = useState(false) const [config, setConfig] = useState(null) const navigate = useNavigate() const location = useLocation() @@ -58,16 +53,10 @@ export function AuthProvider({ children }: AuthProviderProps) { ) useEffect(() => { - if (session?.user) { - const validatedUser = toStoredAuthUser(session.user) - writeStoredAuthUser(validatedUser) - setStoredUser(validatedUser) - return - } if (isPending) return - if (isTransientSessionError(sessionError)) return - clearStoredAuthUser() - setStoredUser(null) + const reconciledUser = reconcileStoredAuthUser(session?.user ?? null, sessionError) + setStoredUser(reconciledUser) + if (reconciledUser !== null) setSignedOut(false) }, [session, sessionError, isPending]) useEffect(() => { @@ -142,24 +131,41 @@ export function AuthProvider({ children }: AuthProviderProps) { }, []) const logout = useCallback(async () => { - clearStoredAuthUser() + setSignedOut(true) setStoredUser(null) - clearPersistedQueryCache() - queryClient.clear() - await signOut({ - fetchOptions: { - onSuccess: () => { - window.location.href = '/login' + clearStoredAuthUser() + try { + await signOut({ + fetchOptions: { + onSuccess: () => { + window.location.href = '/login' + }, }, - }, - }) - }, [queryClient]) + }) + } catch { + return + } + }, []) const refreshSession = useCallback(async () => { await refetch() }, [refetch]) - const user = session?.user ?? storedUser + const user = signedOut ? null : session?.user ?? storedUser + + useEffect(() => { + if (user === null) return + + const handleStorageChange = (event: StorageEvent) => { + if (event.key !== null && event.key !== STORED_AUTH_USER_KEY) return + const nextUser = readStoredAuthUser() + if (nextUser !== null && nextUser.id === user.id) return + void logout() + } + + window.addEventListener('storage', handleStorageChange) + return () => window.removeEventListener('storage', handleStorageChange) + }, [user, logout]) const value = useMemo(() => ({ user, diff --git a/frontend/src/contexts/QueryCachePersistenceContext.test.tsx b/frontend/src/contexts/QueryCachePersistenceContext.test.tsx index 4e3782332..4dfce90b7 100644 --- a/frontend/src/contexts/QueryCachePersistenceContext.test.tsx +++ b/frontend/src/contexts/QueryCachePersistenceContext.test.tsx @@ -2,9 +2,12 @@ import { act, render, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { QueryClient, QueryClientProvider, dehydrate, useIsRestoring } from '@tanstack/react-query' import { createSyncStoragePersister } from '@tanstack/query-sync-storage-persister' -import type { Persister } from '@tanstack/react-query-persist-client' import { QueryCachePersistenceProvider } from './QueryCachePersistenceContext' -import { QUERY_CACHE_STORAGE_KEY, queryCacheBuster } from '@/lib/queryPersistence' +import { + QUERY_CACHE_STORAGE_KEY, + queryCacheBuster, + type QueryCachePersister, +} from '@/lib/queryPersistence' const mocks = vi.hoisted(() => ({ useAuth: vi.fn(), @@ -52,8 +55,16 @@ function createStorage() { type TestStorage = ReturnType['storage'] -function createTestPersister(storage: TestStorage): Persister { - return createSyncStoragePersister({ storage, key: QUERY_CACHE_STORAGE_KEY, throttleTime: 0 }) +function createTestPersister(storage: TestStorage, throttleTime = 0): QueryCachePersister { + const persister = createSyncStoragePersister({ + storage, + key: QUERY_CACHE_STORAGE_KEY, + throttleTime, + }) + return { + ...persister, + hasPersistedClient: () => storage.getItem(QUERY_CACHE_STORAGE_KEY) !== null, + } } function seed( @@ -72,9 +83,12 @@ function seed( } let restoringValue: boolean | undefined +let restoringHistory: boolean[] = [] function RestoringProbe() { - restoringValue = useIsRestoring() + const isRestoring = useIsRestoring() + restoringValue = isRestoring + restoringHistory.push(isRestoring) return null } @@ -86,11 +100,16 @@ function createClient(): QueryClient { return client } -function tree(auth: AuthStub, persister: Persister, client: QueryClient) { +function tree( + auth: AuthStub, + persister: QueryCachePersister, + client: QueryClient, + persistThrottleMs = 0, +) { mocks.useAuth.mockReturnValue(auth) return ( - + @@ -105,6 +124,7 @@ async function settle(): Promise { beforeEach(() => { restoringValue = undefined + restoringHistory = [] mocks.useAuth.mockReset() localStorage.clear() }) @@ -178,7 +198,7 @@ describe('QueryCachePersistenceProvider', () => { it('is restoring until the restore resolves', async () => { let resolveRestore: () => void = () => {} - const persister: Persister = { + const persister: QueryCachePersister = { persistClient: vi.fn(), restoreClient: vi.fn( () => new Promise((resolve) => { @@ -186,6 +206,7 @@ describe('QueryCachePersistenceProvider', () => { }), ), removeClient: vi.fn(), + hasPersistedClient: () => true, } render(tree(signedIn(), persister, createClient())) @@ -225,4 +246,72 @@ describe('QueryCachePersistenceProvider', () => { await waitFor(() => expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(false)) await waitFor(() => expect(restoringValue).toBe(false)) }) + + it('subscribes without a restoring pass when nothing is persisted yet', async () => { + const { store, storage } = createStorage() + const client = createClient() + const persister = createTestPersister(storage) + + render(tree(signedIn(), persister, client)) + + expect(restoringHistory).not.toContain(true) + + act(() => { + client.setQueryData(['repos'], [{ id: 1 }]) + }) + await settle() + + expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(true) + expect(restoringHistory).not.toContain(true) + }) + + it('saves an allowlisted update but not a transcript update', async () => { + const client = createClient() + const persistClient = vi.fn() + const persister: QueryCachePersister = { + persistClient, + restoreClient: () => undefined, + removeClient: vi.fn(), + hasPersistedClient: () => false, + } + + render(tree(signedIn(), persister, client)) + + act(() => { + client.setQueryData(['opencode', 'transcript', 'ses_1'], [{ text: 'streamed' }]) + }) + await settle() + expect(persistClient).not.toHaveBeenCalled() + + act(() => { + client.setQueryData(['repos'], [{ id: 1 }]) + }) + await settle() + + expect(persistClient).toHaveBeenCalledTimes(1) + const persisted = persistClient.mock.calls[0][0] + expect(persisted.buster).toBe(queryCacheBuster('user-1')) + expect(persisted.clientState.queries.map((query: { queryKey: unknown[] }) => query.queryKey)).toEqual([ + ['repos'], + ]) + }) + + it('drops a save scheduled before sign-out instead of rewriting the cache', async () => { + const { store, storage } = createStorage() + const client = createClient() + const persister = createTestPersister(storage) + + const view = render(tree(signedIn(), persister, client, 20)) + act(() => { + client.setQueryData(['repos'], [{ id: 1 }]) + }) + + view.rerender(tree(signedOut(), persister, client, 20)) + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 60)) + }) + + expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(false) + expect(client.getQueryData(['repos'])).toBeUndefined() + }) }) diff --git a/frontend/src/contexts/QueryCachePersistenceContext.tsx b/frontend/src/contexts/QueryCachePersistenceContext.tsx index 5e126c573..b9bbb3abb 100644 --- a/frontend/src/contexts/QueryCachePersistenceContext.tsx +++ b/frontend/src/contexts/QueryCachePersistenceContext.tsx @@ -1,56 +1,76 @@ -import { useEffect, useMemo, useRef, useState, type ReactNode } from 'react' +import { useEffect, useLayoutEffect, useMemo, useRef, useState, type ReactNode } from 'react' import { IsRestoringProvider, useQueryClient } from '@tanstack/react-query' import { persistQueryClientRestore, - persistQueryClientSubscribe, - type Persister, + persistQueryClientSave, } from '@tanstack/react-query-persist-client' import { useAuth } from '@/hooks/useAuth' import { PERSISTED_CACHE_MAX_AGE_MS, + PERSIST_THROTTLE_MS, + createBusterGuardedPersister, createQueryCachePersister, queryCacheBuster, + shouldPersistCacheEvent, shouldPersistQuery, + type QueryCachePersister, } from '@/lib/queryPersistence' export interface QueryCachePersistenceProviderProps { children?: ReactNode - persister?: Persister + persister?: QueryCachePersister + /** Spacing between persisted saves; tests lower it instead of waiting a second per write. */ + persistThrottleMs?: number } /** * Restores the allowlisted part of the React Query cache for the signed-in account and keeps it * persisted while that account stays signed in. * + * This provider owns the cache teardown. Signing out or switching accounts clears both the in-memory + * client and the persisted cache here, and the persister drops any save still queued for the account + * that just left, so a trailing write cannot recreate the previous account's cache. + * * The restore starts only once the session resolves to a signed-in user and uses - * `"|"` as the persister buster, so a cache written for another origin or account - * is discarded instead of hydrating this one. While the restore is in flight the subtree runs under + * `"|"` as the persister buster, so a cache written for another origin or account is + * discarded instead of hydrating this one. While the restore is in flight the subtree runs under * `IsRestoringProvider`, which keeps queries from fetching before the persisted data is available. + * A first sign-in with nothing on disk subscribes straight away, because there is nothing to wait for. * - * A cache stored for a different user id is dropped from memory as well, and leaving the signed-in - * state clears both the in-memory client and the persisted cache. + * Saves are driven by query cache events whose key is allowlisted and, for an update, whose action + * changes data or status, so a streaming transcript update costs no dehydrate. */ export function QueryCachePersistenceProvider({ children, persister, + persistThrottleMs = PERSIST_THROTTLE_MS, }: QueryCachePersistenceProviderProps) { const queryClient = useQueryClient() const { user, isAuthenticated, isLoading } = useAuth() - const defaultPersister = useMemo(() => createQueryCachePersister(), []) - const activePersister = persister ?? defaultPersister + const defaultPersister = useMemo(() => createQueryCachePersister(undefined, 0), []) + const basePersister = persister ?? defaultPersister const userId = isAuthenticated ? user?.id ?? null : null const buster = userId === null ? null : queryCacheBuster(userId) + const activeBusterRef = useRef(null) const [restoredBuster, setRestoredBuster] = useState(null) const previousUserIdRef = useRef(null) - const restoring = buster !== null && restoredBuster !== buster + const hasPersistedCache = useMemo( + () => buster !== null && basePersister.hasPersistedClient(), + [buster, basePersister], + ) + const restoring = buster !== null && restoredBuster !== buster && hasPersistedCache + const activePersister = useMemo( + () => createBusterGuardedPersister({ + persister: basePersister, + readBuster: () => activeBusterRef.current, + throttleTime: persistThrottleMs, + }), + [basePersister, persistThrottleMs], + ) - useEffect(() => { - const previousUserId = previousUserIdRef.current - previousUserIdRef.current = userId - if (previousUserId !== null && userId !== null && previousUserId !== userId) { - queryClient.clear() - } - }, [userId, queryClient]) + useLayoutEffect(() => { + activeBusterRef.current = buster + }, [buster]) useEffect(() => { if (isLoading) return @@ -59,35 +79,53 @@ export function QueryCachePersistenceProvider({ setRestoredBuster(null) void Promise.resolve(activePersister.removeClient()).catch(() => {}) queryClient.clear() + previousUserIdRef.current = null return } + if (previousUserIdRef.current !== null && previousUserIdRef.current !== userId) { + queryClient.clear() + } + previousUserIdRef.current = userId + let cancelled = false let unsubscribe: (() => void) | undefined - void (async () => { - await persistQueryClientRestore({ - queryClient, - persister: activePersister, - maxAge: PERSISTED_CACHE_MAX_AGE_MS, - buster, - }).catch(() => {}) - if (cancelled) return + const subscribe = () => { + unsubscribe = queryClient.getQueryCache().subscribe((event) => { + if (!shouldPersistCacheEvent(event)) return + void persistQueryClientSave({ + queryClient, + persister: activePersister, + buster, + dehydrateOptions: { shouldDehydrateQuery: shouldPersistQuery }, + }).catch(() => {}) + }) + } + if (!hasPersistedCache) { setRestoredBuster(buster) - unsubscribe = persistQueryClientSubscribe({ - queryClient, - persister: activePersister, - buster, - dehydrateOptions: { shouldDehydrateQuery: shouldPersistQuery }, - }) - })() + subscribe() + } else { + void (async () => { + await persistQueryClientRestore({ + queryClient, + persister: activePersister, + maxAge: PERSISTED_CACHE_MAX_AGE_MS, + buster, + }).catch(() => {}) + if (cancelled) return + + setRestoredBuster(buster) + subscribe() + })() + } return () => { cancelled = true unsubscribe?.() } - }, [buster, isLoading, activePersister, queryClient]) + }, [buster, userId, isLoading, hasPersistedCache, activePersister, queryClient]) return {children} } diff --git a/frontend/src/lib/auth-loaders.ts b/frontend/src/lib/auth-loaders.ts index 91bed82ed..1a7beeda0 100644 --- a/frontend/src/lib/auth-loaders.ts +++ b/frontend/src/lib/auth-loaders.ts @@ -1,12 +1,6 @@ import { redirect, type LoaderFunctionArgs } from 'react-router-dom' import { getSession } from './auth-client' -import { - clearStoredAuthUser, - isTransientSessionError, - readStoredAuthUser, - toStoredAuthUser, - writeStoredAuthUser, -} from './authSession' +import { reconcileStoredAuthUser } from './authSession' export interface AuthConfig { enabledProviders: string[] @@ -118,30 +112,26 @@ export async function registerLoader() { } /** - * Guards a signed-in route. A validated user is remembered so the next cold start can open the app - * before the network answers; when the session read fails transiently (network, 5xx, timeout) the - * remembered user stands in for it. A successful read with no user, or a 401, is authoritative and - * deletes the remembered profile. + * Guards a signed-in route through {@link reconcileStoredAuthUser}: a validated user is let through + * and remembered for the next cold start, a transient session failure is let through while a + * remembered user stands in for it, and an authoritative empty or rejected session is redirected to + * login with the requested location. */ export async function protectedLoader({ request }: LoaderFunctionArgs) { const url = new URL(request.url) let session: Awaited> | null = null + let failure: unknown = null try { session = await getSession() - } catch { - session = null - } - - if (session?.data?.user) { - writeStoredAuthUser(toStoredAuthUser(session.data.user)) - return null + } catch (error) { + failure = error } - if (session === null || isTransientSessionError(session.error)) { - if (readStoredAuthUser() !== null) return null - } else { - clearStoredAuthUser() - } + const storedUser = reconcileStoredAuthUser( + session?.data?.user ?? null, + failure ?? session?.error ?? null, + ) + if (storedUser !== null) return null return redirect(loginRedirectPath(`${url.pathname}${url.search}${url.hash}`)) } diff --git a/frontend/src/lib/authSession.test.ts b/frontend/src/lib/authSession.test.ts new file mode 100644 index 000000000..0d408cad6 --- /dev/null +++ b/frontend/src/lib/authSession.test.ts @@ -0,0 +1,75 @@ +import { beforeEach, describe, expect, it } from 'vitest' +import { + STORED_AUTH_USER_KEY, + clearStoredAuthUser, + readStoredAuthUser, + reconcileStoredAuthUser, + writeStoredAuthUser, +} from './authSession' + +const STORED_USER = { id: 'user-1', email: 'user@example.com', name: 'Ada' } + +function storedValue(): unknown { + return JSON.parse(localStorage.getItem(STORED_AUTH_USER_KEY) ?? 'null') +} + +describe('reconcileStoredAuthUser', () => { + beforeEach(() => { + localStorage.clear() + }) + + it('saves a validated user and returns the profile without its extra fields', () => { + const user = { ...STORED_USER, emailVerified: true, sessionToken: 'secret' } + + expect(reconcileStoredAuthUser(user, null)).toEqual(STORED_USER) + expect(storedValue()).toEqual(STORED_USER) + }) + + it('replaces a previously stored profile with the validated user', () => { + writeStoredAuthUser(STORED_USER) + + reconcileStoredAuthUser({ id: 'user-2', email: 'two@example.com', name: 'Grace' }, null) + + expect(storedValue()).toEqual({ id: 'user-2', email: 'two@example.com', name: 'Grace' }) + }) + + it.each([ + ['a network failure', new TypeError('Failed to fetch')], + ['a 5xx response', { status: 503, statusText: 'Service Unavailable' }], + ])('keeps the stored profile on %s', (_label, error) => { + writeStoredAuthUser(STORED_USER) + + expect(reconcileStoredAuthUser(null, error)).toEqual(STORED_USER) + expect(storedValue()).toEqual(STORED_USER) + }) + + it('keeps nothing on a transient failure without a stored profile', () => { + expect(reconcileStoredAuthUser(null, new TypeError('Failed to fetch'))).toBeNull() + expect(localStorage.getItem(STORED_AUTH_USER_KEY)).toBeNull() + }) + + it.each([ + ['a 401 response', { status: 401, statusText: 'Unauthorized' }], + ['a response without a user', null], + ])('clears the stored profile on %s', (_label, error) => { + writeStoredAuthUser(STORED_USER) + + expect(reconcileStoredAuthUser(null, error)).toBeNull() + expect(localStorage.getItem(STORED_AUTH_USER_KEY)).toBeNull() + }) + + it('keeps an unreadable stored profile out of the result', () => { + localStorage.setItem(STORED_AUTH_USER_KEY, '{not json') + + expect(readStoredAuthUser()).toBeNull() + expect(reconcileStoredAuthUser(null, new TypeError('Failed to fetch'))).toBeNull() + }) + + it('clears the stored profile through clearStoredAuthUser', () => { + writeStoredAuthUser(STORED_USER) + + clearStoredAuthUser() + + expect(readStoredAuthUser()).toBeNull() + }) +}) diff --git a/frontend/src/lib/authSession.ts b/frontend/src/lib/authSession.ts index f04a84c20..0389d48c5 100644 --- a/frontend/src/lib/authSession.ts +++ b/frontend/src/lib/authSession.ts @@ -59,3 +59,29 @@ export function isTransientSessionError(error: unknown): boolean { } return true } + +/** + * Applies the stored-profile rule to one session read and keeps storage in step with it. + * + * A validated user replaces the saved profile; a transient failure keeps whatever was saved, so an + * offline cold start still opens the app; anything else is authoritative and deletes the profile. + * + * @param user The user of a validated session, or `null` when the read returned none. + * @param error The session read error, if the read failed. + * @returns The profile that now stands for the session, or `null` when the app is signed out. + */ +export function reconcileStoredAuthUser( + user: StoredAuthUser | null, + error: unknown, +): StoredAuthUser | null { + if (user !== null) { + const storedUser = toStoredAuthUser(user) + writeStoredAuthUser(storedUser) + return storedUser + } + + if (isTransientSessionError(error)) return readStoredAuthUser() + + clearStoredAuthUser() + return null +} diff --git a/frontend/src/lib/queryPersistence.test.ts b/frontend/src/lib/queryPersistence.test.ts index 8a4574e2a..9bed4dece 100644 --- a/frontend/src/lib/queryPersistence.test.ts +++ b/frontend/src/lib/queryPersistence.test.ts @@ -1,20 +1,23 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import { QueryClient, dehydrate, hydrate, type Query } from '@tanstack/react-query' +import { QueryClient, dehydrate, hydrate, type Query, type QueryCacheNotifyEvent } from '@tanstack/react-query' import type { PersistedClient } from '@tanstack/react-query-persist-client' import { SESSION_PINS_QUERY_KEY } from '@/hooks/useSessionPins' import { PERSISTED_CACHE_MAX_AGE_MS, PERSISTED_QUERY_RULES, + PERSIST_THROTTLE_MS, QUERY_CACHE_STORAGE_KEY, applyPersistedQueryDefaults, capInfiniteQueryPages, - clearPersistedQueryCache, + createBusterGuardedPersister, createDropOldestHalfRetryer, createQueryCachePersister, isPersistedQueryKey, queryCacheBuster, serializePersistedClient, + shouldPersistCacheEvent, shouldPersistQuery, + type QueryCachePersister, } from './queryPersistence' type DehydratedQuery = PersistedClient['clientState']['queries'][number] @@ -503,6 +506,27 @@ describe('createQueryCachePersister', () => { }) }) + it('reports whether a cache is already stored', () => { + const { store, storage } = createStorage() + const persister = createQueryCachePersister(storage) + + expect(persister.hasPersistedClient()).toBe(false) + store.set(QUERY_CACHE_STORAGE_KEY, '{}') + expect(persister.hasPersistedClient()).toBe(true) + }) + + it('reports no stored cache when the storage read throws', () => { + const persister = createQueryCachePersister({ + getItem: () => { + throw new Error('denied') + }, + setItem: () => {}, + removeItem: () => {}, + }) + + expect(persister.hasPersistedClient()).toBe(false) + }) + it('still writes after dropping the oldest half when the quota rejects a save', async () => { vi.useFakeTimers() const { store, storage } = createStorage() @@ -560,15 +584,184 @@ describe('applyPersistedQueryDefaults', () => { }) }) -describe('clearPersistedQueryCache', () => { +describe('persister teardown', () => { it('removes only the persisted cache key', () => { const { store, storage } = createStorage() store.set(QUERY_CACHE_STORAGE_KEY, '{}') store.set('ocm.auth.user', '{}') - clearPersistedQueryCache(storage) + createQueryCachePersister(storage).removeClient() expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(false) expect(store.has('ocm.auth.user')).toBe(true) }) }) + +describe('shouldPersistCacheEvent', () => { + const queryClient = createClient() + + function updateEvent( + queryKey: readonly unknown[], + action: { type: string } & Record, + ): QueryCacheNotifyEvent { + return { + type: 'updated', + query: buildQuery(queryClient, queryKey), + action, + } as QueryCacheNotifyEvent + } + + it.each([ + ['added', { type: 'added' }], + ['removed', { type: 'removed' }], + ])('reports an allowlisted query that was %s', (_label, event) => { + expect( + shouldPersistCacheEvent({ + ...event, + query: buildQuery(queryClient, ['repos']), + } as QueryCacheNotifyEvent), + ).toBe(true) + expect( + shouldPersistCacheEvent({ + ...event, + query: buildQuery(queryClient, ['opencode', 'transcript', 'ses_1']), + } as QueryCacheNotifyEvent), + ).toBe(false) + }) + + it.each([ + ['success', { type: 'success', data: null }], + ['error', { type: 'error', error: new Error('boom') }], + ['failed', { type: 'failed', failureCount: 1, error: new Error('boom') }], + ['invalidate', { type: 'invalidate' }], + ])('reports an allowlisted query updated on %s', (_label, action) => { + expect(shouldPersistCacheEvent(updateEvent(['repos'], action))).toBe(true) + }) + + it.each([ + ['fetch', { type: 'fetch' }], + ['pause', { type: 'pause' }], + ['continue', { type: 'continue' }], + ])('ignores a %s action, which only moves the fetch status', (_label, action) => { + expect(shouldPersistCacheEvent(updateEvent(['repos'], action))).toBe(false) + }) + + it('ignores a transcript update, which streams once per frame', () => { + expect( + shouldPersistCacheEvent(updateEvent(['opencode', 'transcript', 'ses_1'], { type: 'success', data: null })), + ).toBe(false) + }) + + it('ignores observer-only events', () => { + expect( + shouldPersistCacheEvent({ + type: 'observerResultsUpdated', + query: buildQuery(queryClient, ['repos']), + } as QueryCacheNotifyEvent), + ).toBe(false) + }) +}) + +describe('createBusterGuardedPersister', () => { + function createBasePersister( + storage: ReturnType['storage'], + ): QueryCachePersister { + return createQueryCachePersister(storage, 0) + } + + it('writes the pending client once the throttle elapses', async () => { + vi.useFakeTimers() + const { store, storage } = createStorage() + const persister = createBusterGuardedPersister({ + persister: createBasePersister(storage), + readBuster: () => 'http://localhost|user-1', + }) + + void persister.persistClient(buildPersistedClient({ mutations: [], queries: [] })) + await vi.advanceTimersByTimeAsync(PERSIST_THROTTLE_MS) + await vi.advanceTimersByTimeAsync(1) + + expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(true) + }) + + it('coalesces the saves inside one throttle window into the last one', async () => { + vi.useFakeTimers() + const { store, storage } = createStorage() + const persister = createBusterGuardedPersister({ + persister: createBasePersister(storage), + readBuster: () => 'http://localhost|user-1', + }) + + void persister.persistClient(buildPersistedClient({ mutations: [], queries: [] })) + void persister.persistClient( + buildPersistedClient({ mutations: [], queries: [buildDehydratedQuery(['repos'], 1)] }), + ) + await vi.advanceTimersByTimeAsync(PERSIST_THROTTLE_MS) + await vi.advanceTimersByTimeAsync(1) + + const persisted = JSON.parse(store.get(QUERY_CACHE_STORAGE_KEY) ?? '{}') + expect(persisted.clientState.queries).toHaveLength(1) + }) + + it('drops a save whose buster is no longer active when the throttle elapses', async () => { + vi.useFakeTimers() + const { store, storage } = createStorage() + let buster: string | null = 'http://localhost|user-1' + const persister = createBusterGuardedPersister({ + persister: createBasePersister(storage), + readBuster: () => buster, + }) + + void persister.persistClient(buildPersistedClient({ mutations: [], queries: [] })) + buster = null + await vi.advanceTimersByTimeAsync(PERSIST_THROTTLE_MS) + await vi.advanceTimersByTimeAsync(1) + + expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(false) + }) + + it('drops a save queued for the previous account after the user changes', async () => { + vi.useFakeTimers() + const { store, storage } = createStorage() + let buster: string | null = 'http://localhost|user-1' + const persister = createBusterGuardedPersister({ + persister: createBasePersister(storage), + readBuster: () => buster, + }) + + void persister.persistClient(buildPersistedClient({ mutations: [], queries: [] })) + buster = 'http://localhost|user-2' + await vi.advanceTimersByTimeAsync(PERSIST_THROTTLE_MS) + await vi.advanceTimersByTimeAsync(1) + + expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(false) + }) + + it('forgets the pending save on removeClient', async () => { + vi.useFakeTimers() + const { store, storage } = createStorage() + const persister = createBusterGuardedPersister({ + persister: createBasePersister(storage), + readBuster: () => 'http://localhost|user-1', + }) + + void persister.persistClient(buildPersistedClient({ mutations: [], queries: [] })) + persister.removeClient() + await vi.advanceTimersByTimeAsync(PERSIST_THROTTLE_MS) + await vi.advanceTimersByTimeAsync(1) + + expect(store.has(QUERY_CACHE_STORAGE_KEY)).toBe(false) + }) + + it('reports whether a cache is stored through the wrapped persister', () => { + const { store, storage } = createStorage() + const persister = createBusterGuardedPersister({ + persister: createBasePersister(storage), + readBuster: () => 'http://localhost|user-1', + }) + + expect(persister.hasPersistedClient()).toBe(false) + store.set(QUERY_CACHE_STORAGE_KEY, '{}') + expect(persister.hasPersistedClient()).toBe(true) + }) +}) diff --git a/frontend/src/lib/queryPersistence.ts b/frontend/src/lib/queryPersistence.ts index 82f93c4ad..6bea51ad4 100644 --- a/frontend/src/lib/queryPersistence.ts +++ b/frontend/src/lib/queryPersistence.ts @@ -1,4 +1,9 @@ -import type { DehydratedState, Query, QueryClient } from '@tanstack/react-query' +import type { + DehydratedState, + Query, + QueryCacheNotifyEvent, + QueryClient, +} from '@tanstack/react-query' import { createSyncStoragePersister } from '@tanstack/query-sync-storage-persister' import type { PersistedClient, @@ -23,6 +28,11 @@ export interface QueryCacheStorage { removeItem: (key: string) => void } +export interface QueryCachePersister extends Persister { + /** Whether a cache for any buster is currently stored, so a first save needs no restore pass. */ + hasPersistedClient: () => boolean +} + /** * Query-key prefixes whose cached data is written to disk so a previously loaded list can paint * before the network answers. @@ -50,10 +60,41 @@ export const PERSISTED_QUERY_RULES: readonly PersistedQueryRule[] = [ export const PERSISTED_CACHE_MAX_AGE_MS = 24 * 60 * 60 * 1000 +/** Minimum spacing between persisted saves, so a burst of cache changes costs one write. */ +export const PERSIST_THROTTLE_MS = 1000 + export const QUERY_CACHE_STORAGE_KEY = 'ocm.query-cache.v1' const SESSION_LIST_OPTIONS_INDEX = 3 +/** + * Cache actions that change stored data or a query status. `fetch`, `pause` and `continue` only move + * `fetchStatus`, which is not persisted, so they never justify a save. + */ +const PERSISTED_UPDATE_ACTIONS: ReadonlySet = new Set([ + 'success', + 'error', + 'failed', + 'invalidate', + 'setState', +]) + +/** + * Whether a query cache event can change the persisted cache: the query key must be allowlisted and, + * for an update, the action must change data or status. + * + * Streaming a transcript rewrites its key once per frame, so filtering here keeps the expensive + * dehydrate away from every non-allowlisted update. + */ +export function shouldPersistCacheEvent(event: QueryCacheNotifyEvent): boolean { + if (event.type === 'added' || event.type === 'removed') { + return isPersistedQueryKey(event.query.queryKey) + } + if (event.type !== 'updated') return false + if (!PERSISTED_UPDATE_ACTIONS.has(event.action.type)) return false + return isPersistedQueryKey(event.query.queryKey) +} + function matchesRule(rule: PersistedQueryRule, queryKey: readonly unknown[]): boolean { if (queryKey.length < rule.prefix.length) return false if (!rule.prefix.every((segment, index) => queryKey[index] === segment)) return false @@ -176,20 +217,78 @@ export function createDropOldestHalfRetryer(): PersistRetryer { /** * Creates the localStorage persister that stores the dehydrated cache under - * {@link QUERY_CACHE_STORAGE_KEY}, saving at most once per second. Infinite queries are capped to - * their first page before the write, and {@link createDropOldestHalfRetryer} keeps a full quota - * from silencing persistence. + * {@link QUERY_CACHE_STORAGE_KEY}. Infinite queries are capped to their first page before the write, + * and {@link createDropOldestHalfRetryer} keeps a full quota from silencing persistence. + * + * Writes are spaced by `throttleTime`; {@link createBusterGuardedPersister} throttles at the buster + * level instead, so the persister it wraps is created with a zero throttle. */ export function createQueryCachePersister( storage: QueryCacheStorage | null = typeof window === 'undefined' ? null : window.localStorage, -): Persister { - return createSyncStoragePersister({ + throttleTime: number = PERSIST_THROTTLE_MS, +): QueryCachePersister { + const persister = createSyncStoragePersister({ storage, key: QUERY_CACHE_STORAGE_KEY, - throttleTime: 1000, + throttleTime, serialize: serializePersistedClient, retry: createDropOldestHalfRetryer(), }) + + return { + ...persister, + hasPersistedClient: () => { + if (storage === null) return false + try { + return storage.getItem(QUERY_CACHE_STORAGE_KEY) !== null + } catch { + return false + } + }, + } +} + +export interface BusterGuardedPersisterOptions { + persister: QueryCachePersister + /** The buster that may still be written, or `null` once the account is signed out. */ + readBuster: () => string | null + throttleTime?: number +} + +/** + * Wraps a persister so a save runs only when the buster that owns it is still active at write time. + * + * The throttle lives here, after the guard, because a save scheduled before sign-out would otherwise + * land after `removeClient` and rewrite the previous account's cache. + */ +export function createBusterGuardedPersister({ + persister, + readBuster, + throttleTime = PERSIST_THROTTLE_MS, +}: BusterGuardedPersisterOptions): QueryCachePersister { + let timer: ReturnType | null = null + let pendingClient: PersistedClient | null = null + + const flush = () => { + timer = null + const client = pendingClient + pendingClient = null + if (client === null || client.buster !== readBuster()) return + void Promise.resolve(persister.persistClient(client)).catch(() => {}) + } + + return { + persistClient: (client) => { + pendingClient = client + if (timer === null) timer = setTimeout(flush, throttleTime) + }, + restoreClient: () => persister.restoreClient(), + removeClient: () => { + pendingClient = null + return persister.removeClient() + }, + hasPersistedClient: () => persister.hasPersistedClient(), + } } /** @@ -206,14 +305,3 @@ export function applyPersistedQueryDefaults(queryClient: QueryClient): void { queryClient.setQueryDefaults(rule.prefix, { gcTime: PERSISTED_CACHE_MAX_AGE_MS }) } } - -/** Removes the persisted cache from storage, e.g. on logout. */ -export function clearPersistedQueryCache( - storage: QueryCacheStorage | null = typeof window === 'undefined' ? null : window.localStorage, -): void { - try { - storage?.removeItem(QUERY_CACHE_STORAGE_KEY) - } catch { - return - } -} diff --git a/frontend/src/pages/RemoteProject.tsx b/frontend/src/pages/RemoteProject.tsx index c172d1e39..964488bbe 100644 --- a/frontend/src/pages/RemoteProject.tsx +++ b/frontend/src/pages/RemoteProject.tsx @@ -28,7 +28,7 @@ import { ToolSidePanel } from '@/components/navigation/ToolSidePanel' function RemoteProjectContent({ serverId, directory }: { serverId: string; directory?: string }) { const navigate = useNavigate() - const { servers, isLoading: serversLoading } = useOpenCodeServers() + const { servers, showsInitialLoading: serversLoading } = useOpenCodeServers() const { openTab } = useSettingsDialog() const [mcpDialogOpen, setMcpDialogOpen] = useDialogParam('mcp') const [skillsDialogOpen, setSkillsDialogOpen] = useDialogParam('skills') diff --git a/frontend/src/pages/__tests__/RemoteProject.test.tsx b/frontend/src/pages/__tests__/RemoteProject.test.tsx index 324677ca0..63701397d 100644 --- a/frontend/src/pages/__tests__/RemoteProject.test.tsx +++ b/frontend/src/pages/__tests__/RemoteProject.test.tsx @@ -98,7 +98,7 @@ describe('RemoteProject', () => { mocks.listProjects.mockResolvedValue([project]) mocks.getVcsInfo.mockResolvedValue({ branch: { current: 'main', default: 'main' } }) mocks.useCreateSession.mockReturnValue({ mutate: mocks.createSession, isPending: false }) - mocks.useOpenCodeServers.mockReturnValue({ servers: [server], isLoading: false }) + mocks.useOpenCodeServers.mockReturnValue({ servers: [server], showsInitialLoading: false }) mocks.useSettingsDialog.mockReturnValue({ openTab: mocks.openTab }) mocks.useSSE.mockReturnValue({ isConnected: true, isReconnecting: false }) }) @@ -175,7 +175,7 @@ describe('RemoteProject', () => { it('shows the disabled state without loading projects', async () => { mocks.useOpenCodeServers.mockReturnValue({ servers: [{ ...server, enabled: false }], - isLoading: false, + showsInitialLoading: false, }) renderProject() @@ -186,17 +186,26 @@ describe('RemoteProject', () => { }) it('shows the missing-server state', async () => { - mocks.useOpenCodeServers.mockReturnValue({ servers: [], isLoading: false }) + mocks.useOpenCodeServers.mockReturnValue({ servers: [], showsInitialLoading: false }) renderProject() expect(await screen.findByText('Server not found')).toBeInTheDocument() }) + it('keeps the loading state instead of reporting a missing server while the server list restores', () => { + mocks.useOpenCodeServers.mockReturnValue({ servers: [], showsInitialLoading: true }) + + renderProject() + + expect(screen.queryByText('Server not found')).not.toBeInTheDocument() + expect(mocks.listProjects).not.toHaveBeenCalled() + }) + it('opens server settings from the disabled state', async () => { mocks.useOpenCodeServers.mockReturnValue({ servers: [{ ...server, enabled: false }], - isLoading: false, + showsInitialLoading: false, }) renderProject() From a1f1212082b4ce801086df48c0dc06efa9b8fbdc Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Sat, 24 Oct 2026 10:37:51 -0400 Subject: [PATCH 4/5] fix(frontend): stop empty persist retries, survive disabled storage and show remote project loading The drop-oldest retryer returns undefined once no query is left, ending the sync persister's retry loop. A throwing localStorage getter yields no persister instead of a crash. useSessionsAcrossDirectories reports isPending under the same condition it enables the query. RemoteProject renders a loader while the server list restores instead of a blank body. --- frontend/src/hooks/useOpenCode.test.tsx | 3 +++ frontend/src/hooks/useOpenCode.ts | 5 ++-- frontend/src/lib/queryPersistence.test.ts | 23 +++++++++++++++++++ frontend/src/lib/queryPersistence.ts | 16 ++++++++++++- frontend/src/pages/RemoteProject.tsx | 10 +++++++- .../pages/__tests__/RemoteProject.test.tsx | 7 ++++-- 6 files changed, 58 insertions(+), 6 deletions(-) diff --git a/frontend/src/hooks/useOpenCode.test.tsx b/frontend/src/hooks/useOpenCode.test.tsx index f187e3991..27cb6a408 100644 --- a/frontend/src/hooks/useOpenCode.test.tsx +++ b/frontend/src/hooks/useOpenCode.test.tsx @@ -183,10 +183,13 @@ describe('useSessionsAcrossDirectories', () => { { wrapper: createWrapper(queryClient) }, ) + expect(result.current.isPending).toBe(true) + await waitFor(() => { expect(result.current.isLoading).toBe(false) }) + expect(result.current.isPending).toBe(false) expect(mocks.listSessionPage).toHaveBeenCalledWith({ limit: 25, order: 'desc', diff --git a/frontend/src/hooks/useOpenCode.ts b/frontend/src/hooks/useOpenCode.ts index e774e8669..6d53ed7fa 100644 --- a/frontend/src/hooks/useOpenCode.ts +++ b/frontend/src/hooks/useOpenCode.ts @@ -90,6 +90,7 @@ export const useSessionsAcrossDirectories = ( const persistedGcTime = normalizedSearch === undefined ? PERSISTED_CACHE_MAX_AGE_MS : undefined; const directoryKey = uniqueDirectories.join('|'); const childSessionsParam = includeChildren ? { includeChildren: true } : {}; + const isQueryEnabled = uniqueDirectories.length > 0 || allDirectories; const query = useInfiniteQuery< SessionListPage, @@ -183,7 +184,7 @@ export const useSessionsAcrossDirectories = ( } : undefined, getNextPageParam: (lastPage) => lastPage.nextParam, - enabled: uniqueDirectories.length > 0 || allDirectories, + enabled: isQueryEnabled, staleTime: 10000, gcTime: persistedGcTime, refetchOnWindowFocus: true, @@ -198,7 +199,7 @@ export const useSessionsAcrossDirectories = ( return { data, isLoading: query.isLoading, - isPending: query.isPending && uniqueDirectories.length > 0, + isPending: query.isPending && isQueryEnabled, isPlaceholderData: query.isPlaceholderData, isError: query.isError, fetchNextPage: query.fetchNextPage, diff --git a/frontend/src/lib/queryPersistence.test.ts b/frontend/src/lib/queryPersistence.test.ts index 9bed4dece..9d1e5d729 100644 --- a/frontend/src/lib/queryPersistence.test.ts +++ b/frontend/src/lib/queryPersistence.test.ts @@ -460,10 +460,12 @@ describe('createDropOldestHalfRetryer', () => { const first = retryer(props) const second = retryer({ ...props, persistedClient: first!, errorCount: 2 }) const third = retryer({ ...props, persistedClient: second!, errorCount: 3 }) + const fourth = retryer({ ...props, persistedClient: third!, errorCount: 4 }) expect(first?.clientState.queries.map((query) => query.state.dataUpdatedAt)).toEqual([3, 4]) expect(second?.clientState.queries.map((query) => query.state.dataUpdatedAt)).toEqual([4]) expect(third?.clientState.queries).toEqual([]) + expect(fourth).toBeUndefined() }) it('sorts once per save and starts over for the next save', () => { @@ -527,6 +529,27 @@ describe('createQueryCachePersister', () => { expect(persister.hasPersistedClient()).toBe(false) }) + it('falls back to no storage when reading localStorage throws', () => { + const originalDescriptor = Object.getOwnPropertyDescriptor(window, 'localStorage') + Object.defineProperty(window, 'localStorage', { + configurable: true, + get: () => { + throw new DOMException('denied', 'SecurityError') + }, + }) + + try { + const persister = createQueryCachePersister(undefined) + + expect(() => + persister.persistClient(buildPersistedClient({ mutations: [], queries: [] })), + ).not.toThrow() + expect(persister.hasPersistedClient()).toBe(false) + } finally { + if (originalDescriptor) Object.defineProperty(window, 'localStorage', originalDescriptor) + } + }) + it('still writes after dropping the oldest half when the quota rejects a save', async () => { vi.useFakeTimers() const { store, storage } = createStorage() diff --git a/frontend/src/lib/queryPersistence.ts b/frontend/src/lib/queryPersistence.ts index 6bea51ad4..ffefd43a3 100644 --- a/frontend/src/lib/queryPersistence.ts +++ b/frontend/src/lib/queryPersistence.ts @@ -187,12 +187,16 @@ export function serializePersistedClient(client: PersistedClient): string { * The sync persister re-serializes after each retry, so removing one query at a time burns a whole * write per query on a large cache. Halving the remaining set converges in a few passes, and the * age order is sorted once per save so the retries only filter it. + * + * Returning `undefined` once no query is left ends the retry loop instead of rewriting an empty + * client that a full quota still rejects. */ export function createDropOldestHalfRetryer(): PersistRetryer { let ageOrderedQueries: readonly DehydratedQuery[] = [] return ({ persistedClient }) => { const queries = persistedClient.clientState.queries + if (queries.length === 0) return undefined const isRetryOfSameSave = queries.length < ageOrderedQueries.length && queries.every((query) => ageOrderedQueries.includes(query)) if (!isRetryOfSameSave) { @@ -215,6 +219,16 @@ export function createDropOldestHalfRetryer(): PersistRetryer { } } +/** Reads `localStorage`, which throws instead of returning `null` when the browser has storage disabled. */ +function readBrowserStorage(): QueryCacheStorage | null { + if (typeof window === 'undefined') return null + try { + return window.localStorage + } catch { + return null + } +} + /** * Creates the localStorage persister that stores the dehydrated cache under * {@link QUERY_CACHE_STORAGE_KEY}. Infinite queries are capped to their first page before the write, @@ -224,7 +238,7 @@ export function createDropOldestHalfRetryer(): PersistRetryer { * level instead, so the persister it wraps is created with a zero throttle. */ export function createQueryCachePersister( - storage: QueryCacheStorage | null = typeof window === 'undefined' ? null : window.localStorage, + storage: QueryCacheStorage | null = readBrowserStorage(), throttleTime: number = PERSIST_THROTTLE_MS, ): QueryCachePersister { const persister = createSyncStoragePersister({ diff --git a/frontend/src/pages/RemoteProject.tsx b/frontend/src/pages/RemoteProject.tsx index 964488bbe..4dc7f1e25 100644 --- a/frontend/src/pages/RemoteProject.tsx +++ b/frontend/src/pages/RemoteProject.tsx @@ -120,7 +120,15 @@ function RemoteProjectContent({ serverId, directory }: { serverId: string; direc - {!serversLoading && !server ? ( + {serversLoading && directory ? ( + + + + ) : !serversLoading && !server ? ( { expect(await screen.findByText('Server not found')).toBeInTheDocument() }) - it('keeps the loading state instead of reporting a missing server while the server list restores', () => { + it('shows a loading state instead of a blank body while the server list restores', () => { mocks.useOpenCodeServers.mockReturnValue({ servers: [], showsInitialLoading: true }) - renderProject() + const { container } = renderProject() + expect(screen.getByRole('status', { name: 'Loading project' })).toBeInTheDocument() + expect(container.querySelectorAll('.animate-spin').length).toBeGreaterThan(0) expect(screen.queryByText('Server not found')).not.toBeInTheDocument() + expect(screen.queryByTestId('session-list')).not.toBeInTheDocument() expect(mocks.listProjects).not.toHaveBeenCalled() }) From 52b38053b9327ed55f865b89970c16fed3d9f615 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Sat, 24 Oct 2026 11:06:27 -0400 Subject: [PATCH 5/5] fix(frontend): show servers unavailable when the initial server fetch is paused offline A paused initial fetch kept showsInitialLoading true, so the remote project page showed a spinner until connectivity returned. The hook now exposes isPaused and excludes paused fetches from initial loading; the project page, project list and sidebar show an offline card with Retry instead of a missing-server state, while cached server data still wins. --- .../navigation/RemoteProjectNav.test.tsx | 23 +++++ .../navigation/RemoteProjectNav.tsx | 7 +- .../components/servers/RemoteProjectList.tsx | 13 ++- .../servers/ServersUnavailableCard.tsx | 22 +++++ .../__tests__/RemoteProjectList.test.tsx | 41 ++++++++- .../src/hooks/useOpenCodeServers.test.tsx | 84 +++++++++++++++++++ frontend/src/hooks/useOpenCodeServers.ts | 10 ++- frontend/src/pages/RemoteProject.tsx | 13 ++- .../pages/__tests__/RemoteProject.test.tsx | 41 ++++++++- 9 files changed, 246 insertions(+), 8 deletions(-) create mode 100644 frontend/src/components/servers/ServersUnavailableCard.tsx create mode 100644 frontend/src/hooks/useOpenCodeServers.test.tsx diff --git a/frontend/src/components/navigation/RemoteProjectNav.test.tsx b/frontend/src/components/navigation/RemoteProjectNav.test.tsx index 0567be327..d07ff2a5a 100644 --- a/frontend/src/components/navigation/RemoteProjectNav.test.tsx +++ b/frontend/src/components/navigation/RemoteProjectNav.test.tsx @@ -384,6 +384,29 @@ describe('RemoteProjectNavList', () => { expect(screen.queryByText('Server not found')).not.toBeInTheDocument() }) + it('reports an unreachable server list instead of a missing server while paused offline', () => { + mocks.useOpenCodeServers.mockReturnValue({ + servers: [], + isLoading: false, + showsInitialLoading: false, + isPaused: true, + }) + + render( + , + { wrapper: createWrapper('/') }, + ) + + expect(screen.getByText('Servers unavailable')).toBeInTheDocument() + expect(screen.queryByText('Server not found')).not.toBeInTheDocument() + expect(mocks.listProjects).not.toHaveBeenCalled() + }) + it('reports an unreachable server on a failed project load', async () => { mocks.listProjects.mockRejectedValue(new Error('offline')) mocks.useOpenCodeServerStatuses.mockReturnValue({ diff --git a/frontend/src/components/navigation/RemoteProjectNav.tsx b/frontend/src/components/navigation/RemoteProjectNav.tsx index 6bb5c068b..1cb679755 100644 --- a/frontend/src/components/navigation/RemoteProjectNav.tsx +++ b/frontend/src/components/navigation/RemoteProjectNav.tsx @@ -360,7 +360,11 @@ export function RemoteProjectNavList({ onOpenProject, onSelectSession, }: RemoteProjectNavListProps) { - const { servers, showsInitialLoading: isServersLoading } = useOpenCodeServers() + const { + servers, + showsInitialLoading: isServersLoading, + isPaused: serversPaused, + } = useOpenCodeServers() const { statusById } = useOpenCodeServerStatuses() const server = servers.find((candidate) => candidate.id === serverId) const { directory: activeDirectory } = useActiveRemoteDirectory(serverId) @@ -369,6 +373,7 @@ export function RemoteProjectNavList({ const trimmedSearch = search.trim() if (isServersLoading) return Loading projects... + if (serversPaused && servers.length === 0) return Servers unavailable if (!server) return Server not found if (!server.enabled) return Server is disabled if (showsInitialLoading) return Loading projects... diff --git a/frontend/src/components/servers/RemoteProjectList.tsx b/frontend/src/components/servers/RemoteProjectList.tsx index 89cdd9db4..b7b1023dd 100644 --- a/frontend/src/components/servers/RemoteProjectList.tsx +++ b/frontend/src/components/servers/RemoteProjectList.tsx @@ -13,6 +13,7 @@ import { ActivityStateIndicator } from '@/components/activity/ActivityStateIndic import { Button } from '@/components/ui/button' import { Card } from '@/components/ui/card' import { LinkedServerStateCard } from '@/components/servers/LinkedServerStateCard' +import { ServersUnavailableCard } from '@/components/servers/ServersUnavailableCard' import { OpenDirectoryDialog } from '@/components/servers/OpenDirectoryDialog' import { RemoteProjectRowActions } from '@/components/servers/RemoteProjectRowActions' import { RepoCardShell } from '@/components/repo/RepoCardShell' @@ -86,12 +87,18 @@ function RemoteProjectListLoading() { } function RemoteProjectListContent({ serverId }: { serverId: string }) { - const { servers, showsInitialLoading: serversLoading } = useOpenCodeServers() + const { + servers, + showsInitialLoading: serversLoading, + isPaused: serversPaused, + refetch: refetchServers, + } = useOpenCodeServers() const { openTab } = useSettingsDialog() const [openDirectoryOpen, setOpenDirectoryOpen] = useState(false) const [searchQuery, setSearchQuery] = useState('') const server = servers.find((candidate) => candidate.id === serverId) + const serversUnavailable = serversPaused && servers.length === 0 const isDisabled = server !== undefined && !server.enabled const { projects, activityById, showsInitialLoading, showsError, error, refetch } = @@ -111,7 +118,9 @@ function RemoteProjectListContent({ serverId }: { serverId: string }) { return ( - {!serversLoading && !server ? ( + {serversUnavailable ? : null} + + {!serversLoading && !serversUnavailable && !server ? ( void +} + +/** + * Shown on a linked-server screen while the server list is paused offline with no cached data, so + * an unreachable server list is not mistaken for a server that was removed. + */ +export function ServersUnavailableCard({ onRetry }: ServersUnavailableCardProps) { + return ( + + ) +} diff --git a/frontend/src/components/servers/__tests__/RemoteProjectList.test.tsx b/frontend/src/components/servers/__tests__/RemoteProjectList.test.tsx index fd709c3f3..17d82b119 100644 --- a/frontend/src/components/servers/__tests__/RemoteProjectList.test.tsx +++ b/frontend/src/components/servers/__tests__/RemoteProjectList.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' -import { act, render, screen, waitFor } from '@testing-library/react' +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { MemoryRouter, Route, Routes } from 'react-router-dom' @@ -12,6 +12,7 @@ const mocks = vi.hoisted(() => ({ useOpenCodeServers: vi.fn(), useSettingsDialog: vi.fn(), openTab: vi.fn(), + refetchServers: vi.fn(), createSession: vi.fn(), isMobile: true, })) @@ -152,6 +153,8 @@ describe('RemoteProjectList', () => { servers: [server], isLoading: false, showsInitialLoading: false, + isPaused: false, + refetch: mocks.refetchServers, }) mocks.useSettingsDialog.mockReturnValue({ openTab: mocks.openTab }) }) @@ -435,6 +438,42 @@ describe('RemoteProjectList', () => { expect(container.querySelectorAll('.animate-pulse').length).toBeGreaterThan(0) }) + it('shows an unavailable state instead of a missing server while the server list is paused offline', () => { + mocks.useOpenCodeServers.mockReturnValue({ + servers: [], + isLoading: false, + showsInitialLoading: false, + isPaused: true, + refetch: mocks.refetchServers, + }) + + const { container } = renderProjectList('srv_kitchen') + + expect(screen.getByText('Servers unavailable')).toBeInTheDocument() + expect(screen.queryByText('Server not found')).not.toBeInTheDocument() + expect(container.querySelectorAll('.animate-pulse')).toHaveLength(0) + + fireEvent.click(screen.getByRole('button', { name: 'Retry' })) + + expect(mocks.refetchServers).toHaveBeenCalled() + expect(mocks.listProjects).not.toHaveBeenCalled() + }) + + it('keeps the cached project list while a server list refetch is paused offline', async () => { + mocks.useOpenCodeServers.mockReturnValue({ + servers: [server], + isLoading: false, + showsInitialLoading: false, + isPaused: true, + refetch: mocks.refetchServers, + }) + + renderProjectList() + + expect(await screen.findByText('App')).toBeInTheDocument() + expect(screen.queryByText('Servers unavailable')).not.toBeInTheDocument() + }) + it('shows an empty state when the server has no projects', async () => { mocks.listProjects.mockResolvedValue([]) diff --git a/frontend/src/hooks/useOpenCodeServers.test.tsx b/frontend/src/hooks/useOpenCodeServers.test.tsx new file mode 100644 index 000000000..23542487f --- /dev/null +++ b/frontend/src/hooks/useOpenCodeServers.test.tsx @@ -0,0 +1,84 @@ +import { QueryClient, QueryClientProvider, onlineManager } from '@tanstack/react-query' +import { act, renderHook, waitFor } from '@testing-library/react' +import type { ReactNode } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useOpenCodeServers } from './useOpenCodeServers' + +const mocks = vi.hoisted(() => ({ listOpenCodeServers: vi.fn() })) + +vi.mock('@/api/opencodeServers', () => ({ + listOpenCodeServers: mocks.listOpenCodeServers, + listOpenCodeServerStatuses: vi.fn(), + createOpenCodeServer: vi.fn(), + deleteOpenCodeServer: vi.fn(), + testOpenCodeServer: vi.fn(), + testOpenCodeServerById: vi.fn(), + updateOpenCodeServer: vi.fn(), +})) + +const kitchen = { + id: 'srv_kitchen', + name: 'Kitchen', + baseUrl: 'https://kitchen.example.com', + username: 'opencode', + enabled: true, + hasPassword: true, + createdAt: 1, + updatedAt: 2, +} + +function createWrapper() { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + return ({ children }: { children: ReactNode }) => ( + {children} + ) +} + +describe('useOpenCodeServers', () => { + beforeEach(() => { + mocks.listOpenCodeServers.mockReset() + }) + + afterEach(() => { + onlineManager.setOnline(true) + }) + + it('reports an in-flight first load as initial loading', () => { + mocks.listOpenCodeServers.mockReturnValue(new Promise(() => {})) + + const { result } = renderHook(() => useOpenCodeServers(), { wrapper: createWrapper() }) + + expect(result.current.isPaused).toBe(false) + expect(result.current.showsInitialLoading).toBe(true) + }) + + it('reports a paused offline fetch as paused instead of initial loading', () => { + onlineManager.setOnline(false) + mocks.listOpenCodeServers.mockReturnValue(new Promise(() => {})) + + const { result } = renderHook(() => useOpenCodeServers(), { wrapper: createWrapper() }) + + expect(result.current.isPaused).toBe(true) + expect(result.current.isLoading).toBe(false) + expect(result.current.servers).toEqual([]) + expect(result.current.showsInitialLoading).toBe(false) + expect(mocks.listOpenCodeServers).not.toHaveBeenCalled() + }) + + it('keeps the cached server list and leaves it unloaded while a refetch is paused', async () => { + mocks.listOpenCodeServers.mockResolvedValueOnce([kitchen]).mockReturnValueOnce(new Promise(() => {})) + const wrapper = createWrapper() + const { result } = renderHook(() => useOpenCodeServers(), { wrapper }) + + await waitFor(() => expect(result.current.servers).toEqual([kitchen])) + + onlineManager.setOnline(false) + act(() => { + void result.current.refetch() + }) + + await waitFor(() => expect(result.current.isPaused).toBe(true)) + expect(result.current.servers).toEqual([kitchen]) + expect(result.current.showsInitialLoading).toBe(false) + }) +}) diff --git a/frontend/src/hooks/useOpenCodeServers.ts b/frontend/src/hooks/useOpenCodeServers.ts index 2a4904a6c..cf8130bfa 100644 --- a/frontend/src/hooks/useOpenCodeServers.ts +++ b/frontend/src/hooks/useOpenCodeServers.ts @@ -57,12 +57,18 @@ export function useOpenCodeServers(options: { enabled?: boolean } = {}) { }) const serversListState = cachedListStates(serversQuery.data, serversQuery.isError) + const isPaused = serversQuery.fetchStatus === 'paused' return { servers: serversQuery.data ?? [], isLoading: serversQuery.isLoading, - /** True only while no server list is cached and no error settled, so a restore is not read as a missing server. */ - showsInitialLoading: serversListState.showsInitialLoading, + /** + * True only while no server list is cached, no error settled and the fetch is not paused, so a + * restore is not read as a missing server and an offline pause is not read as a load. + */ + showsInitialLoading: serversListState.showsInitialLoading && !isPaused, + /** True while TanStack Query holds the server list fetch because the browser is offline. */ + isPaused, isSuccess: serversQuery.isSuccess, error: serversQuery.error, refetch: serversQuery.refetch, diff --git a/frontend/src/pages/RemoteProject.tsx b/frontend/src/pages/RemoteProject.tsx index 4dc7f1e25..b86e8b217 100644 --- a/frontend/src/pages/RemoteProject.tsx +++ b/frontend/src/pages/RemoteProject.tsx @@ -18,6 +18,7 @@ import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' import { SessionList } from '@/components/session/SessionList' import { LinkedServerStateCard } from '@/components/servers/LinkedServerStateCard' +import { ServersUnavailableCard } from '@/components/servers/ServersUnavailableCard' import { RemoteSchedulesButton } from '@/components/schedules/RemoteSchedulesButton' import { RemoteMultiRunButton } from '@/components/repo/RemoteMultiRunButton' import { RepoMcpDialog } from '@/components/repo/RepoMcpDialog' @@ -28,7 +29,12 @@ import { ToolSidePanel } from '@/components/navigation/ToolSidePanel' function RemoteProjectContent({ serverId, directory }: { serverId: string; directory?: string }) { const navigate = useNavigate() - const { servers, showsInitialLoading: serversLoading } = useOpenCodeServers() + const { + servers, + showsInitialLoading: serversLoading, + isPaused: serversPaused, + refetch: refetchServers, + } = useOpenCodeServers() const { openTab } = useSettingsDialog() const [mcpDialogOpen, setMcpDialogOpen] = useDialogParam('mcp') const [skillsDialogOpen, setSkillsDialogOpen] = useDialogParam('skills') @@ -39,6 +45,7 @@ function RemoteProjectContent({ serverId, directory }: { serverId: string; direc const toolPanel = useToolPanel(showRail) const server = servers.find((candidate) => candidate.id === serverId) + const serversUnavailable = serversPaused && servers.length === 0 const isDisabled = server !== undefined && !server.enabled const isReady = server?.enabled === true && Boolean(directory) const createSession = useCreateSession(directory) @@ -128,6 +135,10 @@ function RemoteProjectContent({ serverId, directory }: { serverId: string; direc > + ) : serversUnavailable ? ( + + + ) : !serversLoading && !server ? ( ({ createSession: vi.fn(), lastSessionListProps: { current: undefined as SessionListCapture | undefined }, openTab: vi.fn(), + refetchServers: vi.fn(), })) vi.mock('@/api/opencode', async (importOriginal) => { @@ -98,7 +99,12 @@ describe('RemoteProject', () => { mocks.listProjects.mockResolvedValue([project]) mocks.getVcsInfo.mockResolvedValue({ branch: { current: 'main', default: 'main' } }) mocks.useCreateSession.mockReturnValue({ mutate: mocks.createSession, isPending: false }) - mocks.useOpenCodeServers.mockReturnValue({ servers: [server], showsInitialLoading: false }) + mocks.useOpenCodeServers.mockReturnValue({ + servers: [server], + showsInitialLoading: false, + isPaused: false, + refetch: mocks.refetchServers, + }) mocks.useSettingsDialog.mockReturnValue({ openTab: mocks.openTab }) mocks.useSSE.mockReturnValue({ isConnected: true, isReconnecting: false }) }) @@ -205,6 +211,39 @@ describe('RemoteProject', () => { expect(mocks.listProjects).not.toHaveBeenCalled() }) + it('shows an unavailable state instead of a missing server while the server list is paused offline', async () => { + mocks.useOpenCodeServers.mockReturnValue({ + servers: [], + showsInitialLoading: false, + isPaused: true, + refetch: mocks.refetchServers, + }) + + renderProject() + + expect(await screen.findByText('Servers unavailable')).toBeInTheDocument() + expect(screen.queryByText('Server not found')).not.toBeInTheDocument() + expect(screen.queryByRole('status', { name: 'Loading project' })).not.toBeInTheDocument() + + fireEvent.click(screen.getByRole('button', { name: 'Retry' })) + + expect(mocks.refetchServers).toHaveBeenCalled() + }) + + it('keeps the cached project while a server list refetch is paused offline', async () => { + mocks.useOpenCodeServers.mockReturnValue({ + servers: [server], + showsInitialLoading: false, + isPaused: true, + refetch: mocks.refetchServers, + }) + + renderProject() + + expect(screen.getByTestId('session-list')).toHaveTextContent('/home/me/app') + expect(screen.queryByText('Servers unavailable')).not.toBeInTheDocument() + }) + it('opens server settings from the disabled state', async () => { mocks.useOpenCodeServers.mockReturnValue({ servers: [{ ...server, enabled: false }],
Could not load projects
- {error instanceof Error - ? error.message - : 'The server did not respond.'} + {error instanceof Error ? error.message : 'The server did not respond.'}