From 19a39235a2ce7c387bdb1b55cc1b05c5b5eb1a66 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 3 Sep 2026 20:01:38 -0400 Subject: [PATCH] fix: terminate the OAuth connection-details refresh promise chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The connection-details refresh effect in useOAuthRecovery voided inspectorClient.getOAuthState() with a lone .then and no rejection handler. In the browser that read goes through the remote OAuth store, so it is a network round trip that can reject — a backend that is down or restarting, a 401 on the API token, malformed stored state. void silences no-floating-promises without terminating anything, so every such failure became an unhandled rejection, on the initial refresh and on every oauthComplete refresh. Terminate the chain with a .catch that clears the panel details rather than leaving the last successful read on screen: the panel reports the current OAuth state, and a stale answer is indistinguishable from a fresh one. The cancelled guard is repeated on the catch so a rejection arriving after unmount does not write. The void stays, now with the one-line justification AGENTS.md asks for — a synchronous useEffect body cannot await. Two tests cover it. Both fail against the unfixed source with unhandled rejections, which is the failure mode itself: an unhandled rejection fails the whole vitest run. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GBkPzbuxgyqiz39ytmQzMB Signed-off-by: cliffhall --- .../web/src/hooks/useOAuthRecovery.test.tsx | 48 +++++++++++++++++++ clients/web/src/hooks/useOAuthRecovery.ts | 23 ++++++--- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/clients/web/src/hooks/useOAuthRecovery.test.tsx b/clients/web/src/hooks/useOAuthRecovery.test.tsx index 91b3f10d..0ab47c70 100644 --- a/clients/web/src/hooks/useOAuthRecovery.test.tsx +++ b/clients/web/src/hooks/useOAuthRecovery.test.tsx @@ -393,6 +393,54 @@ describe("useOAuthRecovery", () => { expect(h.api().connectionInfoOAuth).toBeUndefined(); }); + it("clears the details when the state read rejects", async () => { + const client = fakeClient({ + getOAuthState: vi.fn().mockRejectedValue(new Error("backend down")), + }); + const props: HarnessProps = { + servers: [entry("a")], + activeServerId: "a", + client, + }; + const h = harness(props); + await waitFor(() => expect(client.getOAuthState).toHaveBeenCalled()); + expect(h.api().connectionInfoOAuth).toBeUndefined(); + + // The rejection is terminated, not floated: a later resolving read still + // populates the panel, which an unhandled rejection would have prevented + // by failing the run. + client.getOAuthState = vi + .fn() + .mockResolvedValue({ tokens: { access_token: "t" } }); + await act(async () => { + client.emit("oauthComplete", {}); + }); + await waitFor(() => expect(h.api().connectionInfoOAuth).toBeDefined()); + }); + + it("drops a rejected state read that lands after the session ended", async () => { + let fail: (reason: unknown) => void = () => {}; + const client = fakeClient({ + getOAuthState: vi.fn( + () => + new Promise((_resolve, reject) => { + fail = reject; + }), + ), + }); + const props: HarnessProps = { + servers: [entry("a")], + activeServerId: "a", + client, + }; + const h = harness(props); + h.rerender({ ...props, connectionStatus: "disconnected" }); + await act(async () => { + fail(new Error("backend down")); + }); + expect(h.api().connectionInfoOAuth).toBeUndefined(); + }); + it("drops a state read that lands after the session ended", async () => { let settle: (value: unknown) => void = () => {}; const client = fakeClient({ diff --git a/clients/web/src/hooks/useOAuthRecovery.ts b/clients/web/src/hooks/useOAuthRecovery.ts index 428352b7..c6b7d184 100644 --- a/clients/web/src/hooks/useOAuthRecovery.ts +++ b/clients/web/src/hooks/useOAuthRecovery.ts @@ -406,12 +406,23 @@ export function useOAuthRecovery({ let cancelled = false; const refresh = (): void => { - void inspectorClient.getOAuthState().then((state) => { - if (cancelled) return; - setConnectionInfoOAuthWhenConnected( - state ? oauthDetailsFromConnectionState(state) : undefined, - ); - }); + // void: a synchronous useEffect body cannot await. The chain is + // terminated below, so the rejection is handled rather than discarded. + void inspectorClient + .getOAuthState() + .then((state) => { + if (cancelled) return; + setConnectionInfoOAuthWhenConnected( + state ? oauthDetailsFromConnectionState(state) : undefined, + ); + }) + .catch(() => { + // The read failed (backend down, 401 on the API token, malformed + // stored state). Clear rather than keep the last successful read — + // a stale answer is indistinguishable from a fresh one in the panel. + if (cancelled) return; + setConnectionInfoOAuthWhenConnected(undefined); + }); }; const onAmbientAuthChallenge = (): void => {