From 99b5e7dcfa419cf3b450e8f87ffd927a866352b9 Mon Sep 17 00:00:00 2001
From: interacsean <seanhasselback@gmail.com>
Date: Thu, 27 Aug 2026 13:27:22 +1000
Subject: [PATCH 1/4] fix(auth): recover from a failed OAuth callback instead
 of wedging
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

auth-public-client cleans the callback parameters out of the URL only when
the code exchange succeeds. Every failure path — the authorization server
returning an error, a state mismatch, a missing PKCE verifier, a failed
exchange — returned or threw with ?code= or ?error= still in the query
string. attemptAutoLogin read that URL to decide whether a callback was in
progress, so it treated the page as a live callback forever: auto-login
never fired again, the app sat on guardComponent, and reloading only
replayed the same failing callback. Editing the URL by hand was the only
way out.

Clear the parameters on every outcome, preserving unrelated query
parameters and the hash, via replaceState so the back button cannot replay
the failed callback either. Decide auto-login from the callback's status
rather than the URL, which is what the check meant all along — "is an
exchange in flight", not "has this page ever been a callback". The URL is
still consulted while the status is idle, where it means nobody has
claimed these parameters and redirecting would be unsafe.

A callback the server explicitly refuses becomes its own status, "denied",
which deliberately does not re-initiate login: bouncing back would ask the
same question, get the same answer, and loop — worse than the dead end it
replaced. The error stays on useAuth().error, the URL is still cleaned,
and a deliberate reload retries. A state mismatch or failed exchange does
resume auto-login, being the recoverable case.

Each of the four new tests was verified to fail against the unfixed code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .changeset/olive-pugs-repeat.md               |  14 +++
 .../core/src/contexts/auth-context.test.tsx   | 109 ++++++++++++++++++
 packages/core/src/contexts/auth-context.tsx   | 101 ++++++++++++++--
 3 files changed, 212 insertions(+), 12 deletions(-)
 create mode 100644 .changeset/olive-pugs-repeat.md

diff --git a/.changeset/olive-pugs-repeat.md b/.changeset/olive-pugs-repeat.md
new file mode 100644
index 000000000..528e88bc8
--- /dev/null
+++ b/.changeset/olive-pugs-repeat.md
@@ -0,0 +1,14 @@
+---
+"@tailor-platform/app-shell": patch
+---
+
+Fix an app being wedged permanently after a failed OAuth callback.
+
+`@tailor-platform/auth-public-client` cleans the callback parameters out of the URL only when the code exchange succeeds. Every failure path — the authorization server returning an `error`, a state mismatch, a missing PKCE verifier, a failed exchange — left `?code=` or `?error=` in the query string. `AuthProvider` read that URL to decide whether a callback was in progress, so it treated the page as a live callback forever: auto-login never fired again, the app sat on `guardComponent`, and reloading only replayed the same failing callback. The sole escape was editing the URL by hand.
+
+Two changes fix it:
+
+- The callback parameters are now cleared on every outcome, not just success, preserving unrelated query parameters and the hash. Because it uses `replaceState`, the back button cannot replay the failed callback either.
+- Auto-login now decides from the callback's status rather than from the URL, which is what the check meant all along — "is an exchange in flight", not "has this page ever been a callback".
+
+A callback the authorization server explicitly refuses is treated as its own outcome and deliberately does **not** re-initiate login: bouncing straight back would ask the same question, get the same answer, and loop. The error stays available on `useAuth().error` for `guardComponent` to render, the URL is still cleaned, and a deliberate reload retries. Failures that are not the server's verdict — a state mismatch or a failed exchange — do resume auto-login, since those are the recoverable ones.
diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx
index f77e69b5c..a8d1fb68e 100644
--- a/packages/core/src/contexts/auth-context.test.tsx
+++ b/packages/core/src/contexts/auth-context.test.tsx
@@ -955,6 +955,115 @@ describe("createAuthClient", () => {
     expect(mockHandleCallback).toHaveBeenCalledTimes(1);
   });
 
+  describe("failed callback recovery", () => {
+    const renderWithClient = (
+      baseOverrides: Record<string, unknown>,
+      handleCallback: ReturnType<typeof vi.fn>,
+    ) => {
+      vi.mocked(createAuthClientMock).mockReturnValue({
+        ...makeBaseClient(handleCallback),
+        ...baseOverrides,
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      } as any);
+
+      return createAuthClient({ clientId: "test", appUri: "https://test.com" });
+    };
+
+    const readyUnauthenticated = {
+      isAuthenticated: false,
+      error: null as string | null,
+      isReady: true,
+    };
+
+    it("strips callback parameters when the authorization server returns an error", async () => {
+      window.history.replaceState(
+        {},
+        "",
+        "/dashboard?error=access_denied&error_description=No&tab=orders",
+      );
+
+      // The error branch upstream resolves rather than throwing, and never
+      // cleans the URL — only the success path does.
+      renderWithClient({}, vi.fn().mockResolvedValue(undefined));
+
+      await waitFor(() => {
+        expect(window.location.search).toBe("?tab=orders");
+      });
+      // Unrelated parameters and the path survive; only the OAuth ones go.
+      expect(window.location.pathname).toBe("/dashboard");
+    });
+
+    it("strips callback parameters when the code exchange throws", async () => {
+      vi.stubGlobal("console", { ...console, error: vi.fn() });
+      window.history.replaceState({}, "", "/?code=auth-code-123&state=xyz");
+
+      renderWithClient({}, vi.fn().mockRejectedValue(new Error("Missing session data")));
+
+      await waitFor(() => {
+        expect(window.location.search).toBe("");
+      });
+    });
+
+    it("resumes auto-login after a failed code exchange", async () => {
+      // The dead end this guards: before the parameters were cleared, `?code=`
+      // stayed in the URL, attemptAutoLogin treated the page as a live callback
+      // forever, and the app sat on guardComponent with no way back — a reload
+      // just replayed the same failing callback.
+      vi.stubGlobal("console", { ...console, error: vi.fn() });
+      window.history.replaceState({}, "", "/?code=auth-code-123&state=xyz");
+
+      const mockLogin = vi.fn().mockResolvedValue(undefined);
+      const client = renderWithClient(
+        { getState: vi.fn(() => readyUnauthenticated), login: mockLogin },
+        vi.fn().mockRejectedValue(new Error("Missing session data")),
+      );
+
+      render(
+        <AuthProvider client={client} autoLogin={true}>
+          <div>Content</div>
+        </AuthProvider>,
+      );
+
+      await waitFor(() => {
+        expect(mockLogin).toHaveBeenCalledTimes(1);
+      });
+    });
+
+    it("does not re-initiate login after the authorization server denies the callback", async () => {
+      // Recovering from a denial by redirecting straight back to the same
+      // authorization server would ask the same question and get the same
+      // answer — a redirect loop, which is worse than the dead end it replaced.
+      window.history.replaceState({}, "", "/?error=access_denied");
+
+      const mockLogin = vi.fn().mockResolvedValue(undefined);
+      // Stable reference: useSyncExternalStore loops if getSnapshot returns a
+      // fresh object each call.
+      const deniedState = { ...readyUnauthenticated, error: "access_denied" };
+      const client = renderWithClient(
+        { getState: vi.fn(() => deniedState), login: mockLogin },
+        vi.fn().mockResolvedValue(undefined),
+      );
+
+      render(
+        <AuthProvider client={client} autoLogin={true}>
+          <div>Content</div>
+        </AuthProvider>,
+      );
+
+      // Let the callback settle and every queued auto-login check run.
+      await act(async () => {
+        await Promise.resolve();
+      });
+      await act(async () => {
+        await Promise.resolve();
+      });
+
+      expect(mockLogin).not.toHaveBeenCalled();
+      // The URL is still cleaned up, so a deliberate reload can retry.
+      expect(window.location.search).toBe("");
+    });
+  });
+
   it("does not call handleCallback when URL has no OAuth parameters", () => {
     // URL is already "/" from afterEach reset
 
diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx
index 2cb86b7dc..810217476 100644
--- a/packages/core/src/contexts/auth-context.tsx
+++ b/packages/core/src/contexts/auth-context.tsx
@@ -30,8 +30,14 @@ export interface AuthClientConfig {
 
 /**
  * Internal type for tracking OAuth callback handling status.
+ *
+ * "denied" is distinct from "rejected": the authorization server itself
+ * returned an `error` in the callback (the user declined consent, the client
+ * is not permitted, and so on). Re-initiating login from that state would
+ * bounce straight back to the same refusal, so auto-login stays out of it,
+ * whereas "rejected" (a local exchange failure) is worth another attempt.
  */
-type CallbackStatus = "idle" | "pending" | "resolved" | "rejected";
+type CallbackStatus = "idle" | "pending" | "resolved" | "rejected" | "denied";
 
 /**
  * Enhanced auth client with additional helper methods
@@ -94,6 +100,9 @@ const createCallbackStatusManager = () => {
         reject: () => {
           updateStatus("rejected");
         },
+        deny: () => {
+          updateStatus("denied");
+        },
       };
     },
     subscribe: (listener: () => void) => {
@@ -160,13 +169,28 @@ export function createAuthClient(config: AuthClientConfig): EnhancedAuthClient {
     const currentUrl = new URL(window.location.href);
 
     if (isOAuthCallbackUrl(currentUrl)) {
-      const { resolve, reject } = callbackManager.start();
+      // Read this before the callback settles: the parameters are stripped below.
+      const authServerReportedError = currentUrl.searchParams.has("error");
+      const { resolve, reject, deny } = callbackManager.start();
+
+      // auth-public-client only cleans the URL when the code exchange succeeds.
+      // Every failure path returns (or throws) with `code` / `error` still in the
+      // query string, which would otherwise leave the app wedged: the parameters
+      // make this look like a callback URL forever, and a reload just replays the
+      // same failing callback. Strip them on every outcome, before settling, so
+      // whatever observes the settled status already sees a clean URL.
+      const settle = (finish: () => void) => {
+        clearOAuthCallbackParams();
+        finish();
+      };
 
       baseClient
         .handleCallback()
-        .then(resolve)
+        .then(() => {
+          settle(authServerReportedError ? deny : resolve);
+        })
         .catch((error) => {
-          reject();
+          settle(reject);
           console.error("Failed to handle OAuth callback:", error);
         });
     }
@@ -261,6 +285,19 @@ const AuthContext = createContext<AuthContextType | null>(null);
 const isOAuthCallbackUrl = (url: URL) =>
   url.searchParams.has("code") || url.searchParams.has("error");
 
+/**
+ * Query parameters an authorization server may add when redirecting back.
+ * Removed together so a settled callback cannot be mistaken for a pending one.
+ */
+const OAUTH_CALLBACK_PARAMS = [
+  "code",
+  "state",
+  "error",
+  "error_description",
+  "error_uri",
+  "iss",
+] as const;
+
 const isCurrentOAuthCallbackUrl = () => {
   if (typeof window === "undefined") {
     return false;
@@ -269,6 +306,28 @@ const isCurrentOAuthCallbackUrl = () => {
   return isOAuthCallbackUrl(new URL(window.location.href));
 };
 
+/**
+ * Remove the OAuth callback parameters from the current URL, preserving any
+ * unrelated query parameters and the hash. Uses `replaceState` so the failed
+ * callback does not stay in session history for the back button to replay.
+ */
+const clearOAuthCallbackParams = () => {
+  if (typeof window === "undefined") {
+    return;
+  }
+
+  const url = new URL(window.location.href);
+  if (!isOAuthCallbackUrl(url)) {
+    return;
+  }
+
+  for (const param of OAUTH_CALLBACK_PARAMS) {
+    url.searchParams.delete(param);
+  }
+
+  window.history.replaceState({}, document.title, `${url.pathname}${url.search}${url.hash}`);
+};
+
 /**
  * Guard component that shows a fallback UI while auth is not ready or
  * not authenticated. Defined here so that the router layer does not
@@ -321,7 +380,11 @@ type AuthProviderProps = {
  * - initial deferred auto-login attempt
  * - duplicate login prevention
  */
-const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean }) => {
+const useAutoLogin = (props: {
+  client: EnhancedAuthClient;
+  enabled?: boolean;
+  callbackStatus: CallbackStatus;
+}) => {
   // Prevent duplicate login redirects when multiple auth_state_changed
   // events fire before the first login attempt settles.
   const loginInFlightRef = useRef<Promise<void> | null>(null);
@@ -331,7 +394,18 @@ const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean })
     const authState = props.client.getState();
     if (
       !props.enabled ||
-      isCurrentOAuthCallbackUrl() ||
+      // Hold off while a code exchange is in flight, and stand down entirely
+      // once the authorization server has refused: redirecting back would ask
+      // the same question and get the same answer.
+      props.callbackStatus === "pending" ||
+      props.callbackStatus === "denied" ||
+      // Only consult the URL while no callback has been claimed. This client
+      // starts the exchange itself whenever it is constructed on a callback
+      // URL, so "idle" here means nobody is handling these parameters and
+      // redirecting is unsafe. Once a callback has run, its status is the
+      // authority — reading the URL instead is what used to strand the app,
+      // because a failed callback left its parameters in place forever.
+      (props.callbackStatus === "idle" && isCurrentOAuthCallbackUrl()) ||
       !authState.isReady ||
       authState.isAuthenticated ||
       loginInFlightRef.current
@@ -348,7 +422,7 @@ const useAutoLogin = (props: { client: EnhancedAuthClient; enabled?: boolean })
       .finally(() => {
         loginInFlightRef.current = null;
       });
-  }, [props.client, props.enabled]);
+  }, [props.client, props.enabled, props.callbackStatus]);
 
   return {
     subscribeAuthState: useCallback(
@@ -459,21 +533,24 @@ const useCallbackStatus = (client: EnhancedAuthClient): CallbackStatus => {
 export const AuthProvider = (props: React.PropsWithChildren<AuthProviderProps>) => {
   const client = props.client;
 
+  // Read callback status first so it can be passed to both useAutoLogin and
+  // useEnsureAuthInitialized. This lets each retry automatically when a pending
+  // callback settles — the new function references trigger their effects again,
+  // which is what lets auto-login resume after a failed callback instead of
+  // being stranded by leftover parameters in the URL.
+  const callbackStatus = useCallbackStatus(client);
+
   // Set up auth state subscription for auto-login orchestration
   const { subscribeAuthState } = useAutoLogin({
     client,
     enabled: props.autoLogin,
+    callbackStatus,
   });
 
   // Use useSyncExternalStore for state management from auth client.
   const getSnapshot = useCallback(() => client.getState(), [client]);
   const authState = useSyncExternalStore(subscribeAuthState, getSnapshot);
 
-  // Read callback status first so it can be passed to useEnsureAuthInitialized.
-  // This lets initialization retry automatically when a pending callback settles
-  // to rejected — the new ensureInitialized reference triggers the useEffect again.
-  const callbackStatus = useCallbackStatus(client);
-
   // Prepare a shared initialization function so AuthProvider can start the
   // first auth check itself without depending on router navigation.
   const ensureAuthInitialized = useEnsureAuthInitialized(client, callbackStatus);

From 9e409c0a3daff4ba12ccedf9d07ede2db0d96e7d Mon Sep 17 00:00:00 2001
From: interacsean <seanhasselback@gmail.com>
Date: Thu, 27 Aug 2026 13:48:56 +1000
Subject: [PATCH 2/4] fix(auth): bound callback retries and stop clobbering
 router history
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adversarial review of this PR found the first cut traded the wedge for a
redirect loop on exactly the paths it called recoverable.

Bound the retries. A failed callback resumes auto-login, but every attempt
is a full-page redirect to the authorization server, so a deterministic
failure — evicted or blocked browser storage, a misconfigured client, two
tabs overwriting each other's single-slot PKCE verifier — looped forever.
Recoverable failures now get one retry per tab, counted in sessionStorage
because each attempt is a fresh page load, after which the callback is
terminal. Unreadable storage counts as exhausted: without somewhere to
count, the ceiling cannot be enforced, and looping is the worse failure.

Classify the outcome from the resulting auth state rather than from
whether the callback threw. Two failure paths return normally after
recording the error — the server's `error` branch and a state mismatch —
so resolve-vs-throw let both skip the ceiling. It also fixes a denial that
arrives as a throw (the error branch clears OAuth temp data first, which
opens IndexedDB and can reject) being misread as retryable, which put it
straight into the loop the denied state exists to prevent.

Preserve history.state when stripping the parameters. Passing a fresh {}
discards what routers keep there — react-router's {usr, key, idx} and
Next.js's __NA — desyncing both for the rest of the session.

Document the terminal case: the guard is where a failed sign-in becomes
visible, and the guard in our own docs only rendered a spinner.

Tests for the retry ceiling, the throwing denial, history preservation,
and the auto-login gate's idle narrowing, which had no coverage. Each was
verified to fail against the code it guards.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .changeset/olive-pugs-repeat.md               |   6 +-
 docs/concepts/authentication.md               |  25 ++++
 .../core/src/contexts/auth-context.test.tsx   | 135 ++++++++++++++++++
 packages/core/src/contexts/auth-context.tsx   | 103 +++++++++++--
 4 files changed, 256 insertions(+), 13 deletions(-)

diff --git a/.changeset/olive-pugs-repeat.md b/.changeset/olive-pugs-repeat.md
index 528e88bc8..5dc9e2592 100644
--- a/.changeset/olive-pugs-repeat.md
+++ b/.changeset/olive-pugs-repeat.md
@@ -11,4 +11,8 @@ Two changes fix it:
 - The callback parameters are now cleared on every outcome, not just success, preserving unrelated query parameters and the hash. Because it uses `replaceState`, the back button cannot replay the failed callback either.
 - Auto-login now decides from the callback's status rather than from the URL, which is what the check meant all along — "is an exchange in flight", not "has this page ever been a callback".
 
-A callback the authorization server explicitly refuses is treated as its own outcome and deliberately does **not** re-initiate login: bouncing straight back would ask the same question, get the same answer, and loop. The error stays available on `useAuth().error` for `guardComponent` to render, the URL is still cleaned, and a deliberate reload retries. Failures that are not the server's verdict — a state mismatch or a failed exchange — do resume auto-login, since those are the recoverable ones.
+Auto-login will not loop on this. A recoverable failure is retried once per tab; beyond that, and for any refusal the authorization server issues explicitly, AppShell stops rather than sending the user back for the same answer. The outcome is classified from the resulting auth state rather than from whether the callback threw, because two failure paths — an `error` from the server and a state mismatch — return normally after recording the error.
+
+That makes `guardComponent` where a failed sign-in becomes visible: a guard that only renders a spinner will spin indefinitely, so render `useAuth().error` and offer a retry. See the authentication guide.
+
+Stripping preserves the existing `history.state`, which routers rely on (react-router keeps `{usr, key, idx}` there, Next.js keeps `__NA`). One limitation is unchanged from before: a callback URL is detected by the presence of `code` or `error`, which are generic parameter names, so an app already using them for its own purposes on a route can have them removed.
diff --git a/docs/concepts/authentication.md b/docs/concepts/authentication.md
index c1ce3a711..77420bf55 100644
--- a/docs/concepts/authentication.md
+++ b/docs/concepts/authentication.md
@@ -217,6 +217,31 @@ function ChatScreen() {
 | `autoLogin`      | `boolean`               | No       | Automatically redirect unauthenticated users to login |
 | `guardComponent` | `() => React.ReactNode` | No       | Rendered while loading or when not authenticated      |
 
+## When the callback fails
+
+A sign-in that comes back from the authorization server unsuccessfully — the user declined, the client is not permitted, the exchange failed — leaves `isAuthenticated` false with the reason on `useAuth().error`. The callback parameters are cleared from the URL either way, so a reload retries cleanly instead of replaying the failed callback.
+
+With `autoLogin`, a failure that looks recoverable is retried once automatically. Beyond that, and for any refusal the authorization server issues explicitly, AppShell stops: sending the user straight back would ask the same question, get the same answer, and loop.
+
+That makes the guard the place where a failed sign-in becomes visible. A guard that only ever renders a spinner will spin indefinitely in this case, so render the error and offer a way out:
+
+```tsx
+const AuthGate = () => {
+  const { isReady, error, login } = useAuth();
+
+  if (error) {
+    return (
+      <div>
+        <p>Sign-in failed: {error}</p>
+        <button onClick={() => login()}>Try again</button>
+      </div>
+    );
+  }
+
+  return isReady ? <LoginPrompt /> : <LoadingScreen />;
+};
+```
+
 ## Integration with AppShell
 
 The authentication provider works seamlessly with AppShell's data layer, automatically handling:
diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx
index a8d1fb68e..2cc5f530b 100644
--- a/packages/core/src/contexts/auth-context.test.tsx
+++ b/packages/core/src/contexts/auth-context.test.tsx
@@ -23,6 +23,9 @@ afterEach(() => {
   vi.clearAllMocks();
   vi.unstubAllGlobals();
   window.history.replaceState({}, "", "/");
+  // The failed-callback retry counter is tab-scoped and survives page loads by
+  // design, so it has to be reset between tests or it leaks across them.
+  window.sessionStorage.clear();
 });
 
 const LoadingGuard = () => <div>Loading...</div>;
@@ -969,6 +972,21 @@ describe("createAuthClient", () => {
       return createAuthClient({ clientId: "test", appUri: "https://test.com" });
     };
 
+    // A client whose callback has already settled, so the gate must consult the
+    // status rather than the (still dirty) URL.
+    const createSettledCallbackClient = (
+      status: "resolved" | "rejected",
+      overrides: Record<string, unknown>,
+    ) =>
+      ({
+        ...makeBaseClient(vi.fn()),
+        getAppUri: vi.fn(() => "https://api.test.com"),
+        getCallbackStatusSnapshot: () => status,
+        subscribeCallbackStatus: () => () => {},
+        ...overrides,
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      }) as any;
+
     const readyUnauthenticated = {
       isAuthenticated: false,
       error: null as string | null,
@@ -1029,6 +1047,123 @@ describe("createAuthClient", () => {
       });
     });
 
+    it("stops re-initiating login once the callback retry budget is spent", async () => {
+      // Every retry is a full-page redirect to the authorization server, so a
+      // deterministic failure (evicted storage, misconfigured client) would loop
+      // forever without a ceiling.
+      vi.stubGlobal("console", { ...console, error: vi.fn() });
+      window.sessionStorage.setItem("tailor-app-shell:oauth-callback-failures", "1");
+      window.history.replaceState({}, "", "/?code=auth-code-123&state=xyz");
+
+      const mockLogin = vi.fn().mockResolvedValue(undefined);
+      const client = renderWithClient(
+        { getState: vi.fn(() => readyUnauthenticated), login: mockLogin },
+        vi.fn().mockRejectedValue(new Error("Missing session data")),
+      );
+
+      render(
+        <AuthProvider client={client} autoLogin={true}>
+          <div>Content</div>
+        </AuthProvider>,
+      );
+
+      await act(async () => {
+        await Promise.resolve();
+      });
+      await act(async () => {
+        await Promise.resolve();
+      });
+
+      expect(mockLogin).not.toHaveBeenCalled();
+    });
+
+    it("treats a denial that throws as denied rather than retryable", async () => {
+      // The server's `error` branch clears OAuth temp data first, which opens
+      // IndexedDB and can reject. The denial must not be reclassified as a
+      // retryable failure just because it arrived as a throw.
+      vi.stubGlobal("console", { ...console, error: vi.fn() });
+      window.history.replaceState({}, "", "/?error=access_denied");
+
+      const mockLogin = vi.fn().mockResolvedValue(undefined);
+      const client = renderWithClient(
+        { getState: vi.fn(() => readyUnauthenticated), login: mockLogin },
+        vi.fn().mockRejectedValue(new Error("storage unavailable")),
+      );
+
+      render(
+        <AuthProvider client={client} autoLogin={true}>
+          <div>Content</div>
+        </AuthProvider>,
+      );
+
+      await act(async () => {
+        await Promise.resolve();
+      });
+      await act(async () => {
+        await Promise.resolve();
+      });
+
+      expect(mockLogin).not.toHaveBeenCalled();
+      // Nothing was spent from the retry budget: this outcome is terminal.
+      expect(window.sessionStorage.getItem("tailor-app-shell:oauth-callback-failures")).toBeNull();
+    });
+
+    it("preserves the router's history state while stripping parameters", async () => {
+      // react-router keeps {usr, key, idx} here and Next.js keeps __NA; replacing
+      // it with a fresh object desyncs both for the rest of the session.
+      const routerState = { idx: 3, key: "abc", usr: null };
+      window.history.replaceState(routerState, "", "/dashboard?code=auth-code-123");
+
+      renderWithClient({}, vi.fn().mockResolvedValue(undefined));
+
+      await waitFor(() => {
+        expect(window.location.search).toBe("");
+      });
+      expect(window.history.state).toEqual(routerState);
+    });
+
+    it("clears the retry budget once a callback succeeds", async () => {
+      window.sessionStorage.setItem("tailor-app-shell:oauth-callback-failures", "1");
+      window.history.replaceState({}, "", "/?code=auth-code-123&state=xyz");
+
+      const authenticated = { isAuthenticated: true, error: null, isReady: true };
+      renderWithClient(
+        { getState: vi.fn(() => authenticated) },
+        vi.fn().mockResolvedValue(undefined),
+      );
+
+      await waitFor(() => {
+        expect(
+          window.sessionStorage.getItem("tailor-app-shell:oauth-callback-failures"),
+        ).toBeNull();
+      });
+    });
+
+    it("lets a settled callback drive auto-login even if parameters remain in the URL", async () => {
+      // Isolates the narrowing in the auto-login gate: the URL is only an
+      // authority while the status is "idle". Once a callback has run, its
+      // status decides — reading the URL there is what stranded the app, since
+      // a failed callback used to leave its parameters in place permanently.
+      window.history.replaceState({}, "", "/?code=auth-code-123&state=xyz");
+
+      const mockLogin = vi.fn().mockResolvedValue(undefined);
+      const settledState = { isAuthenticated: false, error: "boom", isReady: true };
+      const mockClient = createSettledCallbackClient("rejected", {
+        login: mockLogin,
+        getState: vi.fn(() => settledState),
+      });
+
+      render(
+        <AuthProvider client={mockClient} autoLogin={true}>
+          <div>Content</div>
+        </AuthProvider>,
+      );
+
+      await waitFor(() => {
+        expect(mockLogin).toHaveBeenCalledTimes(1);
+      });
+    });
+
     it("does not re-initiate login after the authorization server denies the callback", async () => {
       // Recovering from a denial by redirecting straight back to the same
       // authorization server would ask the same question and get the same
diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx
index 810217476..2c9318741 100644
--- a/packages/core/src/contexts/auth-context.tsx
+++ b/packages/core/src/contexts/auth-context.tsx
@@ -31,11 +31,11 @@ export interface AuthClientConfig {
 /**
  * Internal type for tracking OAuth callback handling status.
  *
- * "denied" is distinct from "rejected": the authorization server itself
- * returned an `error` in the callback (the user declined consent, the client
- * is not permitted, and so on). Re-initiating login from that state would
- * bounce straight back to the same refusal, so auto-login stays out of it,
- * whereas "rejected" (a local exchange failure) is worth another attempt.
+ * "denied" is distinct from "rejected": it means another automatic login round
+ * trip will not help, so auto-login stays out of it. Two things land there —
+ * the authorization server refusing outright (the user declined consent, the
+ * client is not permitted), and a callback that keeps failing after its retry
+ * budget is spent. "rejected" is a failure that is still worth one more attempt.
  */
 type CallbackStatus = "idle" | "pending" | "resolved" | "rejected" | "denied";
 
@@ -118,6 +118,56 @@ const createCallbackStatusManager = () => {
   };
 };
 
+/**
+ * How many times auto-login may re-run a callback that failed, per browser tab.
+ *
+ * A failed callback is worth one more attempt: the common causes (a PKCE
+ * verifier clobbered by a concurrent teardown, two tabs racing the same
+ * single-slot OAuth storage) clear on a second pass. A failure that survives
+ * the retry is treated as permanent, because every attempt is a full-page
+ * redirect to the authorization server — without a ceiling, a deterministic
+ * failure (blocked or evicted browser storage, a misconfigured client) becomes
+ * an unbounded redirect loop, which is worse than the dead end it replaced.
+ */
+const MAX_AUTOMATIC_CALLBACK_RETRIES = 1;
+
+const CALLBACK_FAILURE_COUNT_KEY = "tailor-app-shell:oauth-callback-failures";
+
+/**
+ * Read the consecutive-failure count for this tab.
+ *
+ * sessionStorage rather than memory, because each retry is a full page load and
+ * an in-memory counter resets with it. Reports failures as exhausted when
+ * storage is unreadable: without somewhere to count, the ceiling cannot be
+ * enforced, and looping is the worse failure. Such a browser cannot complete
+ * the flow anyway — the auth client needs IndexedDB.
+ */
+const readCallbackFailureCount = (): number | null => {
+  try {
+    const raw = window.sessionStorage.getItem(CALLBACK_FAILURE_COUNT_KEY);
+    const count = raw == null ? 0 : Number.parseInt(raw, 10);
+    return Number.isNaN(count) ? null : count;
+  } catch {
+    return null;
+  }
+};
+
+const recordCallbackFailure = (count: number) => {
+  try {
+    window.sessionStorage.setItem(CALLBACK_FAILURE_COUNT_KEY, String(count + 1));
+  } catch {
+    // Nothing to do: the read side already treats unreadable storage as exhausted.
+  }
+};
+
+const clearCallbackFailures = () => {
+  try {
+    window.sessionStorage.removeItem(CALLBACK_FAILURE_COUNT_KEY);
+  } catch {
+    // Best effort — a stale count only costs a retry, and it is tab-scoped.
+  }
+};
+
 /**
  * Create an enhanced authentication client.
  *
@@ -179,18 +229,40 @@ export function createAuthClient(config: AuthClientConfig): EnhancedAuthClient {
       // make this look like a callback URL forever, and a reload just replays the
       // same failing callback. Strip them on every outcome, before settling, so
       // whatever observes the settled status already sees a clean URL.
-      const settle = (finish: () => void) => {
+      //
+      // The outcome is classified from the resulting auth state rather than from
+      // whether the promise resolved. Not every failure throws — an `error` from
+      // the authorization server and a state mismatch both return normally after
+      // recording the error — so resolve-vs-throw is the wrong signal, and using
+      // it would let those two skip the retry ceiling and loop.
+      const settle = () => {
         clearOAuthCallbackParams();
-        finish();
+
+        if (baseClient.getState().isAuthenticated) {
+          clearCallbackFailures();
+          resolve();
+          return;
+        }
+
+        const failureCount = readCallbackFailureCount();
+        if (
+          authServerReportedError ||
+          failureCount === null ||
+          failureCount >= MAX_AUTOMATIC_CALLBACK_RETRIES
+        ) {
+          deny();
+          return;
+        }
+
+        recordCallbackFailure(failureCount);
+        reject();
       };
 
       baseClient
         .handleCallback()
-        .then(() => {
-          settle(authServerReportedError ? deny : resolve);
-        })
+        .then(settle)
         .catch((error) => {
-          settle(reject);
+          settle();
           console.error("Failed to handle OAuth callback:", error);
         });
     }
@@ -325,7 +397,14 @@ const clearOAuthCallbackParams = () => {
     url.searchParams.delete(param);
   }
 
-  window.history.replaceState({}, document.title, `${url.pathname}${url.search}${url.hash}`);
+  // Preserve the existing history state: routers keep their own bookkeeping on
+  // it (react-router stores {usr, key, idx}, Next.js stores __NA), and replacing
+  // it with a fresh object desyncs them for the rest of the session.
+  window.history.replaceState(
+    window.history.state,
+    document.title,
+    `${url.pathname}${url.search}${url.hash}`,
+  );
 };
 
 /**

From 5f5df529058e0b7ec74d03f7b34fe3a510f62866 Mon Sep 17 00:00:00 2001
From: interacsean <seanhasselback@gmail.com>
Date: Fri, 28 Aug 2026 09:55:47 +1000
Subject: [PATCH 3/4] fix(auth): drop the URL-cleanup workaround, keep the gate
 fix
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Scope decision: app-shell should not reimplement upstream's callback URL
cleanup. auth-public-client owns it and performs it only on success — a
long-standing defect present since 0.3.0, now filed as
tailor-platform/auth-public-client#139 together with the replaceState
history.state clobbering.

What remains here is app-shell's own logic error and its consequences:
the auto-login gate read the URL to mean "callback in progress", which a
failed callback made permanently true. The gate now reads the callback's
settled status (the URL stays authoritative only while no callback has
been claimed), an explicit server refusal is terminal, and recoverable
failures get one retry per tab.

Removes clearOAuthCallbackParams and the history.state preservation —
both belong upstream — along with their three tests. The stale parameters
now remain in the URL after a failure until #139 is fixed; app-shell no
longer misbehaves because of them.

With the strip gone the gate is load-bearing, and the mutation results
show it: reverting the gate to the URL check now fails two tests, where
previously the strip masked it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .changeset/olive-pugs-repeat.md               | 13 ++--
 docs/concepts/authentication.md               |  2 +-
 .../core/src/contexts/auth-context.test.tsx   | 48 +-------------
 packages/core/src/contexts/auth-context.tsx   | 63 +++----------------
 4 files changed, 19 insertions(+), 107 deletions(-)

diff --git a/.changeset/olive-pugs-repeat.md b/.changeset/olive-pugs-repeat.md
index 5dc9e2592..a6549258a 100644
--- a/.changeset/olive-pugs-repeat.md
+++ b/.changeset/olive-pugs-repeat.md
@@ -2,17 +2,14 @@
 "@tailor-platform/app-shell": patch
 ---
 
-Fix an app being wedged permanently after a failed OAuth callback.
+Fix `autoLogin` being permanently disabled after a failed OAuth callback.
 
-`@tailor-platform/auth-public-client` cleans the callback parameters out of the URL only when the code exchange succeeds. Every failure path — the authorization server returning an `error`, a state mismatch, a missing PKCE verifier, a failed exchange — left `?code=` or `?error=` in the query string. `AuthProvider` read that URL to decide whether a callback was in progress, so it treated the page as a live callback forever: auto-login never fired again, the app sat on `guardComponent`, and reloading only replayed the same failing callback. The sole escape was editing the URL by hand.
+`@tailor-platform/auth-public-client` cleans the callback parameters out of the URL only when the code exchange succeeds; every failure path leaves `?code=` or `?error=` in the query string (tailor-platform/auth-public-client#139). `AuthProvider` decided whether a callback was in progress by reading that URL, so after any failed callback it treated the page as a live callback forever: auto-login never fired again, the app sat on `guardComponent`, and reloading only replayed the same failing callback.
 
-Two changes fix it:
+Auto-login now decides from the callback's status rather than the URL — which is what the check meant all along: "is an exchange in flight", not "has this page ever been a callback". The URL is still the authority when no callback has been claimed at all, where redirecting away from unconsumed parameters would be unsafe.
 
-- The callback parameters are now cleared on every outcome, not just success, preserving unrelated query parameters and the hash. Because it uses `replaceState`, the back button cannot replay the failed callback either.
-- Auto-login now decides from the callback's status rather than from the URL, which is what the check meant all along — "is an exchange in flight", not "has this page ever been a callback".
-
-Auto-login will not loop on this. A recoverable failure is retried once per tab; beyond that, and for any refusal the authorization server issues explicitly, AppShell stops rather than sending the user back for the same answer. The outcome is classified from the resulting auth state rather than from whether the callback threw, because two failure paths — an `error` from the server and a state mismatch — return normally after recording the error.
+It will not loop, either. A recoverable failure is retried once per tab; beyond that, and for any refusal the authorization server issues explicitly, AppShell stops rather than redirecting the user back for the same answer. The outcome is classified from the resulting auth state rather than from whether the callback threw, because two failure paths — a server `error` and a state mismatch — return normally after recording the error.
 
 That makes `guardComponent` where a failed sign-in becomes visible: a guard that only renders a spinner will spin indefinitely, so render `useAuth().error` and offer a retry. See the authentication guide.
 
-Stripping preserves the existing `history.state`, which routers rely on (react-router keeps `{usr, key, idx}` there, Next.js keeps `__NA`). One limitation is unchanged from before: a callback URL is detected by the presence of `code` or `error`, which are generic parameter names, so an app already using them for its own purposes on a route can have them removed.
+The stale parameters themselves remain in the URL after a failure until the cleanup is fixed upstream (tailor-platform/auth-public-client#139) — app-shell no longer misbehaves because of them, but deliberately does not reimplement the library's URL cleanup.
diff --git a/docs/concepts/authentication.md b/docs/concepts/authentication.md
index 77420bf55..39a924d0d 100644
--- a/docs/concepts/authentication.md
+++ b/docs/concepts/authentication.md
@@ -219,7 +219,7 @@ function ChatScreen() {
 
 ## When the callback fails
 
-A sign-in that comes back from the authorization server unsuccessfully — the user declined, the client is not permitted, the exchange failed — leaves `isAuthenticated` false with the reason on `useAuth().error`. The callback parameters are cleared from the URL either way, so a reload retries cleanly instead of replaying the failed callback.
+A sign-in that comes back from the authorization server unsuccessfully — the user declined, the client is not permitted, the exchange failed — leaves `isAuthenticated` false with the reason on `useAuth().error`. The stale callback parameters may remain in the URL (the auth client currently cleans them only on success — [auth-public-client#139](https://github.com/tailor-platform/auth-public-client/issues/139)), but they no longer affect AppShell's behaviour.
 
 With `autoLogin`, a failure that looks recoverable is retried once automatically. Beyond that, and for any refusal the authorization server issues explicitly, AppShell stops: sending the user straight back would ask the same question, get the same answer, and loop.
 
diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx
index 2cc5f530b..f04d5ad8a 100644
--- a/packages/core/src/contexts/auth-context.test.tsx
+++ b/packages/core/src/contexts/auth-context.test.tsx
@@ -993,35 +993,6 @@ describe("createAuthClient", () => {
       isReady: true,
     };
 
-    it("strips callback parameters when the authorization server returns an error", async () => {
-      window.history.replaceState(
-        {},
-        "",
-        "/dashboard?error=access_denied&error_description=No&tab=orders",
-      );
-
-      // The error branch upstream resolves rather than throwing, and never
-      // cleans the URL — only the success path does.
-      renderWithClient({}, vi.fn().mockResolvedValue(undefined));
-
-      await waitFor(() => {
-        expect(window.location.search).toBe("?tab=orders");
-      });
-      // Unrelated parameters and the path survive; only the OAuth ones go.
-      expect(window.location.pathname).toBe("/dashboard");
-    });
-
-    it("strips callback parameters when the code exchange throws", async () => {
-      vi.stubGlobal("console", { ...console, error: vi.fn() });
-      window.history.replaceState({}, "", "/?code=auth-code-123&state=xyz");
-
-      renderWithClient({}, vi.fn().mockRejectedValue(new Error("Missing session data")));
-
-      await waitFor(() => {
-        expect(window.location.search).toBe("");
-      });
-    });
-
     it("resumes auto-login after a failed code exchange", async () => {
       // The dead end this guards: before the parameters were cleared, `?code=`
       // stayed in the URL, attemptAutoLogin treated the page as a live callback
@@ -1108,20 +1079,6 @@ describe("createAuthClient", () => {
       expect(window.sessionStorage.getItem("tailor-app-shell:oauth-callback-failures")).toBeNull();
     });
 
-    it("preserves the router's history state while stripping parameters", async () => {
-      // react-router keeps {usr, key, idx} here and Next.js keeps __NA; replacing
-      // it with a fresh object desyncs both for the rest of the session.
-      const routerState = { idx: 3, key: "abc", usr: null };
-      window.history.replaceState(routerState, "", "/dashboard?code=auth-code-123");
-
-      renderWithClient({}, vi.fn().mockResolvedValue(undefined));
-
-      await waitFor(() => {
-        expect(window.location.search).toBe("");
-      });
-      expect(window.history.state).toEqual(routerState);
-    });
-
     it("clears the retry budget once a callback succeeds", async () => {
       window.sessionStorage.setItem("tailor-app-shell:oauth-callback-failures", "1");
       window.history.replaceState({}, "", "/?code=auth-code-123&state=xyz");
@@ -1194,8 +1151,9 @@ describe("createAuthClient", () => {
       });
 
       expect(mockLogin).not.toHaveBeenCalled();
-      // The URL is still cleaned up, so a deliberate reload can retry.
-      expect(window.location.search).toBe("");
+      // The parameters stay in the URL until upstream cleans failed callbacks
+      // too (auth-public-client#139) — correctness here rests on the status.
+      expect(window.location.search).toBe("?error=access_denied");
     });
   });
 
diff --git a/packages/core/src/contexts/auth-context.tsx b/packages/core/src/contexts/auth-context.tsx
index 2c9318741..52f5514e7 100644
--- a/packages/core/src/contexts/auth-context.tsx
+++ b/packages/core/src/contexts/auth-context.tsx
@@ -223,21 +223,18 @@ export function createAuthClient(config: AuthClientConfig): EnhancedAuthClient {
       const authServerReportedError = currentUrl.searchParams.has("error");
       const { resolve, reject, deny } = callbackManager.start();
 
-      // auth-public-client only cleans the URL when the code exchange succeeds.
-      // Every failure path returns (or throws) with `code` / `error` still in the
-      // query string, which would otherwise leave the app wedged: the parameters
-      // make this look like a callback URL forever, and a reload just replays the
-      // same failing callback. Strip them on every outcome, before settling, so
-      // whatever observes the settled status already sees a clean URL.
-      //
       // The outcome is classified from the resulting auth state rather than from
       // whether the promise resolved. Not every failure throws — an `error` from
       // the authorization server and a state mismatch both return normally after
       // recording the error — so resolve-vs-throw is the wrong signal, and using
       // it would let those two skip the retry ceiling and loop.
+      //
+      // Note the URL is deliberately NOT cleaned here. auth-public-client owns
+      // callback URL cleanup and currently performs it only on success
+      // (tailor-platform/auth-public-client#139); on failure the parameters
+      // remain until that is fixed upstream. app-shell stays correct regardless,
+      // because auto-login is gated on this settled status, not on the URL.
       const settle = () => {
-        clearOAuthCallbackParams();
-
         if (baseClient.getState().isAuthenticated) {
           clearCallbackFailures();
           resolve();
@@ -357,19 +354,6 @@ const AuthContext = createContext<AuthContextType | null>(null);
 const isOAuthCallbackUrl = (url: URL) =>
   url.searchParams.has("code") || url.searchParams.has("error");
 
-/**
- * Query parameters an authorization server may add when redirecting back.
- * Removed together so a settled callback cannot be mistaken for a pending one.
- */
-const OAUTH_CALLBACK_PARAMS = [
-  "code",
-  "state",
-  "error",
-  "error_description",
-  "error_uri",
-  "iss",
-] as const;
-
 const isCurrentOAuthCallbackUrl = () => {
   if (typeof window === "undefined") {
     return false;
@@ -378,35 +362,6 @@ const isCurrentOAuthCallbackUrl = () => {
   return isOAuthCallbackUrl(new URL(window.location.href));
 };
 
-/**
- * Remove the OAuth callback parameters from the current URL, preserving any
- * unrelated query parameters and the hash. Uses `replaceState` so the failed
- * callback does not stay in session history for the back button to replay.
- */
-const clearOAuthCallbackParams = () => {
-  if (typeof window === "undefined") {
-    return;
-  }
-
-  const url = new URL(window.location.href);
-  if (!isOAuthCallbackUrl(url)) {
-    return;
-  }
-
-  for (const param of OAUTH_CALLBACK_PARAMS) {
-    url.searchParams.delete(param);
-  }
-
-  // Preserve the existing history state: routers keep their own bookkeeping on
-  // it (react-router stores {usr, key, idx}, Next.js stores __NA), and replacing
-  // it with a fresh object desyncs them for the rest of the session.
-  window.history.replaceState(
-    window.history.state,
-    document.title,
-    `${url.pathname}${url.search}${url.hash}`,
-  );
-};
-
 /**
  * Guard component that shows a fallback UI while auth is not ready or
  * not authenticated. Defined here so that the router layer does not
@@ -482,8 +437,10 @@ const useAutoLogin = (props: {
       // starts the exchange itself whenever it is constructed on a callback
       // URL, so "idle" here means nobody is handling these parameters and
       // redirecting is unsafe. Once a callback has run, its status is the
-      // authority — reading the URL instead is what used to strand the app,
-      // because a failed callback left its parameters in place forever.
+      // authority — reading the URL instead is what used to strand the app:
+      // upstream cleans the URL only on success, so a failed callback leaves
+      // its parameters in place (auth-public-client#139) and a URL-based gate
+      // treats the page as a live callback forever.
       (props.callbackStatus === "idle" && isCurrentOAuthCallbackUrl()) ||
       !authState.isReady ||
       authState.isAuthenticated ||

From 5b9f264763a02ee0fa4c481fa0ccd50ce7a2e318 Mon Sep 17 00:00:00 2001
From: interacsean <seanhasselback@gmail.com>
Date: Fri, 28 Aug 2026 09:59:26 +1000
Subject: [PATCH 4/4] docs(auth): reference the private upstream issue as plain
 text

lychee in doc-check runs unauthenticated and 404s on the private
auth-public-client repo, failing link-check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 docs/concepts/authentication.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/concepts/authentication.md b/docs/concepts/authentication.md
index 39a924d0d..7148fbcb6 100644
--- a/docs/concepts/authentication.md
+++ b/docs/concepts/authentication.md
@@ -219,7 +219,7 @@ function ChatScreen() {
 
 ## When the callback fails
 
-A sign-in that comes back from the authorization server unsuccessfully — the user declined, the client is not permitted, the exchange failed — leaves `isAuthenticated` false with the reason on `useAuth().error`. The stale callback parameters may remain in the URL (the auth client currently cleans them only on success — [auth-public-client#139](https://github.com/tailor-platform/auth-public-client/issues/139)), but they no longer affect AppShell's behaviour.
+A sign-in that comes back from the authorization server unsuccessfully — the user declined, the client is not permitted, the exchange failed — leaves `isAuthenticated` false with the reason on `useAuth().error`. The stale callback parameters may remain in the URL (the auth client currently cleans them only on success — tracked upstream as `tailor-platform/auth-public-client#139`; the repo is private, hence no link), but they no longer affect AppShell's behaviour.
 
 With `autoLogin`, a failure that looks recoverable is retried once automatically. Beyond that, and for any refusal the authorization server issues explicitly, AppShell stops: sending the user straight back would ask the same question, get the same answer, and loop.