Let users cancel and retry OAuth sign-in

.changeset/connection-oauth-cancel.md

@@ -0,0 +1,5 @@
+---
+"executor": patch
+---
+
+Let users cancel sign-in from the connection form and prevent cancelled OAuth requests from disrupting a retry.

e2e/scenarios/connection-setup-ux.test.ts

@@ -122,6 +122,11 @@ scenario(
         await step("Sign in with a provider account without naming the connection", async () => {
           await visit(page, `/integrations/${slug}?addAccount=1`);
           await page.getByRole("tab", { name: "OAuth2", exact: true }).click();
+          // Attached previews can contain other compatible apps. Use this
+          // scenario's provider instance for the full callback and token exchange.
+          await page
+            .getByRole("radio", { name: new RegExp(slug.replace("setup-", ""), "i") })
+            .check();
           const opened = page.waitForEvent("popup");
           await page.getByRole("button", { name: "Connect with OAuth", exact: true }).click();
           const popup = await opened;
@@ -288,3 +293,148 @@ scenario(
     }),
   ),
 );
+
+scenario(
+  "Connection setup · an interrupted OAuth popup can be cancelled and retried",
+  {},
+  Effect.scoped(
+    Effect.gen(function* () {
+      const { browser, identity, slug, client, emulator } = yield* fixture;
+      yield* browser.session(identity, async ({ page, step }) => {
+        await step("Start provider sign-in without entering a name", async () => {
+          await visit(page, `/integrations/${slug}?addAccount=1`);
+          await page.getByRole("tab", { name: "OAuth2", exact: true }).click();
+          // Attached previews can contain other compatible apps. Use this
+          // scenario's provider instance for the full callback and token exchange.
+          await page
+            .getByRole("radio", { name: new RegExp(slug.replace("setup-", ""), "i") })
+            .check();
+          const opened = page.waitForEvent("popup");
+          await page.getByRole("button", { name: "Connect with OAuth", exact: true }).click();
+          const popup = await opened;
+          await popup.waitForURL(/oauth\/v2\/authorize/);
+          await popup.close();
+        });
+        await step("Cancel the waiting sign-in and retry", async () => {
+          const cancel = page.getByRole("button", { name: "Cancel sign-in", exact: true });
+          expect(
+            await cancel.isVisible(),
+            "an interrupted provider window must not strand Connecting forever",
+          ).toBe(true);
+          expect(await cancel.isEnabled()).toBe(true);
+          await page.getByText("Continue in the sign-in window", { exact: true }).waitFor();
+          expect(
+            await page.getByRole("tab", { name: "OAuth2", exact: true }).isVisible(),
+            "the authentication form stays visible during sign-in",
+          ).toBe(true);
+          expect(await page.getByRole("textbox", { name: /Display name/ }).isVisible()).toBe(true);
+          expect(
+            await page.getByRole("button", { name: "Connecting…", exact: true }).count(),
+            "waiting for provider consent must not leave a dead Connecting button",
+          ).toBe(0);
+          await cancel.click();
+          expect(
+            await page.getByRole("button", { name: "Connect with OAuth", exact: true }).isEnabled(),
+          ).toBe(true);
+          const opened = page.waitForEvent("popup");
+          await page.getByRole("button", { name: "Connect with OAuth", exact: true }).click();
+          const popup = await opened;
+          await popup.waitForURL(/oauth\/v2\/authorize/);
+          await popup.route("https://emulators.dev/oauth/v2/authorize/callback", (route) =>
+            route.continue({ url: `${emulator.baseUrl}/oauth/v2/authorize/callback` }),
+          );
+          await popup.getByRole("button", { name: /admin/ }).click();
+          await page.getByRole("dialog").waitFor({ state: "hidden", timeout: 30_000 });
+        });
+      });
+      const connections = yield* client.connections.list({ query: { integration: slug } });
+      expect(connections, "retry finishes and persists exactly one connection").toHaveLength(1);
+    }),
+  ),
+);
+
+scenario(
+  "Connection setup · cancelling a pending OAuth start ignores its late response",
+  {},
+  Effect.scoped(
+    Effect.gen(function* () {
+      const { browser, identity, slug, client, emulator } = yield* fixture;
+      yield* browser.session(identity, async ({ page, step }) => {
+        const released = Promise.withResolvers<void>();
+        const started = Promise.withResolvers<string>();
+        const completed = Promise.withResolvers<void>();
+        await page.route(
+          "**/api/oauth/start",
+          async (route) => {
+            const response = await route.fetch();
+            const body: unknown = await response.json();
+            if (
+              typeof body !== "object" ||
+              body === null ||
+              !("state" in body) ||
+              typeof body.state !== "string"
+            ) {
+              expect.fail("the held start response must contain its OAuth session state");
+            }
+            started.resolve(body.state);
+            await released.promise;
+            await route.fulfill({ response });
+            completed.resolve();
+          },
+          { times: 1 },
+        );
+        try {
+          await step("Cancel while the authorization response is still in flight", async () => {
+            await visit(page, `/integrations/${slug}?addAccount=1`);
+            await page
+              .getByRole("radio", { name: new RegExp(slug.replace("setup-", ""), "i") })
+              .check();
+            const opened = page.waitForEvent("popup");
+            await page.getByRole("button", { name: "Connect with OAuth", exact: true }).click();
+            const popup = await opened;
+            const oldState = await started.promise;
+            expect(
+              await page.getByRole("textbox", { name: /Display name/ }).isVisible(),
+              "the connection form stays visible while sign-in is being prepared",
+            ).toBe(true);
+            await page.getByRole("button", { name: "Cancel sign-in", exact: true }).click();
+            await expect.poll(() => popup.isClosed()).toBe(true);
+            const cancelled = page.waitForResponse((response) =>
+              response.url().endsWith("/api/oauth/cancel"),
+            );
+            const retryOpened = page.waitForEvent("popup");
+            await page.getByRole("button", { name: "Connect with OAuth", exact: true }).click();
+            const retryPopup = await retryOpened;
+            await retryPopup.waitForURL(/oauth\/v2\/authorize/);
+            released.resolve();
+            await completed.promise;
+            const cancellation = await cancelled;
+            expect(
+              cancellation.request().postDataJSON(),
+              "only the old session is cancelled",
+            ).toEqual({ state: oldState });
+            expect(
+              retryPopup.isClosed(),
+              "the old response cannot close the new sign-in window",
+            ).toBe(false);
+            expect(
+              page.context().pages(),
+              "only the replacement sign-in window remains",
+            ).toHaveLength(2);
+            await page.getByText("Continue in the sign-in window", { exact: true }).waitFor();
+            await retryPopup.route("https://emulators.dev/oauth/v2/authorize/callback", (route) =>
+              route.continue({ url: `${emulator.baseUrl}/oauth/v2/authorize/callback` }),
+            );
+            await retryPopup.getByRole("button", { name: /admin/ }).click();
+            await page.getByRole("dialog").waitFor({ state: "hidden", timeout: 30_000 });
+          });
+        } finally {
+          released.resolve();
+          await page.unrouteAll({ behavior: "wait" });
+        }
+      });
+      const connections = yield* client.connections.list({ query: { integration: slug } });
+      expect(connections, "the newer attempt survives and completes consent").toHaveLength(1);
+    }),
+  ),
+);

packages/react/src/components/add-account-modal.test.ts

@@ -366,6 +366,7 @@ describe("runCimdConnect", () => {
 
     const outcome = await runCimdConnect(
       {
+        isActive: () => true,
         reserve: (): OAuthPopupReservation => RESERVED,
         release: (): void => {},
         createClient: (args: CimdCreateArgs): Promise<OAuthClientSlug | null> => {
@@ -410,6 +411,7 @@ describe("runCimdConnect", () => {
 
     const outcome = await runCimdConnect(
       {
+        isActive: () => true,
         reserve: (): OAuthPopupReservation => RESERVED,
         release: (): void => {},
         createClient: (): Promise<OAuthClientSlug | null> => {
@@ -1076,6 +1078,7 @@ describe("runCimdConnect popup reservation", () => {
     const popup = popupSpy();
     const outcome = await runCimdConnect(
       {
+        isActive: () => true,
         ...popup,
         createClient: (args: CimdCreateArgs): Promise<OAuthClientSlug> => {
           popup.calls.push("createClient");
@@ -1096,6 +1099,7 @@ describe("runCimdConnect popup reservation", () => {
     const popup = popupSpy();
     const outcome = await runCimdConnect(
       {
+        isActive: () => true,
         ...popup,
         createClient: (): Promise<OAuthClientSlug | null> => Promise.resolve(null),
         start: (): void => {},
@@ -1107,10 +1111,32 @@ describe("runCimdConnect popup reservation", () => {
     expect(popup.calls).toEqual(["reserve", "release"]);
   });
 
+  it("does not start sign-in after cancellation during client setup", async () => {
+    const created = Promise.withResolvers<OAuthClientSlug | null>();
+    const popup = popupSpy();
+    let active = true;
+    const connecting = runCimdConnect(
+      {
+        ...popup,
+        isActive: () => active,
+        createClient: () => created.promise,
+        start: () => {
+          popup.calls.push("start");
+        },
+      },
+      cimdInput,
+    );
+    active = false;
+    created.resolve(OAuthClientSlug.make("cancelled-client"));
+    expect(await connecting).toEqual({ kind: "aborted" });
+    expect(popup.calls).toEqual(["reserve", "release"]);
+  });
+
   it("never claims a window when the method is missing its endpoints", async () => {
     const popup = popupSpy();
     const outcome = await runCimdConnect(
       {
+        isActive: () => true,
         ...popup,
         createClient: (): Promise<OAuthClientSlug | null> => Promise.resolve(null),
         start: (): void => {},

packages/react/src/components/add-account-modal.tsx

@@ -741,6 +741,7 @@ type CimdCreateClientArgs = {
 };
 
 type RunCimdConnectDeps = {
+  readonly isActive: () => boolean;
   readonly createClient: (args: CimdCreateClientArgs) => Promise<OAuthClientSlug | null>;
   readonly start: (args: CimdStartArgs) => void;
   /** Claim the sign-in window before any await. See `useOAuthPopupFlow.reserve`. */
@@ -767,6 +768,7 @@ type RunCimdConnectInput = {
 
 type CimdOutcome =
   | { readonly kind: "started"; readonly client: OAuthClientSlug; readonly reused: boolean }
+  | { readonly kind: "aborted" }
   | { readonly kind: "popup-blocked" }
   | { readonly kind: "failed"; readonly reason: "missing-endpoints" | "create-failed" };
 
@@ -830,6 +832,10 @@ export async function runCimdConnect(
   if (reservation.kind === "blocked") return { kind: "popup-blocked" };
 
   const resolved = await resolveCimdClient(deps, input);
+  if (!deps.isActive()) {
+    deps.release();
+    return { kind: "aborted" };
+  }
   if (resolved.kind === "failed") {
     deps.release();
     return resolved;
@@ -1866,7 +1872,14 @@ function AddAccountModalView(props: AddAccountModalProps) {
   const oauthBusy = ccBusy || oauthPopup.busy;
   const cimdConnecting = cimdBusy || oauthPopup.busy;
   const dcrConnecting = dcrBusy || oauthPopup.busy;
-  const automaticOAuthConnecting = cimdConnecting || dcrConnecting;
+  const signInPending = cimdBusy || dcrBusy || oauthPopup.busy;
+  const automaticAttemptRef = useRef(0);
+  const cancelSignIn = () => {
+    automaticAttemptRef.current += 1;
+    oauthPopup.cancel();
+    setCimdBusy(false);
+    setDcrBusy(false);
+  };
 
   // "Connection saved to" for a PICKED BYO OAuth app. Cloud: a Workspace (`org`)
   // app can mint Personal or Workspace connections; a Personal (`user`) app can
@@ -2359,11 +2372,16 @@ function AddAccountModalView(props: AddAccountModalProps) {
     const cimdOwner = owner;
     const connectionName = previewConnectionName(label, cimdOwner);
     const identityLabel = typedIdentityLabel(label);
+    const attempt = ++automaticAttemptRef.current;
+    const isActive = () => viewMountedRef.current && attempt === automaticAttemptRef.current;
     setCimdBusy(true);
     const outcome = await runCimdConnect(
       {
         reserve: oauthPopup.reserve,
-        release: oauthPopup.releaseReservation,
+        release: () => {
+          if (attempt === automaticAttemptRef.current) oauthPopup.releaseReservation();
+        },
+        isActive,
         createClient: createCimdClient,
         start: (args: CimdStartArgs): void => {
           void oauthPopup.start({
@@ -2398,6 +2416,7 @@ function AddAccountModalView(props: AddAccountModalProps) {
         existingClients: clientSummaries,
       },
     );
+    if (!isActive()) return;
     setCimdBusy(false);
     trackEvent("connection_oauth_started", {
       integration_slug: String(integration),
@@ -2442,16 +2461,18 @@ function AddAccountModalView(props: AddAccountModalProps) {
         setDcrFailed(true);
         return;
       }
+      const attempt = ++automaticAttemptRef.current;
+      const isActive = () => viewMountedRef.current && attempt === automaticAttemptRef.current;
       setDcrBusy(true);
       const outcome = await runAutomaticOAuthConnect(
         {
           reserve: oauthPopup.reserve,
-          release: oauthPopup.releaseReservation,
-          // Closing the modal genuinely unmounts this view (see
-          // `AddAccountModal`), so "still mounted" is exactly "still open".
-          // The sequence checks it between round trips: a close mid-flight
-          // must not register a client or launch the popup afterwards.
-          isActive: () => viewMountedRef.current,
+          release: () => {
+            if (attempt === automaticAttemptRef.current) oauthPopup.releaseReservation();
+          },
+          // A closed modal or cancelled attempt cannot register a client or
+          // launch sign-in after the next round trip. A retry owns a new attempt.
+          isActive,
           probe: async (url: string): Promise<OAuthProbeResult | null> => {
             const exit = await doProbe({ payload: { url }, reactivityKeys: [] });
             if (Exit.isFailure(exit)) return null;
@@ -2560,7 +2581,7 @@ function AddAccountModalView(props: AddAccountModalProps) {
       // The modal closed mid-flight: this view is unmounted, so no state may
       // be written at all — not the fallback below, and not even the busy
       // flag, which belongs to the surface that is gone.
-      if (outcome.kind === "aborted") return;
+      if (!isActive() || outcome.kind === "aborted") return;
       setDcrBusy(false);
       // `connection_oauth_started` measures the connect funnel; a reconnect
       // reports through `connection_reconnected` on the popup callbacks above,
@@ -3371,23 +3392,38 @@ function AddAccountModalView(props: AddAccountModalProps) {
               </p>
             ) : null}
             <DialogFooter>
-              <Button
-                type="button"
-                variant="ghost"
-                onClick={close}
-                disabled={submitting || oauthBusy || automaticOAuthConnecting}
-              >
+              {signInPending ? (
+                <p role="status" className="flex-1 self-center text-xs text-muted-foreground">
+                  {oauthPopup.phase === "authorizing"
+                    ? "Continue in the sign-in window"
+                    : oauthPopup.phase === "saving"
+                      ? "Finishing connection…"
+                      : "Preparing sign-in…"}
+                </p>
+              ) : null}
+              <Button type="button" variant="ghost" onClick={close} disabled={submitting || ccBusy}>
                 {isOAuth ? "Close" : "Cancel"}
               </Button>
               {/* Footer action, in precedence order:
+              - pending sign-in: cancel, or wait for an authorized connection to save;
               - transparent CIMD (no app registration): create/reuse a public
                 metadata-document client and start OAuth;
               - transparent DCR (no picker): a single Connect that runs
                 probe → register → start;
               - registering a BYO app: the form owns its own submit, no footer;
               - picked BYO OAuth app: Connect with OAuth / Connect (client creds);
               - credential/no-auth method: Add connection. */}
-              {cimdActive ? (
+              {signInPending ? (
+                oauthPopup.phase === "saving" ? (
+                  <Button type="button" loading>
+                    Finishing…
+                  </Button>
+                ) : (
+                  <Button type="button" variant="outline" onClick={cancelSignIn}>
+                    Cancel sign-in
+                  </Button>
+                )
+              ) : cimdActive ? (
                 <Button
                   type="button"
                   onClick={() => void handleCimdConnect()}