From 6f2e1faed255a799ec7e3f094e019d383c73f684 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 29 Aug 2026 03:15:42 -0400 Subject: [PATCH] fix(web): install a step-up's command retry only when its prompt opens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot round 6. `handleCommandScopedAuthRecovery` wrote `pendingStepUpRetryRef` before calling `trySetPendingStepUp`, which REFUSES a second prompt while one is already open — so the refused command's operation replaced the open prompt's, and authorizing that prompt ran the command the user had just been told could not start. The retry now goes in only on a prompt that actually opened, which makes the pairing hold in both directions: the prompt and its retry are installed together, and the EMA branch takes them out together. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Quk1hBUtyhkXowwMXzAnAY Signed-off-by: cliffhall --- .../web/src/hooks/useOAuthRecovery.test.tsx | 26 +++++++++++++++++++ clients/web/src/hooks/useOAuthRecovery.ts | 25 ++++++++++++------ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/clients/web/src/hooks/useOAuthRecovery.test.tsx b/clients/web/src/hooks/useOAuthRecovery.test.tsx index 1cc704ab..91b3f10d 100644 --- a/clients/web/src/hooks/useOAuthRecovery.test.tsx +++ b/clients/web/src/hooks/useOAuthRecovery.test.tsx @@ -2059,6 +2059,32 @@ describe("useOAuthRecovery", () => { expect(firstRetry).not.toHaveBeenCalled(); }); + it("keeps the open prompt's retry when a second step-up is refused", async () => { + // `trySetPendingStepUp` refuses the second prompt, so the second + // command's operation must not replace the first's — authorizing the + // open prompt would otherwise run the command the user was just told + // could not start (#2165). + const client = fakeClient({ + handleAuthChallenge: vi.fn().mockResolvedValue({ kind: "satisfied" }), + }); + const h = harness({ + servers: [entry("a", {}, true)], + activeServerId: "a", + client, + }); + const firstRetry = vi.fn().mockResolvedValue(undefined); + const refusedRetry = vi.fn().mockResolvedValue(undefined); + await openStepUp(h, "tool", firstRetry); + await openStepUp(h, "prompt", refusedRetry); + expect(toastTitles()).toContain("Step-up authorization in progress"); + + await act(async () => { + await h.api().handleStepUpAuthorize(); + }); + expect(firstRetry).toHaveBeenCalledTimes(1); + expect(refusedRetry).not.toHaveBeenCalled(); + }); + it("cancels back to the panel that asked", async () => { const client = fakeClient(); const h = harness({ servers: [entry("a")], activeServerId: "a", client }); diff --git a/clients/web/src/hooks/useOAuthRecovery.ts b/clients/web/src/hooks/useOAuthRecovery.ts index f64cd6db..428352b7 100644 --- a/clients/web/src/hooks/useOAuthRecovery.ts +++ b/clients/web/src/hooks/useOAuthRecovery.ts @@ -690,14 +690,23 @@ export function useOAuthRecovery({ ) { return true; } - pendingStepUpRetryRef.current = options.retryOperation ?? null; - trySetPendingStepUp({ - challenge: error.authChallenge, - authorizationUrl: error.authorizationUrl, - serverId: options.serverId, - source: options.source, - enterpriseManaged: isEmaStepUp(error.authChallenge, server), - }); + // The retry belongs to the prompt, so it is installed only once the + // prompt is actually open (#2165). `trySetPendingStepUp` REFUSES a + // second prompt while one is already up — writing the ref first meant + // the refused command's operation replaced the open prompt's, and + // authorizing that prompt then ran the command the user was just told + // could not start. + if ( + trySetPendingStepUp({ + challenge: error.authChallenge, + authorizationUrl: error.authorizationUrl, + serverId: options.serverId, + source: options.source, + enterpriseManaged: isEmaStepUp(error.authChallenge, server), + }) + ) { + pendingStepUpRetryRef.current = options.retryOperation ?? null; + } return false; }