Fix infinite spinner when session endpoint is unreachable

.changeset/hungry-donkeys-repeat.md

@@ -0,0 +1,20 @@
+---
+"@agent-native/core": patch
+---
+
+Stop stranding users on the loading spinner when the session endpoint is
+unreadable. `useSession` retried a failed `/_agent-native/auth/session` every
+second forever while holding `isLoading` true, so a transient 5xx, network
+failure, or timeout produced a spinner that never resolved and carried no error
+anywhere. It now retries a bounded number of times with backoff and then reports
+a distinct `status: "unavailable"` alongside the existing `session`/`isLoading`
+fields.
+
+`RequireSession` keys off that status: unreadable is no longer collapsed into
+signed-out (which would bounce a signed-in user to the sign-in page over a blip)
+nor into loading (which stranded them). It renders a notice with Try again and
+Reload actions instead.
+
+The `DefaultSpinner` stall hint is also environment-aware now. It previously
+told every visitor — including on hosted deployments — to "check the terminal
+running the dev server", which is meaningless outside local development.

packages/core/src/client/app-providers.spec.tsx

@@ -74,9 +74,18 @@ function renderProviders(props: {
   });
 }
 
+// `RequireSession` branches on `useSession().status`, not just `isLoading` —
+// every mock here must supply a status or the gate can neither redirect nor
+// hold the fallback consistently with the real hook.
+const SIGNED_OUT_SESSION = {
+  session: null,
+  isLoading: false,
+  status: "unauthenticated" as const,
+};
+
 describe("AppProviders session gate", () => {
   it("uses Toolkit's theme-aware toaster by default", () => {
-    useSessionMock.mockReturnValue({ session: null, isLoading: false });
+    useSessionMock.mockReturnValue(SIGNED_OUT_SESSION);
 
     act(() => {
       root.render(
@@ -92,7 +101,7 @@ describe("AppProviders session gate", () => {
   });
 
   it("renders public paths directly without resolving or redirecting a session", () => {
-    useSessionMock.mockReturnValue({ session: null, isLoading: false });
+    useSessionMock.mockReturnValue(SIGNED_OUT_SESSION);
 
     renderProviders({ isPublicPath: true });
 
@@ -104,7 +113,7 @@ describe("AppProviders session gate", () => {
   });
 
   it("gates private paths and redirects signed-out visitors after hydration", () => {
-    useSessionMock.mockReturnValue({ session: null, isLoading: false });
+    useSessionMock.mockReturnValue(SIGNED_OUT_SESSION);
 
     renderProviders({});
 
@@ -116,7 +125,7 @@ describe("AppProviders session gate", () => {
   });
 
   it("allows token-authenticated private surfaces to bypass the session gate", () => {
-    useSessionMock.mockReturnValue({ session: null, isLoading: false });
+    useSessionMock.mockReturnValue(SIGNED_OUT_SESSION);
 
     renderProviders({ sessionBypass: true });
 

packages/core/src/client/require-session.spec.tsx

@@ -82,7 +82,11 @@ const Child = () => <div data-testid="protected">inbox</div>;
 
 describe("RequireSession", () => {
   it("shows a loading fallback while the session resolves and never redirects", () => {
-    useSessionMock.mockReturnValue({ session: null, isLoading: true });
+    useSessionMock.mockReturnValue({
+      session: null,
+      isLoading: true,
+      status: "loading",
+    });
     render(
       <RequireSession>
         <Child />
@@ -97,6 +101,7 @@ describe("RequireSession", () => {
     useSessionMock.mockReturnValue({
       session: { userId: "u1", email: "a@b.com" },
       isLoading: false,
+      status: "authenticated",
     });
     render(
       <RequireSession>
@@ -108,7 +113,11 @@ describe("RequireSession", () => {
   });
 
   it("redirects to the framework sign-in page carrying an opaque continuation", () => {
-    useSessionMock.mockReturnValue({ session: null, isLoading: false });
+    useSessionMock.mockReturnValue({
+      session: null,
+      isLoading: false,
+      status: "unauthenticated",
+    });
     render(
       <RequireSession>
         <Child />
@@ -131,7 +140,11 @@ describe("RequireSession", () => {
     // here, which is the only thing left standing between this surface and a
     // same-URL replace loop — it must never gain a fallback.
     stubLocation("/_agent-native/sign-in", "?c=abc");
-    useSessionMock.mockReturnValue({ session: null, isLoading: false });
+    useSessionMock.mockReturnValue({
+      session: null,
+      isLoading: false,
+      status: "unauthenticated",
+    });
     render(
       <RequireSession>
         <Child />
@@ -146,7 +159,11 @@ describe("RequireSession", () => {
     // old marker-only base resolver returned "" and failed to recognise it as
     // an auth entry path — a live, reproducible infinite bounce.
     vi.stubEnv("VITE_APP_BASE_PATH", "/myapp");
-    useSessionMock.mockReturnValue({ session: null, isLoading: false });
+    useSessionMock.mockReturnValue({
+      session: null,
+      isLoading: false,
+      status: "unauthenticated",
+    });
     for (const path of ["/myapp/login", "/myapp/signup"]) {
       stubLocation(path);
       render(
@@ -160,7 +177,11 @@ describe("RequireSession", () => {
   });
 
   it("does not redirect twice across re-renders", () => {
-    useSessionMock.mockReturnValue({ session: null, isLoading: false });
+    useSessionMock.mockReturnValue({
+      session: null,
+      isLoading: false,
+      status: "unauthenticated",
+    });
     render(
       <RequireSession>
         <Child />
@@ -175,7 +196,11 @@ describe("RequireSession", () => {
   });
 
   it("renders `signedOut` instead of redirecting when redirect is disabled", () => {
-    useSessionMock.mockReturnValue({ session: null, isLoading: false });
+    useSessionMock.mockReturnValue({
+      session: null,
+      isLoading: false,
+      status: "unauthenticated",
+    });
     render(
       <RequireSession redirect={false} signedOut={<div>please sign in</div>}>
         <Child />
@@ -185,8 +210,35 @@ describe("RequireSession", () => {
     expect(replaceMock).not.toHaveBeenCalled();
   });
 
+  it("shows a recoverable notice when the session is unreadable", () => {
+    // A transient 5xx must read as neither "signed out" (which bounces a
+    // signed-in user to sign-in) nor "still loading" (which strands them).
+    useSessionMock.mockReturnValue({
+      session: null,
+      // The real hook keeps isLoading true for "unavailable" so legacy
+      // isLoading-only consumers never misread it as signed-out.
+      isLoading: true,
+      status: "unavailable",
+      error: new Error("Could not read the session after 4 attempts."),
+      retry: vi.fn(),
+    });
+    render(
+      <RequireSession>
+        <Child />
+      </RequireSession>,
+    );
+    expect(replaceMock).not.toHaveBeenCalled();
+    expect(container.querySelector('[data-testid="protected"]')).toBeNull();
+    expect(container.querySelector('[aria-label="Loading"]')).toBeNull();
+    expect(container.textContent).toContain("Try again");
+  });
+
   it("bypass renders children even with no session", () => {
-    useSessionMock.mockReturnValue({ session: null, isLoading: false });
+    useSessionMock.mockReturnValue({
+      session: null,
+      isLoading: false,
+      status: "unauthenticated",
+    });
     render(
       <RequireSession bypass>
         <Child />

packages/core/src/client/require-session.tsx

@@ -118,13 +118,13 @@ function ResolvedSessionGate({
   redirect = true,
   signedOut,
 }: Omit<RequireSessionProps, "bypass">) {
-  const { session, isLoading } = useSession();
+  const { session, status, retry } = useSession();
   // Guard against firing the redirect more than once (effect re-runs, React
   // StrictMode double-invoke) — a second navigation while the first is in
   // flight is harmless but noisy.
   const redirectedRef = useRef(false);
 
-  const mustRedirect = !isLoading && !session && redirect;
+  const mustRedirect = status === "unauthenticated" && redirect;
 
   useEffect(() => {
     if (!mustRedirect) return;
@@ -144,10 +144,43 @@ function ResolvedSessionGate({
 
   // Still resolving, or redirect already in flight: show the loading fallback
   // rather than flashing app chrome the visitor can't use.
-  if (isLoading) return <>{fallback ?? <DefaultSpinner />}</>;
+  if (status === "loading") return <>{fallback ?? <DefaultSpinner />}</>;
+  // Unreadable is not signed-out. Redirecting here would bounce a signed-in
+  // user to the sign-in page over a transient 5xx, and rendering the spinner
+  // would strand them on a screen that never resolves.
+  if (status === "unavailable") {
+    return <SessionUnavailableNotice retry={retry} />;
+  }
   if (!session) {
     if (redirect) return <>{fallback ?? <DefaultSpinner />}</>;
     return <>{signedOut ?? null}</>;
   }
   return <>{children}</>;
 }
+
+function SessionUnavailableNotice({ retry }: { retry: () => void }) {
+  return (
+    <div className="flex h-screen w-full flex-col items-center justify-center gap-4 px-6 text-center">
+      <p className="max-w-md text-sm text-muted-foreground">
+        We couldn&apos;t reach the server to confirm you&apos;re signed in. This
+        is usually temporary.
+      </p>
+      <div className="flex gap-2">
+        <button
+          type="button"
+          onClick={retry}
+          className="rounded-md bg-primary px-3 py-1.5 text-sm font-medium text-primary-foreground"
+        >
+          Try again
+        </button>
+        <button
+          type="button"
+          onClick={() => window.location.reload()}
+          className="rounded-md border border-border px-3 py-1.5 text-sm font-medium"
+        >
+          Reload page
+        </button>
+      </div>
+    </div>
+  );
+}

packages/core/src/client/use-session.spec.tsx

@@ -29,6 +29,11 @@ function SessionConsumers({ labels }: { labels: string[] }) {
   return labels.map((label) => <SessionConsumer key={label} label={label} />);
 }
 
+function StatusConsumer() {
+  const { status } = useSession();
+  return <div data-testid="status">{status}</div>;
+}
+
 async function renderConsumers(labels: string[]) {
   await act(async () => {
     root.render(<SessionConsumers labels={labels} />);
@@ -144,6 +149,47 @@ describe("useSession", () => {
     expect(analyticsMocks.trackSessionStatus).toHaveBeenCalledWith(true);
   });
 
+  it("stops retrying and reports unavailable when the endpoint keeps failing", async () => {
+    vi.useFakeTimers();
+    const failingFetch = vi.fn(async () => new Response(null, { status: 503 }));
+    vi.stubGlobal("fetch", failingFetch);
+
+    await act(async () => {
+      root.render(<StatusConsumer />);
+      await Promise.resolve();
+    });
+    expect(container.textContent).toBe("loading");
+
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(30_000);
+    });
+
+    expect(container.textContent).toBe("unavailable");
+    expect(failingFetch).toHaveBeenCalledTimes(4);
+    // An unreadable endpoint must never be reported as a signed-out visitor.
+    expect(analyticsMocks.trackSessionStatus).not.toHaveBeenCalled();
+  });
+
+  it("keeps legacy isLoading consumers from misreading unavailable as signed-out", async () => {
+    // Consumers that only read `isLoading`/`session` (not `status`) must never
+    // see a false "signed out" once retries are exhausted.
+    vi.useFakeTimers();
+    const failingFetch = vi.fn(async () => new Response(null, { status: 503 }));
+    vi.stubGlobal("fetch", failingFetch);
+
+    await act(async () => {
+      root.render(<SessionConsumers labels={["first"]} />);
+      await Promise.resolve();
+    });
+
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(30_000);
+    });
+
+    expect(failingFetch).toHaveBeenCalledTimes(4);
+    expect(container.textContent).toBe("loading");
+  });
+
   it("caches a definitive unauthenticated response", async () => {
     const fetchMock = vi.fn(async () =>
       jsonResponse({ error: "Not authenticated" }),