Fix: discord#580
Merged
graycyrus merged 16 commits intotinyhumansai:mainfrom Apr 15, 2026
Merged
Conversation
…rdConfig component - Added support for managed Discord linking, including new API methods for initiating and checking link status. - Enhanced the DiscordConfig component to manage link tokens and implement polling for link completion. - Introduced new state management for link tokens and improved error handling during connection processes. - Updated the definitions to include a new auth action for managed links, streamlining the onboarding experience for users. - Added tests for the new Discord link functionality to ensure robust integration and error handling. This commit significantly improves the user experience for connecting Discord accounts, providing clearer feedback and handling for managed links.
…rdConfig component - Added support for managed Discord linking, including new API methods for initiating and checking link status. - Enhanced the DiscordConfig component to manage link tokens and implement polling for link status, improving user experience during the linking process. - Introduced new state management for link tokens and error handling, ensuring robust interaction with the Discord API. - Updated the channel definitions to include a new auth action for managed links, streamlining the integration process. - Added tests for the new API methods and updated existing tests to cover the new functionality. This commit significantly improves the integration with Discord, allowing users to link their accounts more effectively and providing better feedback during the linking process.
- Bumped the version of the OpenHuman package from 0.52.7 to 0.52.9 in both Cargo.lock files for the main application and the Tauri app. - This update ensures that the latest features and fixes from the OpenHuman package are included in the project.
…ount - Adjusted the test for the DiscordConfig component to expect three Connect buttons instead of two, aligning with recent changes in the component's authentication modes. - This update ensures that the test accurately reflects the current functionality and improves test reliability.
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 57 minutes and 34 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (16)
📝 WalkthroughWalkthroughAdds Discord managed link functionality enabling users to link personal Discord accounts through a token-based authentication flow. Includes frontend polling mechanism, new backend RPC endpoints for initiating and verifying links, response validation infrastructure, and improvements to error classification for upstream provider issues. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as DiscordConfig<br/>(Frontend)
participant API as channelConnectionsApi<br/>(Frontend)
participant RPCHandler as RPC Handler<br/>(Backend)
participant Ops as Channel Ops<br/>(Backend)
participant Discord as Discord API<br/>(External)
User->>UI: Click "Connect" (managed_dm mode)
UI->>API: discordLinkStart()
API->>RPCHandler: RPC: discord_link_start
RPCHandler->>Ops: discord_link_start(config)
Ops->>Discord: POST /oauth/authorize (generate token)
Discord-->>Ops: link_token, instructions
Ops-->>RPCHandler: DiscordLinkStartResult
RPCHandler-->>API: {linkToken, instructions}
API-->>UI: {linkToken, instructions}
UI->>UI: Show token card, start polling
UI->>UI: Update status to "connecting"
loop Poll until linked or timeout
UI->>API: discordLinkCheck(linkToken)
API->>RPCHandler: RPC: discord_link_check
RPCHandler->>Ops: discord_link_check(config, linkToken)
Ops->>Discord: GET /users/@me
Discord-->>Ops: Check if discordId linked
alt Account linked
Ops->>Ops: Store channel:discord:managed_dm credential
Ops-->>RPCHandler: DiscordLinkCheckResult{linked: true}
RPCHandler-->>API: {linked: true}
API-->>UI: {linked: true}
UI->>UI: Update status to "connected", stop polling
UI->>UI: Hide Connect, show Disconnect
else Token expired
Ops-->>RPCHandler: Error: token expired
RPCHandler-->>API: Error
API-->>UI: Error
UI->>UI: Update status to "error", stop polling
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 2
🧹 Nitpick comments (9)
src/openhuman/channels/providers/discord/api.rs (1)
220-226: Variable shadowing is intentional but could use a clarifying comment.The
bot_user_idat line 222-226 shadows the earlier declaration at line 136. The intent appears to be preferring the ID from the member response (if present) over the one from/users/@me. This is correct behavior, but a brief comment would help future readers understand why.📝 Suggested clarification
+ // Prefer user ID from member response; fall back to the ID from /users/@me let bot_user_id = member .get("user") .and_then(|u| u.get("id")) .and_then(|i| i.as_str()) .unwrap_or(bot_user_id);🤖 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 220 - 226, The local variable bot_user_id inside the block that inspects member.get("user") intentionally shadows the earlier bot_user_id (from the /users/@me response) to prefer the ID returned in the member object when present; add a short clarifying comment immediately above the shadowing assignment (the block using member.get("user")...unwrap_or(bot_user_id)) explaining that this shadowing is deliberate and that the member-provided ID should override the previously fetched bot ID to aid future readers.app/src/services/api/__tests__/channelConnectionsApi.test.ts (1)
46-55: Consider adding tests for the newdiscordLinkStartanddiscordLinkCheckmethods.The test suite validates
listDiscordGuildsandconnectChannelbut doesn't cover the newly added managed-link methods. Adding coverage would help catch regressions in the link flow.🧪 Example test cases
it('unwraps Discord link start from CLI envelope', async () => { mockCallCoreRpc.mockResolvedValueOnce({ result: { linkToken: 'abc123', instructions: 'Send !start abc123' }, logs: ['link token created'], }); await expect(channelConnectionsApi.discordLinkStart()).resolves.toEqual({ linkToken: 'abc123', instructions: 'Send !start abc123', }); }); it('unwraps Discord link check from CLI envelope', async () => { mockCallCoreRpc.mockResolvedValueOnce({ result: { linked: true, details: { discord_user_id: '123' } }, logs: ['link verified'], }); await expect(channelConnectionsApi.discordLinkCheck('abc123')).resolves.toEqual({ linked: true, details: { discord_user_id: '123' }, }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/services/api/__tests__/channelConnectionsApi.test.ts` around lines 46 - 55, Add unit tests in the same suite that exercise channelConnectionsApi.discordLinkStart and channelConnectionsApi.discordLinkCheck: mock mockCallCoreRpc to return the CLI envelope shape (e.g., { result: { linkToken, instructions }, logs: [...] }) and assert discordLinkStart() resolves to the unwrapped { linkToken, instructions }, then mock { result: { linked, details }, logs: [...] } and assert discordLinkCheck('token') resolves to the unwrapped { linked, details }; reuse the existing mocking pattern used in the file to mirror the examples provided.app/src/services/api/channelConnectionsApi.ts (1)
62-69:expectObjectperforms shape validation but not field validation.The function verifies the payload is an object but casts to
Twithout checking that required fields exist. ForDiscordLinkStartResult, if the backend returns{}, the caller would get an object withoutlinkTokenorinstructions.Consider adding field validation for critical result types, similar to
normalizeConnectResultwhich validatesstatus.🛡️ Example field validation for link results
function expectDiscordLinkStart(payload: unknown): DiscordLinkStartResult { const record = expectObject<Record<string, unknown>>(payload, 'Discord link start'); const linkToken = typeof record.linkToken === 'string' ? record.linkToken : ''; const instructions = typeof record.instructions === 'string' ? record.instructions : ''; if (!linkToken) { throw new Error('Discord link start response missing linkToken'); } return { linkToken, instructions }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/services/api/channelConnectionsApi.ts` around lines 62 - 69, The current expectObject<T> only checks that payload is an object and then unsafely casts to T; add explicit field-level validators for critical response types instead of widening expectObject. For example, keep expectObject to unwrap/asRecord, but implement new functions like expectDiscordLinkStart(payload) and expectDiscordLinkComplete(payload) that call expectObject<Record<string, unknown>>('Discord link start' / 'Discord link complete'), then validate required fields (e.g., ensure record.linkToken and record.instructions are strings, or record.status matches expected enum) and throw descriptive errors if missing—mirroring normalizeConnectResult’s pattern of strict field validation and erroring on invalid shapes.src/openhuman/channels/controllers/definitions.rs (1)
358-366: Consider extending test coverage for the newManagedDmmode.The existing test
discord_has_bot_token_and_oauthverifiesBotTokenandOAuthmodes but doesn't assert on the newly addedManagedDm. Adding an assertion would catch accidental regressions.🧪 Suggested test extension
#[test] -fn discord_has_bot_token_and_oauth() { +fn discord_has_bot_token_oauth_and_managed_dm() { let def = find_channel_definition("discord").expect("discord not found"); assert!(def.auth_mode_spec(ChannelAuthMode::BotToken).is_some()); assert!(def.auth_mode_spec(ChannelAuthMode::OAuth).is_some()); + assert!(def.auth_mode_spec(ChannelAuthMode::ManagedDm).is_some()); let oauth = def.auth_mode_spec(ChannelAuthMode::OAuth).unwrap(); assert_eq!(oauth.auth_action, Some("discord_oauth")); + + let managed = def.auth_mode_spec(ChannelAuthMode::ManagedDm).unwrap(); + assert_eq!(managed.auth_action, Some("discord_managed_link")); + assert!(managed.fields.is_empty()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/controllers/definitions.rs` around lines 358 - 366, Extend the test discord_has_bot_token_and_oauth to also assert the new ManagedDm mode is present: call def.auth_mode_spec(ChannelAuthMode::ManagedDm) and assert it is_some(), then unwrap and assert its auth_action equals the expected value (e.g., "discord_managed_dm"); use the existing pattern with find_channel_definition, is_some(), unwrap(), and auth_action to mirror the OAuth check.app/src/components/channels/DiscordConfig.tsx (1)
71-126: Polling implementation is solid, but consider logging persistent failures.The polling logic correctly handles abort signals, timeout expiration, and successful completion. However, the empty catch block at lines 96-98 silently swallows all errors. While this is fine for transient network issues, persistent failures (e.g., auth revoked, backend down) would be invisible.
🔍 Suggested improvement for visibility
try { const check = await channelConnectionsApi.discordLinkCheck(token); if (check.linked) { log('discord managed link completed'); setLinkToken(null); dispatch( upsertChannelConnection({ channel: 'discord', authMode: 'managed_dm', patch: { status: 'connected', lastError: undefined, capabilities: ['dm'] }, }) ); return; } - } catch { - // keep polling + } catch (err) { + log('discord link check failed, will retry: %o', err); }🤖 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 71 - 126, The empty catch inside startLinkPolling (around the channelConnectionsApi.discordLinkCheck call) swallows all errors; change it to log the error so persistent failures are visible—e.g., catch (err) { log('discord link check failed', err); }—and optionally, for non-transient errors, update state via setLinkToken(null) and dispatch upsertChannelConnection({... , patch: { status: 'error', lastError: String(err) }}) where appropriate; keep existing abort/timeout behavior intact and reference startLinkPolling, channelConnectionsApi.discordLinkCheck, setLinkToken, and upsertChannelConnection when making the changes.src/openhuman/channels/controllers/ops.rs (2)
250-255: Variable shadowing:channel_idparameter is shadowed by local binding.The outer
channel_idparameter (value:"discord") is shadowed by the localchannel_idextracted fromcreds_map(the Discord channel ID). While this doesn't cause a bug in the current code, it reduces clarity and could lead to subtle issues if this block is extended later.Consider renaming the local variable to
discord_channel_idfor clarity.♻️ Suggested rename to avoid shadowing
- let channel_id = creds_map + let discord_channel_id = creds_map .get("channel_id") .and_then(|v| v.as_str()) .map(str::trim) .filter(|s| !s.is_empty()) .map(|s| s.to_string());And update the usage at line 276:
persisted.channels_config.discord = Some(DiscordConfig { bot_token, guild_id: guild_id.clone(), - channel_id: channel_id.clone(), + channel_id: discord_channel_id.clone(), allowed_users, listen_to_bots, mention_only, });And at line 290:
- has_channel_id = channel_id.is_some(), + has_channel_id = discord_channel_id.is_some(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/controllers/ops.rs` around lines 250 - 255, The local binding named channel_id inside the creds_map extraction shadows the outer parameter channel_id; rename the local to discord_channel_id (or similar) and update all subsequent uses in this function/block (the places that previously referenced the local channel_id after the extraction) to use discord_channel_id so the original parameter remains unshadowed and intent is clear (update references in the conditional logic that uses the extracted channel id and any downstream calls that passed the local variable).
720-724: Consider redacting user ID from debug logs.The
discord_idis logged at debug level. While this mirrors the existing Telegram pattern (line 562-566), the coding guidelines mention avoiding logging "user identifiers" in debug logs. Discord/Telegram user IDs are numeric identifiers that could be considered user-identifying information.Consider logging only whether the ID is present rather than its value:
🔒 Optional: Redact user ID from logs
log::debug!( - "[discord-link] user profile discordId: {:?}, linked={}", - discord_id, + "[discord-link] user profile has discordId: {}, linked={}", + discord_id.is_some(), linked );Note: This would also apply to the equivalent Telegram code at lines 562-566 for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/controllers/ops.rs` around lines 720 - 724, The debug log currently prints the Discord user identifier (discord_id) in the log::debug call; change it to avoid logging the raw ID and instead log only whether an ID is present (e.g., "discord_id_present={}" or "has_discord_id={}") while keeping the linked flag, updating the code around the log::debug that references discord_id and linked; also make the equivalent change to the Telegram debug logging block that mirrors this behavior so both channels redact user IDs consistently.src/openhuman/tools/impl/agent/spawn_subagent.rs (1)
49-64: Unify upstream-unhealthy matching with provider-level classifier.This helper’s signatures are narrower than
src/openhuman/providers/reliable.rsand can miss equivalent outage errors (for example,"upstream unavailable"/"service unavailable"), causing inconsistent parent-facing classification.♻️ Proposed matcher parity update
fn classify_subagent_failure(message: &str) -> String { let lower = message.to_lowercase(); let upstream_unhealthy = lower.contains("no healthy upstream") + || lower.contains("upstream unavailable") + || lower.contains("service unavailable") || lower.contains("upstream_unhealthy") || lower.contains("provider call failed: all providers/models failed");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/tools/impl/agent/spawn_subagent.rs` around lines 49 - 64, The classify_subagent_failure helper (function classify_subagent_failure) currently only matches a few phrases and should be expanded to mirror the provider-level outage classifier in src/openhuman/providers/reliable.rs; update the matching set used in classify_subagent_failure to include equivalent outage phrases such as "upstream unavailable", "service unavailable", "unavailable", and any other tokens used by the provider classifier (e.g., "503", "rate limited", or normalized variants), so the upstream_unhealthy boolean uses the same normalized checks as the provider-level classifier to produce consistent parent-facing messages.src/openhuman/providers/reliable.rs (1)
78-85: Add direct tests for upstream classification precedence.The new
is_upstream_unhealthy+failure_reasonbranch is good, but there’s no focused unit test asserting thatupstream_unhealthywins over other reasons. A small test pair here would lock behavior and prevent silent regressions.♻️ Proposed test additions
+ #[test] + fn upstream_unhealthy_detection_matches_common_signatures() { + assert!(is_upstream_unhealthy(&anyhow::anyhow!("503 Service Unavailable: no healthy upstream"))); + assert!(is_upstream_unhealthy(&anyhow::anyhow!("upstream unavailable"))); + assert!(is_upstream_unhealthy(&anyhow::anyhow!("service unavailable"))); + assert!(!is_upstream_unhealthy(&anyhow::anyhow!("429 Too Many Requests"))); + } + + #[test] + fn failure_reason_prioritizes_upstream_unhealthy() { + assert_eq!(failure_reason(true, true, true), "upstream_unhealthy"); + assert_eq!(failure_reason(true, true, false), "rate_limited_non_retryable"); + }Also applies to: 178-194
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/providers/reliable.rs` around lines 78 - 85, Add focused unit tests that assert classification precedence so "upstream_unhealthy" wins over other failure reasons: write tests that call is_upstream_unhealthy and the public failure_reason (or the function that returns classification) with error strings that include both an upstream indicator (e.g., "no healthy upstream", "service unavailable") and another plausible reason (e.g., timeout, auth error) and assert the result is upstream_unhealthy; add one test for a direct upstream-only message and one for a mixed message to lock behavior in the is_upstream_unhealthy and failure_reason branches (use the existing functions is_upstream_unhealthy and failure_reason to locate where to hook the tests).
🤖 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/providers/SocketProvider.tsx`:
- Around line 46-52: The call to callCoreRpc({ method:
'openhuman.socket_connect_with_session', ... }) in SocketProvider.tsx currently
swallows errors; modify the catch to log a grep-friendly, development-oriented
message including the method name, that the RPC connection failed but is
non-fatal, and include the full error object/stack for debugging (use the app's
logger if available, e.g., processLogger or console.error, with a unique prefix
like "socket-connect:" or "openhuman.socket_connect_with_session:"). Ensure the
log provides context (where it was attempted — SocketProvider) and retains the
non-fatal behavior.
In `@src/openhuman/tools/impl/agent/spawn_subagent.rs`:
- Around line 247-253: The tracing::error! call in spawn_subagent.rs is logging
raw payloads (variables message and parent_visible_error); change it to avoid
printing full error strings by either omitting those variables or replacing them
with redacted/truncated representations (e.g., use a helper like
redact(&message) or redact_error_summary(&parent_visible_error) that returns
only an error type, short summary, or "[REDACTED]" placeholder), and log only
non-sensitive context (agent_id, task_id) along with the redacted summary so no
secrets/PII are emitted.
---
Nitpick comments:
In `@app/src/components/channels/DiscordConfig.tsx`:
- Around line 71-126: The empty catch inside startLinkPolling (around the
channelConnectionsApi.discordLinkCheck call) swallows all errors; change it to
log the error so persistent failures are visible—e.g., catch (err) {
log('discord link check failed', err); }—and optionally, for non-transient
errors, update state via setLinkToken(null) and dispatch
upsertChannelConnection({... , patch: { status: 'error', lastError: String(err)
}}) where appropriate; keep existing abort/timeout behavior intact and reference
startLinkPolling, channelConnectionsApi.discordLinkCheck, setLinkToken, and
upsertChannelConnection when making the changes.
In `@app/src/services/api/__tests__/channelConnectionsApi.test.ts`:
- Around line 46-55: Add unit tests in the same suite that exercise
channelConnectionsApi.discordLinkStart and
channelConnectionsApi.discordLinkCheck: mock mockCallCoreRpc to return the CLI
envelope shape (e.g., { result: { linkToken, instructions }, logs: [...] }) and
assert discordLinkStart() resolves to the unwrapped { linkToken, instructions },
then mock { result: { linked, details }, logs: [...] } and assert
discordLinkCheck('token') resolves to the unwrapped { linked, details }; reuse
the existing mocking pattern used in the file to mirror the examples provided.
In `@app/src/services/api/channelConnectionsApi.ts`:
- Around line 62-69: The current expectObject<T> only checks that payload is an
object and then unsafely casts to T; add explicit field-level validators for
critical response types instead of widening expectObject. For example, keep
expectObject to unwrap/asRecord, but implement new functions like
expectDiscordLinkStart(payload) and expectDiscordLinkComplete(payload) that call
expectObject<Record<string, unknown>>('Discord link start' / 'Discord link
complete'), then validate required fields (e.g., ensure record.linkToken and
record.instructions are strings, or record.status matches expected enum) and
throw descriptive errors if missing—mirroring normalizeConnectResult’s pattern
of strict field validation and erroring on invalid shapes.
In `@src/openhuman/channels/controllers/definitions.rs`:
- Around line 358-366: Extend the test discord_has_bot_token_and_oauth to also
assert the new ManagedDm mode is present: call
def.auth_mode_spec(ChannelAuthMode::ManagedDm) and assert it is_some(), then
unwrap and assert its auth_action equals the expected value (e.g.,
"discord_managed_dm"); use the existing pattern with find_channel_definition,
is_some(), unwrap(), and auth_action to mirror the OAuth check.
In `@src/openhuman/channels/controllers/ops.rs`:
- Around line 250-255: The local binding named channel_id inside the creds_map
extraction shadows the outer parameter channel_id; rename the local to
discord_channel_id (or similar) and update all subsequent uses in this
function/block (the places that previously referenced the local channel_id after
the extraction) to use discord_channel_id so the original parameter remains
unshadowed and intent is clear (update references in the conditional logic that
uses the extracted channel id and any downstream calls that passed the local
variable).
- Around line 720-724: The debug log currently prints the Discord user
identifier (discord_id) in the log::debug call; change it to avoid logging the
raw ID and instead log only whether an ID is present (e.g.,
"discord_id_present={}" or "has_discord_id={}") while keeping the linked flag,
updating the code around the log::debug that references discord_id and linked;
also make the equivalent change to the Telegram debug logging block that mirrors
this behavior so both channels redact user IDs consistently.
In `@src/openhuman/channels/providers/discord/api.rs`:
- Around line 220-226: The local variable bot_user_id inside the block that
inspects member.get("user") intentionally shadows the earlier bot_user_id (from
the /users/@me response) to prefer the ID returned in the member object when
present; add a short clarifying comment immediately above the shadowing
assignment (the block using member.get("user")...unwrap_or(bot_user_id))
explaining that this shadowing is deliberate and that the member-provided ID
should override the previously fetched bot ID to aid future readers.
In `@src/openhuman/providers/reliable.rs`:
- Around line 78-85: Add focused unit tests that assert classification
precedence so "upstream_unhealthy" wins over other failure reasons: write tests
that call is_upstream_unhealthy and the public failure_reason (or the function
that returns classification) with error strings that include both an upstream
indicator (e.g., "no healthy upstream", "service unavailable") and another
plausible reason (e.g., timeout, auth error) and assert the result is
upstream_unhealthy; add one test for a direct upstream-only message and one for
a mixed message to lock behavior in the is_upstream_unhealthy and failure_reason
branches (use the existing functions is_upstream_unhealthy and failure_reason to
locate where to hook the tests).
In `@src/openhuman/tools/impl/agent/spawn_subagent.rs`:
- Around line 49-64: The classify_subagent_failure helper (function
classify_subagent_failure) currently only matches a few phrases and should be
expanded to mirror the provider-level outage classifier in
src/openhuman/providers/reliable.rs; update the matching set used in
classify_subagent_failure to include equivalent outage phrases such as "upstream
unavailable", "service unavailable", "unavailable", and any other tokens used by
the provider classifier (e.g., "503", "rate limited", or normalized variants),
so the upstream_unhealthy boolean uses the same normalized checks as the
provider-level classifier to produce consistent parent-facing messages.
🪄 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: 828c808a-0e3e-436a-b58d-39f93023b3da
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockapp/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
app/src/components/channels/DiscordConfig.tsxapp/src/components/channels/__tests__/DiscordConfig.test.tsxapp/src/lib/channels/definitions.tsapp/src/providers/SocketProvider.tsxapp/src/services/api/__tests__/channelConnectionsApi.test.tsapp/src/services/api/channelConnectionsApi.tssrc/openhuman/channels/controllers/definitions.rssrc/openhuman/channels/controllers/ops.rssrc/openhuman/channels/controllers/schemas.rssrc/openhuman/channels/providers/discord/api.rssrc/openhuman/providers/reliable.rssrc/openhuman/tools/impl/agent/spawn_subagent.rssrc/openhuman/voice/schemas.rs
…ogging - Extracted the `check_channel_permissions_at_base` function to modularize the permission checking logic for better readability and maintainability. - Updated the API calls to use a dynamic base URL instead of a hardcoded one, improving flexibility. - Enhanced logging for non-success responses from Discord API calls, providing more context for debugging. - Minor adjustment in the hallucination check logic to improve clarity in variable usage.
- Updated error handling in DiscordConfig to log specific errors when link checks fail, improving debugging capabilities. - Enhanced SocketProvider to log non-fatal RPC connection failures, providing clearer context for potential issues with the sidecar or backend connectivity.
…mplete responses - Introduced `expectDiscordLinkStart` and `expectDiscordLinkComplete` functions to validate the structure of responses from Discord link API calls. - Updated `channelConnectionsApi` methods to utilize these new validation functions, improving error handling and response consistency. - Added tests to ensure proper unwrapping and validation of Discord link responses, enhancing overall reliability of the API integration.
…es in touchConnection function - Updated the touchConnection function to conditionally assign lastError and capabilities based on their presence in the patch object, ensuring more accurate state management. - This change enhances the reliability of connection updates by preventing unintended overwrites of existing values.
… set to undefined - Introduced a new test case to verify that the lastError state is cleared when an explicit patch sets it to undefined. This enhances the reliability of the state management in the channelConnectionsSlice reducer.
…eason precedence - Introduced multiple test cases to verify the detection of upstream unavailability and service unavailability errors. - Ensured that the `failure_reason` function correctly prioritizes upstream_unhealthy over other error states, enhancing the reliability of error handling in the system.
- Implemented a new useEffect to handle successful OAuth events for Discord, updating the channel connection status and capabilities accordingly. - This enhancement improves the integration with Discord by ensuring that the application responds correctly to OAuth success events, providing a better user experience.
- Updated the deep link handling logic to also retrieve the skillId from the URL parameters, improving flexibility in OAuth success scenarios. - Added a new simulation example for skillId in the setupDesktopDeepLinkListener function to aid in testing and development.
- Revised the permissions mock to include additional endpoints for better accuracy in testing Discord API interactions. - Adjusted the message dispatch test to utilize a deterministic stub for the agent's response, ensuring consistent timing and behavior during tests. - Reduced the delay in the SlowProvider to enhance test performance while maintaining robustness.
graycyrus
approved these changes
Apr 15, 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
managed_dmauth mode and end-to-end managed link flow in the app UI.discord_link_startanddiscord_link_checkRPC endpoints and wires them into channel schemas/controllers.{ result, logs }) and validate response shapes.Problem
Solution
discord_link_startreturns a short-lived token/instructions.discord_link_checkpolls link completion via profile state and updates local credential markers.discord_managed_link.channelConnectionsApiwith envelope unwrapping + strict object/array normalization for connect/list/check operations.DiscordConfigUI to drive managed-link polling, state transitions, and safer connect/disconnect handling.Submission Checklist
(Any feature related checklist can go in here)
Impact
Related
Summary by CodeRabbit
New Features
Bug Fixes