From e2cbf0de6d827c02907f7bae9880ac022569c52f Mon Sep 17 00:00:00 2001 From: cliffhall Date: Fri, 28 Aug 2026 05:07:00 -0400 Subject: [PATCH] fix(web): lock the OAuth clear so a double click cannot run it twice (#2144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review round 11. The web counterpart of the TUI lock added in round 9. Neither clear control was guarded while the operation was pending, and revocation can take up to five seconds — so a double click started concurrent RFC 7009 requests, concurrent store writes and two contradictory toasts against the same client. Both controls now share one ref-backed in-flight lock in `runClear`, since they drive the same client and the same store entry. A ref rather than state, for the reason the TUI one is: a double click delivers both events before React re-renders, so a state-based guard lets both through. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh Signed-off-by: cliffhall --- clients/web/src/App.tsx | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index b89b2e1d..bb25547f 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -1320,6 +1320,11 @@ function App() { // target isn't resolvable. const settingsModalIsStdio = settingsModalServerType === "stdio"; + /** + * In-flight lock shared by both clear controls (#2144). See `runClear`. + */ + const clearOAuthInFlightRef = useRef(false); + /** * Run a clear from a key/click handler, which cannot await. * @@ -1331,16 +1336,28 @@ function App() { */ const runClear = useCallback( (server: Parameters[0]) => { - clearServerOAuthAndDisconnect(server).catch((err: unknown) => { - notifications.show({ - title: "Could not clear the stored OAuth state", - message: err instanceof Error ? err.message : String(err), - color: "red", - // The tokens may still be on disk and the session may still be up, - // so this is not a notice to let time out. - autoClose: false, + // Shared by BOTH clear controls (Connection Info and Server Settings), + // because they drive the same client and the same store entry. A ref + // rather than state: a double click delivers both events before React + // re-renders, so a state-based guard would let both through — and with + // revocation taking up to five seconds, that means concurrent RFC 7009 + // requests, concurrent store writes, and two contradictory toasts. + if (clearOAuthInFlightRef.current) return; + clearOAuthInFlightRef.current = true; + clearServerOAuthAndDisconnect(server) + .finally(() => { + clearOAuthInFlightRef.current = false; + }) + .catch((err: unknown) => { + notifications.show({ + title: "Could not clear the stored OAuth state", + message: err instanceof Error ? err.message : String(err), + color: "red", + // The tokens may still be on disk and the session may still be up, + // so this is not a notice to let time out. + autoClose: false, + }); }); - }); }, [clearServerOAuthAndDisconnect], );