feat(channels): implement Telegram + Discord UI and connection state (#286)#317
Merged
senamakel merged 11 commits intotinyhumansai:mainfrom Apr 4, 2026
Merged
Conversation
Move FALLBACK_DEFINITIONS, STATUS_STYLES, and AUTH_MODE_LABELS from MessagingPanel into a shared module so both the settings panel and the upcoming Channels page can reuse them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…abilities Reusable UI primitives for the Channels page: status pill badge, credential form input, and capability chip list. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Shared hook that loads channel definitions and status from the core RPC, falls back to local definitions, and syncs Redux state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tab bar component showing Web / Telegram / Discord with connection status badges and active route summary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…flow Channel-specific config panels with per-auth-mode credential fields, connect/disconnect, status badges, and error handling. TelegramConfig includes the managed DM deep link initiation flow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ChannelConfigPanel switches between Telegram/Discord/Web configs based on selection. WebChannelConfig shows a simple always-connected card. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Top-level /channels page with ChannelSelector + ChannelConfigPanel. Updates sidebar nav to point to /channels instead of /settings/messaging. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…onents Replace inline loading logic with useChannelDefinitions hook, inline status badges with ChannelStatusBadge, and inline capability chips with ChannelCapabilities. Reduces duplication with the new Channels page. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds a new Channels page and route plus shared channel infrastructure: definitions, hook, selector, status badges, per-channel config components (Telegram, Discord, Web), reusable inputs/capabilities, tests, and MessagingPanel refactor to use the new hook and components. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as Channels Page
participant Selector as ChannelSelector
participant ConfigPanel as ChannelConfigPanel
participant ChannelCfg as Discord/Telegram Config
participant API as channelConnectionsApi
participant Store as Redux Store
participant RPC as Core RPC
User->>UI: Navigate to /channels
UI->>+UI: call useChannelDefinitions()
UI->>API: listDefinitions() & listStatus()
API-->>UI: definitions, status
UI->>Store: dispatch upsertChannelConnection() for connected entries
UI-->>-User: Render Channels UI
User->>Selector: Click channel tab
Selector->>ConfigPanel: set selectedChannel
ConfigPanel->>ChannelCfg: render channel-specific UI
User->>ChannelCfg: Fill fields, click Connect
ChannelCfg->>Store: set auth_mode status="connecting"
ChannelCfg->>API: connectChannel(credentials)
alt OAuth required (pending_auth with oauth)
API-->>ChannelCfg: pending_auth + auth_action
ChannelCfg->>RPC: openhuman.auth.oauth_connect()
RPC-->>ChannelCfg: oauthUrl
ChannelCfg->>User: openUrl(oauthUrl)
else Credentials accepted
API-->>ChannelCfg: success
ChannelCfg->>Store: upsertChannelConnection(status="connected")
end
ChannelCfg-->>User: show updated status/error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
app/src/components/channels/__tests__/ChannelSelector.test.tsx (1)
2-3: Reset the shared mock between tests for isolation.
onSelectis shared across test cases; clearing it each test prevents future false positives from call-history leakage.♻️ Suggested tweak
-import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; @@ describe('ChannelSelector', () => { const onSelect = vi.fn(); + + beforeEach(() => { + onSelect.mockClear(); + });Also applies to: 8-10, 25-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/channels/__tests__/ChannelSelector.test.tsx` around lines 2 - 3, The tests share the mock onSelect (created as a vi.fn()) which leaks call history across cases; add a test lifecycle hook to reset the mock between tests (e.g., use beforeEach/afterEach to call vi.clearAllMocks() or onSelect.mockClear()) so ChannelSelector tests (the onSelect mock used in the test suite) start with a clean mock for every spec.app/src/lib/channels/__tests__/definitions.test.ts (1)
25-37: Avoid non-null assertions in lookup tests for clearer failures.Using
!on.find(...)can throw before assertions run, which obscures why the test failed.♻️ Suggested tweak
it('telegram has bot_token and managed_dm auth modes', () => { - const tg = FALLBACK_DEFINITIONS.find(d => d.id === 'telegram')!; - const modes = tg.auth_modes.map(m => m.mode); - expect(modes).toContain('bot_token'); - expect(modes).toContain('managed_dm'); + const tg = FALLBACK_DEFINITIONS.find(d => d.id === 'telegram'); + expect(tg).toBeDefined(); + expect(tg?.auth_modes.map(m => m.mode)).toEqual( + expect.arrayContaining(['bot_token', 'managed_dm']) + ); }); it('discord has bot_token and oauth auth modes', () => { - const dc = FALLBACK_DEFINITIONS.find(d => d.id === 'discord')!; - const modes = dc.auth_modes.map(m => m.mode); - expect(modes).toContain('bot_token'); - expect(modes).toContain('oauth'); + const dc = FALLBACK_DEFINITIONS.find(d => d.id === 'discord'); + expect(dc).toBeDefined(); + expect(dc?.auth_modes.map(m => m.mode)).toEqual( + expect.arrayContaining(['bot_token', 'oauth']) + ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/lib/channels/__tests__/definitions.test.ts` around lines 25 - 37, The tests use non-null assertions on FIND results (FALLBACK_DEFINITIONS.find(...)!) which can throw before assertions; change both tests ('telegram has bot_token and managed_dm auth modes' and 'discord has bot_token and oauth auth modes') to first assert the lookup is defined (e.g., expect(result).toBeDefined()) or use a guard, then extract auth_modes and assert on modes—replace the bang usage on the variables tg and dc to ensure failures show clear assertion errors rather than runtime exceptions.app/src/components/channels/__tests__/ChannelStatusBadge.test.tsx (1)
21-24: Prefer querying the rendered badge overcontainer.firstChild.This keeps the assertion closer to user-observable behavior and reduces coupling to DOM structure.
As per coding guidelines: "Prefer testing behavior over implementation details; use existing helpers from `app/src/test/` (`test-utils.tsx`, shared mock backend) before adding new harness code".♻️ Suggested tweak
it('applies custom className', () => { - const { container } = render(<ChannelStatusBadge status="connected" className="extra-class" />); - expect(container.firstChild).toHaveClass('extra-class'); + render(<ChannelStatusBadge status="connected" className="extra-class" />); + expect(screen.getByText('Connected')).toHaveClass('extra-class'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/channels/__tests__/ChannelStatusBadge.test.tsx` around lines 21 - 24, Update the test in ChannelStatusBadge.test.tsx to avoid asserting against container.firstChild; instead import render from the shared test-utils and query the rendered badge element (e.g., using getByRole('status') or getByText('connected') depending on the component's accessible label) and assert that that element has the "extra-class" class; keep the test using ChannelStatusBadge and replace container.firstChild with the queried element in the expect call.app/src/components/channels/ChannelFieldInput.tsx (1)
10-27: Consider adding label-input association for accessibility.The label and input are not programmatically associated. Screen readers may not correctly announce the label when the input is focused.
♿ Proposed fix for accessibility
-const ChannelFieldInput = ({ field, value, onChange, disabled }: ChannelFieldInputProps) => { +const ChannelFieldInput = ({ field, value, onChange, disabled }: ChannelFieldInputProps) => { + const inputId = `channel-field-${field.name}`; return ( <div> - <label className="block text-xs text-stone-400 mb-1"> + <label htmlFor={inputId} className="block text-xs text-stone-400 mb-1"> {field.label} {field.required && <span className="text-coral-400 ml-0.5">*</span>} </label> <input + id={inputId} type={field.field_type === 'secret' ? 'password' : 'text'} value={value} onChange={e => onChange(e.target.value)} placeholder={field.placeholder || field.label} disabled={disabled} className="w-full rounded-lg border border-stone-700 bg-stone-900 px-3 py-2 text-sm text-white placeholder:text-stone-500 focus:outline-none focus:border-primary-500/60 disabled:opacity-50" /> </div> ); };Note: This assumes
FieldRequirementhas anameproperty. If not, you could usefield.labelwith a slugify function or generate a unique ID.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/channels/ChannelFieldInput.tsx` around lines 10 - 27, Add a programmatic label-input association in ChannelFieldInput by giving the input an id and the label an htmlFor; use field.name if available (or derive a slug from field.label or use React's useId inside ChannelFieldInput) and set that id on the input and htmlFor on the label so screen readers announce the label when the input is focused (update references in the ChannelFieldInput component for the label and input elements).app/src/components/channels/__tests__/TelegramConfig.test.tsx (2)
67-68: Button selection by index is brittle.
connectButtons[1]assumes the Managed DM auth mode is second in the DOM. Ifauth_modesorder changes, this test will silently test the wrong button.Consider a more explicit selection, e.g., finding the button within a specific section or using
data-testid.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/channels/__tests__/TelegramConfig.test.tsx` around lines 67 - 68, The test is brittle because it uses screen.getAllByText('Connect')[1] to target the Managed DM button; change the selector to explicitly locate the Managed DM section and then its Connect button (e.g., use within() on the element containing 'Managed DM' to call getByRole('button', { name: /connect/i }) or add a data-testid on the Managed DM connect button and use getByTestId) and then call fireEvent.click on that explicitly selected element instead of indexing connectButtons.
9-9: Non-null assertion could mask test setup failures.If
FALLBACK_DEFINITIONSdoesn't contain a'telegram'entry, this will silently produceundefined, and tests will fail with confusing errors. Consider adding an explicit check.Proposed defensive check
-const telegramDef = FALLBACK_DEFINITIONS.find(d => d.id === 'telegram')!; +const telegramDef = FALLBACK_DEFINITIONS.find(d => d.id === 'telegram'); +if (!telegramDef) { + throw new Error('Test setup error: telegram definition not found in FALLBACK_DEFINITIONS'); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/channels/__tests__/TelegramConfig.test.tsx` at line 9, Replace the non-null assertion on the FIND of FALLBACK_DEFINITIONS with an explicit defensive check: assign const telegramDef = FALLBACK_DEFINITIONS.find(d => d.id === 'telegram'); then immediately assert or throw if telegramDef is undefined (e.g., throw new Error('telegram definition missing in FALLBACK_DEFINITIONS') or use expect(telegramDef).toBeDefined()) so test setup failures produce a clear error; update any subsequent code that used the asserted value to use the checked telegramDef.app/src/components/channels/DiscordConfig.tsx (1)
37-55:runBusyandupdateFieldare duplicated across channel config components.These helpers (
runBusyat lines 37-48,updateFieldat lines 50-55) are nearly identical inTelegramConfig,DiscordConfig, andMessagingPanel. Consider extracting them into a shared hook (e.g.,useChannelFormState) to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/channels/DiscordConfig.tsx` around lines 37 - 55, The runBusy and updateField implementations are duplicated in DiscordConfig (and also present in TelegramConfig and MessagingPanel); extract them into a shared hook (e.g., useChannelFormState) that encapsulates state and helpers (busyKeys, error, fieldValues, setFieldValues) and exports runBusy and updateField (and any setters you need), then replace the local runBusy and updateField in DiscordConfig/TelegramConfig/MessagingPanel with imports from useChannelFormState so each component calls the single shared implementation instead of duplicating logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/channels/ChannelSelector.tsx`:
- Around line 49-53: The channel status resolution assigned to bestStatus
currently checks for 'connected' then 'connecting' then defaults to
'disconnected'; update the resolution logic that computes bestStatus (the
expression using channelModes and Object.values(...) in ChannelSelector.tsx) to
also check for a status of 'error' after 'connecting' (i.e., prefer 'error' over
falling back to 'disconnected') so that failures are surfaced; modify the chain
of checks on channelModes used to set bestStatus to include a find for c?.status
=== 'error' in the correct priority order.
In `@app/src/hooks/useChannelDefinitions.ts`:
- Around line 30-33: The cancellation guard variable `cancelled` is declared
inside the async function `loadDefinitions` (and similarly inside other async
helpers in this hook), so the effect's cleanup cannot set it and state updates
can still run after unmount; move the `let cancelled = false` to the outer scope
of the hook (or create an AbortController) so the cleanup returned by the
`useEffect` can set `cancelled = true` (or call abort()) and ensure every async
helper (e.g., `loadDefinitions` and the other async callbacks referenced) checks
`if (cancelled) return` before calling `setLoading`, `setError`, or other state
setters; finally return the cleanup closure from the `useEffect` to flip the
flag/abort to prevent state updates after unmount.
- Around line 49-56: The code casts external values entry.channel_id and
entry.auth_mode to ChannelType/ChannelAuthMode and dispatches
upsertChannelConnection without runtime validation; validate that
entry.channel_id is one of the allowed ChannelType values and entry.auth_mode is
one of ChannelAuthMode before calling dispatch in useChannelDefinitions (e.g.,
guard using a Set or enum lookup), and if validation fails either skip the
dispatch and log/warn or map to a safe default so you never index
state.connections with unknown keys; reference the symbols entry.channel_id,
entry.auth_mode, upsertChannelConnection, and the reducer access pattern
state.connections[channel][authMode] when adding the guards.
---
Nitpick comments:
In `@app/src/components/channels/__tests__/ChannelSelector.test.tsx`:
- Around line 2-3: The tests share the mock onSelect (created as a vi.fn())
which leaks call history across cases; add a test lifecycle hook to reset the
mock between tests (e.g., use beforeEach/afterEach to call vi.clearAllMocks() or
onSelect.mockClear()) so ChannelSelector tests (the onSelect mock used in the
test suite) start with a clean mock for every spec.
In `@app/src/components/channels/__tests__/ChannelStatusBadge.test.tsx`:
- Around line 21-24: Update the test in ChannelStatusBadge.test.tsx to avoid
asserting against container.firstChild; instead import render from the shared
test-utils and query the rendered badge element (e.g., using getByRole('status')
or getByText('connected') depending on the component's accessible label) and
assert that that element has the "extra-class" class; keep the test using
ChannelStatusBadge and replace container.firstChild with the queried element in
the expect call.
In `@app/src/components/channels/__tests__/TelegramConfig.test.tsx`:
- Around line 67-68: The test is brittle because it uses
screen.getAllByText('Connect')[1] to target the Managed DM button; change the
selector to explicitly locate the Managed DM section and then its Connect button
(e.g., use within() on the element containing 'Managed DM' to call
getByRole('button', { name: /connect/i }) or add a data-testid on the Managed DM
connect button and use getByTestId) and then call fireEvent.click on that
explicitly selected element instead of indexing connectButtons.
- Line 9: Replace the non-null assertion on the FIND of FALLBACK_DEFINITIONS
with an explicit defensive check: assign const telegramDef =
FALLBACK_DEFINITIONS.find(d => d.id === 'telegram'); then immediately assert or
throw if telegramDef is undefined (e.g., throw new Error('telegram definition
missing in FALLBACK_DEFINITIONS') or use expect(telegramDef).toBeDefined()) so
test setup failures produce a clear error; update any subsequent code that used
the asserted value to use the checked telegramDef.
In `@app/src/components/channels/ChannelFieldInput.tsx`:
- Around line 10-27: Add a programmatic label-input association in
ChannelFieldInput by giving the input an id and the label an htmlFor; use
field.name if available (or derive a slug from field.label or use React's useId
inside ChannelFieldInput) and set that id on the input and htmlFor on the label
so screen readers announce the label when the input is focused (update
references in the ChannelFieldInput component for the label and input elements).
In `@app/src/components/channels/DiscordConfig.tsx`:
- Around line 37-55: The runBusy and updateField implementations are duplicated
in DiscordConfig (and also present in TelegramConfig and MessagingPanel);
extract them into a shared hook (e.g., useChannelFormState) that encapsulates
state and helpers (busyKeys, error, fieldValues, setFieldValues) and exports
runBusy and updateField (and any setters you need), then replace the local
runBusy and updateField in DiscordConfig/TelegramConfig/MessagingPanel with
imports from useChannelFormState so each component calls the single shared
implementation instead of duplicating logic.
In `@app/src/lib/channels/__tests__/definitions.test.ts`:
- Around line 25-37: The tests use non-null assertions on FIND results
(FALLBACK_DEFINITIONS.find(...)!) which can throw before assertions; change both
tests ('telegram has bot_token and managed_dm auth modes' and 'discord has
bot_token and oauth auth modes') to first assert the lookup is defined (e.g.,
expect(result).toBeDefined()) or use a guard, then extract auth_modes and assert
on modes—replace the bang usage on the variables tg and dc to ensure failures
show clear assertion errors rather than runtime exceptions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e12b74c6-fdde-4021-8cd7-9f243ee09213
📒 Files selected for processing (20)
app/src/AppRoutes.tsxapp/src/components/MiniSidebar.tsxapp/src/components/channels/ChannelCapabilities.tsxapp/src/components/channels/ChannelConfigPanel.tsxapp/src/components/channels/ChannelFieldInput.tsxapp/src/components/channels/ChannelSelector.tsxapp/src/components/channels/ChannelStatusBadge.tsxapp/src/components/channels/DiscordConfig.tsxapp/src/components/channels/TelegramConfig.tsxapp/src/components/channels/WebChannelConfig.tsxapp/src/components/channels/__tests__/ChannelSelector.test.tsxapp/src/components/channels/__tests__/ChannelStatusBadge.test.tsxapp/src/components/channels/__tests__/DiscordConfig.test.tsxapp/src/components/channels/__tests__/TelegramConfig.test.tsxapp/src/components/settings/panels/MessagingPanel.tsxapp/src/hooks/useChannelDefinitions.tsapp/src/lib/channels/__tests__/definitions.test.tsapp/src/lib/channels/definitions.tsapp/src/pages/Channels.tsxapp/src/pages/__tests__/Channels.test.tsx
Comment on lines
+49
to
+53
| const bestStatus = channelModes | ||
| ? (Object.values(channelModes).find(c => c?.status === 'connected')?.status ?? | ||
| Object.values(channelModes).find(c => c?.status === 'connecting')?.status ?? | ||
| 'disconnected') | ||
| : 'disconnected'; |
Contributor
There was a problem hiding this comment.
Include error in channel status resolution.
Current logic can mask failures as disconnected. Add error after connecting so users can see broken routes.
🐛 Suggested fix
const channelModes = channelConnections.connections[channelId];
- const bestStatus = channelModes
- ? (Object.values(channelModes).find(c => c?.status === 'connected')?.status ??
- Object.values(channelModes).find(c => c?.status === 'connecting')?.status ??
- 'disconnected')
- : 'disconnected';
+ const statuses = channelModes
+ ? Object.values(channelModes).map(connection => connection?.status)
+ : [];
+ const bestStatus = statuses.includes('connected')
+ ? 'connected'
+ : statuses.includes('connecting')
+ ? 'connecting'
+ : statuses.includes('error')
+ ? 'error'
+ : 'disconnected';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const bestStatus = channelModes | |
| ? (Object.values(channelModes).find(c => c?.status === 'connected')?.status ?? | |
| Object.values(channelModes).find(c => c?.status === 'connecting')?.status ?? | |
| 'disconnected') | |
| : 'disconnected'; | |
| const statuses = channelModes | |
| ? Object.values(channelModes).map(connection => connection?.status) | |
| : []; | |
| const bestStatus = statuses.includes('connected') | |
| ? 'connected' | |
| : statuses.includes('connecting') | |
| ? 'connecting' | |
| : statuses.includes('error') | |
| ? 'error' | |
| : 'disconnected'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/channels/ChannelSelector.tsx` around lines 49 - 53, The
channel status resolution assigned to bestStatus currently checks for
'connected' then 'connecting' then defaults to 'disconnected'; update the
resolution logic that computes bestStatus (the expression using channelModes and
Object.values(...) in ChannelSelector.tsx) to also check for a status of 'error'
after 'connecting' (i.e., prefer 'error' over falling back to 'disconnected') so
that failures are surfaced; modify the chain of checks on channelModes used to
set bestStatus to include a find for c?.status === 'error' in the correct
priority order.
Comment on lines
+30
to
+33
| const loadDefinitions = useCallback(async () => { | ||
| let cancelled = false; | ||
| setLoading(true); | ||
| setError(null); |
Contributor
There was a problem hiding this comment.
Cancellation guard is ineffective; async state updates can run after unmount.
cancelled is scoped inside loadDefinitions, and the returned cleanup function is never used by the effect. This makes the guard a no-op.
🐛 Suggested fix
- const loadDefinitions = useCallback(async () => {
- let cancelled = false;
+ const loadDefinitions = useCallback(async (isCancelled: () => boolean = () => false) => {
setLoading(true);
setError(null);
try {
const [defs, statusEntries] = await Promise.all([
channelConnectionsApi.listDefinitions().catch(() => null),
channelConnectionsApi.listStatus().catch(() => null),
]);
- if (cancelled) return;
+ if (isCancelled()) return;
@@
} catch (e) {
const msg = e instanceof Error ? e.message : String(e);
- if (!cancelled) {
+ if (!isCancelled()) {
setDefinitions(FALLBACK_DEFINITIONS);
setError(`Could not load from backend: ${msg}`);
log('fallback to local definitions: %s', msg);
}
} finally {
- if (!cancelled) setLoading(false);
+ if (!isCancelled()) setLoading(false);
}
-
- return () => {
- cancelled = true;
- };
}, [dispatch]);
useEffect(() => {
- void loadDefinitions();
+ let cancelled = false;
+ void loadDefinitions(() => cancelled);
+ return () => {
+ cancelled = true;
+ };
}, [loadDefinitions]);Also applies to: 40-41, 65-76, 79-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/hooks/useChannelDefinitions.ts` around lines 30 - 33, The
cancellation guard variable `cancelled` is declared inside the async function
`loadDefinitions` (and similarly inside other async helpers in this hook), so
the effect's cleanup cannot set it and state updates can still run after
unmount; move the `let cancelled = false` to the outer scope of the hook (or
create an AbortController) so the cleanup returned by the `useEffect` can set
`cancelled = true` (or call abort()) and ensure every async helper (e.g.,
`loadDefinitions` and the other async callbacks referenced) checks `if
(cancelled) return` before calling `setLoading`, `setError`, or other state
setters; finally return the cleanup closure from the `useEffect` to flip the
flag/abort to prevent state updates after unmount.
Comment on lines
+49
to
+56
| const channel = entry.channel_id as ChannelType; | ||
| const authMode = entry.auth_mode as ChannelAuthMode; | ||
| if (entry.connected) { | ||
| dispatch( | ||
| upsertChannelConnection({ | ||
| channel, | ||
| authMode, | ||
| patch: { status: 'connected', capabilities: ['read', 'write'] }, |
Contributor
There was a problem hiding this comment.
Validate backend channel_id / auth_mode before dispatching.
Casting external payload fields with as bypasses runtime safety. An unknown channel_id can break reducer indexing (state.connections[channel][authMode]) and crash the update path.
🛡️ Suggested fix
+const CHANNEL_TYPES: ChannelType[] = ['telegram', 'discord', 'web'];
+const CHANNEL_AUTH_MODES: ChannelAuthMode[] = ['managed_dm', 'oauth', 'bot_token', 'api_key'];
+
+const isChannelType = (value: string): value is ChannelType =>
+ CHANNEL_TYPES.includes(value as ChannelType);
+
+const isChannelAuthMode = (value: string): value is ChannelAuthMode =>
+ CHANNEL_AUTH_MODES.includes(value as ChannelAuthMode);
@@
if (statusEntries && Array.isArray(statusEntries)) {
for (const entry of statusEntries) {
- const channel = entry.channel_id as ChannelType;
- const authMode = entry.auth_mode as ChannelAuthMode;
+ if (!isChannelType(entry.channel_id) || !isChannelAuthMode(entry.auth_mode)) {
+ log('skipping unknown status entry: %o', entry);
+ continue;
+ }
+ const channel = entry.channel_id;
+ const authMode = entry.auth_mode;
if (entry.connected) {
dispatch(
upsertChannelConnection({
channel,
authMode,
patch: { status: 'connected', capabilities: ['read', 'write'] },
})
);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/hooks/useChannelDefinitions.ts` around lines 49 - 56, The code casts
external values entry.channel_id and entry.auth_mode to
ChannelType/ChannelAuthMode and dispatches upsertChannelConnection without
runtime validation; validate that entry.channel_id is one of the allowed
ChannelType values and entry.auth_mode is one of ChannelAuthMode before calling
dispatch in useChannelDefinitions (e.g., guard using a Set or enum lookup), and
if validation fails either skip the dispatch and log/warn or map to a safe
default so you never index state.connections with unknown keys; reference the
symbols entry.channel_id, entry.auth_mode, upsertChannelConnection, and the
reducer access pattern state.connections[channel][authMode] when adding the
guards.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/components/settings/panels/MessagingPanel.tsx (1)
363-370:⚠️ Potential issue | 🔴 CriticalFix
ChannelFieldInputusage: incorrect prop types causing build and runtime failures.The component expects
onChange: (value: string) => void, not aChangeEvent. Remove theplaceholderprop (component derives it fromfield):<ChannelFieldInput key={field.key} field={field} value={fieldValues[compositeKey]?.[field.key] ?? ''} onChange={val => updateField(compositeKey, field.key, val)} className="w-full rounded-lg border border-stone-200 bg-white px-3 py-2 text-sm text-stone-900 placeholder:text-stone-400 focus:outline-none focus:border-primary-500/60" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/settings/panels/MessagingPanel.tsx` around lines 363 - 370, ChannelFieldInput is being passed the wrong onChange type and an unnecessary placeholder prop; change the onChange handler to accept a string (use onChange={val => updateField(compositeKey, field.key, val)}) instead of using the DOM event, and remove the placeholder prop since ChannelFieldInput derives it from the provided field prop; keep key, field, value (from fieldValues[compositeKey]?.[field.key] ?? ''), and className as-is to locate the change near ChannelFieldInput and updateField usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/settings/panels/MessagingPanel.tsx`:
- Line 22: Remove the unused import ChannelCapabilities and delete the dead
constants STATUS_STYLES and FALLBACK_DEFINITIONS from MessagingPanel.tsx; these
are vestigial after moving logic into useChannelDefinitions and reference an
undefined ChannelDefinition type. Open the file, remove the import line that
references ChannelCapabilities, and remove the declarations named STATUS_STYLES
and FALLBACK_DEFINITIONS (and any unused types/constants that only relate to
them), ensuring the component relies on useChannelDefinitions instead and that
no references to those symbols remain.
- Line 4: Remove the duplicate local constant AUTH_MODE_LABELS in
MessagingPanel.tsx and rely on the imported AUTH_MODE_LABELS from
'../../../lib/channels/definitions'; locate the local declaration (the constant
declared around lines 38–43 in the MessagingPanel component) and delete it so
there is only the single imported symbol, ensuring any references in the file
continue to use AUTH_MODE_LABELS from the import.
---
Outside diff comments:
In `@app/src/components/settings/panels/MessagingPanel.tsx`:
- Around line 363-370: ChannelFieldInput is being passed the wrong onChange type
and an unnecessary placeholder prop; change the onChange handler to accept a
string (use onChange={val => updateField(compositeKey, field.key, val)}) instead
of using the DOM event, and remove the placeholder prop since ChannelFieldInput
derives it from the provided field prop; keep key, field, value (from
fieldValues[compositeKey]?.[field.key] ?? ''), and className as-is to locate the
change near ChannelFieldInput and updateField usage.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 00d6c288-b0b5-4ab2-a644-e50bd90bbc8f
📒 Files selected for processing (1)
app/src/components/settings/panels/MessagingPanel.tsx
| import { useCallback, useMemo, useState } from 'react'; | ||
|
|
||
| import { useChannelDefinitions } from '../../../hooks/useChannelDefinitions'; | ||
| import { AUTH_MODE_LABELS } from '../../../lib/channels/definitions'; |
Contributor
There was a problem hiding this comment.
Import declaration conflicts with local declaration of AUTH_MODE_LABELS.
Line 4 imports AUTH_MODE_LABELS from ../../../lib/channels/definitions, but lines 38–43 declare a local constant with the same name. Remove the duplicate local declaration since the import already provides this mapping.
Proposed fix
-const AUTH_MODE_LABELS: Record<string, string> = {
- managed_dm: 'Managed DM',
- oauth: 'OAuth Sign-in',
- bot_token: 'Bot Token',
- api_key: 'API Key',
-};Also applies to: 38-43
🧰 Tools
🪛 GitHub Actions: Build
[error] 4-4: TypeScript (TS2440): Import declaration conflicts with local declaration of 'AUTH_MODE_LABELS'.
🪛 GitHub Actions: Type Check
[error] 4-4: TypeScript (tsc --noEmit) error TS2440: Import declaration conflicts with local declaration of 'AUTH_MODE_LABELS'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/settings/panels/MessagingPanel.tsx` at line 4, Remove the
duplicate local constant AUTH_MODE_LABELS in MessagingPanel.tsx and rely on the
imported AUTH_MODE_LABELS from '../../../lib/channels/definitions'; locate the
local declaration (the constant declared around lines 38–43 in the
MessagingPanel component) and delete it so there is only the single imported
symbol, ensuring any references in the file continue to use AUTH_MODE_LABELS
from the import.
| ChannelType, | ||
| } from '../../../types/channels'; | ||
| import { openUrl } from '../../../utils/openUrl'; | ||
| import ChannelCapabilities from '../../channels/ChannelCapabilities'; |
Contributor
There was a problem hiding this comment.
Remove unused imports and dead code.
The pipeline reports multiple unused declarations:
| Line(s) | Symbol | Issue |
|---|---|---|
| 22 | ChannelCapabilities |
Imported but never used |
| 28–36 | STATUS_STYLES |
Declared but never used |
| 46–134 | FALLBACK_DEFINITIONS |
Declared but never used; also references undefined ChannelDefinition type |
These are leftover artifacts from the refactor that moved logic into useChannelDefinitions. Remove them to fix the build.
Proposed fix for imports (line 22)
-import ChannelCapabilities from '../../channels/ChannelCapabilities';Proposed fix for STATUS_STYLES and FALLBACK_DEFINITIONS (lines 28–134)
-const STATUS_STYLES: Record<ChannelConnectionStatus, { label: string; className: string }> = {
- connected: { label: 'Connected', className: 'bg-sage-50 text-sage-700 border-sage-200' },
- connecting: { label: 'Connecting', className: 'bg-amber-50 text-amber-700 border-amber-200' },
- disconnected: {
- label: 'Disconnected',
- className: 'bg-stone-100 text-stone-600 border-stone-200',
- },
- error: { label: 'Error', className: 'bg-coral-50 text-coral-600 border-coral-200' },
-};
-
-const AUTH_MODE_LABELS: Record<string, string> = {
- managed_dm: 'Managed DM',
- oauth: 'OAuth Sign-in',
- bot_token: 'Bot Token',
- api_key: 'API Key',
-};
-
-/** Fallback definitions used when the core sidecar is unreachable. */
-const FALLBACK_DEFINITIONS: ChannelDefinition[] = [
- {
- id: 'telegram',
- ...
- },
- ...
-];Also applies to: 28-36, 46-134
🧰 Tools
🪛 GitHub Actions: Build
[error] 22-22: TypeScript (TS6133): 'ChannelCapabilities' is declared but its value is never read.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/settings/panels/MessagingPanel.tsx` at line 22, Remove the
unused import ChannelCapabilities and delete the dead constants STATUS_STYLES
and FALLBACK_DEFINITIONS from MessagingPanel.tsx; these are vestigial after
moving logic into useChannelDefinitions and reference an undefined
ChannelDefinition type. Open the file, remove the import line that references
ChannelCapabilities, and remove the declarations named STATUS_STYLES and
FALLBACK_DEFINITIONS (and any unused types/constants that only relate to them),
ensuring the component relies on useChannelDefinitions instead and that no
references to those symbols remain.
senamakel
approved these changes
Apr 4, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Problem
Solution
/channelsroute and sidebar entry for channel configuration.useChannelDefinitionsto load channel metadata/status and sync local Redux state.Submission Checklist
Impact
Related
Summary by CodeRabbit
New Features
Tests