-
-
Notifications
You must be signed in to change notification settings - Fork 254
bit Boilerplate now expose its own mcp tool (#11590) #11598
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
Conversation
|
Important Review skippedAuto incremental 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 Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces MCP (Model Context Protocol) server support to the Boilerplate template by implementing a centralized ChatbotService, registering MCP endpoints with tooling, and refactoring chatbot streaming logic. Updates documentation to remove deprecated pub/sub constants and adds dependency package references. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (SignalR)
participant Hub as AppHub
participant CBS as ChatbotService
participant AI as AI Chat Client
participant MCP as MCP Service
rect rgb(200, 220, 255)
Note over Client,CBS: SignalR Chatbot Flow (via AppHub)
Client->>Hub: Chatbot(request, incomingMessages)
Hub->>CBS: Start(history, culture, deviceInfo)
CBS->>CBS: Load system prompt from DB
loop For each incoming message
Client->>Hub: incomingMessages stream
Hub->>CBS: ProcessMessageAsync(message, serverAddress)
CBS->>AI: Stream AI response with tools
AI-->>CBS: Streaming chunks
CBS->>CBS: Aggregate response & generate suggestions
CBS->>Hub: Response via GetStreamingChannel
Hub-->>Client: Yield streamed chunks
end
Hub->>CBS: Stop()
CBS->>CBS: Complete response channel
end
rect rgb(220, 255, 220)
Note over MCP,AI: MCP Tool Flow (via AppMcpService)
MCP->>AppMcpService: ChatWithBot(message, history, ...)
AppMcpService->>CBS: Start(history, culture, deviceInfo)
AppMcpService->>CBS: ProcessMessageAsync(message, serverAddress)
CBS->>AI: Stream AI response with tools
AI-->>AppMcpService: Streaming chunks
AppMcpService->>AppMcpService: Aggregate chunks
AppMcpService-->>MCP: Return final response string
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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 |
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs (1)
38-74: Serialize ProcessMessageAsync or you corrupt shared state.Each incoming message kicks off
ProcessMessageAsyncand immediately moves on. The previous invocation is only cancelled, not awaited, so the writes overlap. That corrupts the sharedchatMessageslist and the (now multi-writer) channel, andStop()completes the channel while the last task is still writing, yieldingChannelClosedException/ lost chunks. Track the active task, wait for it to finish (after cancelling) before starting the next one, and await it before callingchatbotService.Stop().- CancellationTokenSource? messageSpecificCancellationTokenSrc = null; + CancellationTokenSource? messageSpecificCancellationTokenSrc = null; + Task? processingTask = null; ... - if (messageSpecificCancellationTokenSrc is not null) - await messageSpecificCancellationTokenSrc.CancelAsync(); - - messageSpecificCancellationTokenSrc = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - _ = chatbotService.ProcessMessageAsync( + if (messageSpecificCancellationTokenSrc is not null) + { + await messageSpecificCancellationTokenSrc.CancelAsync(); + if (processingTask is not null) + { + try { await processingTask; } catch { } + } + } + + messageSpecificCancellationTokenSrc = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + processingTask = chatbotService.ProcessMessageAsync( incomingMessage, request.ServerApiAddress, messageSpecificCancellationTokenSrc.Token); ... - messageSpecificCancellationTokenSrc?.Dispose(); - chatbotService.Stop(); + try + { + if (processingTask is not null) + { + try { await processingTask; } catch { } + } + } + finally + { + messageSpecificCancellationTokenSrc?.Dispose(); + chatbotService.Stop(); + } }
🧹 Nitpick comments (3)
src/Templates/Boilerplate/Bit.Boilerplate/.docs/22- Messaging.md (1)
109-127: Consider the trade-off of removing specific message constants from documentation.The documentation now shows only
PROFILE_UPDATEDwith a comment "// ... and more" instead of listing all availableClientPubSubMessagesconstants. While this simplifies maintenance, it may reduce discoverability for developers who need to know what pub/sub messages are available.Consider either:
- Keeping a complete list of available constants
- Adding a reference to the source file location where developers can find all constants
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppMcpService.cs (2)
59-75: Consider adding timeout for streaming operation.The streaming loop reads from the channel indefinitely without a timeout. If the chatbot service hangs or never completes, this method could block indefinitely. Consider adding a timeout or using the
cancellationTokenmore explicitly.Example approach:
var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); timeoutCts.CancelAfter(TimeSpan.FromSeconds(60)); // 60 second timeout await foreach (var chunk in chatbotService.GetStreamingChannel().ReadAllAsync(timeoutCts.Token)) { // ... existing logic }
68-68: Generic error message may not be helpful for debugging.Returning
"Error processing message. Please try again."provides no context about what went wrong. Consider including more diagnostic information or logging the error details for troubleshooting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (11)
src/Templates/Boilerplate/Bit.Boilerplate/.docs/07- ASP.NET Core Identity - Authentication & Authorization.md(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/.docs/22- Messaging.md(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Middlewares.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppMcpService.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/ChatbotService.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/SharedPubSubMessages.cs(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: Agent
- GitHub Check: CodeQL analysis (csharp)
- GitHub Check: build and test
🔇 Additional comments (9)
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/SharedPubSubMessages.cs (2)
12-14: LGTM!The documentation clearly explains when this pub/sub message is used.
23-25: LGTM!The documentation clearly explains when this pub/sub message is used.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj (1)
50-50: LGTM! Package reference follows the established pattern.The new
ModelContextProtocol.AspNetCorepackage reference is correctly conditioned on thesignalRsetting and positioned alongside related packages. The version is managed centrally viaDirectory.Packages.props.src/Templates/Boilerplate/Bit.Boilerplate/.docs/07- ASP.NET Core Identity - Authentication & Authorization.md (1)
259-267: Documentation improvement clarifies claim-based session management.The updated documentation now explicitly states that the max privileged sessions value is stored in the
AppClaimTypes.MAX_PRIVILEGED_SESSIONSclaim, making the dynamic configuration approach clearer.src/Templates/Boilerplate/Bit.Boilerplate/.docs/22- Messaging.md (1)
153-154: Documentation clarification improves understanding.The updated comment explicitly states that the diagnostic modal behavior is achieved by publishing
SHOW_DIAGNOSTIC_MODALto the Pub/Sub service, which is clearer than the previous inline comment.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (1)
71-77: MCP server registration follows the correct pattern.The MCP server is properly configured with HTTP transport and the
AppMcpServicetool provider. TheChatbotServiceis correctly registered as scoped, which is appropriate for maintaining per-request conversation state.src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (1)
56-56: Clarify the AI summary inconsistency and verify preview version constraint.The AI summary incorrectly states "reformatting only" — line 56 introduces a new package reference. However, the latest available version of ModelContextProtocol.AspNetCore is 0.4.0-preview.3 (prerelease) with no stable release available. Pinning to this preview version is unavoidable; verify this dependency and its stability implications are acceptable for your project.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Middlewares.cs (1)
77-77: MCP endpoint is intentionally public but lacks rate limiting protection.The endpoint is explicitly designed for external MCP-enabled AI clients (documented in ChatbotService.cs lines 14-15 referencing CrystaCode.AI). However, the security concern is valid:
- Unauthenticated access intentional: The
.RequireAuthorization()is commented out by design, not oversight- Data exposure limited: Returns AI-processed responses, not raw database queries
- Rate limiting absent: No protection found despite public access to resource-intensive AI processing
Since this endpoint processes messages through ChatbotService (line 31 in AppMcpService.cs) which accesses application data and returns AI-generated responses, adding rate limiting is recommended to prevent abuse/resource exhaustion, even though the endpoint is intentionally public.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs (1)
149-149: Decision required: secure /mcp or document/mitigate public exposure.app.MapMcp("/mcp") is currently unauthenticated (RequireAuthorization() commented out). AppMcpService.ChatWithBot calls ChatbotService which reads system prompts and queries the DB (product recommendations/search) and can log/save conversation data; no server-side rate-limiter was found.
- If /mcp must be private: enable authorization (uncomment .RequireAuthorization() on the MapMcp call in src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs).
- If /mcp must remain public for external MCP clients: restrict exposed tools/data (remove DB-returning or sensitive functions), add rate limiting/throttling, enable monitoring/alerts, and document that /mcp is a public endpoint.
...lates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppMcpService.cs
Outdated
Show resolved
Hide resolved
...lates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppMcpService.cs
Outdated
Show resolved
Hide resolved
...ates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/ChatbotService.cs
Show resolved
Hide resolved
...ates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/ChatbotService.cs
Show resolved
Hide resolved
...ates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/ChatbotService.cs
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/SharedPubSubMessages.cs
Show resolved
Hide resolved
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.
Pull Request Overview
This PR adds Model Context Protocol (MCP) support to the Bit Boilerplate, allowing the chatbot functionality to be exposed as an MCP tool that can be consumed by MCP-enabled clients like CrystaCode.ai. The implementation refactors existing chatbot logic from SignalR hub into a reusable ChatbotService and creates an AppMcpService to expose it via MCP endpoints.
Key Changes
- Refactored chatbot logic from
AppHub.Chatbot.csinto a new reusableChatbotServiceclass - Added MCP server support with
AppMcpServiceexposing the chatbot as an MCP tool - Registered MCP endpoints at
/mcpin both Server.Api and Server.Web projects
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| SharedPubSubMessages.cs | Added XML documentation comments for pub/sub message constants |
| Program.Middlewares.cs (Server.Web) | Mapped MCP endpoint with commented-out authorization |
| ChatbotService.cs | New service extracting chatbot conversation logic for reuse across SignalR and MCP |
| AppMcpService.cs | New MCP tool service exposing chatbot functionality to MCP clients |
| AppHub.Chatbot.cs | Refactored to delegate chatbot logic to ChatbotService |
| Program.Services.cs | Registered MCP server with HTTP transport and ChatbotService |
| Program.Middlewares.cs (Server.Api) | Mapped MCP endpoint with commented-out authorization |
| Boilerplate.Server.Api.csproj | Added ModelContextProtocol.AspNetCore package reference |
| Directory.Packages.props | Added ModelContextProtocol.AspNetCore version 0.4.0-preview.3 |
closes #11590
Summary by CodeRabbit
New Features
Documentation
Refactor