diff --git a/clients/web/src/hooks/useOAuthRecovery.test.tsx b/clients/web/src/hooks/useOAuthRecovery.test.tsx index e2ede1da..6fee4ec2 100644 --- a/clients/web/src/hooks/useOAuthRecovery.test.tsx +++ b/clients/web/src/hooks/useOAuthRecovery.test.tsx @@ -391,6 +391,49 @@ describe("useOAuthRecovery", () => { }); }); + it("does not carry the left server's retry into a later ambient step-up", async () => { + const client = fakeClient({ + handleAuthChallenge: vi.fn().mockResolvedValue({ kind: "satisfied" }), + }); + const servers = [entry("a", {}, true), entry("b", {}, true)]; + const h = harness({ servers, activeServerId: "a", client }); + const retryA = vi.fn().mockResolvedValue(undefined); + + await act(async () => { + await h + .api() + .handleCommandScopedAuthRecovery( + new AuthRecoveryRequiredError( + AUTH_URL, + challenge("insufficient_scope"), + ), + { serverId: "a", source: "tool", retryOperation: retryA }, + ); + }); + expect(h.api().pendingStepUp?.serverId).toBe("a"); + + await act(async () => { + h.rerender({ servers, activeServerId: "b", client }); + }); + expect(h.api().pendingStepUp).toBeNull(); + + // Server B raises its own step-up, which carries no retry of its own. + await act(async () => { + client.emit("authChallengeInteractive", { + challenge: challenge("insufficient_scope"), + authorizationUrl: AUTH_URL, + }); + }); + await waitFor(() => expect(h.api().pendingStepUp?.serverId).toBe("b")); + + await act(async () => { + await h.api().handleStepUpAuthorize(); + }); + // Authorizing B must not run the command A was left mid-way through. + expect(retryA).not.toHaveBeenCalled(); + expect(toastTitles()).toContain("Permissions updated"); + }); + it("refuses a second step-up while one is open", 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 d5a3d891..1acb59a0 100644 --- a/clients/web/src/hooks/useOAuthRecovery.ts +++ b/clients/web/src/hooks/useOAuthRecovery.ts @@ -317,6 +317,19 @@ export function useOAuthRecovery({ useEffect(() => { sessionRef.current.pendingStepUp = pendingStepUp; sessionRef.current.pendingReauth = pendingReauth; + // The retry belongs to the prompt, so it dies with it. Every arm that + // dismisses a prompt deliberately drops the retry inline — but the two + // that clear the slot as a *consequence* of something else (the + // server-switch reset below, and `resetOAuthRecoveryState` on disconnect) + // would otherwise leave it installed, and the next prompt to open without + // one of its own would inherit it: authorizing an ambient step-up on + // server B would run the command server A was left mid-way through + // (Copilot on #2237). Clearing it here covers both without any of them + // having to remember. It is a ref write in an effect, which is where a ref + // write belongs — the render-phase reset above must stay pure. + if (pendingStepUp === null) { + pendingStepUpRetryRef.current = null; + } }); // Both slots are server-scoped, so switching servers has to clear whichever