feat(channels): Discord server/channel wiring and picker (#289)#349
feat(channels): Discord server/channel wiring and picker (#289)#349graycyrus merged 13 commits intotinyhumansai:mainfrom
Conversation
Add optional channel_id field to DiscordConfig for restricting the bot to a specific Discord channel, matching the pattern used by SlackConfig and MattermostConfig. Refs: tinyhumansai#289 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Expose channel_id as an optional field in the Discord BotToken auth mode so users can specify a default channel for outbound messages via the UI. Refs: tinyhumansai#289 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…scovery
Refactor discord.rs into discord/ folder module and add api.rs with:
- list_bot_guilds: GET /users/@me/guilds
- list_guild_channels: GET /guilds/{id}/channels (filtered to text channels)
- check_channel_permissions: compute bot permissions from roles + overwrites
Includes unit tests for type serialization and permission bit constants.
Refs: tinyhumansai#289
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add three new RPC endpoints: - openhuman.channels_discord_list_guilds: list servers the bot is in - openhuman.channels_discord_list_channels: list text channels in a guild - openhuman.channels_discord_check_permissions: validate bot permissions These retrieve the stored Discord bot token from credentials and call the Discord REST API directly from the Rust core. Refs: tinyhumansai#289 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tering Add channel_id field to DiscordChannel struct and update all construction sites. When channel_id is set, the listen loop only processes messages from that specific channel, enabling server+channel-scoped operation. Refs: tinyhumansai#289 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add TypeScript types for DiscordGuild, DiscordTextChannel, and BotPermissionCheck. Wire up three new RPC methods in channelConnectionsApi for listing guilds, listing channels, and checking bot permissions. Refs: tinyhumansai#289 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add server and channel selection UI that loads guilds and channels from the Discord API via the Rust core RPC. Includes permission checking with visual feedback for missing permissions. Refs: tinyhumansai#289 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Show DiscordServerChannelPicker below the connect buttons when the bot_token connection is active. Guild and channel selections flow back into the credential field values for persistence. Refs: tinyhumansai#289 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add tests for: - DiscordConfig TOML/JSON deserialization with and without channel_id - channel_id field storage on DiscordChannel struct - Config roundtrip serialization Refs: tinyhumansai#289 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Test guild loading, rendering, and placeholder states with mocked RPC responses. Verifies the component renders heading, loads guilds from the mock, and shows the select placeholder. Refs: tinyhumansai#289 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds Discord guild/channel discovery and permission checks: new frontend picker wired into DiscordConfig, new channelConnectionsApi RPC wrappers and types, backend Discord provider modules and RPC handlers, config/schema/runtime changes to carry an optional Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Frontend)
participant Picker as DiscordServerChannelPicker
participant API as channelConnectionsApi (RPC client)
participant Server as Backend RPC Handler
participant Discord as Discord API
User->>Picker: mount / interact
activate Picker
Picker->>API: listDiscordGuilds()
activate API
API->>Server: discord_list_guilds RPC
activate Server
Server->>Discord: GET /users/@me/guilds
Discord-->>Server: guild list
Server-->>API: RpcOutcome<Vec<DiscordGuild>>
API-->>Picker: DiscordGuild[]
deactivate Server
deactivate API
Picker->>User: render guilds
User->>Picker: select guild
Picker->>API: listDiscordChannels(guildId)
activate API
API->>Server: discord_list_channels RPC
activate Server
Server->>Discord: GET /guilds/{id}/channels
Discord-->>Server: channels
Server-->>API: RpcOutcome<Vec<DiscordTextChannel>>
API-->>Picker: channels
deactivate Server
deactivate API
Picker->>User: render channels
User->>Picker: select channel
Picker->>API: checkDiscordPermissions(guildId, channelId)
activate API
API->>Server: discord_check_permissions RPC
activate Server
Server->>Discord: GET /guilds/{id}/members/@me
Server->>Discord: GET /guilds/{id}/roles
Server->>Discord: GET /channels/{channelId}
Discord-->>Server: member/roles/channel
Server-->>API: BotPermissionCheck
API-->>Picker: BotPermissionCheck
deactivate Server
deactivate API
Picker->>User: render permission result
deactivate Picker
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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/__tests__/DiscordServerChannelPicker.test.tsx`:
- Around line 48-54: The assertion in the test "shows \"Select a server\"
placeholder after guilds load" uses getByRole(...) || getByText(...), but
getByRole throws when the element is missing so the fallback never runs; change
the lookup so the OR branch can execute (for example replace getByRole with
queryByRole or use findByRole with try/catch) and then assert that either the
combobox (queryByRole('combobox', { name: /server/i })) or the fallback text
(getByText('Select a server')) is in the document; update the assertion in
DiscordServerChannelPicker.test.tsx accordingly to use queryByRole (or handle
findByRole rejection) so the fallback is reachable for the
DiscordServerChannelPicker component test.
In `@app/src/components/channels/DiscordConfig.tsx`:
- Around line 212-217: The parent state keeps a stale channel_id when the
DiscordServerChannelPicker changes guild because the onGuildSelected handler
only calls updateField(compositeKey, 'guild_id', guildId); modify that handler
so it also clears the parent channel selection (e.g., call
updateField(compositeKey, 'channel_id', '' or null) immediately after setting
'guild_id') so the previous channel_id cannot be submitted after a guild switch;
locate the onGuildSelected prop on DiscordServerChannelPicker and update it to
set both 'guild_id' and clear 'channel_id'.
- Around line 211-219: Prettier formatting failed for the JSX block in
DiscordConfig.tsx around the DiscordServerChannelPicker usage; run Prettier (or
your project's formatting command) on app/ or on this file to reformat the block
so it satisfies the project's Prettier rules, then re-stage the formatted
changes; specifically ensure the JSX for DiscordServerChannelPicker and the
inline arrow callbacks (onGuildSelected, onChannelSelected calling updateField
with compositeKey) follow the project's line-break/spacing rules before
committing.
In `@app/src/components/channels/DiscordServerChannelPicker.tsx`:
- Around line 56-68: The component isn't clearing parent-held IDs when the user
clears a server or channel or switches servers; in handleGuildChange and the
channel handler referenced around lines 89-100 (e.g., handleChannelChange / the
callback that calls onChannelSelected), call the parent callbacks with an empty
value to clear persisted IDs: after setSelectedGuildId(guildId) and when guildId
is falsy, invoke onGuildSelected?.('') (or null if parent expects that) and also
invoke onChannelSelected?.('') whenever you clear or change the guild;
similarly, in the channel-selection callback ensure onChannelSelected?.('') is
called when channel is cleared so the parent doesn't retain stale
guild_id/channel_id. Ensure you use the same value type the parent expects and
update both handlers (guild and channel) consistently.
- Around line 70-84: The async loadChannels handler (and the similar async that
sets permissions around lines 101-118) unconditionally updates UI state via
setChannels / setPermissions, allowing stale responses to overwrite the current
selection; fix by tracking whether the response is still relevant before
committing state: generate a per-request token (or use an AbortController if
channelConnectionsApi supports it), store it in a ref like currentRequestRef
when starting loadChannels, capture the token (and the current guildId) inside
the async function, then before calling setChannels, setState or setError
compare the token (and/or guildId) to the stored currentRequestRef and bail out
if they differ; apply the same staleness check to the permissions-fetching async
so only the latest response updates UI.
- Around line 9-12: The component lacks props to receive existing selections, so
add optional props (e.g., selectedGuildId?: string and selectedChannelId?:
string) to DiscordServerChannelPickerProps and use them to initialize and keep
the internal selector state in sync (initialize local state from
selectedGuildId/selectedChannelId and update it when those props change using
useEffect or by making the selectors controlled via the props), then ensure
onGuildSelected/onChannelSelected still fire on user changes; update the
component to prefer prop values over empty defaults so reopening an existing
config shows the saved guild/channel.
In `@src/openhuman/channels/providers/discord/api.rs`:
- Around line 200-231: The current loop applies permission_overwrites in API
order; change it to enforce Discord precedence: first apply the `@everyone`
overwrite (where ow_type==0 and ow_id==guild_id), then compute combined role
denies and allows by iterating role overwrites (ow_type==0 and ow_id in
member_role_ids) OR-ing all deny masks together and OR-ing all allow masks
together, then apply role denies (permissions &= !role_denies) and role allows
(permissions |= role_allows), and finally apply the member overwrite (ow_type==1
and ow_id==bot_user_id); use the existing variables channel_data, member,
member_role_ids, guild_id, bot_user_id, permissions and the existing parsing
logic for allow/deny when extracting masks.
- Around line 140-150: The roles fetch currently swallows errors by turning a
failing roles_resp into an empty guild_roles, causing silent partial results;
change it to fail-fast like the earlier member fetch by propagating the error
from roles_resp.send()/roles_resp.json() instead of unwrap_or_default — e.g.,
check roles_resp.status().is_success() and return an Err (propagate the HTTP
error/result) when it isn’t, and likewise update the channel-overwrite fetch
(the code that requests overwrites) to follow the same pattern so both
roles_resp/guild_roles and the channel overwrite response propagate real errors
rather than returning empty/partial permission data.
In `@src/openhuman/channels/providers/discord/channel.rs`:
- Around line 410-415: The ChannelMessage ID fallback is producing a bare UUID
when message_id is empty, breaking the expected "discord_" namespace; update the
ChannelMessage creation (where channel_msg is built using message_id) so the
empty-ID branch also prefixes the generated UUID with "discord_" (use
Uuid::new_v4() value formatted with the "discord_" prefix) to keep ID shape
consistent with the normal path and tests.
In `@src/openhuman/channels/providers/discord/mod.rs`:
- Around line 1-4: There are two conflicting module definitions for the same
"discord" module (one as a file and one as a mod.rs) causing a duplicate-module
compile error; pick one source-of-truth and delete the other, then ensure the
remaining module exposes the same symbols (api, channel, and pub use
channel::DiscordChannel) and update any parent mod declarations so only one
module is declared.
🪄 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: 214941d7-b468-4624-98f6-f13c85e84736
📒 Files selected for processing (16)
app/src/components/channels/DiscordConfig.tsxapp/src/components/channels/DiscordServerChannelPicker.tsxapp/src/components/channels/__tests__/DiscordServerChannelPicker.test.tsxapp/src/services/api/channelConnectionsApi.tsapp/src/types/channels.tssrc/openhuman/channels/commands.rssrc/openhuman/channels/controllers/definitions.rssrc/openhuman/channels/controllers/ops.rssrc/openhuman/channels/controllers/schemas.rssrc/openhuman/channels/providers/discord/api.rssrc/openhuman/channels/providers/discord/channel.rssrc/openhuman/channels/providers/discord/mod.rssrc/openhuman/channels/runtime/startup.rssrc/openhuman/config/mod.rssrc/openhuman/config/schema/channels.rssrc/openhuman/cron/scheduler.rs
| it('shows "Select a server" placeholder after guilds load', async () => { | ||
| renderWithProviders(<DiscordServerChannelPicker />); | ||
| await waitFor(() => { | ||
| expect( | ||
| screen.getByRole('combobox', { name: /server/i }) || | ||
| screen.getByText('Select a server') | ||
| ).toBeInTheDocument(); |
There was a problem hiding this comment.
The fallback branch is unreachable in this assertion.
getByRole(...) throws when the combobox is missing, so || screen.getByText('Select a server') never runs. This only checks the first lookup in practice.
Proposed test fix
- expect(
- screen.getByRole('combobox', { name: /server/i }) ||
- screen.getByText('Select a server')
- ).toBeInTheDocument();
+ expect(screen.getByRole('combobox', { name: /server/i })).toBeInTheDocument();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/channels/__tests__/DiscordServerChannelPicker.test.tsx`
around lines 48 - 54, The assertion in the test "shows \"Select a server\"
placeholder after guilds load" uses getByRole(...) || getByText(...), but
getByRole throws when the element is missing so the fallback never runs; change
the lookup so the OR branch can execute (for example replace getByRole with
queryByRole or use findByRole with try/catch) and then assert that either the
combobox (queryByRole('combobox', { name: /server/i })) or the fallback text
(getByText('Select a server')) is in the document; update the assertion in
DiscordServerChannelPicker.test.tsx accordingly to use queryByRole (or handle
findByRole rejection) so the fallback is reachable for the
DiscordServerChannelPicker component test.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/openhuman/channels/providers/discord/api.rs (1)
228-229:⚠️ Potential issue | 🟠 MajorApply deny before allow when processing permission overwrites.
Discord's permission overwrite semantics require deny to be applied before allow. The current code applies allow first (
permissions |= allow;), then deny (permissions &= !deny;), which is inverted. When an overwrite contains both deny and allow bits, or when multiple overwrites interact, this produces incorrect permission results.Proposed fix
- permissions |= allow; - permissions &= !deny; + permissions &= !deny; + permissions |= allow;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/discord/api.rs` around lines 228 - 229, The permission overwrite logic applies allow before deny, which is incorrect; in the overwrite processing (where the variables permissions, allow, and deny are used) change the order so deny is applied first and allow after — i.e., first mask out denied bits from permissions using deny, then set allowed bits — ensuring the code path in the overwrite handling that currently does "permissions |= allow; permissions &= !deny;" is updated to apply deny before allow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/openhuman/channels/providers/discord/api.rs`:
- Around line 228-229: The permission overwrite logic applies allow before deny,
which is incorrect; in the overwrite processing (where the variables
permissions, allow, and deny are used) change the order so deny is applied first
and allow after — i.e., first mask out denied bits from permissions using deny,
then set allowed bits — ensuring the code path in the overwrite handling that
currently does "permissions |= allow; permissions &= !deny;" is updated to apply
deny before allow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 042be4f7-6ea3-4c8e-8473-37aadc1be652
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
app/src/components/channels/DiscordConfig.tsxapp/src/components/channels/DiscordServerChannelPicker.tsxapp/src/components/channels/__tests__/DiscordServerChannelPicker.test.tsxsrc/openhuman/channels/controllers/ops.rssrc/openhuman/channels/controllers/schemas.rssrc/openhuman/channels/providers/discord/api.rs
✅ Files skipped from review due to trivial changes (1)
- app/src/components/channels/DiscordServerChannelPicker.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/components/channels/DiscordConfig.tsx
- src/openhuman/channels/controllers/ops.rs
- app/src/components/channels/tests/DiscordServerChannelPicker.test.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/components/channels/__tests__/DiscordServerChannelPicker.test.tsx (1)
40-53: Tests cover initial render and guild loading; consider adding workflow coverage.The current tests verify the component renders and loads guilds correctly. For more complete coverage, consider adding tests for:
- Selecting a guild triggers channel loading
- Selecting a channel triggers permission check
- Permission check results are displayed
This is optional since the current tests verify the core functionality works.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/channels/__tests__/DiscordServerChannelPicker.test.tsx` around lines 40 - 53, Add tests for the user workflows in DiscordServerChannelPicker: write a test that renders DiscordServerChannelPicker, simulates selecting a guild (use userEvent.selectOptions or click on the combobox options) and assert that the channel-loading function/API (mock the guild->channels fetch used by the component) is called and the channel list appears; add a test that selects a channel and asserts the permission-check function (mock the permission-check API or method used by the component) is invoked and then assert the permission result UI is shown (e.g., allow/deny message or disabled state). Reference the component DiscordServerChannelPicker and the code paths that call the channel-fetch and permission-check helpers so you can mock those responses and assert DOM updates after await waitFor().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/components/channels/__tests__/DiscordServerChannelPicker.test.tsx`:
- Around line 40-53: Add tests for the user workflows in
DiscordServerChannelPicker: write a test that renders
DiscordServerChannelPicker, simulates selecting a guild (use
userEvent.selectOptions or click on the combobox options) and assert that the
channel-loading function/API (mock the guild->channels fetch used by the
component) is called and the channel list appears; add a test that selects a
channel and asserts the permission-check function (mock the permission-check API
or method used by the component) is invoked and then assert the permission
result UI is shown (e.g., allow/deny message or disabled state). Reference the
component DiscordServerChannelPicker and the code paths that call the
channel-fetch and permission-check helpers so you can mock those responses and
assert DOM updates after await waitFor().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 492d15c6-ef77-457c-8c07-7e9f5a78cd41
📒 Files selected for processing (5)
app/src/components/channels/DiscordConfig.tsxapp/src/components/channels/DiscordServerChannelPicker.tsxapp/src/components/channels/__tests__/DiscordServerChannelPicker.test.tsxsrc/openhuman/channels/providers/discord/api.rssrc/openhuman/channels/providers/discord/channel.rs
✅ Files skipped from review due to trivial changes (1)
- app/src/components/channels/DiscordConfig.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/components/channels/DiscordServerChannelPicker.tsx
Brings in Discord server/channel picker (tinyhumansai#349) and autocomplete observability improvements (tinyhumansai#308) from main. Resolves conflict in tests/json_rpc_e2e.rs by keeping both the screen intelligence tests (this branch) and the autocomplete runtime settings test (main). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Notes
Verification
Summary by CodeRabbit
New Features
Improvements
Tests
Backend/API