fix(auth): invalidate stale browser OAuth operations

src/client/src/controllers/authController.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, vi } from "vitest";
 import { api as defaultApi, type AuthProviderOption, type OAuthFlowState, type SessionInfo, type SessionStatus } from "../api";
 import { initialAppState, type AppState } from "../appState";
 import { AuthController, parseAuthSlashCommand } from "./authController";
@@ -134,6 +134,108 @@ describe("AuthController", () => {
     });
   });
 
+  it("does not recreate an OAuth dialog when a pending response settles during cancellation", async () => {
+    const prompt = { requestId: "request-1", message: "Paste callback", kind: "manual" } as const;
+    const flow = oauthFlow({ prompt });
+    const response = deferred<OAuthFlowState>();
+    const cancellation = deferred<OAuthFlowState>();
+    const { controller, getState } = createController(
+      { authDialog: { step: "oauth", flow, inputValue: "https://callback" } },
+      {
+        respondOAuthFlow: () => response.promise,
+        cancelOAuthFlow: () => cancellation.promise,
+      },
+    );
+
+    const responsePending = controller.respondOAuth();
+    const cancellationPending = controller.cancelOAuth();
+    const dialogAfterCancel = getState().authDialog;
+
+    response.resolve(oauthFlow({ prompt, progress: ["Stale response"] }));
+    await responsePending;
+    const dialogAfterResponse = getState().authDialog;
+
+    cancellation.resolve(oauthFlow({ status: "cancelled" }));
+    await cancellationPending;
+
+    expect(dialogAfterCancel).toBeUndefined();
+    expect(dialogAfterResponse).toBeUndefined();
+    expect(getState().authDialog).toBeUndefined();
+  });
+
+  it("does not let a stale OAuth response overwrite a newer flow", async () => {
+    vi.stubGlobal("window", { setInterval: () => 1, clearInterval: () => undefined });
+    const oldPrompt = { requestId: "request-1", message: "Paste callback", kind: "manual" } as const;
+    const oldFlow = oauthFlow({ prompt: oldPrompt });
+    const newFlow = oauthFlow({ flowId: "flow-2", prompt: { requestId: "request-2", message: "Paste callback", kind: "manual" } });
+    const response = deferred<OAuthFlowState>();
+    const providers = [authProvider("anthropic", "oauth")];
+    const { controller, getState } = createController(
+      { authDialog: { step: "oauth", flow: oldFlow, inputValue: "https://old-callback" } },
+      {
+        respondOAuthFlow: () => response.promise,
+        authProviders: () => Promise.resolve({ providers }),
+        startOAuthLogin: () => Promise.resolve(newFlow),
+      },
+    );
+
+    try {
+      const responsePending = controller.respondOAuth();
+      await controller.openLogin("anthropic");
+      const dialogAfterNewFlow = getState().authDialog;
+
+      response.resolve(oauthFlow({ prompt: oldPrompt, progress: ["Stale response"] }));
+      await responsePending;
+
+      expect(dialogAfterNewFlow).toMatchObject({ step: "oauth", flow: { flowId: "flow-2" } });
+      expect(getState().authDialog).toMatchObject({ step: "oauth", flow: { flowId: "flow-2" } });
+    } finally {
+      response.resolve(oldFlow);
+      controller.dispose();
+      vi.unstubAllGlobals();
+    }
+  });
+
+  it("does not let an older poll restore a running flow after a newer poll stops polling", async () => {
+    vi.useFakeTimers();
+    vi.stubGlobal("window", { setInterval: globalThis.setInterval, clearInterval: globalThis.clearInterval });
+    const prompt = { requestId: "request-1", message: "Paste callback", kind: "manual" } as const;
+    const runningFlow = oauthFlow({ prompt });
+    const stalePoll = deferred<OAuthFlowState>();
+    const providers = [authProvider("anthropic", "oauth")];
+    let pollCalls = 0;
+    const { controller, getState } = createController(
+      {},
+      {
+        authProviders: () => Promise.resolve({ providers }),
+        startOAuthLogin: () => Promise.resolve(runningFlow),
+        oauthFlow: () => {
+          pollCalls += 1;
+          return pollCalls === 1 ? stalePoll.promise : Promise.resolve(oauthFlow({ status: "cancelled", prompt }));
+        },
+      },
+    );
+
+    try {
+      await controller.openLogin("anthropic");
+      await vi.advanceTimersByTimeAsync(1000);
+      await vi.advanceTimersByTimeAsync(1000);
+      const dialogAfterPollingStopped = getState().authDialog;
+
+      stalePoll.resolve(oauthFlow({ prompt, progress: ["Stale running poll"] }));
+      await flushMicrotasks();
+
+      expect(pollCalls).toBe(2);
+      expect(dialogAfterPollingStopped).toMatchObject({ step: "oauth", flow: { status: "cancelled" } });
+      expect(getState().authDialog).toMatchObject({ step: "oauth", flow: { status: "cancelled" } });
+    } finally {
+      stalePoll.resolve(runningFlow);
+      controller.dispose();
+      vi.unstubAllGlobals();
+      vi.useRealTimers();
+    }
+  });
+
   it("cancels the active OAuth flow and closes the dialog even when cancellation fails", async () => {
     const flow = oauthFlow({ prompt: { requestId: "request-1", message: "Paste callback", kind: "manual" } });
     const cancelCalls: { flowId: string; machineId: string | undefined }[] = [];
@@ -246,6 +348,13 @@ async function flushMicrotasks(): Promise<void> {
   await Promise.resolve();
 }
 
+function deferred<T>(): { promise: Promise<T>; resolve: (value: T) => void } {
+  let resolveDeferred: ((value: T) => void) | undefined;
+  const promise = new Promise<T>((resolve) => { resolveDeferred = resolve; });
+  if (resolveDeferred === undefined) throw new Error("Deferred promise was not initialized");
+  return { promise, resolve: resolveDeferred };
+}
+
 function remoteMachine(id: string): NonNullable<AppState["selectedMachine"]> {
   return {
     id,

src/client/src/controllers/authController.ts

@@ -1,6 +1,9 @@
 import { api as defaultApi, type AuthProviderOption, type AuthType, type OAuthFlowState, type SessionStatus } from "../api";
+import type { AuthDialogState } from "../appState";
 import { selectedMachineId, type GetState, type SetState } from "./types";
 
+type OAuthDialogState = Extract<AuthDialogState, { step: "oauth" }>;
+
 export interface AuthControllerDependencies {
   api?: typeof defaultApi;
   pollIntervalMs?: number;
@@ -9,6 +12,8 @@ export interface AuthControllerDependencies {
 export class AuthController {
   private readonly api: typeof defaultApi;
   private readonly pollIntervalMs: number;
+  private oauthOperationGeneration = 0;
+  private pollGeneration = 0;
   private pollTimer: number | undefined;
 
   constructor(
@@ -22,6 +27,7 @@ export class AuthController {
   }
 
   dispose(): void {
+    this.oauthOperationGeneration += 1;
     this.stopPolling();
   }
 
@@ -128,15 +134,22 @@ export class AuthController {
     if (dialog?.step !== "oauth") return;
     const request = dialog.flow.prompt ?? dialog.flow.select;
     if (request === undefined) return;
+    const operationGeneration = this.oauthOperationGeneration;
+    const flowId = dialog.flow.flowId;
+    const requestId = request.requestId;
     const responseValue = value ?? dialog.inputValue ?? "";
     const clean = { ...dialog };
     delete clean.error;
     this.setState({ authDialog: { ...clean, responding: true } });
     try {
-      const flow = await this.api.respondOAuthFlow(dialog.flow.flowId, request.requestId, responseValue, selectedMachineId(this.getState()));
+      const flow = await this.api.respondOAuthFlow(flowId, requestId, responseValue, selectedMachineId(this.getState()));
+      const current = this.currentOAuthDialog(operationGeneration, flowId);
+      if (flow.flowId !== flowId || current === undefined || oauthRequestId(current.flow) !== requestId) return;
       this.updateOAuthFlow(flow);
     } catch (error) {
-      this.setState({ authDialog: { ...dialog, responding: false, error: String(error) } });
+      const current = this.currentOAuthDialog(operationGeneration, flowId);
+      if (current === undefined || oauthRequestId(current.flow) !== requestId) return;
+      this.setState({ authDialog: { ...current, responding: false, error: String(error) } });
     }
   }
 
@@ -146,16 +159,18 @@ export class AuthController {
       this.closeDialog();
       return;
     }
-    this.stopPolling();
-    try {
-      await this.api.cancelOAuthFlow(dialog.flow.flowId, selectedMachineId(this.getState()));
-    } catch {
-      // Best-effort cancel. The dialog closes either way.
-    }
+    const flowId = dialog.flow.flowId;
+    const machineId = selectedMachineId(this.getState());
     this.closeDialog();
+    try {
+      await this.api.cancelOAuthFlow(flowId, machineId);
+    } catch {
+      // Best-effort cancel. The dialog is already closed either way.
+    }
   }
 
   closeDialog(): void {
+    this.oauthOperationGeneration += 1;
     this.stopPolling();
     this.setState({ authDialog: undefined });
   }
@@ -183,12 +198,15 @@ export class AuthController {
 
   private async startOAuth(provider: AuthProviderOption): Promise<void> {
     if (this.rejectRemoteOAuth("login", provider)) return;
+    const operationGeneration = ++this.oauthOperationGeneration;
+    this.stopPolling();
     try {
       const flow = await this.api.startOAuthLogin(provider.id, selectedMachineId(this.getState()));
+      if (operationGeneration !== this.oauthOperationGeneration) return;
       this.updateOAuthFlow(flow);
-      this.startPolling(flow.flowId);
+      if (flow.status === "running") this.startPolling(flow.flowId);
     } catch (error) {
-      this.setState({ error: String(error) });
+      if (operationGeneration === this.oauthOperationGeneration) this.setState({ error: String(error) });
     }
   }
 
@@ -207,11 +225,14 @@ export class AuthController {
       void this.refreshStatus();
       return;
     }
-    if (flow.status === "error" || flow.status === "cancelled") this.stopPolling();
+    if (flow.status === "error" || flow.status === "cancelled") {
+      this.oauthOperationGeneration += 1;
+      this.stopPolling();
+    }
     const existing = this.getState().authDialog;
     const previousInput = existing?.step === "oauth" && existing.flow.flowId === flow.flowId ? existing.inputValue ?? "" : "";
-    const previousRequestId = existing?.step === "oauth" ? existing.flow.prompt?.requestId ?? existing.flow.select?.requestId : undefined;
-    const newRequestId = flow.prompt?.requestId ?? flow.select?.requestId;
+    const previousRequestId = existing?.step === "oauth" ? oauthRequestId(existing.flow) : undefined;
+    const newRequestId = oauthRequestId(flow);
     const sameRequest = previousRequestId !== undefined && previousRequestId === newRequestId;
     const inputValue = sameRequest ? previousInput : "";
     const responding = sameRequest && existing?.step === "oauth" ? existing.responding === true : false;
@@ -220,29 +241,45 @@ export class AuthController {
 
   private startPolling(flowId: string): void {
     this.stopPolling();
-    this.pollTimer = window.setInterval(() => { void this.poll(flowId); }, this.pollIntervalMs);
+    const operationGeneration = this.oauthOperationGeneration;
+    const pollGeneration = this.pollGeneration;
+    this.pollTimer = window.setInterval(() => { void this.poll(flowId, operationGeneration, pollGeneration); }, this.pollIntervalMs);
   }
 
   private stopPolling(): void {
+    this.pollGeneration += 1;
     if (this.pollTimer === undefined) return;
     window.clearInterval(this.pollTimer);
     this.pollTimer = undefined;
   }
 
-  private async poll(flowId: string): Promise<void> {
-    const dialog = this.getState().authDialog;
-    if (dialog?.step !== "oauth" || dialog.flow.flowId !== flowId) {
+  private async poll(flowId: string, operationGeneration: number, pollGeneration: number): Promise<void> {
+    if (pollGeneration !== this.pollGeneration) return;
+    const dialog = this.currentOAuthDialog(operationGeneration, flowId);
+    if (dialog === undefined) {
       this.stopPolling();
       return;
     }
+    const requestId = oauthRequestId(dialog.flow);
     try {
-      this.updateOAuthFlow(await this.api.oauthFlow(flowId, selectedMachineId(this.getState())));
+      const flow = await this.api.oauthFlow(flowId, selectedMachineId(this.getState()));
+      const current = this.currentOAuthDialog(operationGeneration, flowId);
+      if (flow.flowId !== flowId || pollGeneration !== this.pollGeneration || current === undefined || oauthRequestId(current.flow) !== requestId) return;
+      this.updateOAuthFlow(flow);
     } catch (error) {
+      const current = this.currentOAuthDialog(operationGeneration, flowId);
+      if (pollGeneration !== this.pollGeneration || current === undefined || oauthRequestId(current.flow) !== requestId) return;
       this.stopPolling();
-      this.setState({ authDialog: { ...dialog, error: String(error) } });
+      this.setState({ authDialog: { ...current, error: String(error) } });
     }
   }
 
+  private currentOAuthDialog(operationGeneration: number, flowId: string): OAuthDialogState | undefined {
+    if (operationGeneration !== this.oauthOperationGeneration) return undefined;
+    const dialog = this.getState().authDialog;
+    return dialog?.step === "oauth" && dialog.flow.flowId === flowId ? dialog : undefined;
+  }
+
   private async refreshStatus(): Promise<void> {
     const session = this.session();
     if (session === undefined) return;
@@ -260,6 +297,10 @@ export class AuthController {
   }
 }
 
+function oauthRequestId(flow: OAuthFlowState): string | undefined {
+  return flow.prompt?.requestId ?? flow.select?.requestId;
+}
+
 export function parseAuthSlashCommand(text: string): { command: "login" | "logout"; providerId?: string } | undefined {
   const trimmed = text.trim();
   const match = /^\/(login|logout)(?:\s+(\S+))?\s*$/u.exec(trimmed);