fix(mosaic): put contact verify, MFA and account actions under one section lock

.changeset/friendly-identifier-lock.md

@@ -0,0 +1,2 @@
+---
+---

.changeset/goofy-parents-trade.md

@@ -0,0 +1,2 @@
+---
+---

packages/mosaic/AGENTS.md

@@ -66,6 +66,8 @@ Most problems a feature hits already have a shared answer. Find the row for your
 | Deciding which part renders `Card.Root`                                  | Only the surface, never a screen or `*Steps`, see [Views](docs/views.md)                                                               |
 | An interaction with an async lifecycle or two values that move together  | A machine in the controller file, via `setup()` (`src/machine/`)                                                                       |
 | Styles several components share                                          | `src/styles/*.styles.ts`                                                                                                               |
+| Direct actions and confirmation dialogs that must exclude each other     | `usePendingAction.guard` on the confirmation (`src/hooks/`, `src/blocks/confirm/`)                                                     |
+| A machine-driven step that a lock or caller must await                   | `waitFor` (`src/machine/wait-for.ts`), inside `usePendingAction.run`                                                                   |
 
 ## Signs you are about to invent a pattern
 

packages/mosaic/docs/controllers.md

@@ -45,17 +45,52 @@ It owns the error (see "Errors"):
 const actions = usePendingAction({ errorFallback: messages.errors.generic });
 
 return {
-  rows: rows.map(row => (actions.errorKey === row.id ? { ...row, error: actions.error } : row)),
-  pendingId: actions.pendingKey,
+  rows: rows.map(row => ({
+    ...row,
+    isPending: actions.pendingKey === row.id,
+    error: actions.errorKey === row.id ? actions.error : undefined,
+  })),
   onRemove: id => void actions.run(id, () => onRemove(id)),
 };
 ```
 
+When a direct action and a confirmation must not run together, give both
+operations one `usePendingAction` owner:
+
+```ts
+const actions = usePendingAction();
+const remove = useConfirmController<UserProfileEmail>({
+  onConfirm: contact => removalFocus.remove(contact.id),
+});
+
+return {
+  contacts: contacts.map(contact => ({ ...contact, isPrimaryPending: actions.pendingKey === contact.id })),
+  isDisabled: actions.isPending,
+  remove: actions.guard(remove),
+};
+```
+
+Keep this coordination in the section controller. Run direct actions with
+`actions.run(key, action)` and pass the confirmation through `actions.guard`, which
+runs its `confirm` under the same lock. Pass `actions.isPending` to the section
+controls' `disabled` props.
+
+`confirm` resolves `true` when the operation succeeds, or `false` when it fails or
+the dialog ignores the call. Its promise follows the operation even if the
+dialog closes during cancellation. The shared pending owner waits for that
+promise, so closing the dialog does not enable another action before cancellation
+settles. Removal keeps its dialog errors, and inline row errors and focus keys
+stay unchanged. Put each row's pending state on the row the controller returns,
+so views render it without comparing pending keys.
+
 `run` resolves `true` on success, so the caller can close a dialog or move
 focus. To hold pending through an OAuth redirect, await a delay inside the
-action (`user-profile-connected-accounts-section.controller.ts`). When the user
-cancels a reverification challenge, `run` resolves `false` without showing an
-error and releases the lock. It is not for:
+action (`user-profile-connected-accounts-section.controller.ts`). To hold it
+through a step a machine drives, run an action that awaits `waitFor`
+(`machine/wait-for.ts`) until the machine leaves that state
+(`user-profile-add-email.controller.ts`). When the user cancels a
+reverification challenge, `run` resolves `false` without showing an error and
+releases the lock. It is not for:
 
 - a form → `useForm`
 - a confirm step → `useConfirmController` with `Confirmation` / `Destructive`

packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts

@@ -91,6 +91,45 @@ describe('confirmMachine', () => {
 });
 
 describe('useConfirmController', () => {
+  it.each(['resolve', 'reject'])(
+    'keeps its completion pending after cancellation until the action can %s',
+    async outcome => {
+      const pending = deferred<void>();
+      const { result } = renderHook(() =>
+        useConfirmController({
+          onConfirm: () => pending.promise,
+          reverification: { status: 'loading', visible: true, onCancel: vi.fn() },
+        }),
+      );
+      const completed = vi.fn();
+      act(() => result.current.onOpenChange(true));
+      let completion: ReturnType<typeof result.current.confirm> | undefined;
+      act(() => {
+        completion = result.current.confirm();
+      });
+      expect(completion).toBeInstanceOf(Promise);
+      void completion?.then(completed);
+      act(() => result.current.onOpenChange(false));
+      await act(async () => {
+        await Promise.resolve();
+      });
+      expect(result.current.isOpen).toBe(false);
+      expect(completed).not.toHaveBeenCalled();
+
+      await act(async () => {
+        if (outcome === 'reject') {
+          pending.reject(cancelled);
+        } else {
+          pending.resolve();
+        }
+        await completion;
+      });
+
+      expect(completed).toHaveBeenCalledWith(outcome === 'resolve');
+      expect(result.current.errorMessage).toBeUndefined();
+    },
+  );
+
   it('ignores open while no block is mounted on the controller, and keeps one handle across renders', () => {
     const { result, rerender } = renderHook(() => useConfirmController({ onConfirm: () => Promise.resolve() }));
     const { handle } = result.current;
@@ -107,17 +146,23 @@ describe('useConfirmController', () => {
     const onConfirm = vi.fn(() => pending.promise);
     const { result } = renderHook(() => useConfirmController({ onConfirm }));
 
-    act(() => result.current.confirm());
+    await act(async () => expect(await result.current.confirm()).toBe(false));
     expect(onConfirm).not.toHaveBeenCalled();
 
     act(() => result.current.onOpenChange(true));
-    act(() => result.current.confirm());
-    act(() => result.current.confirm());
+    const confirm = result.current.confirm;
+    let completion: ReturnType<typeof confirm> | undefined;
+    let duplicate: ReturnType<typeof confirm> | undefined;
+    act(() => {
+      completion = confirm();
+      duplicate = confirm();
+    });
     expect(onConfirm).toHaveBeenCalledOnce();
+    expect(await duplicate).toBe(false);
 
     await act(async () => {
       pending.resolve();
-      await pending.promise;
+      expect(await completion).toBe(true);
     });
     await waitFor(() => expect(result.current.isOpen).toBe(false));
   });

packages/mosaic/src/blocks/confirm/confirm-shell.tsx

@@ -69,7 +69,7 @@ function ConfirmCard<Payload>({
     cancelLabel,
     isPending: controller.isPending,
     errorMessage: controller.errorMessage,
-    confirm: () => controller.confirm(payload),
+    confirm: () => void controller.confirm(payload),
   });
 
   return (

packages/mosaic/src/blocks/confirm/confirm.controller.ts

@@ -80,8 +80,8 @@ export interface ConfirmController<Payload = void> {
   /** Opens the dialog about `payload` */
   open: (payload: Payload) => void;
   onOpenChange: (open: boolean) => void;
-  /** Runs the action for `payload`. The dialog closes when it resolves and shows why when it rejects. */
-  confirm: (payload: Payload) => void;
+  /** Runs the action for `payload`. The dialog closes when it resolves and shows why when it rejects. Resolves whether the action succeeded. */
+  confirm: (payload: Payload) => Promise<boolean>;
 }
 
 export interface ConfirmControllerOptions<Payload = void> {
@@ -136,15 +136,21 @@ export function useConfirmController<Payload = void>({
       send({ type: 'CANCEL' });
     },
     confirm: payload => {
-      if (snapshot.value !== 'confirming') {
-        return;
-      }
+      let completion: Promise<boolean> | undefined;
       send({
         type: 'CONFIRM',
-        run: async () => {
-          await onConfirm(payload);
+        run: () => {
+          const operation = (async () => {
+            await onConfirm(payload);
+          })();
+          completion = operation.then(
+            () => true,
+            () => false,
+          );
+          return operation;
         },
       });
+      return completion ?? Promise.resolve(false);
     },
   };
 }

packages/mosaic/src/blocks/confirmation/__tests__/confirmation.feature.test.tsx

@@ -82,6 +82,7 @@ describe('Confirmation', () => {
     await user.click(confirmButton());
     expect(onConfirm).toHaveBeenCalledOnce();
     expect(confirmButton()).toHaveAttribute('aria-busy', 'true');
+    expect(confirmButton()).toHaveFocus();
 
     run.resolve();
     await closed();

packages/mosaic/src/features/user-profile/user-profile-active-devices-section/user-profile-active-devices-section.controller.ts

@@ -84,7 +84,7 @@ export function useUserProfileActiveDevicesController({
     registerTrigger: removalFocus.registerTrigger,
     finalFocus: removalFocus.finalFocus,
     details,
-    signOutFromDetails: revoke ? device => details.confirm(device) : undefined,
+    signOutFromDetails: revoke ? device => void details.confirm(device) : undefined,
     signOut: revoke ? signOut : undefined,
     signOutAll: revokeAll ? signOutAll : undefined,
     // Confirming removes the other devices and the trigger with them — hence the dialog mounted

packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/__tests__/user-profile-connected-accounts.connect.feature.test.tsx

@@ -158,6 +158,27 @@ describe('connected accounts', () => {
     request.fail();
   });
 
+  it('holds connected account menus while a connection starts', async () => {
+    await renderSection([google]);
+    const request = holdRequests('post', '/v1/me/external_accounts');
+    const user = userEvent.setup();
+
+    try {
+      await user.click(screen.getByRole('button', { name: 'Connect GitHub' }));
+      await waitFor(() => expect(request.requests).toHaveLength(1));
+      const manage = screen.getByRole('button', { name: 'Manage Google' });
+      expect(manage).toHaveAttribute('aria-disabled', 'true');
+      await user.click(manage);
+      expect(screen.queryByRole('menu')).toBeNull();
+    } finally {
+      request.fail();
+    }
+
+    await waitFor(() =>
+      expect(screen.getByRole('button', { name: 'Manage Google' })).not.toHaveAttribute('aria-disabled', 'true'),
+    );
+  });
+
   it('verifies the session before connecting a provider, then continues to it', async () => {
     serveFapi(signedIn([], { verification: { secrets: { password: 'hunter2' } } }));
     const view = await renderWithClerk(<UserProfileConnectedAccountsSection />);

packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-account-row.view.tsx

@@ -19,13 +19,11 @@ import type {
 
 export function UserProfileConnectProviderRowView({
   provider,
-  isPending,
   isDisabled,
   onConnect,
   triggerRef,
 }: {
   provider: UserProfileConnectionProvider;
-  isPending: boolean;
   isDisabled: boolean;
   onConnect: (id: string) => void;
   triggerRef?: Ref<HTMLButtonElement>;
@@ -58,8 +56,8 @@ export function UserProfileConnectProviderRowView({
             variant='outline'
             color='neutral'
             aria-label={fill(m.connectLabel, { provider: provider.provider })}
-            isPending={isPending}
-            disabled={isDisabled && !isPending}
+            isPending={provider.isPending}
+            disabled={isDisabled && !provider.isPending}
             onClick={() => onConnect(provider.id)}
           >
             {m.connect}
@@ -91,7 +89,7 @@ export function UserProfileConnectedAccountRowView({
 }) {
   const m = useMessages('userProfile').connectedAccounts;
   const actions: ActionMenuAction[] = [];
-  if ((account.status === 'reconnect' || account.status === 'error') && onReconnect && !isDisabled) {
+  if ((account.status === 'reconnect' || account.status === 'error') && onReconnect) {
     actions.push({
       label: account.status === 'reconnect' ? m.reconnect : m.tryAgain,
       onClick: () => onReconnect(account.id),
@@ -131,6 +129,8 @@ export function UserProfileConnectedAccountRowView({
         {actions.length > 0 ? (
           <Section.Actions>
             <ActionMenu
+              disabled={isDisabled}
+              focusableWhenDisabled
               triggerRef={triggerRef}
               label={fill(m.manageLabel, { provider: account.provider })}
               actions={actions}

packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts

@@ -30,7 +30,7 @@ export interface UserProfileConnectedAccountsControllerOptions {
 export interface UserProfileConnectedAccountsController {
   accounts: UserProfileConnectedAccount[];
   availableProviders: UserProfileConnectionProvider[];
-  pendingId: string | undefined;
+  isDisabled: boolean;
   onConnect: (id: string) => void;
   onReconnect: (id: string) => void;
   sectionRef: Ref<HTMLDivElement>;
@@ -81,15 +81,17 @@ export function useUserProfileConnectedAccountsController({
     accounts: accounts.map(account =>
       actions.errorKey === account.id ? { ...account, reconnectError: actions.error } : account,
     ),
-    availableProviders: availableProviders.map(provider =>
-      actions.errorKey === provider.id ? { ...provider, connectError: actions.error } : provider,
-    ),
-    pendingId: actions.pendingKey,
+    availableProviders: availableProviders.map(provider => ({
+      ...provider,
+      isPending: actions.pendingKey === provider.id,
+      connectError: actions.errorKey === provider.id ? actions.error : undefined,
+    })),
+    isDisabled: actions.isPending,
     onConnect: id => run(id, onConnect),
     onReconnect: id => run(id, onReconnect),
     sectionRef,
     registerTrigger: removalFocus.registerTrigger,
-    remove: onRemove ? remove : undefined,
+    remove: onRemove ? actions.guard(remove) : undefined,
     finalFocus: removalFocus.finalFocus,
     reconnectReverification,
     reconnectFinalFocus: () => (actions.lastKey ? removalFocus.trigger(actions.lastKey) : null),

packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.types.ts

@@ -16,6 +16,7 @@ export interface ConnectedAccountProviderDisplay {
 export interface UserProfileConnectionProvider extends ConnectedAccountProviderDisplay {
   id: string;
   connectError?: string;
+  isPending?: boolean;
 }
 
 export interface UserProfileConnectedAccount extends ConnectedAccountProviderDisplay {
@@ -30,7 +31,7 @@ export interface UserProfileConnectedAccountsSectionViewProps {
   accounts: UserProfileConnectedAccount[];
   sectionRef?: Ref<HTMLDivElement>;
   availableProviders?: UserProfileConnectionProvider[];
-  pendingId?: string;
+  isDisabled?: boolean;
   onConnect?: (id: string) => void;
   onReconnect?: (id: string) => void;
   registerTrigger: (id: string) => Ref<HTMLButtonElement>;

packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.view.tsx

@@ -18,7 +18,7 @@ export function UserProfileConnectedAccountsSectionView({
   accounts,
   sectionRef,
   availableProviders = [],
-  pendingId,
+  isDisabled = false,
   onConnect,
   onReconnect,
   registerTrigger,
@@ -31,7 +31,6 @@ export function UserProfileConnectedAccountsSectionView({
 }: UserProfileConnectedAccountsSectionViewProps) {
   const m = useMessages('userProfile').connectedAccounts;
   const hasRows = accounts.length > 0 || (availableProviders.length > 0 && Boolean(onConnect));
-  const isBusy = pendingId !== undefined;
 
   return (
     <>
@@ -52,7 +51,7 @@ export function UserProfileConnectedAccountsSectionView({
                   key={account.id}
                   account={account}
                   triggerRef={registerTrigger(account.id)}
-                  isDisabled={isBusy}
+                  isDisabled={isDisabled}
                   onReconnect={onReconnect}
                   onRemove={remove ? account => remove.open(account) : undefined}
                 />
@@ -63,8 +62,7 @@ export function UserProfileConnectedAccountsSectionView({
                       key={provider.id}
                       provider={provider}
                       triggerRef={registerTrigger(provider.id)}
-                      isPending={pendingId === provider.id}
-                      isDisabled={isBusy}
+                      isDisabled={isDisabled}
                       onConnect={onConnect}
                     />
                   ))