fix(channels): recover Discord/OAuth channel stuck in Connecting (#4299) (#4306)

Co-authored-by: M3gA-Mind <megamind@mahadao.com>

app/src/components/channels/DiscordConfig.tsx

@@ -219,9 +219,38 @@ const DiscordConfig = ({ definition }: DiscordConfigProps) => {
               });
               if (oauthResponse.result?.oauthUrl) {
                 await openUrl(oauthResponse.result.oauthUrl);
+              } else {
+                // No URL means the core couldn't start the flow. Surface it now
+                // instead of leaving the badge pinned at `connecting` until the
+                // listener's timeout fires (#4299).
+                log('oauth_connect returned no oauthUrl for discord');
+                dispatch(
+                  setChannelConnectionStatus({
+                    channel: 'discord',
+                    authMode: spec.mode,
+                    status: 'error',
+                    lastError: t(
+                      'channels.discord.oauthStartFailed',
+                      "Couldn't start Discord sign-in. Try again."
+                    ),
+                  })
+                );
               }
-            } catch {
-              // best-effort
+            } catch (err) {
+              // Opening the browser / starting the flow failed — don't swallow
+              // it, or the badge hangs at `connecting` (#4299).
+              log('oauth_connect failed for discord: %o', err);
+              dispatch(
+                setChannelConnectionStatus({
+                  channel: 'discord',
+                  authMode: spec.mode,
+                  status: 'error',
+                  lastError: t(
+                    'channels.discord.oauthStartFailed',
+                    "Couldn't start Discord sign-in. Try again."
+                  ),
+                })
+              );
             }
           }
           return;

app/src/components/channels/__tests__/DiscordConfig.test.tsx

@@ -3,8 +3,10 @@ import { afterEach, describe, expect, it, vi } from 'vitest';
 
 import { FALLBACK_DEFINITIONS } from '../../../lib/channels/definitions';
 import { channelConnectionsApi } from '../../../services/api/channelConnectionsApi';
+import { callCoreRpc } from '../../../services/coreRpcClient';
 import { upsertChannelConnection } from '../../../store/channelConnectionsSlice';
 import { createTestStore, renderWithProviders } from '../../../test/test-utils';
+import { openUrl } from '../../../utils/openUrl';
 import DiscordConfig from '../DiscordConfig';
 
 const coreStateMock = vi.hoisted(() => vi.fn(() => ({ snapshot: { sessionToken: 'jwt-abc' } })));
@@ -182,4 +184,48 @@ describe('DiscordConfig', () => {
     expect(screen.queryByText('Login with OpenHuman')).not.toBeInTheDocument();
     expect(screen.getAllByText('Bot Token').length).toBeGreaterThanOrEqual(1);
   });
+
+  // #4299: an OAuth init failure (oauth_connect throws, or returns no URL) must
+  // surface as an error instead of leaving the badge pinned at `connecting`.
+  it('surfaces an error when oauth_connect throws', async () => {
+    const store = createTestStore();
+    vi.mocked(channelConnectionsApi.connectChannel).mockResolvedValue({
+      status: 'pending_auth',
+      auth_action: 'discord_oauth',
+      restart_required: false,
+    });
+    vi.mocked(callCoreRpc).mockRejectedValue(new Error('rpc boom'));
+
+    renderWithProviders(<DiscordConfig definition={discordDef} />, { store });
+
+    // Auth modes render in definition order: bot_token (0), oauth (1).
+    fireEvent.click(screen.getAllByRole('button', { name: 'Connect' })[1]);
+
+    await waitFor(() => {
+      const oauth = store.getState().channelConnections.connections.discord.oauth;
+      expect(oauth?.status).toBe('error');
+      expect(oauth?.lastError).toMatch(/Couldn't start Discord sign-in/);
+    });
+  });
+
+  it('surfaces an error when oauth_connect returns no oauthUrl', async () => {
+    const store = createTestStore();
+    vi.mocked(channelConnectionsApi.connectChannel).mockResolvedValue({
+      status: 'pending_auth',
+      auth_action: 'discord_oauth',
+      restart_required: false,
+    });
+    vi.mocked(callCoreRpc).mockResolvedValue({ result: {} });
+
+    renderWithProviders(<DiscordConfig definition={discordDef} />, { store });
+
+    fireEvent.click(screen.getAllByRole('button', { name: 'Connect' })[1]);
+
+    await waitFor(() => {
+      const oauth = store.getState().channelConnections.connections.discord.oauth;
+      expect(oauth?.status).toBe('error');
+      expect(oauth?.lastError).toMatch(/Couldn't start Discord sign-in/);
+    });
+    expect(openUrl).not.toHaveBeenCalled();
+  });
 });

app/src/hooks/__tests__/useOAuthConnectionListener.test.tsx

@@ -1,13 +1,17 @@
-import { renderHook } from '@testing-library/react';
+import { act, renderHook } from '@testing-library/react';
 import type { ReactNode } from 'react';
 import { Provider } from 'react-redux';
-import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import { store } from '../../store';
 import {
   resetChannelConnectionsState,
   setChannelConnectionStatus,
 } from '../../store/channelConnectionsSlice';
+import {
+  beginDeepLinkAuthProcessing,
+  completeDeepLinkAuthProcessing,
+} from '../../store/deepLinkAuthState';
 import { useOAuthConnectionListener } from '../useOAuthConnectionListener';
 
 const wrapper = ({ children }: { children: ReactNode }) => (
@@ -144,3 +148,147 @@ describe('useOAuthConnectionListener (#2128)', () => {
     expect(store.getState().channelConnections.connections.discord.oauth).toBeUndefined();
   });
 });
+
+// #4299: recover from a `connecting` badge that no OAuth deep link ever resolves
+// (Discord rejected the redirect_uri, user cancelled / closed the browser).
+const GRACE_MS = 2_500;
+const TIMEOUT_MS = 120_000;
+
+const setConnecting = () =>
+  store.dispatch(
+    setChannelConnectionStatus({ channel: 'discord', authMode: 'oauth', status: 'connecting' })
+  );
+
+const oauthStatus = () => store.getState().channelConnections.connections.discord.oauth?.status;
+
+describe('useOAuthConnectionListener stuck-connecting recovery (#4299)', () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+    store.dispatch(resetChannelConnectionsState());
+  });
+
+  afterEach(() => {
+    completeDeepLinkAuthProcessing();
+    vi.useRealTimers();
+    store.dispatch(resetChannelConnectionsState());
+  });
+
+  it('flips to error when the user returns (focus) and no deep link arrives', () => {
+    setConnecting();
+    renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), {
+      wrapper,
+    });
+
+    act(() => {
+      window.dispatchEvent(new Event('focus'));
+    });
+    act(() => {
+      vi.advanceTimersByTime(GRACE_MS);
+    });
+
+    const connection = store.getState().channelConnections.connections.discord.oauth;
+    expect(connection?.status).toBe('error');
+    expect(connection?.lastError).toMatch(/Couldn't finish connecting Discord/);
+  });
+
+  it('flips to error on the visibilitychange path too', () => {
+    setConnecting();
+    renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), {
+      wrapper,
+    });
+
+    act(() => {
+      document.dispatchEvent(new Event('visibilitychange'));
+    });
+    act(() => {
+      vi.advanceTimersByTime(GRACE_MS);
+    });
+
+    expect(oauthStatus()).toBe('error');
+  });
+
+  it('does NOT flip when a success deep link lands within the grace window', () => {
+    setConnecting();
+    renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), {
+      wrapper,
+    });
+
+    act(() => {
+      window.dispatchEvent(new Event('focus'));
+    });
+    act(() => {
+      vi.advanceTimersByTime(GRACE_MS - 500);
+      dispatchOAuthSuccess('discord');
+    });
+    act(() => {
+      vi.advanceTimersByTime(GRACE_MS + TIMEOUT_MS);
+    });
+
+    expect(oauthStatus()).toBe('connected');
+  });
+
+  it('flips to error after the absolute timeout when the user never returns', () => {
+    setConnecting();
+    renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), {
+      wrapper,
+    });
+
+    act(() => {
+      vi.advanceTimersByTime(TIMEOUT_MS);
+    });
+    act(() => {
+      vi.advanceTimersByTime(GRACE_MS);
+    });
+
+    expect(oauthStatus()).toBe('error');
+  });
+
+  it('does not arm recovery when the channel is not connecting', () => {
+    store.dispatch(
+      setChannelConnectionStatus({ channel: 'discord', authMode: 'oauth', status: 'connected' })
+    );
+    renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), {
+      wrapper,
+    });
+
+    act(() => {
+      window.dispatchEvent(new Event('focus'));
+      vi.advanceTimersByTime(TIMEOUT_MS + GRACE_MS);
+    });
+
+    expect(oauthStatus()).toBe('connected');
+  });
+
+  it('skips recovery while a deep-link auth round-trip is in flight', () => {
+    setConnecting();
+    renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), {
+      wrapper,
+    });
+
+    beginDeepLinkAuthProcessing();
+    act(() => {
+      window.dispatchEvent(new Event('focus'));
+      vi.advanceTimersByTime(GRACE_MS);
+    });
+
+    expect(oauthStatus()).toBe('connecting');
+  });
+
+  it('clears timers on unmount so a late timeout cannot mutate state', () => {
+    setConnecting();
+    const { unmount } = renderHook(
+      () => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }),
+      { wrapper }
+    );
+
+    act(() => {
+      window.dispatchEvent(new Event('focus'));
+    });
+    unmount();
+    act(() => {
+      vi.advanceTimersByTime(TIMEOUT_MS + GRACE_MS);
+    });
+
+    expect(oauthStatus()).toBe('connecting');
+  });
+});

app/src/hooks/useOAuthConnectionListener.ts

@@ -16,15 +16,25 @@
  *
  * Centralising this means new channels with OAuth auth modes inherit correct
  * pending-state transitions for free.
+ *
+ * It also recovers from the *no deep link ever arrives* case (#4299): when
+ * Discord rejects the redirect_uri, the user cancels, or the browser lands on
+ * an error page, neither `oauth:success` nor `oauth:error` fires, so the badge
+ * would otherwise stay pinned at `connecting` forever. While a channel sits at
+ * `connecting` this hook watches for the user returning to the app (window
+ * focus / tab visible) and an absolute timeout, then flips to a retryable
+ * `error` if no OAuth event resolved it.
  */
 import debug from 'debug';
-import { useEffect } from 'react';
+import { useEffect, useRef } from 'react';
 
+import { useT } from '../lib/i18n/I18nContext';
 import {
   setChannelConnectionStatus,
   upsertChannelConnection,
 } from '../store/channelConnectionsSlice';
-import { useAppDispatch } from '../store/hooks';
+import { getDeepLinkAuthState } from '../store/deepLinkAuthState';
+import { useAppDispatch, useAppSelector } from '../store/hooks';
 import type { ChannelAuthMode, ChannelType } from '../types/channels';
 
 const log = debug('channels:oauth-listener');
@@ -35,6 +45,23 @@ const log = debug('channels:oauth-listener');
 // render. (CodeRabbit on PR #2256.)
 const DEFAULT_OAUTH_CAPABILITIES = ['read', 'write'] as const;
 
+// How long a channel may sit at `connecting` with no OAuth deep link before we
+// give up and surface a retryable error. Covers the "user never came back to
+// the app" path (closed the browser tab on Discord's redirect_uri error page,
+// switched away permanently). Mirrors `OAuthProviderButton`'s 300s login
+// fallback but shorter — a channel connect that hasn't round-tripped in 2min is
+// not coming back. (#4299)
+const OAUTH_CONNECTING_TIMEOUT_MS = 120_000;
+
+// Grace window applied after a recovery trigger (window focus / tab visible /
+// absolute timeout) before flipping `connecting` → `error`. A *successful*
+// OAuth deep link arrives shortly after the OS refocuses the app, but its
+// dispatch is delayed by `focusMainWindow()` + the async app-version gate in
+// `desktopDeepLinkListener.handleOAuthDeepLink`. Waiting out this grace lets a
+// real success/error event win the race (and cancel the timer) so we never
+// flash a false error on the happy path. (#4299)
+const OAUTH_RECOVER_GRACE_MS = 2_500;
+
 interface OAuthSuccessDetail {
   integrationId?: string;
   toolkit?: string;
@@ -73,15 +100,37 @@ export function useOAuthConnectionListener({
   capabilitiesOnSuccess = DEFAULT_OAUTH_CAPABILITIES,
 }: UseOAuthConnectionListenerOptions): void {
   const dispatch = useAppDispatch();
+  const { t } = useT();
+
+  // Current status for this channel/authMode. Drives whether recovery watchers
+  // are armed (only while `connecting`). `statusRef` mirrors it so the deferred
+  // grace callback reads the freshest value without itself being an effect dep.
+  const status = useAppSelector(
+    state => state.channelConnections.connections[channel]?.[authMode]?.status
+  );
+  const statusRef = useRef(status);
+  statusRef.current = status;
 
   useEffect(() => {
     const channelKey = channel.toLowerCase();
 
+    // Shared timers so a real oauth:success / oauth:error can cancel a pending
+    // recovery, guaranteeing the deep link always wins the race.
+    let graceTimer: ReturnType<typeof setTimeout> | undefined;
+    let absoluteTimer: ReturnType<typeof setTimeout> | undefined;
+    const clearTimers = () => {
+      if (graceTimer !== undefined) clearTimeout(graceTimer);
+      if (absoluteTimer !== undefined) clearTimeout(absoluteTimer);
+      graceTimer = undefined;
+      absoluteTimer = undefined;
+    };
+
     const handleSuccess = (event: Event) => {
       const detail = (event as CustomEvent<OAuthSuccessDetail>).detail;
       const toolkit = detail?.toolkit?.toLowerCase();
       if (!toolkit || toolkit !== channelKey) return;
 
+      clearTimers();
       log('oauth success for channel=%s authMode=%s', channel, authMode);
       dispatch(
         upsertChannelConnection({
@@ -101,6 +150,7 @@ export function useOAuthConnectionListener({
       const provider = detail?.provider?.toLowerCase();
       if (!provider || provider !== channelKey) return;
 
+      clearTimers();
       const lastError =
         detail?.message ||
         'OAuth sign-in did not complete. Try again and approve access to continue.';
@@ -110,9 +160,64 @@ export function useOAuthConnectionListener({
 
     window.addEventListener('oauth:success', handleSuccess);
     window.addEventListener('oauth:error', handleError);
+
+    // Recovery is only relevant while the badge is pinned at `connecting`.
+    if (status !== 'connecting') {
+      return () => {
+        window.removeEventListener('oauth:success', handleSuccess);
+        window.removeEventListener('oauth:error', handleError);
+        clearTimers();
+      };
+    }
+
+    // Flip to a retryable error, but only if the flow is *still* unresolved:
+    // a deep link landing during the grace window updates the store and makes
+    // this a no-op. Skip while a deep-link auth round-trip is in flight (the
+    // callback flips `isProcessing` before dispatching its event, same guard as
+    // `OAuthProviderButton`).
+    const recover = (label: string) => {
+      if (statusRef.current !== 'connecting') return;
+      if (getDeepLinkAuthState().isProcessing) {
+        log('[%s] recover via %s skipped (deep-link processing)', channel, label);
+        return;
+      }
+      const channelLabel = channel.charAt(0).toUpperCase() + channel.slice(1);
+      log('[%s] recover via %s -> error (no oauth deep link arrived)', channel, label);
+      dispatch(
+        setChannelConnectionStatus({
+          channel,
+          authMode,
+          status: 'error',
+          lastError: t(
+            'channels.oauth.connectRecoverFailed',
+            "Couldn't finish connecting {channel}. The sign-in window closed before access was granted — try again."
+          ).replace('{channel}', channelLabel),
+        })
+      );
+    };
+
+    const scheduleRecover = (label: string) => {
+      if (graceTimer !== undefined) clearTimeout(graceTimer);
+      graceTimer = setTimeout(() => recover(label), OAUTH_RECOVER_GRACE_MS);
+    };
+
+    // User returned to the app from the system browser without a deep link.
+    const handleFocus = () => scheduleRecover('focus');
+    const handleVisibility = () => {
+      if (document.visibilityState === 'visible') scheduleRecover('visibilitychange');
+    };
+
+    window.addEventListener('focus', handleFocus);
+    document.addEventListener('visibilitychange', handleVisibility);
+    // Backstop for the user who never refocuses the app at all.
+    absoluteTimer = setTimeout(() => scheduleRecover('timeout'), OAUTH_CONNECTING_TIMEOUT_MS);
+
     return () => {
       window.removeEventListener('oauth:success', handleSuccess);
       window.removeEventListener('oauth:error', handleError);
+      window.removeEventListener('focus', handleFocus);
+      document.removeEventListener('visibilitychange', handleVisibility);
+      clearTimers();
     };
-  }, [dispatch, channel, authMode, capabilitiesOnSuccess]);
+  }, [dispatch, channel, authMode, capabilitiesOnSuccess, status, t]);
 }