History

Make OAuth sign-in easy to cancel and retry

Rhys Sullivan committed 1 month ago

Commit 1b19b371 parent c6d10835 files · +321 −58Browse files

Unified · Split

5 files
  1. .changeset/connection-oauth-cancel.md +5 −0
  2. e2e/scenarios/connection-setup-ux.test.ts +128 −0
  3. packages/react/src/components/add-account-modal.test.ts +26 −0
  4. packages/react/src/components/add-account-modal.tsx +83 −17
  5. packages/react/src/plugins/oauth-sign-in.tsx +79 −41
.changeset/connection-oauth-cancel.md+5−0
@@ -0,0 +1,5 @@
1---
2"executor": patch
3---
4
5Show a clear, cancellable sign-in state and prevent cancelled OAuth requests from disrupting a retry.
e2e/scenarios/connection-setup-ux.test.ts+128−0
@@ -288,3 +288,131 @@ scenario( · 287 unmodified lines
288 }),288 }),
289 ),289 ),
290);290);
291
292scenario(
293 "Connection setup · an interrupted OAuth popup can be cancelled and retried",
294 {},
295 Effect.scoped(
296 Effect.gen(function* () {
297 const { browser, identity, slug, client, emulator } = yield* fixture;
298 yield* browser.session(identity, async ({ page, step }) => {
299 await step("Start provider sign-in without entering a name", async () => {
300 await visit(page, `/integrations/${slug}?addAccount=1`);
301 await page.getByRole("tab", { name: "OAuth2", exact: true }).click();
302 const opened = page.waitForEvent("popup");
303 await page.getByRole("button", { name: "Connect with OAuth", exact: true }).click();
304 const popup = await opened;
305 await popup.waitForURL(/oauth\/v2\/authorize/);
306 await popup.close();
307 });
308 await step("Cancel the waiting sign-in and retry", async () => {
309 const cancel = page.getByRole("button", { name: "Cancel sign-in", exact: true });
310 expect(
311 await cancel.isVisible(),
312 "an interrupted provider window must not strand Connecting forever",
313 ).toBe(true);
314 expect(await cancel.isEnabled()).toBe(true);
315 await page.getByText("Continue in the sign-in window", { exact: true }).waitFor();
316 expect(
317 await page.getByRole("button", { name: "Connecting…", exact: true }).count(),
318 "waiting for provider consent must not leave a dead Connecting button",
319 ).toBe(0);
320 await cancel.click();
321 expect(
322 await page.getByRole("button", { name: "Connect with OAuth", exact: true }).isEnabled(),
323 ).toBe(true);
324 const opened = page.waitForEvent("popup");
325 await page.getByRole("button", { name: "Connect with OAuth", exact: true }).click();
326 const popup = await opened;
327 await popup.waitForURL(/oauth\/v2\/authorize/);
328 await popup.route("https://emulators.dev/oauth/v2/authorize/callback", (route) =>
329 route.continue({ url: `${emulator.baseUrl}/oauth/v2/authorize/callback` }),
330 );
331 await popup.getByRole("button", { name: /admin/ }).click();
332 await page.getByRole("dialog").waitFor({ state: "hidden", timeout: 30_000 });
333 });
334 });
335 const connections = yield* client.connections.list({ query: { integration: slug } });
336 expect(connections, "retry finishes and persists exactly one connection").toHaveLength(1);
337 }),
338 ),
339);
340
341scenario(
342 "Connection setup · cancelling a pending OAuth start ignores its late response",
343 {},
344 Effect.scoped(
345 Effect.gen(function* () {
346 const { browser, identity, slug, client, emulator } = yield* fixture;
347 yield* browser.session(identity, async ({ page, step }) => {
348 const released = Promise.withResolvers<void>();
349 const started = Promise.withResolvers<string>();
350 const completed = Promise.withResolvers<void>();
351 await page.route(
352 "**/api/oauth/start",
353 async (route) => {
354 const response = await route.fetch();
355 const body: unknown = await response.json();
356 if (
357 typeof body !== "object" ||
358 body === null ||
359 !("state" in body) ||
360 typeof body.state !== "string"
361 ) {
362 expect.fail("the held start response must contain its OAuth session state");
363 }
364 started.resolve(body.state);
365 await released.promise;
366 await route.fulfill({ response });
367 completed.resolve();
368 },
369 { times: 1 },
370 );
371 try {
372 await step("Cancel while the authorization response is still in flight", async () => {
373 await visit(page, `/integrations/${slug}?addAccount=1`);
374 const opened = page.waitForEvent("popup");
375 await page.getByRole("button", { name: "Connect with OAuth", exact: true }).click();
376 const popup = await opened;
377 const oldState = await started.promise;
378 await page.getByRole("button", { name: "Cancel sign-in", exact: true }).click();
379 await expect.poll(() => popup.isClosed()).toBe(true);
380 const cancelled = page.waitForResponse((response) =>
381 response.url().endsWith("/api/oauth/cancel"),
382 );
383 const retryOpened = page.waitForEvent("popup");
384 await page.getByRole("button", { name: "Connect with OAuth", exact: true }).click();
385 const retryPopup = await retryOpened;
386 await retryPopup.waitForURL(/oauth\/v2\/authorize/);
387 released.resolve();
388 await completed.promise;
389 const cancellation = await cancelled;
390 expect(
391 cancellation.request().postDataJSON(),
392 "only the old session is cancelled",
393 ).toEqual({ state: oldState });
394 expect(
395 retryPopup.isClosed(),
396 "the old response cannot close the new sign-in window",
397 ).toBe(false);
398 expect(
399 page.context().pages(),
400 "only the replacement sign-in window remains",
401 ).toHaveLength(2);
402 await page.getByText("Continue in the sign-in window", { exact: true }).waitFor();
403 await retryPopup.route("https://emulators.dev/oauth/v2/authorize/callback", (route) =>
404 route.continue({ url: `${emulator.baseUrl}/oauth/v2/authorize/callback` }),
405 );
406 await retryPopup.getByRole("button", { name: /admin/ }).click();
407 await page.getByRole("dialog").waitFor({ state: "hidden", timeout: 30_000 });
408 });
409 } finally {
410 released.resolve();
411 await page.unrouteAll({ behavior: "wait" });
412 }
413 });
414 const connections = yield* client.connections.list({ query: { integration: slug } });
415 expect(connections, "the newer attempt survives and completes consent").toHaveLength(1);
416 }),
417 ),
418);
packages/react/src/components/add-account-modal.test.ts+26−0
@@ -366,6 +366,7 @@ describe("runCimdConnect", () => { · 365 unmodified lines
366 366
367 const outcome = await runCimdConnect(367 const outcome = await runCimdConnect(
368 {368 {
369 isActive: () => true,
369 reserve: (): OAuthPopupReservation => RESERVED,370 reserve: (): OAuthPopupReservation => RESERVED,
370 release: (): void => {},371 release: (): void => {},
371 createClient: (args: CimdCreateArgs): Promise<OAuthClientSlug | null> => {372 createClient: (args: CimdCreateArgs): Promise<OAuthClientSlug | null> => {
@@ -410,6 +411,7 @@ describe("runCimdConnect", () => { · 38 unmodified lines
410 411
411 const outcome = await runCimdConnect(412 const outcome = await runCimdConnect(
412 {413 {
414 isActive: () => true,
413 reserve: (): OAuthPopupReservation => RESERVED,415 reserve: (): OAuthPopupReservation => RESERVED,
414 release: (): void => {},416 release: (): void => {},
415 createClient: (): Promise<OAuthClientSlug | null> => {417 createClient: (): Promise<OAuthClientSlug | null> => {
@@ -1076,6 +1078,7 @@ describe("runCimdConnect popup reservation", () => { · 660 unmodified lines
1076 const popup = popupSpy();1078 const popup = popupSpy();
1077 const outcome = await runCimdConnect(1079 const outcome = await runCimdConnect(
1078 {1080 {
1081 isActive: () => true,
1079 ...popup,1082 ...popup,
1080 createClient: (args: CimdCreateArgs): Promise<OAuthClientSlug> => {1083 createClient: (args: CimdCreateArgs): Promise<OAuthClientSlug> => {
1081 popup.calls.push("createClient");1084 popup.calls.push("createClient");
@@ -1096,6 +1099,7 @@ describe("runCimdConnect popup reservation", () => { · 14 unmodified lines
1096 const popup = popupSpy();1099 const popup = popupSpy();
1097 const outcome = await runCimdConnect(1100 const outcome = await runCimdConnect(
1098 {1101 {
1102 isActive: () => true,
1099 ...popup,1103 ...popup,
1100 createClient: (): Promise<OAuthClientSlug | null> => Promise.resolve(null),1104 createClient: (): Promise<OAuthClientSlug | null> => Promise.resolve(null),
1101 start: (): void => {},1105 start: (): void => {},
@@ -1107,10 +1111,32 @@ describe("runCimdConnect popup reservation", () => { · 5 unmodified lines
1107 expect(popup.calls).toEqual(["reserve", "release"]);1111 expect(popup.calls).toEqual(["reserve", "release"]);
1108 });1112 });
1109 1113
1114 it("does not start sign-in after cancellation during client setup", async () => {
1115 const created = Promise.withResolvers<OAuthClientSlug | null>();
1116 const popup = popupSpy();
1117 let active = true;
1118 const connecting = runCimdConnect(
1119 {
1120 ...popup,
1121 isActive: () => active,
1122 createClient: () => created.promise,
1123 start: () => {
1124 popup.calls.push("start");
1125 },
1126 },
1127 cimdInput,
1128 );
1129 active = false;
1130 created.resolve(OAuthClientSlug.make("cancelled-client"));
1131 expect(await connecting).toEqual({ kind: "aborted" });
1132 expect(popup.calls).toEqual(["reserve", "release"]);
1133 });
1134
1110 it("never claims a window when the method is missing its endpoints", async () => {1135 it("never claims a window when the method is missing its endpoints", async () => {
1111 const popup = popupSpy();1136 const popup = popupSpy();
1112 const outcome = await runCimdConnect(1137 const outcome = await runCimdConnect(
1113 {1138 {
1139 isActive: () => true,
1114 ...popup,1140 ...popup,
1115 createClient: (): Promise<OAuthClientSlug | null> => Promise.resolve(null),1141 createClient: (): Promise<OAuthClientSlug | null> => Promise.resolve(null),
1116 start: (): void => {},1142 start: (): void => {},
packages/react/src/components/add-account-modal.tsx+83−17
@@ -100,7 +100,15 @@ import { · 99 unmodified lines
100 DropdownMenuItem,100 DropdownMenuItem,
101 DropdownMenuTrigger,101 DropdownMenuTrigger,
102} from "./dropdown-menu";102} from "./dropdown-menu";
103import { ChevronDown, EyeIcon, EyeOffIcon, PlusIcon, XIcon } from "lucide-react";103import {
104 ChevronDown,
105 EyeIcon,
106 EyeOffIcon,
107 LoaderCircleIcon,
108 PanelsTopLeftIcon,
109 PlusIcon,
110 XIcon,
111} from "lucide-react";
104import {112import {
105 Dialog,113 Dialog,
106 DialogContent,114 DialogContent,
@@ -741,6 +749,7 @@ type CimdCreateClientArgs = { · 634 unmodified lines
741};749};
742 750
743type RunCimdConnectDeps = {751type RunCimdConnectDeps = {
752 readonly isActive: () => boolean;
744 readonly createClient: (args: CimdCreateClientArgs) => Promise<OAuthClientSlug | null>;753 readonly createClient: (args: CimdCreateClientArgs) => Promise<OAuthClientSlug | null>;
745 readonly start: (args: CimdStartArgs) => void;754 readonly start: (args: CimdStartArgs) => void;
746 /** Claim the sign-in window before any await. See `useOAuthPopupFlow.reserve`. */755 /** Claim the sign-in window before any await. See `useOAuthPopupFlow.reserve`. */
@@ -767,6 +776,7 @@ type RunCimdConnectInput = { · 20 unmodified lines
767 776
768type CimdOutcome =777type CimdOutcome =
769 | { readonly kind: "started"; readonly client: OAuthClientSlug; readonly reused: boolean }778 | { readonly kind: "started"; readonly client: OAuthClientSlug; readonly reused: boolean }
779 | { readonly kind: "aborted" }
770 | { readonly kind: "popup-blocked" }780 | { readonly kind: "popup-blocked" }
771 | { readonly kind: "failed"; readonly reason: "missing-endpoints" | "create-failed" };781 | { readonly kind: "failed"; readonly reason: "missing-endpoints" | "create-failed" };
772 782
@@ -830,6 +840,10 @@ export async function runCimdConnect( · 57 unmodified lines
830 if (reservation.kind === "blocked") return { kind: "popup-blocked" };840 if (reservation.kind === "blocked") return { kind: "popup-blocked" };
831 841
832 const resolved = await resolveCimdClient(deps, input);842 const resolved = await resolveCimdClient(deps, input);
843 if (!deps.isActive()) {
844 deps.release();
845 return { kind: "aborted" };
846 }
833 if (resolved.kind === "failed") {847 if (resolved.kind === "failed") {
834 deps.release();848 deps.release();
835 return resolved;849 return resolved;
@@ -1866,7 +1880,14 @@ function AddAccountModalView(props: AddAccountModalProps) { · 1030 unmodified lines
1866 const oauthBusy = ccBusy || oauthPopup.busy;1880 const oauthBusy = ccBusy || oauthPopup.busy;
1867 const cimdConnecting = cimdBusy || oauthPopup.busy;1881 const cimdConnecting = cimdBusy || oauthPopup.busy;
1868 const dcrConnecting = dcrBusy || oauthPopup.busy;1882 const dcrConnecting = dcrBusy || oauthPopup.busy;
1869 const automaticOAuthConnecting = cimdConnecting || dcrConnecting;1883 const signInPending = cimdBusy || dcrBusy || oauthPopup.busy;
1884 const automaticAttemptRef = useRef(0);
1885 const cancelSignIn = () => {
1886 automaticAttemptRef.current += 1;
1887 oauthPopup.cancel();
1888 setCimdBusy(false);
1889 setDcrBusy(false);
1890 };
1870 1891
1871 // "Connection saved to" for a PICKED BYO OAuth app. Cloud: a Workspace (`org`)1892 // "Connection saved to" for a PICKED BYO OAuth app. Cloud: a Workspace (`org`)
1872 // app can mint Personal or Workspace connections; a Personal (`user`) app can1893 // app can mint Personal or Workspace connections; a Personal (`user`) app can
@@ -2359,11 +2380,16 @@ function AddAccountModalView(props: AddAccountModalProps) { · 486 unmodified lines
2359 const cimdOwner = owner;2380 const cimdOwner = owner;
2360 const connectionName = previewConnectionName(label, cimdOwner);2381 const connectionName = previewConnectionName(label, cimdOwner);
2361 const identityLabel = typedIdentityLabel(label);2382 const identityLabel = typedIdentityLabel(label);
2383 const attempt = ++automaticAttemptRef.current;
2384 const isActive = () => viewMountedRef.current && attempt === automaticAttemptRef.current;
2362 setCimdBusy(true);2385 setCimdBusy(true);
2363 const outcome = await runCimdConnect(2386 const outcome = await runCimdConnect(
2364 {2387 {
2365 reserve: oauthPopup.reserve,2388 reserve: oauthPopup.reserve,
2366 release: oauthPopup.releaseReservation,2389 release: () => {
2390 if (attempt === automaticAttemptRef.current) oauthPopup.releaseReservation();
2391 },
2392 isActive,
2367 createClient: createCimdClient,2393 createClient: createCimdClient,
2368 start: (args: CimdStartArgs): void => {2394 start: (args: CimdStartArgs): void => {
2369 void oauthPopup.start({2395 void oauthPopup.start({
@@ -2398,6 +2424,7 @@ function AddAccountModalView(props: AddAccountModalProps) { · 28 unmodified lines
2398 existingClients: clientSummaries,2424 existingClients: clientSummaries,
2399 },2425 },
2400 );2426 );
2427 if (!isActive()) return;
2401 setCimdBusy(false);2428 setCimdBusy(false);
2402 trackEvent("connection_oauth_started", {2429 trackEvent("connection_oauth_started", {
2403 integration_slug: String(integration),2430 integration_slug: String(integration),
@@ -2442,16 +2469,18 @@ function AddAccountModalView(props: AddAccountModalProps) { · 38 unmodified lines
2442 setDcrFailed(true);2469 setDcrFailed(true);
2443 return;2470 return;
2444 }2471 }
2472 const attempt = ++automaticAttemptRef.current;
2473 const isActive = () => viewMountedRef.current && attempt === automaticAttemptRef.current;
2445 setDcrBusy(true);2474 setDcrBusy(true);
2446 const outcome = await runAutomaticOAuthConnect(2475 const outcome = await runAutomaticOAuthConnect(
2447 {2476 {
2448 reserve: oauthPopup.reserve,2477 reserve: oauthPopup.reserve,
2449 release: oauthPopup.releaseReservation,2478 release: () => {
2450 // Closing the modal genuinely unmounts this view (see2479 if (attempt === automaticAttemptRef.current) oauthPopup.releaseReservation();
2451 // `AddAccountModal`), so "still mounted" is exactly "still open".2480 },
2452 // The sequence checks it between round trips: a close mid-flight2481 // A closed modal or cancelled attempt cannot register a client or
2453 // must not register a client or launch the popup afterwards.2482 // launch sign-in after the next round trip. A retry owns a new attempt.
2454 isActive: () => viewMountedRef.current,2483 isActive,
2455 probe: async (url: string): Promise<OAuthProbeResult | null> => {2484 probe: async (url: string): Promise<OAuthProbeResult | null> => {
2456 const exit = await doProbe({ payload: { url }, reactivityKeys: [] });2485 const exit = await doProbe({ payload: { url }, reactivityKeys: [] });
2457 if (Exit.isFailure(exit)) return null;2486 if (Exit.isFailure(exit)) return null;
@@ -2560,7 +2589,7 @@ function AddAccountModalView(props: AddAccountModalProps) { · 102 unmodified lines
2560 // The modal closed mid-flight: this view is unmounted, so no state may2589 // The modal closed mid-flight: this view is unmounted, so no state may
2561 // be written at all — not the fallback below, and not even the busy2590 // be written at all — not the fallback below, and not even the busy
2562 // flag, which belongs to the surface that is gone.2591 // flag, which belongs to the surface that is gone.
2563 if (outcome.kind === "aborted") return;2592 if (!isActive() || outcome.kind === "aborted") return;
2564 setDcrBusy(false);2593 setDcrBusy(false);
2565 // `connection_oauth_started` measures the connect funnel; a reconnect2594 // `connection_oauth_started` measures the connect funnel; a reconnect
2566 // reports through `connection_reconnected` on the popup callbacks above,2595 // reports through `connection_reconnected` on the popup callbacks above,
@@ -2720,7 +2749,49 @@ function AddAccountModalView(props: AddAccountModalProps) { · 153 unmodified lines
2720 : "sm:max-w-xl",2749 : "sm:max-w-xl",
2721 )}2750 )}
2722 >2751 >
2723 {addingMethod && createCustomMethod ? (2752 {signInPending ? (
2753 <>
2754 <DialogHeader>
2755 <DialogTitle>Add connection · {integrationName}</DialogTitle>
2756 </DialogHeader>
2757 <div role="status" className="flex flex-col items-center px-4 py-8 text-center">
2758 <div className="mb-5 flex size-12 items-center justify-center rounded-xl border border-border/70 bg-muted/30 text-foreground shadow-sm">
2759 {oauthPopup.phase === "authorizing" ? (
2760 <PanelsTopLeftIcon aria-hidden="true" className="size-5" />
2761 ) : (
2762 <LoaderCircleIcon
2763 aria-hidden="true"
2764 className="size-5 animate-spin motion-reduce:animate-none"
2765 />
2766 )}
2767 </div>
2768 <p className="text-sm font-medium text-foreground">
2769 {oauthPopup.phase === "authorizing"
2770 ? "Continue in the sign-in window"
2771 : oauthPopup.phase === "saving"
2772 ? "Finishing connection"
2773 : "Preparing sign-in"}
2774 </p>
2775 <DialogDescription className="mt-2 max-w-xs text-sm leading-relaxed">
2776 {oauthPopup.phase === "authorizing"
2777 ? "If the window closed or sign-in stalled, cancel and try again."
2778 : oauthPopup.phase === "saving"
2779 ? "Sign-in complete. Updating your connections."
2780 : "The sign-in window will be ready shortly. You can cancel at any time."}
2781 </DialogDescription>
2782 </div>
2783 <DialogFooter className="border-t border-border/60 pt-4 sm:justify-between">
2784 <Button type="button" variant="ghost" onClick={close}>
2785 Close
2786 </Button>
2787 {oauthPopup.phase !== "saving" ? (
2788 <Button type="button" variant="outline" onClick={cancelSignIn}>
2789 Cancel sign-in
2790 </Button>
2791 ) : null}
2792 </DialogFooter>
2793 </>
2794 ) : addingMethod && createCustomMethod ? (
2724 <>2795 <>
2725 <DialogHeader className="border-b border-border/60 px-5 py-4">2796 <DialogHeader className="border-b border-border/60 px-5 py-4">
2726 <DialogTitle className="text-base">Add authentication method</DialogTitle>2797 <DialogTitle className="text-base">Add authentication method</DialogTitle>
@@ -3371,12 +3442,7 @@ function AddAccountModalView(props: AddAccountModalProps) { · 644 unmodified lines
3371 </p>3442 </p>
3372 ) : null}3443 ) : null}
3373 <DialogFooter>3444 <DialogFooter>
3374 <Button3445 <Button type="button" variant="ghost" onClick={close} disabled={submitting || ccBusy}>
3375 type="button"
3376 variant="ghost"
3377 onClick={close}
3378 disabled={submitting || oauthBusy || automaticOAuthConnecting}
3379 >
3380 {isOAuth ? "Close" : "Cancel"}3446 {isOAuth ? "Close" : "Cancel"}
3381 </Button>3447 </Button>
3382 {/* Footer action, in precedence order:3448 {/* Footer action, in precedence order:
packages/react/src/plugins/oauth-sign-in.tsx+79−41
@@ -1,10 +1,12 @@
1import { useCallback, useEffect, useRef, useState } from "react";1import { useCallback, useContext, useEffect, useRef, useState } from "react";
2import { useAtomSet } from "@effect/atom-react";2import { RegistryContext, useAtomSet } from "@effect/atom-react";
3import * as Data from "effect/Data";3import * as Data from "effect/Data";
4import * as Effect from "effect/Effect";4import * as Effect from "effect/Effect";
5import * as Exit from "effect/Exit";5import * as Exit from "effect/Exit";
6import * as AtomRegistry from "effect/unstable/reactivity/AtomRegistry";
6 7
7import { cancelOAuth, oauthConnectionCompleted, startOAuth } from "../api/atoms";8import { oauthConnectionCompleted } from "../api/atoms";
9import { ExecutorApiClient } from "../api/client";
8import { trackEvent } from "../api/analytics";10import { trackEvent } from "../api/analytics";
9import { messageFromExit, messageFromUnknown, useReportHandledError } from "../api/error-reporting";11import { messageFromExit, messageFromUnknown, useReportHandledError } from "../api/error-reporting";
10import {12import {
@@ -186,12 +188,11 @@ export function useOAuthPopupFlow< · 175 unmodified lines
186 popupName,188 popupName,
187 startErrorMessage,189 startErrorMessage,
188 } = options;190 } = options;
189 const doStartOAuth = useAtomSet(startOAuth, { mode: "promiseExit" });191 const registry = useContext(RegistryContext);
190 const doCancelOAuth = useAtomSet(cancelOAuth, { mode: "promiseExit" });
191 const doOAuthConnectionCompleted = useAtomSet(oauthConnectionCompleted, { mode: "promiseExit" });192 const doOAuthConnectionCompleted = useAtomSet(oauthConnectionCompleted, { mode: "promiseExit" });
192 const reportHandledError = useReportHandledError();193 const reportHandledError = useReportHandledError();
193 const blockedMessage = popupBlockedMessage ?? POPUP_BLOCKED_MESSAGE;194 const blockedMessage = popupBlockedMessage ?? POPUP_BLOCKED_MESSAGE;
194 const [busy, setBusy] = useState(false);195 const [phase, setPhase] = useState<"idle" | "preparing" | "authorizing" | "saving">("idle");
195 const [error, setError] = useState<string | null>(null);196 const [error, setError] = useState<string | null>(null);
196 const cleanupRef = useRef<(() => void) | null>(null);197 const cleanupRef = useRef<(() => void) | null>(null);
197 const sessionRef = useRef<{ readonly state: string } | null>(null);198 const sessionRef = useRef<{ readonly state: string } | null>(null);
@@ -200,6 +201,7 @@ export function useOAuthPopupFlow< · 2 unmodified lines
200 // exists. Hold it here so cancel and unmount can close it, or abandoning a201 // exists. Hold it here so cancel and unmount can close it, or abandoning a
201 // connect mid-probe strands a blank popup on screen.202 // connect mid-probe strands a blank popup on screen.
202 const reservationRef = useRef<ReservedOAuthPopup | null>(null);203 const reservationRef = useRef<ReservedOAuthPopup | null>(null);
204 const generationRef = useRef(0);
203 205
204 const releaseReservation = useCallback(() => {206 const releaseReservation = useCallback(() => {
205 reservationRef.current?.popup.close();207 reservationRef.current?.popup.close();
@@ -208,23 +210,33 @@ export function useOAuthPopupFlow< · 2 unmodified lines
208 210
209 const cancelSession = useCallback(211 const cancelSession = useCallback(
210 (state: string) => {212 (state: string) => {
211 void doCancelOAuth({ payload: { state: OAuthState.make(state) } });213 // Each cancellation owns its request; cancelling an older attempt must
214 // not interrupt cleanup of another session.
215 const request = ExecutorApiClient.runtime.atom(
216 ExecutorApiClient.use((client) =>
217 client.oauth.cancel({ payload: { state: OAuthState.make(state) } }),
218 ),
219 );
220 void Effect.runPromiseExit(AtomRegistry.getResult(registry, request));
212 },221 },
213 [doCancelOAuth],222 [registry],
214 );223 );
215 224
216 const cancel = useCallback(() => {225 const cancel = useCallback(() => {
226 generationRef.current += 1;
227 setError(null);
217 const session = sessionRef.current;228 const session = sessionRef.current;
218 cleanupRef.current?.();229 cleanupRef.current?.();
219 cleanupRef.current = null;230 cleanupRef.current = null;
220 sessionRef.current = null;231 sessionRef.current = null;
221 releaseReservation();232 releaseReservation();
222 if (session) cancelSession(session.state);233 if (session) cancelSession(session.state);
223 setBusy(false);234 setPhase("idle");
224 }, [cancelSession, releaseReservation]);235 }, [cancelSession, releaseReservation]);
225 236
226 useEffect(237 useEffect(
227 () => () => {238 () => () => {
239 generationRef.current += 1;
228 const session = sessionRef.current;240 const session = sessionRef.current;
229 cleanupRef.current?.();241 cleanupRef.current?.();
230 cleanupRef.current = null;242 cleanupRef.current = null;
@@ -267,21 +279,20 @@ export function useOAuthPopupFlow< · 36 unmodified lines
267 // `reserve`; cancelling again here would close the window it reserved.279 // `reserve`; cancelling again here would close the window it reserved.
268 const reservation = input.reservation ?? reserve();280 const reservation = input.reservation ?? reserve();
269 if (reservation.kind === "blocked") {281 if (reservation.kind === "blocked") {
270 setBusy(false);282 setPhase("idle");
271 setError(blockedMessage);283 setError(blockedMessage);
272 input.onError?.(blockedMessage);284 input.onError?.(blockedMessage);
273 return;285 return;
274 }286 }
275 setBusy(true);287 setPhase("preparing");
276 setError(null);288 setError(null);
277 // Desktop hosts open the auth URL in the user's real browser, so they289 // Desktop hosts open the auth URL in the user's real browser, so they
278 // reserve no in-page window and rely on the polling channel for the290 // reserve no in-page window and rely on the polling channel for the
279 // result.291 // result.
280 const desktopBridge = reservation.kind === "desktop" ? reservation.bridge : null;292 const desktopBridge = reservation.kind === "desktop" ? reservation.bridge : null;
281 const reservedPopup = reservation.kind === "window" ? reservation.popup : null;293 const reservedPopup = reservation.kind === "window" ? reservation.popup : null;
282 // The window's lifetime now belongs to this flow's teardown, which closes294 // Cancel/unmount owns this window even while the start request is pending.
283 // it on every failure path below.295 const generation = generationRef.current;
284 reservationRef.current = null;
285 const startExit = await Effect.runPromiseExit(296 const startExit = await Effect.runPromiseExit(
286 Effect.tryPromise({297 Effect.tryPromise({
287 try: input.run,298 try: input.run,
@@ -292,6 +303,14 @@ export function useOAuthPopupFlow< · 4 unmodified lines
292 }),303 }),
293 }),304 }),
294 );305 );
306 if (generation !== generationRef.current) {
307 reservedPopup?.popup.close();
308 if (Exit.isSuccess(startExit) && startExit.value.authorizationUrl !== null) {
309 cancelSession(startExit.value.state);
310 }
311 return;
312 }
313 reservationRef.current = null;
295 if (Exit.isFailure(startExit)) {314 if (Exit.isFailure(startExit)) {
296 const message = messageFromExit(startExit, startErrorMessage ?? "Failed to start sign-in");315 const message = messageFromExit(startExit, startErrorMessage ?? "Failed to start sign-in");
297 reportHandledError(startExit.cause, {316 reportHandledError(startExit.cause, {
@@ -301,7 +320,7 @@ export function useOAuthPopupFlow< · 3 unmodified lines
301 metadata: input.reportMetadata,320 metadata: input.reportMetadata,
302 });321 });
303 reservedPopup?.popup.close();322 reservedPopup?.popup.close();
304 setBusy(false);323 setPhase("idle");
305 setError(message);324 setError(message);
306 input.onError?.(message);325 input.onError?.(message);
307 return;326 return;
@@ -311,29 +330,33 @@ export function useOAuthPopupFlow< · 3 unmodified lines
311 const message =330 const message =
312 noAuthorizationUrlMessage ?? "OAuth start did not produce an authorization URL";331 noAuthorizationUrlMessage ?? "OAuth start did not produce an authorization URL";
313 reservedPopup?.popup.close();332 reservedPopup?.popup.close();
314 setBusy(false);333 setPhase("idle");
315 setError(message);334 setError(message);
316 input.onError?.(message);335 input.onError?.(message);
317 return;336 return;
318 }337 }
319 338
339 setPhase("authorizing");
320 sessionRef.current = { state: response.state };340 sessionRef.current = { state: response.state };
321 input.onAuthorizationStarted?.(response);341 input.onAuthorizationStarted?.(response);
322 const handleResult = async (result: OAuthPopupResult<TPayload>) => {342 const handleResult = async (result: OAuthPopupResult<TPayload>) => {
343 if (generation !== generationRef.current) return;
323 cleanupRef.current = null;344 cleanupRef.current = null;
324 sessionRef.current = null;345 sessionRef.current = null;
325 346
326 if (!result.ok) {347 if (!result.ok) {
327 trackEvent("oauth_completed", { success: false });348 trackEvent("oauth_completed", { success: false });
328 setBusy(false);349 setPhase("idle");
329 setError(result.error);350 setError(result.error);
330 input.onError?.(result.error, result.errorDetails);351 input.onError?.(result.error, result.errorDetails);
331 return;352 return;
332 }353 }
333 354
355 setPhase("saving");
334 const refreshExit = await doOAuthConnectionCompleted({356 const refreshExit = await doOAuthConnectionCompleted({
335 reactivityKeys: connectionWriteKeys,357 reactivityKeys: connectionWriteKeys,
336 });358 });
359 if (generation !== generationRef.current) return;
337 if (Exit.isFailure(refreshExit)) {360 if (Exit.isFailure(refreshExit)) {
338 const message = messageFromExit(refreshExit, "Failed to refresh connection");361 const message = messageFromExit(refreshExit, "Failed to refresh connection");
339 reportHandledError(refreshExit.cause, {362 reportHandledError(refreshExit.cause, {
@@ -343,7 +366,7 @@ export function useOAuthPopupFlow< · 3 unmodified lines
343 metadata: input.reportMetadata,366 metadata: input.reportMetadata,
344 });367 });
345 trackEvent("oauth_completed", { success: false });368 trackEvent("oauth_completed", { success: false });
346 setBusy(false);369 setPhase("idle");
347 setError(message);370 setError(message);
348 input.onError?.(message);371 input.onError?.(message);
349 return;372 return;
@@ -362,15 +385,18 @@ export function useOAuthPopupFlow< · 12 unmodified lines
362 metadata: input.reportMetadata,385 metadata: input.reportMetadata,
363 });386 });
364 trackEvent("oauth_completed", { success: false });387 trackEvent("oauth_completed", { success: false });
365 setBusy(false);388 if (generation === generationRef.current) {
366 setError(message);389 setPhase("idle");
367 input.onError?.(message);390 setError(message);
391 input.onError?.(message);
392 }
368 return;393 return;
369 }394 }
370 trackEvent("oauth_completed", { success: true });395 trackEvent("oauth_completed", { success: true });
371 setBusy(false);396 if (generation === generationRef.current) setPhase("idle");
372 };397 };
373 const handleClosed = () => {398 const handleClosed = () => {
399 if (generation !== generationRef.current) return;
374 cleanupRef.current = null;400 cleanupRef.current = null;
375 sessionRef.current = null;401 sessionRef.current = null;
376 // `popup.closed` is advisory: COOP redirects can make a live popup402 // `popup.closed` is advisory: COOP redirects can make a live popup
@@ -379,16 +405,17 @@ export function useOAuthPopupFlow< · 2 unmodified lines
379 const message =405 const message =
380 popupClosedMessage ?? "Sign-in cancelled - popup was closed before completing the flow.";406 popupClosedMessage ?? "Sign-in cancelled - popup was closed before completing the flow.";
381 trackEvent("oauth_completed", { success: false });407 trackEvent("oauth_completed", { success: false });
382 setBusy(false);408 setPhase("idle");
383 setError(message);409 setError(message);
384 input.onError?.(message);410 input.onError?.(message);
385 };411 };
386 const handleOpenFailed = () => {412 const handleOpenFailed = () => {
413 if (generation !== generationRef.current) return;
387 cleanupRef.current = null;414 cleanupRef.current = null;
388 sessionRef.current = null;415 sessionRef.current = null;
389 cancelSession(response.state);416 cancelSession(response.state);
390 trackEvent("oauth_completed", { success: false });417 trackEvent("oauth_completed", { success: false });
391 setBusy(false);418 setPhase("idle");
392 setError(blockedMessage);419 setError(blockedMessage);
393 input.onError?.(blockedMessage);420 input.onError?.(blockedMessage);
394 };421 };
@@ -442,20 +469,29 @@ export function useOAuthPopupFlow< · 47 unmodified lines
442 name: String(input.payload.name),469 name: String(input.payload.name),
443 owner: input.payload.owner,470 owner: input.payload.owner,
444 },471 },
445 run: () =>472 run: () => {
446 doStartOAuth({473 // A shared mutation atom returns its latest result to every waiter.
447 payload: {474 // A fast retry could give the cancelled attempt the NEW session's
448 client: input.payload.client,475 // state and make it cancel that session. Keep one atom per attempt;
449 clientOwner: input.payload.clientOwner,476 // its subscription lives until this request settles, even on cancel.
450 owner: input.payload.owner,477 const request = ExecutorApiClient.runtime.atom(
451 name: input.payload.name,478 ExecutorApiClient.use((client) =>
452 integration: input.payload.integration,479 client.oauth.start({
453 template: input.payload.template,480 payload: {
454 identityLabel: input.payload.identityLabel,481 client: input.payload.client,
455 newConnection: input.payload.newConnection,482 clientOwner: input.payload.clientOwner,
456 redirectUri: input.payload.redirectUri ?? oauthCallbackUrl(callbackPath),483 owner: input.payload.owner,
457 },484 name: input.payload.name,
458 }).then((exit) =>485 integration: input.payload.integration,
486 template: input.payload.template,
487 identityLabel: input.payload.identityLabel,
488 newConnection: input.payload.newConnection,
489 redirectUri: input.payload.redirectUri ?? oauthCallbackUrl(callbackPath),
490 },
491 }),
492 ),
493 );
494 return Effect.runPromiseExit(AtomRegistry.getResult(registry, request)).then((exit) =>
459 Exit.isSuccess(exit)495 Exit.isSuccess(exit)
460 ? // The redirect branch carries `authorizationUrl` + `state`; the496 ? // The redirect branch carries `authorizationUrl` + `state`; the
461 // inline "connected" (client_credentials) branch has no URL to497 // inline "connected" (client_credentials) branch has no URL to
@@ -469,14 +505,16 @@ export function useOAuthPopupFlow< · 7 unmodified lines
469 message: messageFromExit(exit, startErrorMessage ?? "Failed to start sign-in"),505 message: messageFromExit(exit, startErrorMessage ?? "Failed to start sign-in"),
470 }),506 }),
471 ),507 ),
472 ),508 );
509 },
473 });510 });
474 },511 },
475 [callbackPath, doStartOAuth, openAuthorization, startErrorMessage],512 [callbackPath, registry, openAuthorization, startErrorMessage],
476 );513 );
477 514
478 return {515 return {
479 busy,516 busy: phase !== "idle",
517 phase,
480 error,518 error,
481 setError,519 setError,
482 start,520 start,