From 9ecd7b876b1c5dba5f449afae3876f53f34279bb Mon Sep 17 00:00:00 2001
From: Steven <stevenlgtm@gmail.com>
Date: Mon, 23 Feb 2026 14:08:59 +0800
Subject: [PATCH] fix(web): fix spurious logout on page reload with expired
 access token
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two bugs caused users to be redirected to /auth too frequently:

1. Race condition in Promise.all([initInstance(), initAuth()]):
   initInstance() makes a gRPC request whose auth interceptor calls
   getAccessToken() synchronously. When the access token was expired,
   getAccessToken() eagerly deleted it from localStorage as a "cleanup"
   side-effect. By the time initAuth() ran and checked hasStoredToken(),
   localStorage was already empty, so it skipped the getCurrentUser()
   call and the token refresh cycle entirely — logging the user out even
   when the refresh-token cookie was still valid. Fix: remove the
   localStorage deletion from getAccessToken(); clearAccessToken()
   (called on confirmed auth failure and logout) handles proper cleanup.

2. React Query retry: 1 caused a second refresh+redirect attempt after
   auth failures. The auth interceptor already handles token refresh and
   request retry internally. If it still throws Unauthenticated, the
   redirect is already in flight — a React Query retry only fires another
   failed refresh and a redundant redirectOnAuthFailure() call. Fix: use
   a shouldRetry function that skips retries for Unauthenticated errors
   while keeping the existing once-retry behaviour for other errors.
---
 web/src/auth-state.ts       | 10 ++++++----
 web/src/lib/query-client.ts | 15 +++++++++++++--
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/web/src/auth-state.ts b/web/src/auth-state.ts
index f71dfc36c..50ec6326e 100644
--- a/web/src/auth-state.ts
+++ b/web/src/auth-state.ts
@@ -17,11 +17,13 @@ export const getAccessToken = (): string | null => {
         if (expiresAt > new Date()) {
           accessToken = storedToken;
           tokenExpiresAt = expiresAt;
-        } else {
-          // Token expired, clean up
-          localStorage.removeItem(TOKEN_KEY);
-          localStorage.removeItem(EXPIRES_KEY);
         }
+        // Do NOT remove expired tokens here. Callers such as InstanceContext.initialize()
+        // run concurrently with AuthContext.initialize() via Promise.all. If we eagerly
+        // delete the expired token from localStorage, hasStoredToken() (called synchronously
+        // inside AuthContext.initialize()) finds nothing and skips the refresh attempt,
+        // logging the user out even when the refresh-token cookie is still valid.
+        // clearAccessToken() handles proper cleanup after a confirmed auth failure or logout.
       }
     } catch (e) {
       // localStorage might not be available (e.g., in some privacy modes)
diff --git a/web/src/lib/query-client.ts b/web/src/lib/query-client.ts
index af7e9713c..2918e2c65 100644
--- a/web/src/lib/query-client.ts
+++ b/web/src/lib/query-client.ts
@@ -1,5 +1,16 @@
+import { Code, ConnectError } from "@connectrpc/connect";
 import { QueryClient } from "@tanstack/react-query";
 
+// Don't retry requests that failed due to authentication errors.
+// The auth interceptor in connect.ts already handles token refresh and request retry.
+// If the interceptor still throws Unauthenticated, the session is truly gone and the
+// user will be redirected to /auth. A React Query retry would only fire a second
+// failed refresh attempt and a second redirect call while navigation is already in progress.
+const shouldRetry = (failureCount: number, error: unknown): boolean => {
+  if (error instanceof ConnectError && error.code === Code.Unauthenticated) return false;
+  return failureCount < 1;
+};
+
 export const queryClient = new QueryClient({
   defaultOptions: {
     queries: {
@@ -7,12 +18,12 @@ export const queryClient = new QueryClient({
       // Individual queries can override with shorter staleTime if needed (e.g., notifications)
       staleTime: 1000 * 30, // 30 seconds (increased from 10s for better performance)
       gcTime: 1000 * 60 * 5, // 5 minutes (formerly cacheTime)
-      retry: 1,
+      retry: shouldRetry,
       refetchOnWindowFocus: true, // Refetch when user returns to tab
       refetchOnReconnect: true, // Refetch when network reconnects
     },
     mutations: {
-      retry: 1,
+      retry: shouldRetry,
     },
   },
 });