fix(api): renew the session once, and not at all for guests

docs/QA.md

@@ -297,6 +297,10 @@ The refresh queue is the most critical path: a 401 triggers a single token refre
 
 Flagging a credential endpoint `isPublic` sends every rejected attempt twice and then reports the session as expired — see `auth-api.test.ts` for the four regressions that guard against it.
 
+**Refresh is single-flight, and two tests keep it that way.** Every caller that needs a renewed session shares one in-flight `POST /auth/refresh`. The public branch used to call refresh directly and so escaped the queue the authenticated path uses: opening an article fires several public reads at once — the article, its comments, the trending rail — and with a stale token each one asked for its own. That spends a five-a-minute budget three at a time, and where the refresh token rotates the later calls present one the first has already consumed, fail, and sign the reader out.
+
+The second test covers the guest case: a public request that carried **no** token is already anonymous, so its 401 says nothing about a session. Renewing one that was never opened ends by reporting it expired, which puts the sign-in modal in front of a reader who never signed in.
+
 #### `authApi` (`src/features/auth/api/auth-api.test.ts`)
 
 7 tests. The only `*.api.ts` spec in the suite, because these thunks are the one place where the _choice_ of client flag is itself the behaviour under test.

src/core/api/client.test.ts

@@ -251,6 +251,72 @@ describe("apiClient", () => {
             expect(result).toEqual([{ id: "public-1" }]);
         });
 
+        // Opening an article fires several public reads at once — the article,
+        // its comments, the trending rail. With a stale token each one takes
+        // this branch, and each used to call refresh directly, bypassing the
+        // single-flight guard the authenticated path uses. Refresh is limited
+        // to five a minute, and a rotating refresh token means the second and
+        // third present one the first has already spent: they fail, and a
+        // failed refresh signs the reader out.
+        it("refreshes once for several concurrent public 401s", async () => {
+            localStorage.setItem("access_token", "expired-token");
+            let refreshes = 0;
+            const seen = new Set<string>();
+
+            server.use(
+                http.get(`${BASE}/articles/:slug`, ({ request }) => {
+                    const key = new URL(request.url).pathname;
+                    if (!seen.has(key)) {
+                        seen.add(key);
+                        return new HttpResponse(null, { status: 401 });
+                    }
+                    return HttpResponse.json({ data: { id: "a" } });
+                }),
+                http.post(`${BASE}/auth/refresh`, async () => {
+                    refreshes += 1;
+                    await new Promise((resolve) => setTimeout(resolve, 20));
+                    return HttpResponse.json({
+                        data: { accessToken: "fresh" },
+                    });
+                }),
+            );
+
+            await Promise.all([
+                api.get("/articles/one", { isPublic: true }),
+                api.get("/articles/two", { isPublic: true }),
+                api.get("/articles/three", { isPublic: true }),
+            ]);
+            await new Promise((resolve) => setTimeout(resolve, 60));
+
+            expect(refreshes).toBe(1);
+        });
+
+        // A reader who never signed in has no session to renew. Asking anyway
+        // spends a request and then reports the session as expired, which
+        // opens the sign-in modal at someone who never signed in.
+        it("does not try to refresh when no token was sent", async () => {
+            let refreshes = 0;
+            const onExpired = vi.fn();
+            registerSessionExpiredHandler(onExpired);
+
+            server.use(
+                http.get(
+                    `${BASE}/articles`,
+                    () => new HttpResponse(null, { status: 401 }),
+                ),
+                http.post(`${BASE}/auth/refresh`, () => {
+                    refreshes += 1;
+                    return new HttpResponse(null, { status: 401 });
+                }),
+            );
+
+            await api.get("/articles", { isPublic: true }).catch(() => {});
+            await new Promise((resolve) => setTimeout(resolve, 40));
+
+            expect(refreshes).toBe(0);
+            expect(onExpired).not.toHaveBeenCalled();
+        });
+
         it("reports a dropped connection on the retry as a NetworkError", async () => {
             localStorage.setItem("access_token", "expired-token");
             let hit = 0;

src/core/api/client.ts

@@ -36,6 +36,19 @@ interface ApiOptions extends RequestInit {
 }
 
 let isRefreshing = false;
+
+/**
+ * The in-flight refresh, shared by every caller that needs one.
+ *
+ * The queue below serialises *authenticated* retries, but the public branch
+ * used to call `attemptTokenRefresh` directly and so escaped it entirely.
+ * Opening an article fires several public reads at once — the article, its
+ * comments, the trending rail — and with a stale token each one asked for its
+ * own refresh. That spends a five-a-minute budget three at a time, and where
+ * the refresh token rotates, the later calls present one the first has
+ * already consumed: they fail, and a failed refresh signs the reader out.
+ */
+let refreshPromise: Promise<string | null> | null = null;
 let failedQueue: Array<{
     resolve: (token: string | null) => void;
     reject: (error: unknown) => void;
@@ -102,6 +115,15 @@ const attemptTokenRefresh = async (): Promise<string | null> => {
     return null;
 };
 
+const refreshOnce = (): Promise<string | null> => {
+    if (!refreshPromise) {
+        refreshPromise = attemptTokenRefresh().finally(() => {
+            refreshPromise = null;
+        });
+    }
+    return refreshPromise;
+};
+
 export const apiClient = async <T>(
     endpoint: string,
     options: ApiOptions = {},
@@ -134,10 +156,21 @@ export const apiClient = async <T>(
         headers,
     });
 
+    // A public request that carried no token was already anonymous, so its
+    // 401 is the endpoint's own answer rather than a stale session. Retrying
+    // it unchanged only repeats the same failure, and renewing a session that
+    // was never opened ends by reporting it expired — which puts the sign-in
+    // modal in front of a reader who never signed in.
+    const isStaleSession =
+        response.status === 401 &&
+        !_retry &&
+        !isAnonymous &&
+        !(isPublic && !token);
+
     // `isAnonymous` endpoints answer 401 to mean "wrong credentials", so the
     // whole recovery apparatus below is skipped and the problem document
     // falls through to the caller.
-    if (response.status === 401 && !_retry && !isAnonymous) {
+    if (isStaleSession) {
         // Public endpoints: retry without token so the request succeeds
         // as unauthenticated, then attempt a background refresh.
         if (isPublic) {
@@ -147,8 +180,10 @@ export const apiClient = async <T>(
                 headers,
             });
 
-            // Background refresh so subsequent authenticated calls work
-            attemptTokenRefresh().then((newToken) => {
+            // Background refresh so subsequent authenticated calls work.
+            // Shared with every other caller, so a page full of public reads
+            // renews the session once rather than once each.
+            refreshOnce().then((newToken) => {
                 if (!newToken) {
                     localStorage.removeItem("access_token");
                     _onSessionExpired?.();
@@ -172,7 +207,7 @@ export const apiClient = async <T>(
 
         isRefreshing = true;
 
-        const newToken = await attemptTokenRefresh();
+        const newToken = await refreshOnce();
         if (newToken) {
             processQueue(null, newToken);
             isRefreshing = false;