-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix(channels): clear stale OAuth Connecting badges across auth modes (#2128) #2256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
senamakel
merged 3 commits into
tinyhumansai:main
from
sanil-23:fix/2128-oauth-badge-pending-state
May 20, 2026
+445
−22
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
146 changes: 146 additions & 0 deletions
146
app/src/hooks/__tests__/useOAuthConnectionListener.test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| import { renderHook } from '@testing-library/react'; | ||
| import type { ReactNode } from 'react'; | ||
| import { Provider } from 'react-redux'; | ||
| import { afterEach, beforeEach, describe, expect, it } from 'vitest'; | ||
|
|
||
| import { store } from '../../store'; | ||
| import { | ||
| resetChannelConnectionsState, | ||
| setChannelConnectionStatus, | ||
| } from '../../store/channelConnectionsSlice'; | ||
| import { useOAuthConnectionListener } from '../useOAuthConnectionListener'; | ||
|
|
||
| const wrapper = ({ children }: { children: ReactNode }) => ( | ||
| <Provider store={store}>{children}</Provider> | ||
| ); | ||
|
|
||
| const dispatchOAuthSuccess = (toolkit: string, integrationId = 'integration-123') => { | ||
| window.dispatchEvent(new CustomEvent('oauth:success', { detail: { integrationId, toolkit } })); | ||
| }; | ||
|
|
||
| const dispatchOAuthError = (provider: string, errorCode = 'access_denied', message?: string) => { | ||
| window.dispatchEvent( | ||
| new CustomEvent('oauth:error', { detail: { provider, errorCode, message } }) | ||
| ); | ||
| }; | ||
|
|
||
| describe('useOAuthConnectionListener (#2128)', () => { | ||
| beforeEach(() => { | ||
| store.dispatch(resetChannelConnectionsState()); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| store.dispatch(resetChannelConnectionsState()); | ||
| }); | ||
|
|
||
| it('transitions matching channel to connected on oauth:success', () => { | ||
| store.dispatch( | ||
| setChannelConnectionStatus({ channel: 'discord', authMode: 'oauth', status: 'connecting' }) | ||
| ); | ||
|
|
||
| renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), { | ||
| wrapper, | ||
| }); | ||
| dispatchOAuthSuccess('discord'); | ||
|
|
||
| const connection = store.getState().channelConnections.connections.discord.oauth; | ||
| expect(connection?.status).toBe('connected'); | ||
| expect(connection?.lastError).toBeUndefined(); | ||
| expect(connection?.capabilities).toEqual(['read', 'write']); | ||
| }); | ||
|
|
||
| it('ignores oauth:success for a different channel', () => { | ||
| store.dispatch( | ||
| setChannelConnectionStatus({ channel: 'discord', authMode: 'oauth', status: 'connecting' }) | ||
| ); | ||
|
|
||
| renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), { | ||
| wrapper, | ||
| }); | ||
| dispatchOAuthSuccess('telegram'); | ||
|
|
||
| expect(store.getState().channelConnections.connections.discord.oauth?.status).toBe( | ||
| 'connecting' | ||
| ); | ||
| }); | ||
|
|
||
| it('matches toolkit case-insensitively', () => { | ||
| renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), { | ||
| wrapper, | ||
| }); | ||
| dispatchOAuthSuccess('Discord'); | ||
|
|
||
| expect(store.getState().channelConnections.connections.discord.oauth?.status).toBe('connected'); | ||
| }); | ||
|
|
||
| it('transitions to error on oauth:error and surfaces the message', () => { | ||
| store.dispatch( | ||
| setChannelConnectionStatus({ channel: 'telegram', authMode: 'oauth', status: 'connecting' }) | ||
| ); | ||
|
|
||
| renderHook(() => useOAuthConnectionListener({ channel: 'telegram', authMode: 'oauth' }), { | ||
| wrapper, | ||
| }); | ||
| dispatchOAuthError('telegram', 'access_denied', 'User cancelled'); | ||
|
|
||
| const connection = store.getState().channelConnections.connections.telegram.oauth; | ||
| expect(connection?.status).toBe('error'); | ||
| expect(connection?.lastError).toBe('User cancelled'); | ||
| }); | ||
|
|
||
| it('falls back to a generic error message when none is provided', () => { | ||
| renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), { | ||
| wrapper, | ||
| }); | ||
| dispatchOAuthError('discord', 'unknown_error'); | ||
|
|
||
| const connection = store.getState().channelConnections.connections.discord.oauth; | ||
| expect(connection?.status).toBe('error'); | ||
| expect(connection?.lastError).toMatch(/OAuth sign-in did not complete/); | ||
| }); | ||
|
|
||
| it('ignores oauth:error for a different channel', () => { | ||
| store.dispatch( | ||
| setChannelConnectionStatus({ channel: 'discord', authMode: 'oauth', status: 'connecting' }) | ||
| ); | ||
|
|
||
| renderHook(() => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), { | ||
| wrapper, | ||
| }); | ||
| dispatchOAuthError('telegram', 'access_denied'); | ||
|
|
||
| expect(store.getState().channelConnections.connections.discord.oauth?.status).toBe( | ||
| 'connecting' | ||
| ); | ||
| }); | ||
|
|
||
| it('records custom capabilities on success when provided', () => { | ||
| renderHook( | ||
| () => | ||
| useOAuthConnectionListener({ | ||
| channel: 'discord', | ||
| authMode: 'oauth', | ||
| capabilitiesOnSuccess: ['dm'], | ||
| }), | ||
| { wrapper } | ||
| ); | ||
| dispatchOAuthSuccess('discord'); | ||
|
|
||
| expect(store.getState().channelConnections.connections.discord.oauth?.capabilities).toEqual([ | ||
| 'dm', | ||
| ]); | ||
| }); | ||
|
|
||
| it('unsubscribes on unmount so further events do not mutate state', () => { | ||
| const { unmount } = renderHook( | ||
| () => useOAuthConnectionListener({ channel: 'discord', authMode: 'oauth' }), | ||
| { wrapper } | ||
| ); | ||
| unmount(); | ||
| dispatchOAuthSuccess('discord'); | ||
|
|
||
| // No listener mounted any more — the slice stays at its initial state for | ||
| // discord.oauth (undefined, not connected). | ||
| expect(store.getState().channelConnections.connections.discord.oauth).toBeUndefined(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| /** | ||
| * OAuth Connection Listener Hook | ||
| * | ||
| * Bridges the global `oauth:success` / `oauth:error` deep-link CustomEvents | ||
| * (dispatched from `utils/desktopDeepLinkListener.ts`) into the | ||
| * `channelConnections` Redux slice so that the right channel/authMode badge | ||
| * transitions out of `connecting` when the OAuth flow finishes in the system | ||
| * browser. | ||
| * | ||
| * Per-channel config panels (`DiscordConfig`, `TelegramConfig`, …) call this | ||
| * hook with their channel + the auth mode that owns the OAuth path. Each panel | ||
| * used to roll its own effect, which is how #2128 happened: `DiscordConfig` | ||
| * had a success listener, `TelegramConfig` had none, neither handled errors, | ||
| * so failed or completed OAuth flows could leave the badge pinned at | ||
| * `Connecting` forever. | ||
| * | ||
| * Centralising this means new channels with OAuth auth modes inherit correct | ||
| * pending-state transitions for free. | ||
| */ | ||
| import debug from 'debug'; | ||
| import { useEffect } from 'react'; | ||
|
|
||
| import { | ||
| setChannelConnectionStatus, | ||
| upsertChannelConnection, | ||
| } from '../store/channelConnectionsSlice'; | ||
| import { useAppDispatch } from '../store/hooks'; | ||
| import type { ChannelAuthMode, ChannelType } from '../types/channels'; | ||
|
|
||
| const log = debug('channels:oauth-listener'); | ||
|
|
||
| // Module-level constant so the default identity is stable across renders. | ||
| // Without this, an inline default array literal would land in the effect's | ||
| // dep array and re-subscribe the global oauth:* listeners on every parent | ||
| // render. (CodeRabbit on PR #2256.) | ||
| const DEFAULT_OAUTH_CAPABILITIES = ['read', 'write'] as const; | ||
|
|
||
| interface OAuthSuccessDetail { | ||
| integrationId?: string; | ||
| toolkit?: string; | ||
| } | ||
|
|
||
| interface OAuthErrorDetail { | ||
| provider?: string; | ||
| errorCode?: string; | ||
| message?: string; | ||
| } | ||
|
|
||
| export interface UseOAuthConnectionListenerOptions { | ||
| /** Channel that owns the OAuth flow (e.g. 'discord', 'telegram'). */ | ||
| channel: ChannelType; | ||
| /** Auth mode that the OAuth deep-link should resolve to. */ | ||
| authMode: ChannelAuthMode; | ||
| /** | ||
| * Capabilities to record on the connection when OAuth succeeds. Mirrors the | ||
| * existing per-channel defaults; kept explicit so each call site stays | ||
| * self-documenting. | ||
| */ | ||
| capabilitiesOnSuccess?: readonly string[]; | ||
| } | ||
|
|
||
| /** | ||
| * Subscribe to OAuth completion / failure deep-link events for one channel. | ||
| * | ||
| * Match key: the event's `toolkit` (success) or `provider` (error) field is | ||
| * compared case-insensitively to `channel`. Events for other channels are | ||
| * ignored so multiple panels can mount the hook simultaneously without | ||
| * stepping on each other. | ||
| */ | ||
| export function useOAuthConnectionListener({ | ||
| channel, | ||
| authMode, | ||
| capabilitiesOnSuccess = DEFAULT_OAUTH_CAPABILITIES, | ||
| }: UseOAuthConnectionListenerOptions): void { | ||
| const dispatch = useAppDispatch(); | ||
|
|
||
| useEffect(() => { | ||
| const channelKey = channel.toLowerCase(); | ||
|
|
||
| const handleSuccess = (event: Event) => { | ||
| const detail = (event as CustomEvent<OAuthSuccessDetail>).detail; | ||
| const toolkit = detail?.toolkit?.toLowerCase(); | ||
| if (!toolkit || toolkit !== channelKey) return; | ||
|
|
||
| log('oauth success for channel=%s authMode=%s', channel, authMode); | ||
| dispatch( | ||
| upsertChannelConnection({ | ||
| channel, | ||
| authMode, | ||
| patch: { | ||
| status: 'connected', | ||
| lastError: undefined, | ||
| capabilities: [...capabilitiesOnSuccess], | ||
| }, | ||
| }) | ||
| ); | ||
| }; | ||
|
|
||
| const handleError = (event: Event) => { | ||
| const detail = (event as CustomEvent<OAuthErrorDetail>).detail; | ||
| const provider = detail?.provider?.toLowerCase(); | ||
| if (!provider || provider !== channelKey) return; | ||
|
|
||
| const lastError = | ||
| detail?.message || | ||
| 'OAuth sign-in did not complete. Try again and approve access to continue.'; | ||
| log('oauth error for channel=%s authMode=%s code=%s', channel, authMode, detail?.errorCode); | ||
| dispatch(setChannelConnectionStatus({ channel, authMode, status: 'error', lastError })); | ||
| }; | ||
|
|
||
| window.addEventListener('oauth:success', handleSuccess); | ||
| window.addEventListener('oauth:error', handleError); | ||
| return () => { | ||
| window.removeEventListener('oauth:success', handleSuccess); | ||
| window.removeEventListener('oauth:error', handleError); | ||
| }; | ||
| }, [dispatch, channel, authMode, capabilitiesOnSuccess]); | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.