fix(auth): scope clearAllAppData to active user; fix re-onboarding race; drop dead API call (#1816)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>

app/src/components/DefaultRedirect.tsx

@@ -23,6 +23,24 @@ const DefaultRedirect = () => {
     return <Navigate to="/" replace />;
   }
 
+  // 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 <RouteLoadingScreen />;
+  }
+
   if (DEV_FORCE_ONBOARDING || !snapshot.onboardingCompleted) {
     return <Navigate to="/onboarding" replace />;
   }

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(
+    <MemoryRouter initialEntries={[`/${initialEntry}`]}>
+      <Routes>
+        <Route path="/" element={<div>Welcome</div>} />
+        <Route path="/onboarding" element={<div>Onboarding</div>} />
+        <Route path="/home" element={<div>Home</div>} />
+        <Route path="*" element={<DefaultRedirect />} />
+      </Routes>
+    </MemoryRouter>
+  );
+}
+
+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();
+  });
+});

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) {

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<User> => {
     return await callCoreCommand<User>('openhuman.auth_get_me');
   },
-
-  /**
-   * Mark onboarding complete for the current user.
-   * POST /settings/onboarding-complete
-   */
-  onboardingComplete: async (): Promise<void> => {
-    await apiClient.post<{ success: boolean; data: unknown }>('/settings/onboarding-complete', {});
-  },
 };

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 () => {

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();