Fix cold-start drive page SWR deadlock and auth loading regression

apps/web/src/hooks/__tests__/useAuth.test.ts

@@ -25,6 +25,7 @@ type MockAuthStoreState = {
   isRefreshing: boolean;
   hasHydrated: boolean;
   authFailedPermanently: boolean;
+  _authPromise: Promise<void> | null;
   setUser: ReturnType<typeof vi.fn<(user: AuthUser | null) => void>>;
   setLoading: ReturnType<typeof vi.fn<(loading: boolean) => void>>;
   setHydrated: ReturnType<typeof vi.fn<(hydrated: boolean) => void>>;
@@ -46,6 +47,7 @@ const {
   mockAuthStore,
   mockLoadSession,
   mockGetSessionDuration,
+  mockShouldLoadSession,
   mockInitializeEventListeners,
 } = vi.hoisted(() => {
   const store: MockAuthStoreState = {
@@ -55,6 +57,7 @@ const {
     isRefreshing: false,
     hasHydrated: true,
     authFailedPermanently: false,
+    _authPromise: null,
     // Simulate actual state transitions
     setUser: vi.fn<(user: AuthUser | null) => void>((user) => {
       store.user = user;
@@ -88,6 +91,7 @@ const {
     mockAuthStore: store,
     mockLoadSession: vi.fn(),
     mockGetSessionDuration: vi.fn(() => 0),
+    mockShouldLoadSession: vi.fn(() => false),
     mockInitializeEventListeners: vi.fn(),
   };
 });
@@ -136,6 +140,7 @@ vi.mock('@/stores/useAuthStore', () => {
     authStoreHelpers: {
       loadSession: mockLoadSession,
       getSessionDuration: mockGetSessionDuration,
+      shouldLoadSession: mockShouldLoadSession,
       initializeEventListeners: mockInitializeEventListeners,
     },
   };
@@ -177,6 +182,8 @@ describe('useAuth', () => {
     mockAuthStore.isRefreshing = false;
     mockAuthStore.hasHydrated = true;
     mockAuthStore.authFailedPermanently = false;
+    mockAuthStore._authPromise = null;
+    mockShouldLoadSession.mockReturnValue(false);
 
     global.fetch = vi.fn();
   });
@@ -427,12 +434,10 @@ describe('useAuth', () => {
 
     it('given already loading, should skip redundant auth check', async () => {
       mockAuthStore.isLoading = true;
+      mockLoadSession.mockClear();
 
       const { result } = renderHook(() => useAuth());
 
-      // Clear calls from the initial mount effect (loadSession always runs on mount)
-      mockLoadSession.mockClear();
-
       await act(async () => {
         await result.current.actions.checkAuth();
       });
@@ -530,5 +535,23 @@ describe('useAuth', () => {
       // Observable: hydration state updated
       expect(mockAuthStore.setHydrated).toHaveBeenCalledWith(true);
     });
+
+    it('given no session load needed and no in-flight auth, should clear loading', () => {
+      mockShouldLoadSession.mockReturnValue(false);
+      mockAuthStore._authPromise = null;
+
+      renderHook(() => useAuth());
+
+      expect(mockAuthStore.setLoading).toHaveBeenCalledWith(false);
+    });
+
+    it('given shared auth promise in flight, should not clear loading', () => {
+      mockShouldLoadSession.mockReturnValue(false);
+      mockAuthStore._authPromise = Promise.resolve();
+
+      renderHook(() => useAuth());
+
+      expect(mockAuthStore.setLoading).not.toHaveBeenCalledWith(false);
+    });
   });
 });

apps/web/src/hooks/__tests__/usePageTree.test.ts

@@ -200,7 +200,7 @@ describe('usePageTree', () => {
       expect(updatedTree).toBe(mockTree); // Same reference, unchanged
     });
 
-    it('given no data loaded yet, should return undefined without error', () => {
+    it('given no data loaded yet, should no-op without mutating cache', () => {
       mockSWRState.data = undefined;
 
       const { result } = renderHook(() => usePageTree('drive-123'));
@@ -211,10 +211,19 @@ describe('usePageTree', () => {
         });
       }).not.toThrow();
 
-      // mutate is called, and updater returns undefined when no data
-      expect(mockMutate).toHaveBeenCalledWith(expect.any(Function), { revalidate: false });
-      const updaterFn = mockMutate.mock.calls[0][0];
-      expect(updaterFn(undefined)).toBeUndefined();
+      // No data means no optimistic mutation attempt.
+      expect(mockMutate).not.toHaveBeenCalled();
+    });
+
+    it('given tree data changes, should keep updateNode callback stable', () => {
+      mockSWRState.data = [createMockTreePage({ id: 'page-1' })];
+      const { result, rerender } = renderHook(() => usePageTree('drive-123'));
+      const firstUpdateNode = result.current.updateNode;
+
+      mockSWRState.data = [createMockTreePage({ id: 'page-2' })];
+      rerender();
+
+      expect(result.current.updateNode).toBe(firstUpdateNode);
     });
   });
 

apps/web/src/hooks/useAuth.ts

@@ -326,29 +326,46 @@ export function useAuth(): {
     }
   }, []);
 
-  // Initial auth check - always run loadSession() on cold start
-  // This ensures warmSessionCache() runs before CenterPanel mounts,
-  // preventing SWR stuck detection on Electron/Capacitor cold starts.
-  // loadSession() has internal deduplication, circuit breaker, and permanent failure guards.
+  // Initial auth check - simplified with store-level deduplication
+  // Desktop OAuth now uses secure exchange codes handled in Electron main process
   useEffect(() => {
     // Wait for hydration
     if (!hasHydrated) return;
 
-    const loadAndCleanup = async () => {
-      try {
-        await authStoreHelpers.loadSession(isOAuthSuccess); // Force reload for OAuth success
-      } finally {
-        // Clean up OAuth success parameter from URL after session loads
-        if (isOAuthSuccess && typeof window !== 'undefined') {
-          console.log('[AUTH_HOOK] Cleaning up OAuth success parameter from URL');
-          const newUrl = new URL(window.location.href);
-          newUrl.searchParams.delete('auth');
-          window.history.replaceState({}, '', newUrl.toString());
-          setIsOAuthSuccess(false); // Clear the flag to exit loading state
+    // Use store helper to determine if session load is needed
+    const shouldLoad = authStoreHelpers.shouldLoadSession() || isOAuthSuccess;
+
+    if (shouldLoad) {
+      console.log(`[AUTH_HOOK] Loading session - hasHydrated: ${hasHydrated}, isOAuthSuccess: ${isOAuthSuccess}`);
+
+      // Await loadSession to ensure isLoading is set before clearing OAuth flag
+      const loadAndCleanup = async () => {
+        try {
+          await authStoreHelpers.loadSession(isOAuthSuccess); // Force reload for OAuth success
+        } catch (error) {
+          console.error('[AUTH_HOOK] Failed to load session during initial auth check:', error);
+        } finally {
+          // Clean up OAuth success parameter from URL after session loads
+          if (isOAuthSuccess && typeof window !== 'undefined') {
+            console.log('[AUTH_HOOK] Cleaning up OAuth success parameter from URL');
+            const newUrl = new URL(window.location.href);
+            newUrl.searchParams.delete('auth');
+            window.history.replaceState({}, '', newUrl.toString());
+            setIsOAuthSuccess(false); // Clear the flag to exit loading state
+          }
         }
-      }
-    };
-    loadAndCleanup();
+      };
+      void loadAndCleanup();
+    } else {
+      // Multiple components can mount useAuth simultaneously.
+      // If another instance already started loadSession(), keep loading true
+      // until that shared auth promise settles.
+      const hasInFlightSessionLoad = !!useAuthStore.getState()._authPromise;
+      if (hasInFlightSessionLoad) return;
+
+      // Session check not needed (e.g., lastAuthCheck is recent) — unblock the UI
+      useAuthStore.getState().setLoading(false);
+    }
   }, [hasHydrated, isOAuthSuccess]);
 
   // Initialize auth event listeners once (moved to store level for deduplication)

apps/web/src/hooks/usePageTree.ts

@@ -78,6 +78,8 @@ export function usePageTree(driveId?: string, trashView?: boolean) {
     }
   );
   const { cache } = useSWRConfig();
+  const hasTreeSnapshotRef = useRef(!!data);
+  hasTreeSnapshotRef.current = !!data;
 
   // Self-healing: detect when SWR gets stuck (valid key, no data, no error, not fetching).
   // On desktop/Capacitor, async token retrieval in fetchWithAuth can cause SWR to lose track
@@ -137,6 +139,11 @@ export function usePageTree(driveId?: string, trashView?: boolean) {
   }, [swrKey, cache, mutate]);
 
   const updateNode = useCallback((nodeId: string, updates: Partial<TreePage>) => {
+    // Avoid optimistic mutation before the first tree snapshot exists.
+    // A no-op mutate on undefined data can stamp SWR mutation state and drop
+    // the in-flight initial fetch result, leaving the tree in a stuck skeleton state.
+    if (!hasTreeSnapshotRef.current) return;
+
     mutate((currentData) => {
       if (!currentData) return currentData;
 

apps/web/src/stores/__tests__/useAuthStore.test.ts

@@ -511,6 +511,45 @@ describe('useAuthStore', () => {
       expect(global.fetch).toHaveBeenCalled();
     });
 
+    it('given overlapping force loads, should keep latest auth promise active', async () => {
+      const user = createMockUser();
+      let resolveFirstFetch!: (value: Response) => void;
+      let resolveSecondFetch!: (value: Response) => void;
+      const firstFetch = new Promise<Response>((resolve) => {
+        resolveFirstFetch = resolve;
+      });
+      const secondFetch = new Promise<Response>((resolve) => {
+        resolveSecondFetch = resolve;
+      });
+
+      vi.mocked(global.fetch)
+        .mockImplementationOnce(() => firstFetch)
+        .mockImplementationOnce(() => secondFetch);
+
+      const firstLoad = useAuthStore.getState().loadSession(true);
+      const secondLoad = useAuthStore.getState().loadSession(true);
+      const activePromise = useAuthStore.getState()._authPromise;
+
+      // Latest request should own dedupe slot.
+      expect(activePromise).not.toBeNull();
+
+      resolveFirstFetch({
+        ok: true,
+        json: () => Promise.resolve(user),
+      } as Response);
+      await firstLoad;
+
+      // Completing first request must not clear second in-flight promise.
+      expect(useAuthStore.getState()._authPromise).toBe(activePromise);
+
+      resolveSecondFetch({
+        ok: true,
+        json: () => Promise.resolve(user),
+      } as Response);
+      await secondLoad;
+      expect(useAuthStore.getState()._authPromise).toBeNull();
+    });
+
     it('given network error, should record failed attempt', async () => {
       vi.mocked(global.fetch).mockRejectedValue(new Error('Network error'));
       const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {});
@@ -814,6 +853,63 @@ describe('authStoreHelpers', () => {
     });
   });
 
+  describe('shouldLoadSession', () => {
+    it('given not hydrated, should return true', () => {
+      useAuthStore.setState({ hasHydrated: false });
+
+      expect(authStoreHelpers.shouldLoadSession()).toBe(true);
+    });
+
+    it('given no server initialization and recent auth check, should return true', () => {
+      useAuthStore.setState({
+        hasHydrated: true,
+        _serverSessionInitialized: false,
+        lastAuthCheck: Date.now(),
+      });
+
+      expect(authStoreHelpers.shouldLoadSession()).toBe(true);
+    });
+
+    it('given circuit breaker active, should return false', () => {
+      useAuthStore.setState({
+        hasHydrated: true,
+        failedAuthAttempts: 5, // MAX_FAILED_AUTH_ATTEMPTS (web threshold)
+        lastFailedAuthCheck: Date.now(),
+      });
+
+      expect(authStoreHelpers.shouldLoadSession()).toBe(false);
+    });
+
+    it('given already loading, should return false', () => {
+      useAuthStore.setState({
+        hasHydrated: true,
+        _authPromise: Promise.resolve(),
+      });
+
+      expect(authStoreHelpers.shouldLoadSession()).toBe(false);
+    });
+
+    it('given server initialized and stale check, should return true', () => {
+      useAuthStore.setState({
+        hasHydrated: true,
+        _serverSessionInitialized: true,
+        lastAuthCheck: Date.now() - 16 * 60 * 1000, // Stale
+      });
+
+      expect(authStoreHelpers.shouldLoadSession()).toBe(true);
+    });
+
+    it('given server initialized and recent check, should return false', () => {
+      useAuthStore.setState({
+        hasHydrated: true,
+        _serverSessionInitialized: true,
+        lastAuthCheck: Date.now(),
+      });
+
+      expect(authStoreHelpers.shouldLoadSession()).toBe(false);
+    });
+  });
+
   describe('trackActivity', () => {
     it('should call updateActivity on the store', () => {
       useAuthStore.setState({

apps/web/src/stores/useAuthStore.ts

@@ -296,11 +296,8 @@ export const useAuthStore = create<AuthState>()(
           return;
         }
 
-        // Set loading state to prevent premature redirects (critical for OAuth flow)
-        set({ isLoading: true });
-
-        // Create new auth promise
-        const authPromise = (async () => {
+        // Create and publish promise before async work starts to avoid races.
+        const authPromise = Promise.resolve().then(async () => {
           try {
             const isDesktop = typeof window !== 'undefined' && window.electron?.isDesktop;
             const headers: Record<string, string> = {};
@@ -536,13 +533,19 @@ export const useAuthStore = create<AuthState>()(
               lastFailedAuthCheck: Date.now(),
             });
           } finally {
-            // Clear loading state and promise when done
-            set({ isLoading: false, _authPromise: null });
+            // Clear loading state. Only clear _authPromise if this request is still current.
+            set((currentState) => {
+              if (currentState._authPromise !== authPromise) {
+                return { isLoading: false };
+              }
+
+              return { isLoading: false, _authPromise: null };
+            });
           }
-        })();
+        });
 
-        // Store promise for deduplication
-        set({ _authPromise: authPromise });
+        // Set loading and store promise for deduplication.
+        set({ isLoading: true, _authPromise: authPromise });
         return authPromise;
       },
     }),
@@ -628,6 +631,35 @@ export const authStoreHelpers = {
     return state.failedAuthAttempts >= maxAttempts;
   },
 
+  // Check if auth check is needed (considering server initialization)
+  shouldLoadSession: (): boolean => {
+    const state = useAuthStore.getState();
+
+    // Skip if circuit breaker is active
+    if (authStoreHelpers.shouldSkipAuthCheck()) {
+      return false;
+    }
+
+    // Skip if already loading
+    if (state._authPromise) {
+      return false;
+    }
+
+    // Always load if not hydrated yet
+    if (!state.hasHydrated) {
+      return true;
+    }
+
+    // If server session was not initialized this app boot, force one revalidation.
+    // Persisted auth state (user/isAuthenticated/lastAuthCheck) can be stale.
+    if (!state._serverSessionInitialized) {
+      return true;
+    }
+
+    // Server session initialized on this boot, so normal staleness policy applies.
+    return authStoreHelpers.needsAuthCheck();
+  },
+
   // Initialize store from server session (called during app startup)
   initializeFromServer: (initialUser: User | null): void => {
     useAuthStore.getState().initializeFromServer(initialUser);