-
Notifications
You must be signed in to change notification settings - Fork 10
Improve Discord bot reliability and Claude Code integration #3
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: main
Are you sure you want to change the base?
Conversation
This update enhances the Discord bot's reliability and fixes several issues: **Core Improvements:** - Update to Claude Sonnet 4.5 (claude-sonnet-4-5-20250929) - Add --permission-mode acceptEdits for autonomous operation - Fix exit code 143 false error reporting (SIGTERM is normal shutdown) - Add hook disabling mechanism for Discord bot sessions **Bug Fixes:** - Fix MCP bridge port mismatch (now uses MCP_SERVER_PORT env variable) - Truncate long error messages to fit Discord's 4096 character limit - Prevent sound notifications during Discord bot operations **Configuration:** - Add MCP_SERVER_PORT to environment configuration - Pass port configuration through to MCP bridge - Add CLAUDE_DISABLE_HOOKS environment variable **Documentation:** - Add troubleshooting section for Claude Code hooks - Document MCP port configuration - Add hook disabling instructions **Testing:** - Add tests for exit code handling (0, 143, and error codes) - Improve test coverage for process lifecycle 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThis PR introduces configurable MCP server port via environment variable, sets Changes
Sequence DiagramsequenceDiagram
participant Bot as Discord Bot
participant Manager as ClaudeManager
participant Child as Child Process
participant Discord as Discord Channel
Bot->>Manager: Trigger Claude action
Manager->>Child: Spawn with CLAUDE_DISABLE_HOOKS=1
activate Child
Child->>Child: Code execution (hooks disabled)
Child-->>Manager: Result/stderr message
deactivate Child
alt Exit Code: 0 or 143
Manager->>Manager: Suppress error notification
else Exit Code: Non-zero
Manager->>Manager: truncateForEmbed(error)
Manager->>Discord: Post error embed
end
Manager->>Discord: Post result (truncated if needed)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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
🧹 Nitpick comments (1)
src/claude/manager.ts (1)
9-17: Consider adding validation for small maxLength values.The truncation logic is sound for the Discord embed use case (4096 chars). However, if
maxLengthis less than 50, the substring operation would use a negative index, which would work but might produce unexpected results.Consider adding a guard:
function truncateForEmbed(text: string, maxLength: number = 4096): string { if (text.length <= maxLength) { return text; } + const truncationNotice = '\n...\n[Message truncated]'; + const availableLength = Math.max(0, maxLength - truncationNotice.length); - return text.substring(0, maxLength - 50) + '\n...\n[Message truncated]'; + return text.substring(0, availableLength) + truncationNotice; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.env.example(1 hunks).gitignore(1 hunks)README.md(1 hunks)mcp-bridge.cjs(2 hunks)src/claude/manager.ts(5 hunks)src/utils/shell.ts(3 hunks)test/claude/manager.test.ts(3 hunks)
🧰 Additional context used
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 14-14: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🔇 Additional comments (13)
.gitignore (1)
17-17: LGTM!The addition of
log.txtto the ignore patterns aligns with the logging behavior introduced insrc/claude/manager.ts(lines 164-168), where Claude output is appended to this file..env.example (1)
12-14: LGTM!The MCP_SERVER_PORT configuration is well-documented and aligns with the implementation in
mcp-bridge.cjs(line 10) andsrc/utils/shell.ts(line 75), where the port is consumed with the same default value.README.md (1)
168-190: LGTM!The troubleshooting documentation clearly explains the hook-disabling mechanism and provides a practical shell script example. This aligns perfectly with the
CLAUDE_DISABLE_HOOKS=1environment variable set insrc/claude/manager.ts(line 116).mcp-bridge.cjs (2)
10-11: LGTM!The configurable MCP server port is properly externalized from the environment with a sensible default. This aligns with the configuration in
.env.example(line 14) and usage insrc/utils/shell.ts(line 75).
52-52: LGTM!The port is correctly parsed to an integer for the HTTP request options, matching the port used in the URL at line 11.
src/utils/shell.ts (2)
35-37: LGTM!The model upgrade to
claude-sonnet-4-5-20250929(Sonnet 4.5) provides explicit versioning for reproducibility. The--permission-mode acceptEditsaddition enables autonomous editing without prompts, which is essential for the Discord bot's unattended operation.
75-75: LGTM!The MCP_SERVER_PORT environment variable is properly passed to the MCP bridge configuration, with a consistent default of "3001" matching
mcp-bridge.cjs(line 10) and.env.example(line 14).test/claude/manager.test.ts (2)
18-23: LGTM!The shell utility mocks provide appropriate test isolation for ClaudeManager tests, with simple implementations sufficient for validating the manager's behavior without invoking actual shell command construction.
209-327: LGTM!The exit code handling test suite comprehensively validates the refined error reporting behavior introduced in
src/claude/manager.ts(lines 211-224):
- Exit code 0 (success) suppresses error messaging
- Exit code 143 (SIGTERM) treats termination as normal shutdown
- Actual failure codes trigger error embeds with the exit code
The test setup correctly simulates the process lifecycle and verifies Discord messaging behavior.
src/claude/manager.ts (4)
116-116: LGTM!Setting
CLAUDE_DISABLE_HOOKS=1prevents hook execution during Discord bot operations, which is well-documented in the README troubleshooting section (lines 172-189). This avoids unwanted side effects like audio notifications during automated bot interactions.
211-224: LGTM!The refined exit code handling correctly distinguishes between normal shutdowns and actual failures:
- Exit code 0: success
- Exit code null: process terminated without code
- Exit code 143: SIGTERM signal (128 + 15), indicating graceful termination
Only non-zero, non-null, non-143 codes trigger error embeds. This prevents false error reporting during normal bot shutdown operations and is well-covered by the test suite in
test/claude/manager.test.ts(lines 209-327).
241-241: LGTM!The stderr output is appropriately truncated to respect Discord's 4096-character embed description limit, preventing message delivery failures for long error outputs.
261-261: LGTM!Process error messages are consistently truncated using the same helper, ensuring all error embeds respect Discord's character limits.
|
|
||
| # MCP Server Port (optional, defaults to 3001) | ||
| # Change this if port 3001 is already in use | ||
| MCP_SERVER_PORT=3001 |
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.
Add blank line at end of file.
The dotenv-linter correctly flags a missing blank line at the end of the file, which is a common convention for text files.
Apply this diff:
MCP_SERVER_PORT=3001
+📝 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.
| MCP_SERVER_PORT=3001 | |
| MCP_SERVER_PORT=3001 | |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 14-14: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
In .env.example around line 14, the file is missing a trailing blank line; add a
single newline character at the end of the file so the file terminates with a
blank line (save the file with an ending newline).
This update enhances the Discord bot's reliability and fixes several issues:
Core Improvements:
Bug Fixes:
Configuration:
Documentation:
Testing:
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Improvements