BugFix: Refactor Discord bot to use slash commands and use ephemeral message #112
Conversation
WalkthroughThe Discord bot integration was refactored to use slash commands instead of prefix-based commands, with improved separation of control flow for slash commands, thread messages, and new messages. Command handlers were updated for the new interaction model, cog loading was made dynamic and asynchronous, and error handling was enhanced throughout. The Discord library dependency was specified in the project configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Discord
participant DiscordBot
participant DevRelCommands
User->>Discord: Sends message or uses slash command
Discord->>DiscordBot: Delivers message or interaction
alt Slash Command
DiscordBot->>DevRelCommands: Handles interaction (slash command)
DevRelCommands->>DiscordBot: Responds via interaction response
else Message in Thread
DiscordBot->>DiscordBot: Detects active thread, handles as follow-up
DiscordBot->>User: Responds in thread
else New Message in Main Channel
DiscordBot->>DiscordBot: Classifies message
DiscordBot->>DiscordBot: Handles DevRel message (may create thread)
DiscordBot->>User: Responds in new thread
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~80 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
pyproject.toml (1)
11-11: Critical: Conflicting Discord library dependenciesThe project already has
py-cordon line 11, which is a fork of discord.py. Addingdiscord-pycreates a conflict as both libraries provide the samediscordmodule namespace. This will cause import errors and runtime issues.You should use either
py-cordordiscord-py, not both. Since the codebase appears to be using py-cord features (based on the imports in other files), remove this line.Apply this diff to fix the dependency conflict:
- "discord-py (>=2.5.2,<3.0.0)",Also applies to: 28-28
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
backend/integrations/discord/bot.py(4 hunks)backend/integrations/discord/cogs.py(7 hunks)backend/main.py(3 hunks)pyproject.toml(1 hunks)
🧠 Learnings (2)
backend/main.py (2)
Learnt from: smokeyScraper
PR: #87
File: backend/app/api/v1/health.py:1-12
Timestamp: 2025-06-28T23:14:21.089Z
Learning: In the Devr.AI project, the application is designed to run from the backend directory, making import paths like from main import DevRAIApplication correct for the current setup. Import path adjustments for deployment will be configured later during the deployment process.
Learnt from: smokeyScraper
PR: #87
File: tests/test_supabase.py:1-3
Timestamp: 2025-06-28T23:15:13.374Z
Learning: In the Devr.AI project, smokeyScraper prefers to defer test updates and fixes (like missing imports after module reorganization) to separate PRs rather than expanding the scope of module update/chore PRs to include comprehensive test refactoring.
backend/integrations/discord/bot.py (2)
Learnt from: smokeyScraper
PR: #75
File: backend/app/agents/devrel/agent.py:34-35
Timestamp: 2025-06-13T21:56:19.183Z
Learning: In the Devr.AI backend, the DevRelAgent follows a singleton pattern where only one instance exists for the entire application lifetime, using InMemorySaver with thread-based conversation management to persist user conversations across sessions.
Learnt from: smokeyScraper
PR: #76
File: backend/app/agents/shared/base_agent.py:22-44
Timestamp: 2025-06-14T05:57:43.872Z
Learning: In the Devr.AI codebase, user_id directly maps as thread_id for memory persistence in the agent system, ensuring thread_id is always non-empty and unique per user.
🧬 Code Graph Analysis (1)
backend/integrations/discord/bot.py (2)
backend/app/classification/classification_router.py (1)
ClassificationRouter(10-59)backend/app/core/orchestration/queue_manager.py (2)
QueuePriority(12-15)enqueue(73-92)
🧰 Additional context used
🧠 Learnings (2)
backend/main.py (2)
Learnt from: smokeyScraper
PR: #87
File: backend/app/api/v1/health.py:1-12
Timestamp: 2025-06-28T23:14:21.089Z
Learning: In the Devr.AI project, the application is designed to run from the backend directory, making import paths like from main import DevRAIApplication correct for the current setup. Import path adjustments for deployment will be configured later during the deployment process.
Learnt from: smokeyScraper
PR: #87
File: tests/test_supabase.py:1-3
Timestamp: 2025-06-28T23:15:13.374Z
Learning: In the Devr.AI project, smokeyScraper prefers to defer test updates and fixes (like missing imports after module reorganization) to separate PRs rather than expanding the scope of module update/chore PRs to include comprehensive test refactoring.
backend/integrations/discord/bot.py (2)
Learnt from: smokeyScraper
PR: #75
File: backend/app/agents/devrel/agent.py:34-35
Timestamp: 2025-06-13T21:56:19.183Z
Learning: In the Devr.AI backend, the DevRelAgent follows a singleton pattern where only one instance exists for the entire application lifetime, using InMemorySaver with thread-based conversation management to persist user conversations across sessions.
Learnt from: smokeyScraper
PR: #76
File: backend/app/agents/shared/base_agent.py:22-44
Timestamp: 2025-06-14T05:57:43.872Z
Learning: In the Devr.AI codebase, user_id directly maps as thread_id for memory persistence in the agent system, ensuring thread_id is always non-empty and unique per user.
🧬 Code Graph Analysis (1)
backend/integrations/discord/bot.py (2)
backend/app/classification/classification_router.py (1)
ClassificationRouter(10-59)backend/app/core/orchestration/queue_manager.py (2)
QueuePriority(12-15)enqueue(73-92)
🔇 Additional comments (9)
backend/main.py (1)
14-16: Good refactoring to address race conditionsRemoving the direct import and loading the cog dynamically after the event loop starts is the correct approach to fix the race condition issues mentioned in the PR objectives.
backend/integrations/discord/bot.py (4)
21-21: Correct implementation to disable prefix commandsSetting
command_prefix=Noneproperly disables traditional prefix commands, aligning with the migration to slash commands.
39-43: Good addition of slash command syncingThe implementation correctly syncs slash commands on bot startup with appropriate error handling. This ensures slash commands are registered with Discord.
50-61: Well-structured message filtering logicThe implementation correctly:
- Ignores slash command interactions to prevent duplicate processing
- Detects messages in active threads and processes them without re-classification
- Maintains conversation context in threads
This aligns perfectly with the slash command migration objectives.
120-134: Improved thread management and user experienceThe thread management enhancements are well-implemented:
- Proper validation of thread existence and archive status
- Cleanup of stale thread references
- Concise, professional welcome message
These changes improve reliability and user experience.
backend/integrations/discord/cogs.py (4)
20-26: Excellent fix for the token cleanup race conditionMoving the cleanup task initialization from
__init__toon_readyproperly addresses the race condition mentioned in the PR objectives. Theis_running()check prevents duplicate task instances, which is a good defensive programming practice.
45-57: Correct implementation of slash commands with ephemeral responsesThe slash command conversion is properly implemented:
- Uses
@app_commands.commanddecorator- Handles
discord.Interactioncorrectly- Sets
ephemeral=Truefor privacyThis aligns with the PR objective of using ephemeral messages for enhanced security.
104-180: Well-implemented OAuth flow with proper defer handlingThe
/verify_githubcommand implementation is excellent:
- Correctly uses
defer(ephemeral=True)for potentially long operations- Uses
followup.send()after deferring- Comprehensive error handling and user feedback
- Maintains privacy with ephemeral messages throughout
This provides a secure and user-friendly verification experience.
182-184: Correct setup function for dynamic cog loadingThe async setup function properly integrates with Discord.py's extension loading system, enabling the dynamic loading implemented in main.py.
|
Hi @smokeyScraper I have added the coderabbit suggestions, Please have a look |
DhruvK278
changed the title
DhruvK278
changed the title
DhruvK278
changed the title
smokeyScraper
left a comment
smokeyScraper
left a comment
There was a problem hiding this comment.
could you please address the requested change?
rest of the logic seems GTG.
backend/integrations/discord/bot.py
Outdated
| if is_in_active_thread: | ||
| # If it's an active thread, it's part of an ongoing conversation. | ||
| # We don't need to classify it again, just handle it. | ||
| await self._handle_devrel_message(message, {"needs_devrel": True}) | ||
| return |
There was a problem hiding this comment.
!!! BUG here !!!
Any message within the thread is classified to be handled by DevRel as we are skipping classification logic.
Interaction
Please revert to the previous logic itself.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/main.py (1)
47-53: Consider making Discord cog loading failure more explicitThe dynamic cog loading implementation correctly addresses the race condition by ensuring the event loop is running. The error handling is well-structured and catches appropriate exceptions.
However, consider whether Discord cog loading failure should be treated as a fatal error since the Discord integration appears to be a core feature. Currently, the application continues running even if the cog fails to load.
If Discord functionality is essential, consider this approach:
try: await self.discord_bot.load_extension("integrations.discord.cogs") + logger.info("Discord cog extension loaded successfully") except (ImportError, commands.ExtensionError) as e: logger.error("Failed to load Discord cog extension: %s", e) + # Uncomment the next line if Discord functionality is essential + # raise RuntimeError(f"Critical Discord cog loading failed: {e}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/integrations/discord/bot.py(4 hunks)backend/integrations/discord/cogs.py(7 hunks)backend/main.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/integrations/discord/bot.py
- backend/integrations/discord/cogs.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#87
File: tests/test_supabase.py:1-3
Timestamp: 2025-06-28T23:15:13.374Z
Learning: In the Devr.AI project, smokeyScraper prefers to defer test updates and fixes (like missing imports after module reorganization) to separate PRs rather than expanding the scope of module update/chore PRs to include comprehensive test refactoring.
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#85
File: tests/test_supabase.py:1-3
Timestamp: 2025-06-28T14:45:55.244Z
Learning: In the Devr.AI project, smokeyScraper prefers to defer comprehensive test refactoring to separate PRs/efforts when doing major backend restructuring, rather than expanding the scope of the current refactoring PR to include test updates.
backend/main.py (2)
Learnt from: smokeyScraper
PR: #87
File: backend/app/api/v1/health.py:1-12
Timestamp: 2025-06-28T23:14:21.089Z
Learning: In the Devr.AI project, the application is designed to run from the backend directory, making import paths like from main import DevRAIApplication correct for the current setup. Import path adjustments for deployment will be configured later during the deployment process.
Learnt from: smokeyScraper
PR: #87
File: tests/test_supabase.py:1-3
Timestamp: 2025-06-28T23:15:13.374Z
Learning: In the Devr.AI project, smokeyScraper prefers to defer test updates and fixes (like missing imports after module reorganization) to separate PRs rather than expanding the scope of module update/chore PRs to include comprehensive test refactoring.
🔇 Additional comments (3)
backend/main.py (3)
15-17: LGTM: Clean migration to dynamic cog loadingThe import changes correctly reflect the shift from synchronous to asynchronous cog loading. The comment clearly documents the change in approach, which will help future maintainers understand the architecture.
61-61: Excellent error handling improvementRe-raising the exception after cleanup ensures proper resource management. This prevents resource leaks if startup fails and follows the cleanup-then-reraise pattern, which is a best practice for exception handling in resource management scenarios.
67-67: Good refactoring for improved readabilityRemoving the intermediate variable makes the code more concise while maintaining the same functionality. This is a clean improvement that enhances readability.
DhruvK278
force-pushed
the
Bug/Bot_leaks_auth#108
branch
from
b3bd8e2 to
1356976
Compare
Migrates Discord bot commands from legacy prefix-based to modern slash commands using discord.py app_commands. Refactors DevRelCommands cog to use slash commands, improves token cleanup task management, and updates verification flows for ephemeral responses. Discord bot now loads cogs dynamically during async startup, and the Discord dependency is added to pyproject.toml.
|
Merged! This is a great improvement on the UX side. |
2 participants
Migrates Discord bot commands from legacy prefix-based commands to Discord slash commands using app_commands. Enabled ephemeral messages for enhanced security. Refactors cog startup to load dynamically after the event loop starts, improving reliability and startup order. Updates thread handling, message classification, and agent response logic for better maintainability and user experience. Adds 'discord-py' dependency in pyproject.toml.
Closes #108
📝 Description
This pull request refactors the entire Discord bot integration to modernize its command structure and improve its reliability and user experience. The primary change is the migration from legacy prefix commands '!' to the officially recommended slash commands '/', which allows for enhanced security features like ephemeral messages for sensitive information.
The verification process has been updated to no longer rely on Direct Messages (DMs). Instead, the authentication link is sent as a private, ephemeral message directly in the channel where the command was used, improving both security and user experience.
Additionally, the application's startup logic has been overhauled to fix critical race conditions, ensuring background tasks and command cogs are loaded only after the main asynchronous event loop is running. This resolves persistent startup errors and makes the bot's initialization more robust.
🔧 Changes Made
✅ Migrated to Slash Commands
!verify_github) have been converted to use slash commands (e.g.,/verify_github) viadiscord.app_commands.📬 Removed Direct Messaging
👻 Enabled Ephemeral Responses
/verify_githuband/verification_statuscommands now use ephemeral responses, ensuring that replies are only visible to the user who triggered the command.⚙️ Refactored Startup Logic
RuntimeError: no running event loopand ensures background tasks initialize correctly.🧹 Robust Task Management
Cog.listener()on theon_readyevent.📦 Dependency Update
discord-pytopyproject.tomlto guarantee the correct version is installed during setup.📷 Screenshots or Visual Changes (if applicable)
Before
After
🤝 Collaboration
This issue was fixed by @DhruvK278
✅ Checklist
Summary by CodeRabbit
New Features
Improvements
Chores