From b98b7cb86f84f1b3d97ce43c4b516164316d3d74 Mon Sep 17 00:00:00 2001
From: Erwan Leboucher <erwanleboucher@gmail.com>
Date: Wed, 2 Sep 2026 14:12:42 +0200
Subject: [PATCH] fix(crypto): stop stale verification flows from swallowing
 new requests

---
 src/app/components/DeviceVerification.tsx     | 15 ++++-
 src/app/crypto/engineCrypto/EngineCrypto.ts   | 47 +++++++++++++---
 .../incomingVerificationRequest.test.ts       | 56 +++++++++++++++++++
 src/app/hooks/useVerificationRequest.test.tsx | 26 ++++++++-
 src/app/hooks/useVerificationRequest.ts       |  4 ++
 5 files changed, 134 insertions(+), 14 deletions(-)

diff --git a/src/app/components/DeviceVerification.tsx b/src/app/components/DeviceVerification.tsx
index c28a1cc27b..9a2f5ce9fa 100644
--- a/src/app/components/DeviceVerification.tsx
+++ b/src/app/components/DeviceVerification.tsx
@@ -62,12 +62,16 @@ type VerificationAcceptProps = {
   onAccept: () => Promise<void>;
 };
 function VerificationAccept({ onAccept }: VerificationAcceptProps) {
-  const [acceptState, accept] = useAsyncCallback(onAccept);
+  const [acceptState, accept] = useAsyncCallback<void, Error, []>(onAccept);
 
   const accepting = acceptState.status === AsyncStatus.Loading;
   return (
     <Box direction="Column" gap="400">
-      <Text>Click accept to start the verification process.</Text>
+      <Text>
+        {acceptState.status === AsyncStatus.Error
+          ? acceptState.error.message
+          : 'Click accept to start the verification process.'}
+      </Text>
       <Button
         variant="Primary"
         fill="Solid"
@@ -317,7 +321,12 @@ export function ReceiveSelfDeviceVerification() {
   const mx = useMatrixClient();
   const [request, setRequest] = useState<VerificationRequest>();
 
-  useVerificationRequestReceived(setRequest);
+  useVerificationRequestReceived(
+    useCallback((received: VerificationRequest) => {
+      if (!received.isSelfVerification || received.initiatedByMe || !received.pending) return;
+      setRequest(received);
+    }, [])
+  );
 
   useEffect(() => {
     if (request) return undefined;
diff --git a/src/app/crypto/engineCrypto/EngineCrypto.ts b/src/app/crypto/engineCrypto/EngineCrypto.ts
index 7ede813a8f..ae0e9201f5 100644
--- a/src/app/crypto/engineCrypto/EngineCrypto.ts
+++ b/src/app/crypto/engineCrypto/EngineCrypto.ts
@@ -779,6 +779,7 @@ export class EngineCrypto
     }
     const request = new EngineVerificationRequest(this.#engineCall, state);
     this.#verificationRequests.set(transactionId, request);
+    if (state.phase === EnginePhase.Done || state.phase === EnginePhase.Cancelled) return true;
     traceVerification('Surfacing an incoming verification request', {
       sender,
       transactionId,
@@ -2013,17 +2014,45 @@ export class EngineCrypto
   }
 
   async #cancelStaleRequests(userId: string): Promise<void> {
-    const stale = [...this.#verificationRequests.values()].filter(
-      (request) => request.otherUserId === userId && request.pending
-    );
-    for (const request of stale) {
-      traceVerification('Cancelling a stale verification request', {
-        flowId: request.transactionId ?? null,
+    let states: EngineVerificationState[] = [];
+    try {
+      states = ((await this.#call('getVerificationRequests', { userId })) ??
+        []) as EngineVerificationState[];
+    } catch (error) {
+      warnVerification('Could not list stale verification requests', {
+        reason: error instanceof Error ? error.message : String(error),
       });
-      // eslint-disable-next-line no-await-in-loop
-      await request.cancel().catch(() => undefined);
-      if (request.transactionId) this.#verificationRequests.delete(request.transactionId);
     }
+
+    const stale = new Map<string, EngineVerificationRequest>();
+    for (const state of states) {
+      if (state.phase === EnginePhase.Done || state.phase === EnginePhase.Cancelled) continue;
+      stale.set(
+        state.flowId,
+        this.#verificationRequests.get(state.flowId) ??
+          new EngineVerificationRequest(this.#engineCall, state)
+      );
+    }
+    for (const request of this.#verificationRequests.values()) {
+      if (request.otherUserId === userId && request.pending && request.transactionId) {
+        stale.set(request.transactionId, request);
+      }
+    }
+
+    for (const [flowId, request] of stale) {
+      traceVerification('Cancelling a stale verification request', { flowId });
+      try {
+        // eslint-disable-next-line no-await-in-loop
+        await request.cancel();
+        this.#verificationRequests.delete(flowId);
+      } catch (error) {
+        warnVerification('Could not cancel a stale verification request', {
+          flowId,
+          reason: error instanceof Error ? error.message : String(error),
+        });
+      }
+    }
+    await this.#flushOutgoingRequests();
   }
 
   async requestOwnUserVerification(): Promise<VerificationRequest> {
diff --git a/src/app/crypto/engineCrypto/incomingVerificationRequest.test.ts b/src/app/crypto/engineCrypto/incomingVerificationRequest.test.ts
index 8215940ec9..a7d3b82713 100644
--- a/src/app/crypto/engineCrypto/incomingVerificationRequest.test.ts
+++ b/src/app/crypto/engineCrypto/incomingVerificationRequest.test.ts
@@ -291,3 +291,59 @@ describe('incoming verification request', () => {
     expect(received).toHaveBeenCalledOnce();
   });
 });
+
+describe('stale verification requests the JS map never saw', () => {
+  beforeEach(() => {
+    mockInvoke.mockReset();
+    vi.mocked(traceVerification).mockClear();
+  });
+
+  it('cancels a pending flow the engine reports on its own', async () => {
+    const { mx } = clientSpy();
+    const cancelled: string[] = [];
+    mockInvoke.mockImplementation(async (_identity, method, args) => {
+      if (method === 'getVerificationRequests') {
+        return [{ ...requestState, flowId: '$ghost' }];
+      }
+      if (method === 'verificationRequest.cancel') {
+        cancelled.push((args as { flowId: string }).flowId);
+        return null;
+      }
+      if (method === 'queryKeysForUsers') {
+        return { id: 'q1', type: 1, className: 'KeysQueryRequest', body: '{}' };
+      }
+      if (method === 'device.requestVerification') {
+        return { request: { ...requestState, flowId: '$new' }, outgoingRequest: null };
+      }
+      return [];
+    });
+
+    const crypto = new EngineCrypto(mx, { userId: '@me:e.org', deviceId: 'D' });
+    await crypto.requestDeviceVerification('@me:e.org', 'OTHER');
+
+    expect(cancelled).toEqual(['$ghost']);
+  });
+});
+
+describe('already terminal incoming request', () => {
+  beforeEach(() => mockInvoke.mockReset());
+
+  it('does not raise a modal for a flow the engine already cancelled', async () => {
+    const { mx } = clientSpy();
+    mockInvoke.mockImplementation(async (_identity, method) => {
+      if (method === 'receiveSyncChanges') {
+        return [{ type: 3, rawEvent: JSON.stringify(REQUEST_EVENT) }];
+      }
+      if (method === 'getVerificationRequest') return { ...requestState, phase: 5 };
+      return [];
+    });
+
+    const crypto = new EngineCrypto(mx, { userId: '@me:e.org', deviceId: 'D' });
+    const received = vi.fn<(request: unknown) => void>();
+    crypto.on(CryptoEvent.VerificationRequestReceived, received);
+
+    await crypto.preprocessToDeviceMessages([REQUEST_EVENT as never]);
+
+    expect(received).not.toHaveBeenCalled();
+  });
+});
diff --git a/src/app/hooks/useVerificationRequest.test.tsx b/src/app/hooks/useVerificationRequest.test.tsx
index 571a5821ce..a41f23e20e 100644
--- a/src/app/hooks/useVerificationRequest.test.tsx
+++ b/src/app/hooks/useVerificationRequest.test.tsx
@@ -1,7 +1,7 @@
 import { renderHook } from '@testing-library/react';
 import { describe, expect, it, vi } from 'vitest';
-import { VerifierEvent } from '$types/matrix-sdk';
-import { useVerifierShowSas } from './useVerificationRequest';
+import { VerificationPhase, VerifierEvent } from '$types/matrix-sdk';
+import { useVerificationRequestPhase, useVerifierShowSas } from './useVerificationRequest';
 
 describe('useVerifierShowSas', () => {
   it('publishes SAS callbacks that were ready before the listener subscribed', () => {
@@ -33,3 +33,25 @@ describe('useVerifierShowSas', () => {
     expect(removeListener).toHaveBeenCalledWith(VerifierEvent.ShowSas, onCallback);
   });
 });
+
+describe('useVerificationRequestPhase', () => {
+  it('reports the new request phase when the request is swapped out', () => {
+    const stub = (phase: VerificationPhase) => ({
+      phase,
+      on: vi.fn<() => void>(),
+      removeListener: vi.fn<() => void>(),
+    });
+    const cancelled = stub(VerificationPhase.Cancelled);
+    const fresh = stub(VerificationPhase.Requested);
+
+    const { result, rerender } = renderHook(
+      ({ request }) => useVerificationRequestPhase(request as never),
+      { initialProps: { request: cancelled } }
+    );
+
+    expect(result.current).toBe(VerificationPhase.Cancelled);
+
+    rerender({ request: fresh });
+    expect(result.current).toBe(VerificationPhase.Requested);
+  });
+});
diff --git a/src/app/hooks/useVerificationRequest.ts b/src/app/hooks/useVerificationRequest.ts
index 187748da7c..40ede6fdcc 100644
--- a/src/app/hooks/useVerificationRequest.ts
+++ b/src/app/hooks/useVerificationRequest.ts
@@ -38,6 +38,10 @@ const useVerificationRequestChange = (
 export const useVerificationRequestPhase = (request: VerificationRequest): VerificationPhase => {
   const [phase, setPhase] = useState(() => request.phase);
 
+  useEffect(() => {
+    setPhase(request.phase);
+  }, [request]);
+
   useVerificationRequestChange(
     request,
     useCallback(() => {