refactor(mosaic): keep the last reverification card in the component, not the controller

.changeset/funky-suns-press.md

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

packages/mosaic/eslint-suppressions.json

@@ -29,11 +29,6 @@
       "count": 1
     }
   },
-  "src/features/reverification/reverification.controller.ts": {
-    "sonarjs/cognitive-complexity": {
-      "count": 1
-    }
-  },
   "src/features/reverification/use-reverification-with-state.ts": {
     "@typescript-eslint/consistent-type-assertions": {
       "count": 1

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

@@ -88,7 +88,7 @@ function ConfirmCard<Payload>({
               <Flow.Step ids={['confirm']}>{confirmStep}</Flow.Step>
               <Flow.Step ids={['verify']}>
                 <Reverification
-                  {...reverification}
+                  controller={reverification}
                   onClose={() => controller.onOpenChange(false)}
                 />
               </Flow.Step>

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

@@ -139,7 +139,6 @@ export function useConfirmController<Payload = void>({
       if (snapshot.value !== 'confirming') {
         return;
       }
-      reverification?.reset();
       send({
         type: 'CONFIRM',
         run: async () => {

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

@@ -297,12 +297,10 @@ describe('Confirmation shared by rows', () => {
 function ReverifiedHost({
   action,
   cleanupMs,
-  settleMs,
   beforeRunMs,
 }: {
   action: () => Promise<unknown>;
   cleanupMs?: number;
-  settleMs?: number;
   beforeRunMs?: number;
 }) {
   const [run, reverification] = useReverificationFlow(action);
@@ -312,11 +310,7 @@ function ReverifiedHost({
         await new Promise(resolve => setTimeout(resolve, beforeRunMs));
       }
       try {
-        const result = await run();
-        if (settleMs !== undefined) {
-          await new Promise(resolve => setTimeout(resolve, settleMs));
-        }
-        return result;
+        return await run();
       } catch (error) {
         if (cleanupMs !== undefined) {
           await new Promise(resolve => setTimeout(resolve, cleanupMs));
@@ -435,9 +429,13 @@ describe('Confirmation with reverification', () => {
   }
 
   for (const settleMs of [undefined, 400]) {
-    it(`does not go back to the confirm step after a successful verification (host settles after ${settleMs ?? 0}ms)`, async () => {
+    it(`does not go back to the confirm step after a successful verification (action settles after ${settleMs ?? 0}ms)`, async () => {
       serveFapi(passwordVerification);
-      const { user } = await renderReverified({ action: interruptedOnce(), settleMs });
+      const action = vi
+        .fn()
+        .mockResolvedValueOnce(reverificationError())
+        .mockImplementation(() => new Promise(resolve => setTimeout(() => resolve({ done: true }), settleMs ?? 0)));
+      const { user } = await renderReverified({ action });
       await confirmIntoVerification(user);
 
       const field = await screen.findByLabelText('Password');

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

@@ -7,7 +7,7 @@ import { describe, expect, it, vi } from 'vitest';
 import { deferred } from '../../../__tests__/async';
 import { clerkApiError } from '../../../__tests__/clerk-errors';
 import { openDialog } from '../../../__tests__/feature/dialog';
-import { serveFapi } from '../../../__tests__/feature/fake-fapi';
+import { holdRequests, serveFapi } from '../../../__tests__/feature/fake-fapi';
 import { fapiClient, fapiSession, fapiUser } from '../../../__tests__/feature/fapi';
 import { renderWithClerk } from '../../../__tests__/feature/render';
 import { Button } from '../../../components/button';
@@ -207,9 +207,17 @@ describe('Destructive shared by rows', () => {
   });
 });
 
-function ReverifiedHost({ action }: { action: () => Promise<unknown> }) {
+function ReverifiedHost({ action, before }: { action: () => Promise<unknown>; before?: () => Promise<void> }) {
   const [run, reverification] = useReverificationFlow(action);
-  const controller = useConfirmController({ onConfirm: run, reverification });
+  const controller = useConfirmController({
+    onConfirm: before
+      ? async () => {
+          await before();
+          await run();
+        }
+      : run,
+    reverification,
+  });
 
   return (
     <Destructive
@@ -253,3 +261,136 @@ describe('Destructive with reverification', () => {
     expect(action).toHaveBeenCalledTimes(2);
   });
 });
+
+function recordFrames() {
+  const frames: string[] = [];
+  const frame = () => {
+    const dialog = document.querySelector('[role="alertdialog"]');
+    if (!dialog) {
+      return 'gone';
+    }
+    const step = dialog.querySelector('.cl-flow-root')?.getAttribute('data-value') ?? 'none';
+    return `${step}/${dialog.hasAttribute('data-open') ? 'open' : 'closing'}`;
+  };
+  const push = () => {
+    const current = frame();
+    if (frames.at(-1) !== current) {
+      frames.push(current);
+    }
+  };
+  const observer = new MutationObserver(push);
+  observer.observe(document.body, {
+    subtree: true,
+    childList: true,
+    attributes: true,
+    attributeFilter: ['data-value', 'data-open', 'data-closed'],
+  });
+  push();
+  return { frames, stop: () => observer.disconnect() };
+}
+
+const VERIFY = '/v1/client/sessions/sess_1/verify';
+
+describe('Destructive with reverification frames', () => {
+  async function renderReverified(action: () => Promise<unknown>, before?: () => Promise<void>) {
+    serveFapi({
+      client: fapiClient([session]),
+      verification: { secrets: { password: 'hunter2' }, firstFactors: [{ strategy: 'password' as const }] },
+    });
+    await renderWithClerk(
+      <ReverifiedHost
+        action={action}
+        before={before}
+      />,
+    );
+    const user = userEvent.setup();
+    const open = () =>
+      openDialog(user, screen.getByRole('button', { name: 'Delete my account' }), {
+        role: 'alertdialog',
+        name: TITLE,
+      });
+    const closed = () => waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull());
+    return { user, open, closed };
+  }
+
+  it('closes from the verify step when the retried action succeeds', async () => {
+    const action = vi.fn().mockResolvedValueOnce(reverificationError()).mockResolvedValue({ done: true });
+    const { user, open, closed } = await renderReverified(action);
+    await open();
+    await user.type(field(), 'Delete{Enter}');
+    const password = await screen.findByLabelText('Password');
+    await waitFor(() => expect(screen.queryByText(TITLE)).toBeNull());
+
+    const recorder = recordFrames();
+    await user.type(password, 'hunter2{Enter}');
+    await closed();
+    recorder.stop();
+
+    expect(action).toHaveBeenCalledTimes(2);
+    expect(recorder.frames[0]).toBe('verify/open');
+    expect(recorder.frames).not.toContain('confirm/open');
+  });
+
+  it('closes from the verify step when the challenge is cancelled', async () => {
+    const action = vi.fn().mockResolvedValue(reverificationError());
+    const { user, open, closed } = await renderReverified(action);
+    await open();
+    await user.type(field(), 'Delete{Enter}');
+    await screen.findByLabelText('Password');
+    await waitFor(() => expect(screen.queryByText(TITLE)).toBeNull());
+
+    const recorder = recordFrames();
+    await user.click(screen.getByRole('button', { name: 'Close' }));
+    await closed();
+    recorder.stop();
+
+    expect(recorder.frames[0]).toBe('verify/open');
+    expect(recorder.frames).not.toContain('confirm/open');
+  });
+
+  it('shows the confirm step until the next challenge is ready when confirming again after a cancel', async () => {
+    const action = vi.fn().mockResolvedValue(reverificationError());
+    const { user, open, closed } = await renderReverified(action);
+    await open();
+    await user.type(field(), 'Delete{Enter}');
+    await screen.findByLabelText('Password');
+    await user.click(screen.getByRole('button', { name: 'Close' }));
+    await closed();
+
+    await open();
+    const verify = holdRequests('post', VERIFY);
+    const recorder = recordFrames();
+    await user.type(field(), 'Delete{Enter}');
+    await waitFor(() => expect(verify.requests).toHaveLength(1));
+    expect(recorder.frames).toEqual(['confirm/open']);
+    verify.release();
+
+    await screen.findByLabelText('Password');
+    recorder.stop();
+    expect(recorder.frames).toEqual(['confirm/open', 'verify/open']);
+    expect(action).toHaveBeenCalledTimes(2);
+  });
+
+  it('shows the confirm step while an action does other work before the run, after a cancel', async () => {
+    const action = vi.fn().mockResolvedValue(reverificationError());
+    const gates = [deferred<void>(), deferred<void>()];
+    const { user, open, closed } = await renderReverified(action, () => gates[action.mock.calls.length].promise);
+    await open();
+    await user.type(field(), 'Delete{Enter}');
+    gates[0].resolve();
+    await screen.findByLabelText('Password');
+    await user.click(screen.getByRole('button', { name: 'Close' }));
+    await closed();
+
+    await open();
+    const recorder = recordFrames();
+    await user.type(field(), 'Delete{Enter}');
+    await waitFor(() => expect(screen.getByRole('alertdialog').querySelector('[aria-busy="true"]')).not.toBeNull());
+    expect(recorder.frames).toEqual(['confirm/open']);
+    gates[1].resolve();
+
+    await screen.findByLabelText('Password');
+    recorder.stop();
+    expect(recorder.frames).toEqual(['confirm/open', 'verify/open']);
+  });
+});

packages/mosaic/src/features/reverification/__tests__/reverification.feature.test.tsx

@@ -142,7 +142,7 @@ function Host({ action }: { action: () => Promise<unknown> }) {
       <output aria-label='Outcome'>{outcome}</output>
       {running ? (
         <Card.Root renderBranding={false}>
-          <Reverification {...reverification} />
+          <Reverification controller={reverification} />
           {reverification.onCancel ? (
             <button
               type='button'
@@ -174,16 +174,10 @@ function HoldingHost({ action }: { action: () => Promise<unknown> }) {
       >
         Run action
       </button>
-      <button
-        type='button'
-        onClick={reverification.reset}
-      >
-        Reset
-      </button>
       <output aria-label='Outcome'>{outcome}</output>
       <output aria-label='Visible'>{String(reverification.visible)}</output>
       <Card.Root renderBranding={false}>
-        <Reverification {...reverification} />
+        <Reverification controller={reverification} />
         {reverification.onCancel ? (
           <button
             type='button'
@@ -1093,8 +1087,8 @@ describe('Reverification', () => {
       return renderWithClerk(<HoldingHost action={action} />);
     };
 
-    it('keeps the last card on screen, inert, after a dismissal until it is reset', async () => {
-      await renderHolding(guardedAction());
+    it('keeps the last card on screen, inert, after a dismissal, until the next challenge replaces it', async () => {
+      await renderHolding(vi.fn().mockResolvedValue(reverificationError()));
       const user = await startVerification();
       await user.type(await screen.findByLabelText('Password'), 'half-typed');
 
@@ -1106,24 +1100,36 @@ describe('Reverification', () => {
       expect(passwordField()).toHaveAttribute('aria-disabled', 'true');
       expect(passwordField()).toHaveAttribute('readonly');
 
-      await user.click(screen.getByRole('button', { name: 'Reset' }));
-      expect(screen.queryByText(STEP.password)).toBeNull();
+      await user.click(screen.getByRole('button', { name: 'Run action' }));
+      await waitFor(() => expect(passwordField()).toBeEnabled());
+      expect(passwordField()).toHaveValue('');
+      expect(screen.getByRole('button', { name: 'Dismiss' })).toBeInTheDocument();
     });
 
-    it('keeps the last card on screen after the action completes until the next run', async () => {
-      await renderHolding(guardedAction());
+    it('keeps the last card on screen after the action completes, until the next challenge replaces it', async () => {
+      await renderHolding(
+        vi
+          .fn()
+          .mockResolvedValueOnce(reverificationError())
+          .mockResolvedValueOnce({ done: true })
+          .mockResolvedValueOnce(reverificationError())
+          .mockResolvedValue({ done: true }),
+      );
       const user = await startVerification();
       await user.type(await screen.findByLabelText('Password'), `${PASSWORD}{Enter}`);
       await untilResolved();
 
       expect(screen.getByText(STEP.password)).toBeVisible();
       expect(screen.queryByRole('button', { name: 'Dismiss' })).toBeNull();
+      expect(passwordField()).toHaveAttribute('aria-disabled', 'true');
 
       await user.click(screen.getByRole('button', { name: 'Run action' }));
-      await waitFor(() => expect(screen.queryByText(STEP.password)).toBeNull());
+      await waitFor(() => expect(passwordField()).toBeEnabled());
+      expect(passwordField()).toHaveValue('');
+      expect(screen.getByRole('button', { name: 'Dismiss' })).toBeInTheDocument();
     });
 
-    it('is visible once there is a card of its own to show, until it is reset', async () => {
+    it('is visible once there is a card of its own to show, until the run ends', async () => {
       await renderHolding(guardedAction());
       const start = holdRequests('post', VERIFY);
       const visible = () => screen.getByLabelText('Visible').textContent;
@@ -1140,10 +1146,8 @@ describe('Reverification', () => {
 
       await dismiss(user);
       await waitFor(() => expect(outcome()).toBe('rejected: reverification_cancelled'));
-      expect(visible()).toBe('true');
-
-      await user.click(screen.getByRole('button', { name: 'Reset' }));
       expect(visible()).toBe('false');
+      expect(screen.getByText(STEP.password)).toBeVisible();
     });
 
     it('does not keep anything on screen when no reverification was needed', async () => {