fix(channels): clear stale OAuth Connecting badges across auth modes (#2128)#2256
Conversation
📝 WalkthroughWalkthroughCentralizes OAuth event bridging into Redux via a new ChangesOAuth connection lifecycle and state management
Sequence DiagramsequenceDiagram
participant User
participant BrowserWindow as Window
participant Hook as useOAuthConnectionListener
participant Redux as channelConnectionsSlice
participant Component as DiscordConfig/TelegramConfig
User->>BrowserWindow: complete OAuth redirect
BrowserWindow->>Hook: emit oauth:success / oauth:error (detail.toolkit/provider, message)
Hook->>Redux: dispatch upsertChannelConnection / setChannelConnectionStatus
Component->>Redux: dispatch clearOtherPendingForChannel when starting new flow
Redux->>Redux: update sibling auth modes (connecting -> disconnected) and set statuses
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/components/channels/DiscordConfig.tsx`:
- Around line 142-145: The Redux clearing call
dispatch(clearOtherPendingForChannel({ channel: 'discord', exceptAuthMode:
spec.mode })) can race with an already-running startLinkPolling loop that later
dispatches managed_dm back to connected/error; cancel that polling before
clearing state by invoking the cancellation/stop mechanism for startLinkPolling
(e.g., call stopLinkPolling or cancelLinkPollingTask associated with the discord
managed_dm poll) for the channel/auth mode(s) being cleared, ensure the poller
is awaited/confirmed stopped, then proceed to dispatch
clearOtherPendingForChannel and subsequent dispatches so the old poll cannot
revive the deprecated flow (referencing startLinkPolling, managed_dm, and
clearOtherPendingForChannel to locate the logic).
In `@app/src/components/channels/TelegramConfig.tsx`:
- Around line 167-170: When starting a new Telegram auth attempt (just before
the dispatch(clearOtherPendingForChannel({ channel: 'telegram', exceptAuthMode:
spec.mode }))) ensure any in-flight managed_dm polling is stopped so an old poll
cannot later dispatch connected/error and leak into state; add a call to the
existing poll-cleanup API (e.g. dispatch(cancelManagedDMPoll()) or call
stopManagedDMPoll()) or clear the stored poll subscription/timer from state
before dispatching clearOtherPendingForChannel, and ensure the cleanup
unsubscribes the poll's callbacks so managed_dm polling cannot update Redux
after cancellation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e9d62e3e-635f-4c28-9c4e-79fae093b917
📒 Files selected for processing (6)
app/src/components/channels/DiscordConfig.tsxapp/src/components/channels/TelegramConfig.tsxapp/src/hooks/__tests__/useOAuthConnectionListener.test.tsxapp/src/hooks/useOAuthConnectionListener.tsapp/src/store/__tests__/channelConnectionsSlice.test.tsapp/src/store/channelConnectionsSlice.ts
…ngs (tinyhumansai#2128) CodeRabbit on PR tinyhumansai#2256 flagged a real race in the new clearOtherPendingForChannel flow: when the user switches auth modes, the slice row for the prior method is reset to `disconnected`, but the underlying managed-DM polling loop is still alive and can later dispatch the cleared row back to `connected` or `error`, leaking the old attempt into state. Fix in both panels: cancel the in-flight poll *before* dispatching the clear. - DiscordConfig: unconditionally `pollAbort.current?.abort()` + `setLinkToken(null)` at the top of handleConnect. Safe in the same- mode case because startLinkPolling spawns a fresh controller. - TelegramConfig: when starting a non-managed-dm mode, call `stopManagedDmPolling('telegram:managed_dm')` before the slice clear. (Same-mode case is already handled inside startManagedDmPolling.) Added `stopManagedDmPolling` to handleConnect's dep array per react-hooks/exhaustive-deps. Co-Authored-By: Claude <noreply@anthropic.com>
CI status — infra-only failureAfter CodeRabbit's approving re-review on
The first CI run on CodeRabbit's APPROVED review of the latest commit covers the reviewer-comments gate. |
…inyhumansai#2128) OAuth connection badges could stay pinned at `Connecting` indefinitely: - DiscordConfig had a per-component `oauth:success` effect but no error listener; TelegramConfig had neither, so completed and failed OAuth flows never transitioned its badge out of `connecting`. - Starting a second OAuth method on the same channel left the first method's pending state in place, so two methods could appear active at once (the reproducer from the issue). This change: - Adds a shared `useOAuthConnectionListener` hook that bridges the global `oauth:success` / `oauth:error` deep-link CustomEvents from desktopDeepLinkListener.ts into the channelConnections slice, filtered case-insensitively by channel. DiscordConfig and TelegramConfig now both subscribe via this hook; future channels inherit correct pending-state transitions for free. - Adds a `clearOtherPendingForChannel` reducer that cancels any sibling auth mode still mid-`connecting` (transitions it to `disconnected`, not `error`, so the UI doesn't surface a misleading failure) when a new connect flow starts in either panel. - Covers the new reducer (4 cases) and hook (8 cases) with Vitest. Co-Authored-By: Claude <noreply@anthropic.com>
…ngs (tinyhumansai#2128) CodeRabbit on PR tinyhumansai#2256 flagged a real race in the new clearOtherPendingForChannel flow: when the user switches auth modes, the slice row for the prior method is reset to `disconnected`, but the underlying managed-DM polling loop is still alive and can later dispatch the cleared row back to `connected` or `error`, leaking the old attempt into state. Fix in both panels: cancel the in-flight poll *before* dispatching the clear. - DiscordConfig: unconditionally `pollAbort.current?.abort()` + `setLinkToken(null)` at the top of handleConnect. Safe in the same- mode case because startLinkPolling spawns a fresh controller. - TelegramConfig: when starting a non-managed-dm mode, call `stopManagedDmPolling('telegram:managed_dm')` before the slice clear. (Same-mode case is already handled inside startManagedDmPolling.) Added `stopManagedDmPolling` to handleConnect's dep array per react-hooks/exhaustive-deps. Co-Authored-By: Claude <noreply@anthropic.com>
sanil-23
force-pushed
the
fix/2128-oauth-badge-pending-state
branch
from
5936c8d to
a559bd8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/hooks/useOAuthConnectionListener.ts`:
- Line 67: The default array literal for the capabilitiesOnSuccess parameter is
causing the effect to re-run each render; extract that literal into a
module-level constant (e.g. DEFAULT_CAPABILITIES = ['read','write']) and use
that constant as the default value for the capabilitiesOnSuccess param in
useOAuthConnectionListener, so the reference is stable and the effect that
registers/unregisters the 'oauth:success' and 'oauth:error' listeners will not
resubscribe on every render.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9431d6c-efef-4b12-9918-2de1b3ded009
📒 Files selected for processing (6)
app/src/components/channels/DiscordConfig.tsxapp/src/components/channels/TelegramConfig.tsxapp/src/hooks/__tests__/useOAuthConnectionListener.test.tsxapp/src/hooks/useOAuthConnectionListener.tsapp/src/store/__tests__/channelConnectionsSlice.test.tsapp/src/store/channelConnectionsSlice.ts
…mansai#2128) CodeRabbit on PR tinyhumansai#2256 flagged that the inline default array literal `['read', 'write']` for `capabilitiesOnSuccess` creates a fresh reference on every parent render. Since that prop is in the effect's dep array, the global `oauth:success` / `oauth:error` listeners would tear down and re-subscribe on every render of any panel using the default — same class of bug as tinyhumansai#2177. Hoist to a module-level `DEFAULT_OAUTH_CAPABILITIES` constant so the identity is stable. No behaviour change at the call sites. Co-Authored-By: Claude <noreply@anthropic.com>
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Nice work on this one — the shared useOAuthConnectionListener hook is a clean extraction, the clearOtherPendingForChannel reducer is scoped correctly (only touching connecting rows), and all three CodeRabbit findings have been addressed. Tests cover the important paths well.
| File | Area | Summary |
|---|---|---|
useOAuthConnectionListener.ts |
Frontend (new) | Shared hook bridging oauth:success/oauth:error deep-link events into Redux |
channelConnectionsSlice.ts |
Frontend | New clearOtherPendingForChannel reducer |
DiscordConfig.tsx |
Frontend | Replaced inline OAuth effect with shared hook + sibling cancellation |
TelegramConfig.tsx |
Frontend | Added previously-missing OAuth listener + sibling cancellation |
useOAuthConnectionListener.test.tsx |
Tests (new) | 8 hook test cases |
channelConnectionsSlice.test.ts |
Tests | 4 reducer test cases |
[major] PR-Issue alignment gap — GitHub/GitLab provider coverage
Issue #2128 acceptance criteria explicitly requires: "Provider coverage — The fix covers Discord, GitHub, GitLab, and other multi-method OAuth connection panels using the same state path." This PR only wires DiscordConfig and TelegramConfig. If GitHub/GitLab channel configs have OAuth auth modes going through the same channelConnections slice, they need useOAuthConnectionListener mounted too — otherwise the issue's acceptance criteria aren't met and those panels remain vulnerable to the same stale-badge bug.
The shared hook makes this straightforward to add. If you've verified those channels don't have the OAuth badge issue (e.g. they use a different state path or don't have multi-method OAuth), a note in the PR description explaining why they're excluded would close this out.
Everything else looks solid. The hook's channel-matching logic, the module-level DEFAULT_OAUTH_CAPABILITIES constant, the poll-abort ordering in both configs, and the disconnected (not error) transition for cancelled siblings are all good choices.
|
@graycyrus thanks for the read. You called it correctly — and the answer is the second option you offered: GitHub / GitLab don't have channel-config panels in this codebase today, so there's nothing to wire the new hook into for those providers. I've added a Provider coverage section to the PR description with the full evidence. Short version:
If the stale- Let me know if the body update lands the concern or if you'd rather see the missing-panel detection asserted in a regression test as well. |
|
huge thanks @sanil-23, this one's a gem 🙌 centralising the oauth badge transitions behind that new listener hook is such a clean fix, no more stale "connecting" badges hanging around ✨ really appreciate you back here again 💚 |
4 participants
Summary
useOAuthConnectionListenerhook so every channel panel handles bothoauth:successandoauth:errorconsistently.clearOtherPendingForChannelreducer so starting a connect flow onone auth mode drops any sibling auth mode that's still mid-
connectingonthe same channel.
DiscordConfigandTelegramConfigonto the shared hook; futurechannels with an OAuth auth mode inherit correct pending-state transitions
automatically.
Problem
OAuth badges on the channel connection panels could get pinned at
Connectingindefinitely (issue #2128):DiscordConfighad a per-componentoauth:successlistener but nooauth:errorlistener — failed OAuth attempts never transitioned the badgeout of
connecting.TelegramConfighad neither — completed and failed OAuth attempts leftthe badge pinned.
connectingon the chosen auth mode but never cancelledany sibling auth mode that was already pending. Triggering a second OAuth
method on Discord (
OAuth Sign-inthenLogin with OpenHuman, or thereverse) left both methods badged
Connectingsimultaneously.This is the exact repro from the issue. The same shape was visible across
GitHub/GitLab style multi-method panels because the underlying state model
(
channelConnections, keyed by(channel, authMode)) had no notion ofmutual exclusion.
Solution
Shared listener hook —
app/src/hooks/useOAuthConnectionListener.tssubscribes to both
oauth:successandoauth:errorwindow events(dispatched from
utils/desktopDeepLinkListener.ts), filters bytoolkit/providercase-insensitively, and dispatches the matching slice action.Per-channel panels mount it once with
{ channel, authMode }; cleanup onunmount is deterministic. New channels with an OAuth auth mode inherit the
behaviour without copying any logic.
Pending-state cancellation reducer —
clearOtherPendingForChannel({ channel, exceptAuthMode })inchannelConnectionsSlice.tswalks the auth-mode mapfor one channel and transitions every
connectingrow (except theexception) to
disconnectedwithlastError: undefined. Cancelled rows goto
disconnectedrather thanerrorso the UI doesn't surface a misleadingfailure — the user explicitly switched methods, they didn't experience an
error.
Per-panel wiring —
DiscordConfigandTelegramConfigeach:useOAuthConnectionListener({ channel: <name>, authMode: 'oauth' })at the top of the component (replacing the bespoke effect on Discord;
net-new on Telegram).
clearOtherPendingForChannelat the start ofhandleConnectbefore setting their own auth mode to
connecting.Tradeoffs
disconnected, not a newcancelledstate.Adding a dedicated state would expand the
ChannelConnectionStatusunionacross many call sites for marginal UX value.
{ integrationId, toolkit }forsuccess,
{ provider, errorCode, message }for error) is unchanged, sono symmetric change in the Tauri-side handler is needed.
Submission Checklist
pnpm test:coveragelocally over the new files reaches 100% on changed lines (every branch in the hook + reducer is exercised by the suite).N/A: behaviour-only fix on existing surfaces (channel connection pending state).## Related—N/A: no feature ID changes.N/A: no release-cut surface touched (channels panel is part of the always-shipped settings UX).Closes #NNNin the## Relatedsection — see below.Impact
(
desktopDeepLinkListener.ts) is Tauri-gated; the hook is a no-op outsideTauri because no deep-link events fire.
channelConnectionsslice schema(
SCHEMA_VERSION = 1) is unchanged. The new reducer only mutates existingrows; no migration needed.
identifier and never reads tokens. Existing
[DeepLink][oauth:*]logsremain the canonical diagnostic surface; the hook adds its own
channels:oauth-listenerdebug namespace per the project'sverbose-diagnostics rule.
Related
Provider coverage
The issue body mentions Discord, GitHub, and GitLab. The Channels page in this codebase only exposes three multi-method channel-config panels today:
DiscordConfig.tsx,TelegramConfig.tsx, andWebChannelConfig.tsx(the last is not OAuth-driven). There is noGitHubConfig.tsx/GitLabConfig.tsx— verified viafind app/src -name "*Config.tsx".GitHub OAuth does appear elsewhere in the app, but on different state slices that this PR's
channelConnections-bound hook does not (and should not) touch:BootCheckGate.tsx, OAuth callbackdeepLinkAuthsliceInstallSkillDialog.tsx,services/api/skillsApi.tscomponents/composio/TriggerToggles.tsx,composio/providerConfigs.tsxDiscordConfig.tsx,TelegramConfig.tsxchannelConnectionssliceSo this PR's
useOAuthConnectionListenercovers every multi-method OAuth panel that actually exists on the Channels surface. The shared hook is also the right shape for any futureGitHubConfig.tsx/GitLabConfig.tsxchannel panels — wiring them in becomes a one-lineuseOAuthConnectionListener({ channelId, capabilities, ... })import.If the stale-
Connectingsymptom also surfaces in the app-level / skills / Composio OAuth flows, those are separate fixes against different state slices and out of scope for this PR — I'm happy to file follow-up issues if any are observed.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/2128-oauth-badge-pending-state2d93f7c0Validation Run
pnpm --filter openhuman-app format:check—All matched files use Prettier code style!on the 6 changed filespnpm typecheck— clean (tsc --noEmit)pnpm --filter openhuman-app exec vitest run --config test/vitest.config.ts src/store/__tests__/channelConnectionsSlice.test.ts src/hooks/__tests__/useOAuthConnectionListener.test.tsx src/components/channels/__tests__/DiscordConfig.test.tsx src/components/channels/__tests__/TelegramConfig.test.tsx→ 4 files, 27 tests passN/A: no Rust changesN/A: no Tauri shell changesValidation Blocked
command:git pushpre-push hook (app:lint:commands-tokens)error:lint:commands-tokens requires ripgrep—rgnot installed on the dev environmentimpact:zero — the check greps a directory I did not modify (src/components/commands/). Pushed with--no-verifyper the CLAUDE.md guidance for environment-related hook failures unrelated to the diff. Maintainers can re-run on CI to validate.Behavior Changes
connectingwhen the OAuth flow completes or fails, and starting a new method cancels the previous method'sconnectingrow.Connectingsimultaneously, Telegram OAuth never clearing) goes away. No new UI elements; only badge state transitions are affected.Parity Contract
connectedanderrortransitions are unchanged;disconnectChannelConnection,upsertChannelConnection,setChannelConnectionStatusare all untouched. The Discordoauth:successpath still produces the same final state (status: 'connected',capabilities: ['read', 'write']); the inline effect was just refactored behind the shared hook.toolkit(success) orprovider(error) field matches the subscribed channel — siblings on other channels, and mismatched dispatches, are no-ops.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes
Tests