Skip to content

Conversation

@ysmoradi
Copy link
Member

@ysmoradi ysmoradi commented Nov 12, 2025

closes #11590

Summary by CodeRabbit

  • New Features

    • Added Model Context Protocol (MCP) integration enabling external tool access
    • Enhanced chatbot with improved AI streaming capabilities
  • Documentation

    • Updated authentication system documentation for claim-based privilege session management
    • Added messaging system guidelines and best practices
  • Refactor

    • Streamlined chatbot architecture through service consolidation
    • Simplified internal pub/sub messaging system

@ysmoradi ysmoradi requested a review from Copilot November 12, 2025 22:31
@coderabbitai
Copy link

coderabbitai bot commented Nov 12, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

Introduces 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

Cohort / File(s) Summary
Documentation Updates
src/Templates/Boilerplate/Bit.Boilerplate/.docs/07- ASP.NET Core Identity - Authentication & Authorization.md, src/Templates/Boilerplate/Bit.Boilerplate/.docs/22- Messaging.md
Updated authentication docs to describe claim-based privileged session configuration. Removed client-only and server-to-client PubSub message constants; clarified diagnostic modal publishing behavior.
Package & Dependency Configuration
src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props, src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj
Reformatted indentation in central package management. Added ModelContextProtocol.AspNetCore package reference (conditional on signalR).
MCP Endpoint Registration
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Middlewares.cs, src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs
Added app.MapMcp("/mcp") endpoint mapping in both Api and Web middleware configurations.
MCP Service Registration
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs
Registered AddMcpServer() with HTTP transport and AppMcpService tools. Registered SignalR.ChatbotService as scoped dependency.
Chatbot Service Infrastructure
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/ChatbotService.cs, src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppMcpService.cs
Introduced ChatbotService managing conversation history, streaming channels, and AI interactions. Introduced AppMcpService exposing ChatWithBot as MCP tool via static method.
Chatbot Hub Refactoring
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs
Refactored Chatbot method to depend on ChatbotService instead of direct AppDbContext/IChatClient; removed channel orchestration and database lookups; delegated logic to centralized service.
Pub/Sub Messaging Documentation
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/SharedPubSubMessages.cs
Added XML documentation summaries for DASHBOARD_DATA_CHANGED, SESSION_REVOKED, and PROFILE_UPDATED constants.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • ChatbotService.cs — Dense logic with streaming channel management, message processing, database interactions, AI tool creation, and follow-up suggestion generation. Verify state machine (Start/ProcessMessage/Stop) handles concurrent access safely.
  • AppHub.Chatbot.cs — Significant refactoring from direct dependencies to service-based architecture. Confirm error handling, cancellation propagation, and streaming integrity align with prior behavior.
  • AppMcpService.cs — New public static MCP tool method with service scope management and special token handling (MESSAGE_RPOCESS_SUCESS/MESSAGE_RPOCESS_ERROR). Verify error cases and resource cleanup.
  • Program.Services.cs & Program.Middlewares.cs — MCP service registration and endpoint configuration conditional logic should work correctly across Api and Web projects.
  • Documentation changes — Validate that removed pub/sub constants are not referenced elsewhere in codebase and that new messaging documentation accurately reflects the revised architecture.

Poem

🐰 A chatbot service hops into place,
MCP tools now expose with grace,
Streaming channels dance and flow,
While legacy messages let go,
The Boilerplate template's embrace,
Grows smarter, efficient, and bold in the race! 🚀

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly reflects the main objective: exposing MCP tools in the bit Boilerplate project template.
Linked Issues check ✅ Passed The PR implements both objectives: adds MCP tool exposure via AppMcpService and ChatbotService, and refactors messaging documentation to reflect the changes.
Out of Scope Changes check ✅ Passed All changes support the primary objective of exposing MCP tools. Minor updates to authentication docs and package formatting are supporting changes aligned with the overall refactoring.
Docstring Coverage ✅ Passed Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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 ProcessMessageAsync and immediately moves on. The previous invocation is only cancelled, not awaited, so the writes overlap. That corrupts the shared chatMessages list and the (now multi-writer) channel, and Stop() completes the channel while the last task is still writing, yielding ChannelClosedException / lost chunks. Track the active task, wait for it to finish (after cancelling) before starting the next one, and await it before calling chatbotService.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();
+                }
             }

(learn.microsoft.com)

🧹 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_UPDATED with a comment "// ... and more" instead of listing all available ClientPubSubMessages constants. 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 cancellationToken more 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

📥 Commits

Reviewing files that changed from the base of the PR and between bae1d3e and 2d1bf9f.

📒 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.AspNetCore package reference is correctly conditioned on the signalR setting and positioned alongside related packages. The version is managed centrally via Directory.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_SESSIONS claim, 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_MODAL to 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 AppMcpService tool provider. The ChatbotService is 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.

Copy link
Contributor

Copilot AI left a 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.cs into a new reusable ChatbotService class
  • Added MCP server support with AppMcpService exposing the chatbot as an MCP tool
  • Registered MCP endpoints at /mcp in 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

@ysmoradi ysmoradi merged commit 748225e into bitfoundation:develop Nov 13, 2025
21 checks passed
@ysmoradi ysmoradi deleted the 11590 branch November 13, 2025 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bit Boilerplate project template must expose its own set of mcp tools

1 participant