-
Notifications
You must be signed in to change notification settings - Fork 46k
fix(backend): Add channel ID support to SendDiscordMessageBlock for consistency with other Discord blocks #11055
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughUpdated Discord Send message block to accept either channel ID or channel name, with optional server_name only needed for name-based lookup. Implemented hybrid lookup in on_ready: try ID via get_channel, otherwise search guilds (optionally filtered by server_name), validate channel is sendable, chunk and send message, or set status to "Channel not found." Updated docs accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Block as SendDiscordMessageBlock
participant Discord as Discord Client/API
User->>Block: Provide token, channel_name (ID or name), server_name (optional), message
Block->>Discord: Login and on_ready
rect rgb(235, 245, 255)
note over Block: Resolve channel
Block->>Block: Is channel_name numeric?
alt ID-based lookup
Block->>Discord: get_channel(channel_id)
Discord-->>Block: Channel or null
else Name-based lookup
Block->>Discord: List guilds (filter by server_name if provided)
Block->>Discord: Find text channel by name
Discord-->>Block: Channel or null
end
end
alt Channel found and sendable
loop Chunk message
Block->>Discord: channel.send(chunk)
Discord-->>Block: ack
end
Block-->>User: Status: sent
else Channel not found or not text
Block-->>User: Status: Channel not found
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
✅ Deploy Preview for auto-gpt-docs canceled.
|
Here's the code health analysis summary for commits Analysis Summary
|
This PR is marked as Work in Progress and doesn't contain any code changes yet. Please update the PR once you have implemented the requested changes - allowing SendDiscordMessage to accept a channel ID under the channelname parameter similar to other Discord blocks. When submitting your changes, please ensure you:
Looking forward to reviewing your implementation once it's ready! |
Thank you for creating this PR to address the SendDiscordMessageBlock enhancement. I can see this is marked as Work In Progress (WIP), and there are no code changes submitted yet. Here's some feedback to help you prepare for when you're ready to submit the actual changes:
This looks like a straightforward enhancement that will improve consistency across our Discord blocks. Looking forward to reviewing the implementation when it's ready! |
Co-authored-by: ntindle <[email protected]>
Thanks for making these changes to improve the SendDiscordMessageBlock! The implementation looks good and follows the pattern from the other Discord blocks. Before this can be merged, please update the PR title to follow our conventional commit format. The title should use one of these types: feat, fix, refactor, ci, dx - followed by an appropriate scope. For example, a good title might be: Also, I notice the PR is marked as [WIP] (Work in Progress). Once you're ready for review and merge, please remove that prefix from the title. The code changes themselves look great and match the requirements described in the linked issue. |
Thank you for submitting this PR that enhances the SendDiscordMessageBlock to accept both channel IDs and channel names. The implementation looks solid and follows the pattern from other Discord blocks. Before we can merge this PR:
Otherwise, your implementation looks good - you've maintained backward compatibility while adding the ability to use channel IDs, added appropriate error handling, and updated both the code and documentation. Nice work! |
Thank you for this PR enhancing the SendDiscordMessageBlock functionality! Your implementation looks good - I can see you've followed the pattern from other Discord blocks to make the channel_name field accept both IDs and channel names. The code changes look appropriate with proper error handling and type checking. Before this can be merged, please update the PR title to follow our conventional commit format. Since this is enhancing an existing feature, a title like:
would be appropriate. Also, you might want to remove the [WIP] tag when you're ready for final review. Everything else looks good to go! |
@coderabbitai review |
✅ Actions performedReview triggered.
|
@claude review this follows our block best practices |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
autogpt_platform/backend/backend/blocks/discord/bot_blocks.py
(2 hunks)docs/content/platform/blocks/discord.md
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (3.11)
- GitHub Check: Check PR Status
- GitHub Check: Check PR Status
🔇 Additional comments (1)
docs/content/platform/blocks/discord.md (1)
46-46
: Docs change looks good.Updated wording accurately reflects the new flexibility in the backend.
print(f"Logged in as {client.user}") | ||
for guild in client.guilds: | ||
if server_name and guild.name != server_name: | ||
continue | ||
for channel in guild.text_channels: | ||
if channel.name == channel_name: | ||
# Split message into chunks if it exceeds 2000 characters | ||
chunks = self.chunk_message(message_content) | ||
last_message = None | ||
for chunk in chunks: | ||
last_message = await channel.send(chunk) | ||
result["status"] = "Message sent" | ||
result["message_id"] = ( | ||
str(last_message.id) if last_message else "" | ||
) | ||
result["channel_id"] = str(channel.id) | ||
await client.close() | ||
return | ||
|
||
result["status"] = "Channel not found" | ||
channel = None | ||
|
||
# Try to parse as channel ID first | ||
try: | ||
channel_id = int(channel_name) | ||
channel = client.get_channel(channel_id) | ||
except ValueError: | ||
# Not an ID, treat as channel name | ||
for guild in client.guilds: | ||
if server_name and guild.name != server_name: | ||
continue | ||
for ch in guild.text_channels: | ||
if ch.name == channel_name: | ||
channel = ch | ||
break | ||
if channel: | ||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fallback to name lookup when ID cache miss occurs.
If channel_name
is something numeric like "12345"
, int(channel_name)
succeeds but client.get_channel()
returns None
, so we never drop into the name-based search. Those channels worked before and now always hit “Channel not found,” which is a regression. Make the name-search fallback run whenever the ID lookup fails to resolve a channel, not only when int()
raises.
channel = None
# Try to parse as channel ID first
- try:
- channel_id = int(channel_name)
- channel = client.get_channel(channel_id)
- except ValueError:
- # Not an ID, treat as channel name
- for guild in client.guilds:
- if server_name and guild.name != server_name:
- continue
- for ch in guild.text_channels:
- if ch.name == channel_name:
- channel = ch
- break
- if channel:
- break
+ try:
+ channel_id = int(channel_name)
+ channel = client.get_channel(channel_id)
+ except ValueError:
+ channel = None
+
+ if not channel:
+ # Treat as channel name
+ for guild in client.guilds:
+ if server_name and guild.name != server_name:
+ continue
+ for ch in guild.text_channels:
+ if ch.name == channel_name:
+ channel = ch
+ break
+ if channel:
+ break
📝 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.
print(f"Logged in as {client.user}") | |
for guild in client.guilds: | |
if server_name and guild.name != server_name: | |
continue | |
for channel in guild.text_channels: | |
if channel.name == channel_name: | |
# Split message into chunks if it exceeds 2000 characters | |
chunks = self.chunk_message(message_content) | |
last_message = None | |
for chunk in chunks: | |
last_message = await channel.send(chunk) | |
result["status"] = "Message sent" | |
result["message_id"] = ( | |
str(last_message.id) if last_message else "" | |
) | |
result["channel_id"] = str(channel.id) | |
await client.close() | |
return | |
result["status"] = "Channel not found" | |
channel = None | |
# Try to parse as channel ID first | |
try: | |
channel_id = int(channel_name) | |
channel = client.get_channel(channel_id) | |
except ValueError: | |
# Not an ID, treat as channel name | |
for guild in client.guilds: | |
if server_name and guild.name != server_name: | |
continue | |
for ch in guild.text_channels: | |
if ch.name == channel_name: | |
channel = ch | |
break | |
if channel: | |
break | |
print(f"Logged in as {client.user}") | |
channel = None | |
# Try to parse as channel ID first | |
try: | |
channel_id = int(channel_name) | |
channel = client.get_channel(channel_id) | |
except ValueError: | |
channel = None | |
if not channel: | |
# Treat as channel name | |
for guild in client.guilds: | |
if server_name and guild.name != server_name: | |
continue | |
for ch in guild.text_channels: | |
if ch.name == channel_name: | |
channel = ch | |
break | |
if channel: | |
break |
🤖 Prompt for AI Agents
In autogpt_platform/backend/backend/blocks/discord/bot_blocks.py around lines
233–250, the code currently only performs the name-based guild/channel search
when int(channel_name) raises; change the logic so that after attempting to
parse channel_name as an int and calling client.get_channel(channel_id), if that
returns None you still fall back to the name-based search (and likewise if
parsing raises ValueError). Implement a single fallback path that runs the
existing guild/text_channel name loop whenever the ID lookup yields None or
parsing fails.
Claude finished @ntindle's task —— View job Code Review: SendDiscordMessageBlock Channel ID SupportI've reviewed the PR against your block best practices. Here's my analysis: ✅ Strengths
|
@claude fix that issue you found |
Claude finished @ntindle's task —— View job ✅ Fixed the channel ID fallback logicCompleted Tasks
SummaryThe issue has been fixed! The channel resolution logic now properly falls back to name-based search when:
This ensures backward compatibility for numeric channel names like "12345" that previously worked as channel names. They will now correctly fall back to name-based search if not found as an ID. The fix is simple but critical: the name-based search now runs whenever View job | |
When a numeric string was provided as channel_name, the code would try to parse it as an ID, but if get_channel() returned None (channel not in cache), it would never fall back to name-based search. This broke backward compatibility for channel names that happened to be numeric strings. The fix ensures that name-based search runs whenever ID lookup yields None, not only when int() raises ValueError. Co-authored-by: Nicholas Tindle <[email protected]>
✅ Deploy Preview for auto-gpt-docs-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Problem
The
SendDiscordMessageBlock
only accepted channel names, while other Discord blocks likeSendDiscordFileBlock
andSendDiscordEmbedBlock
accept both channel IDs and channel names. This inconsistency made it difficult to use channel IDs with the message sending block, which is often more reliable and direct than name-based lookup.Solution
Updated
SendDiscordMessageBlock
to accept both channel IDs and channel names through thechannel_name
field, matching the implementation pattern used in other Discord blocks.Changes Made
Enhanced channel resolution logic to try parsing the input as a channel ID first, then fall back to name-based search:
Updated field descriptions to clarify the dual functionality:
channel_name
: Now describes that it accepts "Channel ID or channel name"server_name
: Clarified as "only needed if using channel name"Added type checking to ensure the resolved channel can send messages before attempting to send
Updated documentation to reflect the new capability
Backward Compatibility
✅ Fully backward compatible: The field name remains
channel_name
(not renamed), and all existing workflows using channel names will continue to work exactly as before.✅ New capability: Users can now also provide channel IDs (e.g.,
"123456789012345678"
) for more direct channel targeting.Testing
SendDiscordMessageBlock
and all other Discord block testsSendDiscordFileBlock
andSendDiscordEmbedBlock
Fixes #10909
Original prompt
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.
Summary by CodeRabbit
New Features
Documentation