Conversation
WalkthroughThis PR adds per-transport ping control flags (ssePingEnabled and streamPingEnabled) to MCPConfig, enabling selective enabling/disabling of server pings for SSE and stream protocols, with guards and error handling around ping invocations in MCPControllerRegister. Changes
Sequence DiagramsequenceDiagram
participant Init as MCP Initialization
participant Config as MCPConfig
participant Register as MCPControllerRegister
participant Server as MCP Server
Init->>Config: getStreamPingEnabled(name)?
Config-->>Init: returns config flag
rect rgba(100, 200, 150, 0.1)
note over Register: New Guard Logic
Init->>Register: check if ping enabled
alt Ping Enabled
Register->>Register: setup ping interval
loop Periodic Ping
rect rgba(100, 150, 200, 0.1)
note over Register: Error Handling
Register->>Server: ping()
alt Success
Server-->>Register: pong
else Failure
Server-->>Register: error
Register->>Register: log warning
end
Register->>Register: cleanup in finally
end
end
else Ping Disabled
Register->>Register: skip ping setup
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
Summary of ChangesHello @akitaSummer, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces new configuration parameters to enable or disable pinging for Server-Sent Events (SSE) and general stream connections within the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces configuration options to enable or disable pinging for SSE and stream connections, which is a useful feature for managing connection health. The implementation is generally sound, but I have identified a couple of areas for improvement. Specifically, there's a design issue in MCPConfig.ts where per-server configurations do not fall back to the global configuration, making it less intuitive. I've also suggested a minor enhancement to logging in MCPControllerRegister.ts to provide more context for easier debugging. Overall, good work on adding this functionality.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugin/controller/lib/impl/mcp/MCPConfig.ts(4 hunks)plugin/controller/lib/impl/mcp/MCPControllerRegister.ts(3 hunks)plugin/controller/test/fixtures/apps/mcp-app/config/config.default.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugin/controller/lib/impl/mcp/MCPConfig.ts (2)
plugin/controller/test/fixtures/apps/mcp-app/config/config.default.js (1)
config(8-38)plugin/controller/config/config.default.ts (1)
config(3-15)
⏰ 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). (8)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-macos (16)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-macos (18)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (typescript)
🔇 Additional comments (3)
plugin/controller/test/fixtures/apps/mcp-app/config/config.default.js (1)
11-12: LGTM! Test configuration aligns with new ping control flags.The addition of
ssePingEnabledandstreamPingEnabledflags to the test fixture configuration properly exercises the new per-transport ping control functionality.plugin/controller/lib/impl/mcp/MCPControllerRegister.ts (2)
347-349: LGTM! Ping invocations properly gated by feature flags.The conditional execution of
mcpServerPingbased ongetStreamPingEnabled(name)andgetSsePingEnabled(name)correctly implements per-transport ping control, preventing unnecessary network calls when pings are disabled.Also applies to: 501-503
672-683: LGTM! Robust error handling for ping operations.Wrapping
server.ping()in try/catch/finally ensures:
- Ping failures are logged without crashing the interval
- Duration-based cleanup (clearing the interval) always executes regardless of ping outcome
This is a solid improvement for reliability.
2 participants
Checklist
npm testpassesAffected core subsystem(s)
Description of change
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.