fix(modal): hold a dialog from the input that starts its work

A confirm that fired a mutation and returned `false` relied on the mutation's
`isPending` to hold the dialog, but query-core delivers that on a setTimeout(0).
The confirm's own `pending` had already ended by then, so for several
milliseconds the dialog read idle: an Escape right behind the click dismissed a
running ban, Cancel and Confirm were live, and focus jumped back to the field.

Every dialog passing `pending` to a confirm that starts work now returns that
work's promise, which holds it from the click's own commit.

A refused ban, role change or unban also read as success: better-auth's client
resolves with `{ error }` rather than throwing, so the mutation succeeded and
closed the dialog. They now throw, as impersonation already did.

The e2e check presses Escape right behind the click, past the point Chromium
closes natively, and on the refused ban. It replaces the jsdom test, which stubbed
showModal and close and so proved nothing about the browser.

Author: waleedlatif1

apps/sim/app/workspace/[workspaceId]/files/components/share-modal/use-share-dialog.ts

@@ -123,7 +123,8 @@ export function useShareDialog({
     onOpenChange(false)
   }
 
-  const submitPrimaryAction = () => {
+  /** Settles with the save, for a dialog to hold on. A failure stays on `upsertShare.error`. */
+  const submitPrimaryAction = async () => {
     if (!shareReadReady || upsertShare.isPending) return
 
     const base = { workspaceId, fileId, token: saved ? undefined : generateShortId() }
@@ -145,21 +146,22 @@ export function useShareDialog({
             }
           : { ...base, isActive: true as const, authType: 'public' as const }
 
-    upsertShare.mutate(vars, {
-      onSuccess: () => {
-        toast.success(PRIMARY_ACTION_SUCCESS_MESSAGES[primaryAction])
-        setUnshareConfirmOpen(false)
-        resetDraft()
-      },
-    })
+    try {
+      await upsertShare.mutateAsync(vars)
+    } catch {
+      return
+    }
+    toast.success(PRIMARY_ACTION_SUCCESS_MESSAGES[primaryAction])
+    setUnshareConfirmOpen(false)
+    resetDraft()
   }
 
-  const handlePrimaryAction = () => {
+  const handlePrimaryAction = async () => {
     if (isUnshareAction) {
       setUnshareConfirmOpen(true)
       return
     }
-    submitPrimaryAction()
+    await submitPrimaryAction()
   }
 
   const accessHint = (() => {

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/use-connector-actions.ts

@@ -121,13 +121,18 @@ export function useConnectorActions({
       pending: remove.isPending,
       disabled: actionsDisabled,
       error: remove.error,
-      onConfirm: () => {
+      /** Settles with the removal, for a dialog to hold on. A refusal stays on `removal.error`. */
+      onConfirm: async () => {
         if (!canEdit || actionsDisabled) return
-        remove.mutate({
-          knowledgeBaseId,
-          connectorId: connector.id,
-          deleteDocuments: requiresDocumentDeletion || deleteDocuments,
-        })
+        try {
+          await remove.mutateAsync({
+            knowledgeBaseId,
+            connectorId: connector.id,
+            deleteDocuments: requiresDocumentDeletion || deleteDocuments,
+          })
+        } catch {
+          return
+        }
       },
     },
   }

apps/sim/app/workspace/[workspaceId]/settings/components/admin/use-admin-console.ts

@@ -178,24 +178,30 @@ export function useAdminConsole() {
     setBanReason('')
   }
 
-  const handleConfirmBan = () => {
+  /** Settles with the ban, for a dialog to hold on. A refusal stays on `banUser.error`. */
+  const handleConfirmBan = async () => {
     if (pendingAction?.type !== 'ban') return
     const trimmedReason = banReason.trim()
-    banUser.mutate(
-      {
+    try {
+      await banUser.mutateAsync({
         userId: pendingAction.userId,
         ...(trimmedReason ? { banReason: trimmedReason } : {}),
-      },
-      { onSuccess: closePendingAction }
-    )
+      })
+      closePendingAction()
+    } catch {
+      return
+    }
   }
 
-  const handleConfirmRoleChange = () => {
+  /** Settles with the change, for a dialog to hold on. A refusal stays on `setUserRole.error`. */
+  const handleConfirmRoleChange = async () => {
     if (pendingAction?.type !== 'role') return
-    setUserRole.mutate(
-      { userId: pendingAction.userId, role: pendingAction.nextRole },
-      { onSuccess: closePendingAction }
-    )
+    try {
+      await setUserRole.mutateAsync({ userId: pendingAction.userId, role: pendingAction.nextRole })
+      closePendingAction()
+    } catch {
+      return
+    }
   }
 
   const pendingUserIds = new Set<string>()

apps/sim/components/integrations/use-slack-search-setup.ts

@@ -57,35 +57,41 @@ export function useSlackSearchSetup({
           (!configuredAppId || configuredAppId === prepare.data.sharedAppId)
       )
 
+  /**
+   * Settles with the start of consent, for a dialog to hold on. On desktop consent runs in the
+   * user's browser and they may close the dialog meanwhile, so there is nothing to hold on.
+   */
   function installShared() {
     const controller = new AbortController()
     nativeAbort.current = controller
-    oauth.mutate(
-      {
+    const started = oauth
+      .mutateAsync({
         organizationId,
         installationId,
         name,
         description,
         mode: 'shared',
         signal: controller.signal,
-      },
-      {
-        onSuccess: (result) => {
+      })
+      .then(
+        (result) => {
           if (result) window.location.assign(result.authorizationUrl)
           else onClose()
         },
-      }
-    )
+        () => undefined
+      )
+    return isDesktopApp() ? undefined : started
   }
 
-  function advance() {
+  /** A step forward is immediate; the last one settles with the connection, for a dialog to hold on. */
+  async function advance() {
     if (step === 'manifest') {
       setStep('token')
     } else if (step === 'token') {
       setStep('credentials')
     } else {
-      connect.mutate(
-        {
+      try {
+        await connect.mutateAsync({
           organizationId,
           installationId,
           name,
@@ -94,11 +100,11 @@ export function useSlackSearchSetup({
           ...(clientId.trim() ? { clientId: clientId.trim() } : {}),
           ...(clientSecret.trim() ? { clientSecret: clientSecret.trim() } : {}),
           ...(signingSecret.trim() ? { signingSecret: signingSecret.trim() } : {}),
-        },
-        {
-          onSuccess: onClose,
-        }
-      )
+        })
+      } catch {
+        return
+      }
+      onClose()
     }
   }
 

apps/sim/hooks/queries/admin-users.ts

@@ -219,6 +219,7 @@ export function useSetUserRole() {
   return useMutation({
     mutationFn: async ({ userId, role }: { userId: string; role: 'user' | 'admin' }) => {
       const result = await client.admin.setRole({ userId, role })
+      if (result.error) throw new Error(result.error.message ?? 'Failed to change the user role')
       return result
     },
     onSettled: () => queryClient.invalidateQueries({ queryKey: adminUserKeys.lists() }),
@@ -236,6 +237,7 @@ export function useBanUser() {
         userId,
         ...(banReason ? { banReason } : {}),
       })
+      if (result.error) throw new Error(result.error.message ?? 'Failed to ban user')
       return result
     },
     onSettled: () => queryClient.invalidateQueries({ queryKey: adminUserKeys.lists() }),
@@ -250,6 +252,7 @@ export function useUnbanUser() {
   return useMutation({
     mutationFn: async ({ userId }: { userId: string }) => {
       const result = await client.admin.unbanUser({ userId })
+      if (result.error) throw new Error(result.error.message ?? 'Failed to unban user')
       return result
     },
     onSettled: () => queryClient.invalidateQueries({ queryKey: adminUserKeys.lists() }),

apps/sim/platform/app/(shell)/_components/conversation/share-dialog.tsx

@@ -176,8 +176,8 @@ export function ShareDialog(props: ShareDialogProps) {
         confirmLabel={dialog.primaryLabel}
         confirmDisabled={dialog.primaryDisabled}
         destructive={dialog.isUnshareAction}
-        onConfirm={() => {
-          dialog.runPrimaryAction()
+        onConfirm={async () => {
+          await dialog.runPrimaryAction()
           return false
         }}
       >
@@ -255,8 +255,8 @@ export function ShareDialog(props: ShareDialogProps) {
         confirmLabel={pending ? 'Unsharing...' : 'Unshare'}
         confirmDisabled={pending}
         pending={pending}
-        onConfirm={() => {
-          dialog.confirmUnshare()
+        onConfirm={async () => {
+          await dialog.confirmUnshare()
           return false
         }}
       >

apps/sim/platform/app/(shell)/settings/_components/sections/admin/admin-section.tsx

@@ -302,8 +302,8 @@ export default function AdminSection() {
           confirmLabel='Ban'
           pendingLabel='Banning…'
           pending={admin.banUser.isPending}
-          onConfirm={() => {
-            admin.handleConfirmBan()
+          onConfirm={async () => {
+            await admin.handleConfirmBan()
             return false
           }}
         >
@@ -331,8 +331,8 @@ export default function AdminSection() {
           confirmLabel={admin.isDemotion ? 'Demote' : 'Promote'}
           pendingLabel={admin.isDemotion ? 'Demoting…' : 'Promoting…'}
           pending={admin.setUserRole.isPending}
-          onConfirm={() => {
-            admin.handleConfirmRoleChange()
+          onConfirm={async () => {
+            await admin.handleConfirmRoleChange()
             return false
           }}
         >

apps/sim/platform/app/(shell)/settings/_components/sections/connected-accounts/group-access.tsx

@@ -134,17 +134,16 @@ function WorkspaceAccessList({
         throw new Error(
           `Workspace access cannot exceed ${ORGANIZATION_ACCOUNT_WORKSPACE_LIMIT} workspaces`
         )
-      void save([...access.grants, grant], editor.revision)
-    } else {
-      if (grant.workspaceId !== editor.grant.workspaceId)
-        throw new Error('Cannot change the workspace of an existing grant')
-      void save(
-        access.grants.map((existing) =>
-          existing.workspaceId === grant.workspaceId ? grant : existing
-        ),
-        editor.revision
-      )
+      return save([...access.grants, grant], editor.revision)
     }
+    if (grant.workspaceId !== editor.grant.workspaceId)
+      throw new Error('Cannot change the workspace of an existing grant')
+    return save(
+      access.grants.map((existing) =>
+        existing.workspaceId === grant.workspaceId ? grant : existing
+      ),
+      editor.revision
+    )
   }
 
   return (

apps/sim/platform/app/(shell)/settings/_components/sections/connected-accounts/workspace-grant-modal.tsx

@@ -18,7 +18,8 @@ interface WorkspaceGrantModalBaseProps {
   credentialTypes: OrganizationAccountWorkspaceAccess['credentialTypes']
   disabled: boolean
   error?: string
-  onSave: (grant: Grant) => void
+  /** Settles with the save, for the dialog to hold on. */
+  onSave: (grant: Grant) => Promise<void>
   onClose: () => void
 }
 
@@ -38,7 +39,7 @@ export function WorkspaceGrantModal(props: WorkspaceGrantModalProps) {
     props.mode === 'edit' ? props.grant.access : { mode: 'selected', credentialTypes: [] }
   )
 
-  function save() {
+  async function save() {
     if (!workspaceId) throw new Error('Select a workspace before granting access')
     if (
       props.mode === 'create' &&
@@ -47,7 +48,7 @@ export function WorkspaceGrantModal(props: WorkspaceGrantModalProps) {
       throw new Error('Selected workspace is unavailable')
     if (access.mode === 'selected' && !access.credentialTypes.length)
       throw new Error('Select at least one integration')
-    onSave({ workspaceId, access })
+    await onSave({ workspaceId, access })
     return false
   }
 

apps/sim/platform/app/(shell)/settings/_components/sections/integrations/policy-modal.test.tsx

@@ -15,13 +15,15 @@ const mocks = vi.hoisted(() => ({ save: vi.fn() }))
 vi.mock('next/navigation', () => nextNavigationMock)
 vi.mock('@/hooks/queries/kb/connectors', () => kbConnectorsQueriesMock)
 vi.mock('@/hooks/queries/search-integrations', () => ({
-  useUpdateSearchIntegration: () => ({ mutate: mocks.save }),
+  useUpdateSearchIntegration: () => ({ mutateAsync: mocks.save }),
 }))
 const mockPush = nextNavigationMockFns.router.push
 const mockSources = kbConnectorsQueriesMockFns.mockUseSearchSources
 let root: Root
 let container: HTMLDivElement
 beforeEach(() => {
+  mocks.save.mockReset()
+  mocks.save.mockReturnValue(new Promise(() => {}))
   mockSources.mockReturnValue({ data: [], hasNextPage: false })
   vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true)
   Element.prototype.checkVisibility = () => true
@@ -80,15 +82,12 @@ describe('live search policy source preservation', () => {
       document.querySelector<HTMLInputElement>('input[type="radio"][value="member"]')?.click()
     )
     await act(async () => button('Save settings').click())
-    expect(mocks.save).toHaveBeenCalledWith(
-      {
-        organizationId: 'org',
-        connectorType: 'gmail',
-        approved: true,
-        policy: defaultLiveSearchPolicy(),
-      },
-      expect.any(Object)
-    )
+    expect(mocks.save).toHaveBeenCalledWith({
+      organizationId: 'org',
+      connectorType: 'gmail',
+      approved: true,
+      policy: defaultLiveSearchPolicy(),
+    })
   })
   it('can add another connection without clearing the currently configured source', async () => {
     mockSources.mockReturnValue({
@@ -120,15 +119,20 @@ describe('live search policy source preservation', () => {
         />
       )
     )
+    let saved = () => {}
+    mocks.save.mockReturnValue(
+      new Promise<void>((resolve) => {
+        saved = resolve
+      })
+    )
     await act(async () => button('Add connection').click())
     expect(mocks.save).toHaveBeenCalledWith(
       expect.objectContaining({
         policy: expect.objectContaining({ sourceId: 'current-source' }),
-      }),
-      expect.any(Object)
+      })
     )
     expect(mockPush).not.toHaveBeenCalled()
-    await act(async () => mocks.save.mock.calls.at(-1)?.[1].onSuccess())
+    await act(async () => saved())
     expect(mockPush).toHaveBeenCalledWith(
       '/o/org/settings/integrations/providers/gmail?addConnector=gmail'
     )
@@ -159,8 +163,7 @@ describe('live search policy source preservation', () => {
           accessMode: 'service_account',
           sourceId: 'configured-source',
         }),
-      }),
-      expect.any(Object)
+      })
     )
   })
 })