diff --git a/app/src/components/DefaultRedirect.tsx b/app/src/components/DefaultRedirect.tsx index b9727231e..b42b378cb 100644 --- a/app/src/components/DefaultRedirect.tsx +++ b/app/src/components/DefaultRedirect.tsx @@ -23,6 +23,24 @@ const DefaultRedirect = () => { return ; } + // Guard against the post-login race where the session token has arrived + // (via `core-state:session-token-updated` or `storeSessionToken`) but the + // snapshot hasn't been refreshed from the core yet. `toSignedOutSnapshot` + // clears `currentUser` to null on logout, and it stays null until the + // first post-login `refresh()` resolves with the real snapshot — including + // the correct `onboardingCompleted` value. Routing to /onboarding here + // would be wrong for any returning user whose flag is already true. + if (!snapshot.currentUser) { + // Diagnostic: this branch should resolve within one `refresh()` cycle. + // If a user reports a stuck loading screen post-login, this log lets us + // confirm the stuck state and trace it back to a failed/never-resolved + // refresh in `CoreStateProvider`. + console.debug( + '[default-redirect] waiting for currentUser — sessionToken set but snapshot not yet refreshed' + ); + return ; + } + if (DEV_FORCE_ONBOARDING || !snapshot.onboardingCompleted) { return ; } diff --git a/app/src/components/__tests__/DefaultRedirect.test.tsx b/app/src/components/__tests__/DefaultRedirect.test.tsx new file mode 100644 index 000000000..4bb7fb9da --- /dev/null +++ b/app/src/components/__tests__/DefaultRedirect.test.tsx @@ -0,0 +1,98 @@ +import { render, screen } from '@testing-library/react'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { describe, expect, it, vi } from 'vitest'; + +import DefaultRedirect from '../DefaultRedirect'; + +vi.mock('../../utils/config', () => ({ DEV_FORCE_ONBOARDING: false })); + +const mockUseCoreState = vi.fn(); +vi.mock('../../providers/CoreStateProvider', () => ({ useCoreState: () => mockUseCoreState() })); + +function renderRedirect(initialEntry = '*') { + return render( + + + Welcome} /> + Onboarding} /> + Home} /> + } /> + + + ); +} + +describe('DefaultRedirect', () => { + it('shows loading while bootstrapping', () => { + mockUseCoreState.mockReturnValue({ + isBootstrapping: true, + snapshot: { sessionToken: null, currentUser: null, onboardingCompleted: false }, + }); + + renderRedirect(); + + expect(screen.queryByText('Welcome')).not.toBeInTheDocument(); + expect(screen.queryByText('Onboarding')).not.toBeInTheDocument(); + expect(screen.queryByText('Home')).not.toBeInTheDocument(); + }); + + it('redirects to / when no session token', () => { + mockUseCoreState.mockReturnValue({ + isBootstrapping: false, + snapshot: { sessionToken: null, currentUser: null, onboardingCompleted: false }, + }); + + renderRedirect(); + + expect(screen.getByText('Welcome')).toBeInTheDocument(); + }); + + it('shows loading when session token arrived but currentUser is not yet set (post-login race)', () => { + // This is the race: token set by core-state:session-token-updated but + // refresh() hasn't resolved yet — currentUser is still null from + // toSignedOutSnapshot(), onboardingCompleted is still false. + mockUseCoreState.mockReturnValue({ + isBootstrapping: false, + snapshot: { sessionToken: 'token-abc', currentUser: null, onboardingCompleted: false }, + }); + + renderRedirect(); + + // Must NOT navigate to /onboarding — that would be the stale-snapshot bug + expect(screen.queryByText('Onboarding')).not.toBeInTheDocument(); + expect(screen.queryByText('Home')).not.toBeInTheDocument(); + expect(screen.queryByText('Welcome')).not.toBeInTheDocument(); + // Positively assert the loading screen rendered (not just "nothing visible") + expect(screen.getByText('Initializing OpenHuman...')).toBeInTheDocument(); + }); + + it('redirects to /onboarding for a genuinely new user (currentUser set, onboarding false)', () => { + mockUseCoreState.mockReturnValue({ + isBootstrapping: false, + snapshot: { + sessionToken: 'token-abc', + currentUser: { _id: 'user-1', email: 'new@test.com' }, + onboardingCompleted: false, + }, + }); + + renderRedirect(); + + expect(screen.getByText('Onboarding')).toBeInTheDocument(); + }); + + it('redirects to /home for a returning user who already completed onboarding', () => { + mockUseCoreState.mockReturnValue({ + isBootstrapping: false, + snapshot: { + sessionToken: 'token-abc', + currentUser: { _id: 'user-1', email: 'returning@test.com' }, + onboardingCompleted: true, + }, + }); + + renderRedirect(); + + expect(screen.getByText('Home')).toBeInTheDocument(); + }); +}); diff --git a/app/src/pages/onboarding/OnboardingLayout.tsx b/app/src/pages/onboarding/OnboardingLayout.tsx index 0a75b0846..0fd137c6e 100644 --- a/app/src/pages/onboarding/OnboardingLayout.tsx +++ b/app/src/pages/onboarding/OnboardingLayout.tsx @@ -12,7 +12,6 @@ import { setWalkthroughPending } from '../../components/walkthrough/AppWalkthrou // import { ONBOARDING_WELCOME_THREAD_LABEL } from '../../constants/onboardingChat'; import { useCoreState } from '../../providers/CoreStateProvider'; import { trackEvent } from '../../services/analytics'; -import { userApi } from '../../services/api/userApi'; import { getDefaultEnabledTools } from '../../utils/toolDefinitions'; import BetaBanner from './components/BetaBanner'; import { OnboardingContext, type OnboardingDraft } from './OnboardingContext'; @@ -79,12 +78,6 @@ const OnboardingLayout = () => { console.warn('[onboarding] Failed to persist onboarding tasks; continuing completion', e); } - try { - await userApi.onboardingComplete(); - } catch { - console.warn('[onboarding] Failed to notify backend of onboarding completion'); - } - try { await setOnboardingCompletedFlag(true); } catch (e) { diff --git a/app/src/services/api/userApi.ts b/app/src/services/api/userApi.ts index 5ee33b550..a589c2153 100644 --- a/app/src/services/api/userApi.ts +++ b/app/src/services/api/userApi.ts @@ -1,5 +1,4 @@ import type { User } from '../../types/api'; -import { apiClient } from '../apiClient'; import { callCoreCommand } from '../coreCommandClient'; /** @@ -13,12 +12,4 @@ export const userApi = { getMe: async (): Promise => { return await callCoreCommand('openhuman.auth_get_me'); }, - - /** - * Mark onboarding complete for the current user. - * POST /settings/onboarding-complete - */ - onboardingComplete: async (): Promise => { - await apiClient.post<{ success: boolean; data: unknown }>('/settings/onboarding-complete', {}); - }, }; diff --git a/app/src/utils/__tests__/clearAllAppData.test.ts b/app/src/utils/__tests__/clearAllAppData.test.ts index c7fb93594..e82ffc306 100644 --- a/app/src/utils/__tests__/clearAllAppData.test.ts +++ b/app/src/utils/__tests__/clearAllAppData.test.ts @@ -23,31 +23,50 @@ describe('clearAllAppData', () => { mockReset.mockReset().mockResolvedValue(undefined); mockRestart.mockReset().mockResolvedValue(undefined); mockPurgeCef.mockReset().mockResolvedValue(undefined); - window.localStorage.setItem('persisted', '1'); - window.sessionStorage.setItem('session-persisted', '1'); + window.localStorage.clear(); + window.sessionStorage.clear(); }); it('runs the full wipe sequence and restarts the app', async () => { const clearSession = vi.fn().mockResolvedValue(undefined); + // Seed active-user key + user-1-scoped data + another user's data + window.localStorage.setItem('OPENHUMAN_ACTIVE_USER_ID', 'user-1'); + window.localStorage.setItem('user-1:persist:accounts', 'a'); + window.localStorage.setItem('user-1:persist:coreMode', 'b'); + window.localStorage.setItem('user-2:persist:accounts', 'other'); + window.sessionStorage.setItem('session-persisted', '1'); + await clearAllAppData({ clearSession, userId: 'user-1' }); expect(mockPurgeCef).toHaveBeenCalledWith('user-1'); expect(clearSession).toHaveBeenCalledTimes(1); expect(mockReset).toHaveBeenCalledTimes(1); expect(mockPurge).toHaveBeenCalledTimes(1); - expect(window.localStorage.getItem('persisted')).toBeNull(); + // user-1's own keys are gone + expect(window.localStorage.getItem('OPENHUMAN_ACTIVE_USER_ID')).toBeNull(); + expect(window.localStorage.getItem('user-1:persist:accounts')).toBeNull(); + expect(window.localStorage.getItem('user-1:persist:coreMode')).toBeNull(); + // user-2's keys are untouched (#983: other accounts must not lose data) + expect(window.localStorage.getItem('user-2:persist:accounts')).toBe('other'); expect(window.sessionStorage.getItem('session-persisted')).toBeNull(); expect(mockRestart).toHaveBeenCalledTimes(1); }); - it('defaults to a null user scope when no userId is provided', async () => { + it('falls back to localStorage.clear() when no userId is provided', async () => { + window.localStorage.setItem('user-1:persist:accounts', 'a'); + window.localStorage.setItem('user-2:persist:accounts', 'b'); + window.sessionStorage.setItem('session-persisted', '1'); + await clearAllAppData(); expect(mockPurgeCef).toHaveBeenCalledWith(null); // No clearSession was provided — call sequence still completes. expect(mockReset).toHaveBeenCalledTimes(1); expect(mockRestart).toHaveBeenCalledTimes(1); + // Without a userId we have no way to scope, so everything is cleared + expect(window.localStorage.getItem('user-1:persist:accounts')).toBeNull(); + expect(window.localStorage.getItem('user-2:persist:accounts')).toBeNull(); }); it('continues if scheduleCefProfilePurge fails (best-effort)', async () => { diff --git a/app/src/utils/clearAllAppData.ts b/app/src/utils/clearAllAppData.ts index 98a87e2bb..5027a060d 100644 --- a/app/src/utils/clearAllAppData.ts +++ b/app/src/utils/clearAllAppData.ts @@ -5,6 +5,53 @@ import { scheduleCefProfilePurge, } from './tauriCommands'; +const ACTIVE_USER_KEY = 'OPENHUMAN_ACTIVE_USER_ID'; + +/** + * Selectively purge localStorage keys belonging to a single user. + * + * Removes: + * - `${userId}:persist:*` — per-user Redux-persist blobs + * - `${userId}:*` — any other user-scoped keys + * - `OPENHUMAN_ACTIVE_USER_ID` — the boot-time user seed (only when a userId + * is supplied so we don't wipe it on pre-login + * recovery where userId is null) + * + * Intentionally leaves other users' scoped keys untouched so that + * "clear my data" on account B does not silently destroy account A's + * persisted state (#983). + */ +function clearUserScopedStorage(userId: string | null): void { + try { + if (userId) { + const prefix = `${userId}:`; + const toRemove: string[] = []; + for (let i = 0; i < localStorage.length; i++) { + const key = localStorage.key(i); + if (key && key.startsWith(prefix)) { + toRemove.push(key); + } + } + for (const key of toRemove) { + localStorage.removeItem(key); + } + localStorage.removeItem(ACTIVE_USER_KEY); + } else { + // No known user (pre-login recovery) — fall back to clearing everything + // so we don't leave orphaned blobs with no way to scope the deletion. + localStorage.clear(); + } + } catch (err) { + console.warn('[clearAllAppData] storage clear failed:', err); + } finally { + try { + sessionStorage.clear(); + } catch { + // best-effort + } + } +} + export interface ClearAllAppDataOptions { // Optional core-side session clear (e.g. `auth_clear_session`). Best-effort — // skipped silently if the caller cannot/does not provide it (e.g. pre-login @@ -59,11 +106,10 @@ export const clearAllAppData = async ({ await resetOpenHumanDataAndRestartCore(); // 4. Purge redux-persist + browser storage. `persistor.purge()` wipes the - // persisted backend; localStorage/sessionStorage clear everything else - // (auth flags, theme, etc.). + // persisted backend; `clearUserScopedStorage` removes only the active + // user's localStorage keys so other accounts' data is not destroyed. await persistor.purge(); - window.localStorage.clear(); - window.sessionStorage.clear(); + clearUserScopedStorage(userId); // 5. Full app restart so CEF reboots into the fresh pre-login profile. await restartApp();