diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index 1ac0f73f..003cc498 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -468,6 +468,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({ readResourceState?: { status?: string }; currentLogLevel?: string; activeTab?: string; + activeServer?: string; erroredServerId?: string; initializeResult?: { serverInfo: { name: string; version: string } }; onActiveTabChange: (tab: string) => void; @@ -549,6 +550,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({ {props.erroredServerId ?? "none"} + {props.activeServer ?? "none"} @@ -2604,8 +2606,15 @@ describe("App OAuth callback issuer-binding failures (#1808)", () => { await waitFor(() => expect(screen.getByTestId("errored-server")).toHaveTextContent("A"), ); - // `resumeAfterOAuth` carries the reconnect, which can reject while holding - // the status at "connecting", so the attempt is torn down explicitly. + // The active server is deliberately retained. "Authorize again" on the + // re-auth banner only hands `clearServerOAuthState` the live client when + // the banner's server is the active one, so releasing it here would leave + // the stale tokens on the client that holds them. Asserted so a later + // "cleanup" of the apparent leak fails here rather than silently breaking + // the recovery. + expect(screen.getByTestId("active-server")).toHaveTextContent("A"); + // The teardown still runs, for the case where the reconnect is what + // rejected and the status really is left at "connecting". const client = clientInstances[0] as EventTarget & { disconnect: ReturnType; }; diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index b66a92d9..64a3968a 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -2909,6 +2909,19 @@ function App() { // active-server lock is never released. Before the EMA guard, since a // stuck session is worth clearing whichever way the error classifies. await client.disconnect().catch(() => {}); + // `activeServerId` is deliberately NOT cleared here, even though the + // disconnect above often cannot announce itself: it emits only on a + // status *change*, and the commonest failure — a rejected token + // exchange — throws inside `completeOAuthFlow` before the reconnect + // runs, so this freshly built client is still at its initial + // `"disconnected"` and the listener that would clear it never fires. + // + // That looks like a leak and is not. The next step after a callback + // failure is the re-auth banner below, and its "Authorize again" hands + // `clearServerOAuthState` the live client only when the banner's server + // *is* the active one. Releasing it here would pass `null` instead, and + // the stale tokens would never be cleared from the client that holds + // them — the one thing that recovery exists to do. if (isEmaClientNotConfiguredError(err)) { notifications.show({ title: `Cannot connect to "${server.name}"`,