Feat/enhance mcp#86
Conversation
WalkthroughThis update introduces OpenAPI support for the MCP project, adds a custom OpenAPI document transformer, and refines MCP endpoint routing and configuration. It adjusts versioning and metadata handling, updates attributes for clarity, and expands global usings for OpenAPI-related functionality. Minor dictionary and configuration enhancements are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP API
participant OpenAPI Middleware
participant McpDocumentTransformer
Client->>MCP API: POST /mcp (with JSON-RPC payload)
MCP API->>OpenAPI Middleware: Generate OpenAPI docs
OpenAPI Middleware->>McpDocumentTransformer: Transform OpenAPI document
McpDocumentTransformer-->>OpenAPI Middleware: Add /mcp POST operation, set server URL
OpenAPI Middleware-->>Client: Serve OpenAPI spec with custom /mcp operation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
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.yaml 📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (10)
⏰ 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). (4)
✨ 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
Documentation and Community
|
✅ Deploy Preview for bookwormdev canceled.
|
3ab48e4 to
bdf424f
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the MCP (Model Context Protocol) integration by improving configuration, documentation, and API specification. The changes focus on better version handling, endpoint configuration, and OpenAPI documentation for the MCP tools server.
Key changes include:
- Updated MCP client endpoint configuration and version handling
- Added OpenAPI documentation and transformation for MCP endpoints
- Enhanced tool and prompt definitions with titles and descriptions
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Extensions.cs (Chat) | Updates MCP client version and endpoint configuration |
| appsettings.Development.json | Adds document metadata for MCP tools server |
| Product.cs | Adds title to SearchCatalog tool definition |
| Instruction.cs | Adds title to SystemPrompt definition |
| Program.cs | Updates MCP endpoint mapping and adds OpenAPI support |
| McpDocumentTransformer.cs | New OpenAPI document transformer for MCP protocol |
| GlobalUsings.cs | Adds necessary using statements for OpenAPI functionality |
| Extensions.cs (McpTools) | Implements OpenAPI configuration for MCP server |
| OpenApiOptionsExtensions.cs | Changes visibility from internal to public |
| AppHost.cs | Adds MCP service to OpenAPI configuration |
| BookWorm.sln.DotSettings | Adds "jsonrpc" to dictionary |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/Integrations/BookWorm.McpTools/OpenApi/McpDocumentTransformer.cs (2)
29-91: Consider extracting schema definitions for maintainability.The OpenAPI operation definition is comprehensive, but the inline schema creation makes it verbose. Consider extracting schema definitions to improve maintainability and reusability.
Create a separate schema factory:
public static class McpSchemaFactory { public static OpenApiSchema CreateMcpRequestSchema() => new() { Type = "object", Properties = new Dictionary<string, OpenApiSchema> { [McpConstants.Method] = new() { Type = "string" }, [McpConstants.Params] = CreateParamsSchema(), [McpConstants.JsonRpc] = CreateJsonRpcSchema(), [McpConstants.Id] = new() { Type = "integer" } }, Required = new HashSet<string> { McpConstants.Method, McpConstants.Params, McpConstants.JsonRpc, McpConstants.Id } }; // Additional helper methods... }This approach aligns with DDD principles by creating a dedicated schema domain.
10-98: Consider implementing enterprise patterns for better maintainability.While the implementation follows Clean Architecture principles, consider these enterprise-level improvements:
- Domain-Driven Design: Extract MCP schema definitions into a dedicated domain service
- Configuration Pattern: Use IOptions for configuration management instead of hardcoded values
- Observability: Add structured logging for transformation operations
- Error Handling: Implement comprehensive error handling with custom exceptions
Example domain service extraction:
public interface IMcpSchemaService { OpenApiSchema CreateRequestSchema(); OpenApiOperation CreateInvokeOperation(); } public sealed class McpDocumentTransformer( IHttpContextAccessor accessor, IMcpSchemaService schemaService, ILogger<McpDocumentTransformer> logger) : IOpenApiDocumentTransformer { // Implementation using injected services }This aligns with SOLID principles and improves testability and maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
BookWorm.sln.DotSettings(1 hunks)src/Aspire/BookWorm.AppHost/AppHost.cs(1 hunks)src/Aspire/BookWorm.ServiceDefaults/ApiSpecification/OpenApi/OpenApiOptionsExtensions.cs(1 hunks)src/Integrations/BookWorm.McpTools/Extensions/Extensions.cs(3 hunks)src/Integrations/BookWorm.McpTools/GlobalUsings.cs(1 hunks)src/Integrations/BookWorm.McpTools/OpenApi/McpDocumentTransformer.cs(1 hunks)src/Integrations/BookWorm.McpTools/Program.cs(1 hunks)src/Integrations/BookWorm.McpTools/Prompts/Instruction.cs(1 hunks)src/Integrations/BookWorm.McpTools/Tools/Product.cs(1 hunks)src/Integrations/BookWorm.McpTools/appsettings.Development.json(1 hunks)src/Services/Chat/BookWorm.Chat/Infrastructure/ChatStreaming/Extensions.cs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/Aspire/**
⚙️ CodeRabbit Configuration File
src/Aspire/**: .NET Aspire orchestration review:
- Service discovery and configuration
- Health checks and observability
- Resource allocation and scaling
- Development environment setup
Files:
src/Aspire/BookWorm.ServiceDefaults/ApiSpecification/OpenApi/OpenApiOptionsExtensions.cssrc/Aspire/BookWorm.AppHost/AppHost.cs
src/Services/**/Infrastructure/**
⚙️ CodeRabbit Configuration File
src/Services/**/Infrastructure/**: Examine infrastructure implementations:
- Repository pattern adherence
- Database migrations and EF Core configurations
- External service integrations
- Caching strategies and performance
Files:
src/Services/Chat/BookWorm.Chat/Infrastructure/ChatStreaming/Extensions.cs
🧠 Learnings (1)
src/Integrations/BookWorm.McpTools/GlobalUsings.cs (3)
Learnt from: CR
PR: foxminchan/BookWorm#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-02T07:45:53.434Z
Learning: Applies to **/*.cs : Prefer file-scoped namespace declarations and single-line using directives.
Learnt from: CR
PR: foxminchan/BookWorm#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-23T18:48:45.052Z
Learning: Prefer file-scoped namespace declarations and single-line using directives in C# for cleaner and more concise code.
Learnt from: CR
PR: foxminchan/BookWorm#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-23T18:48:43.455Z
Learning: For C# code, prefer file-scoped namespace declarations and single-line using directives to improve readability and reduce nesting.
⏰ 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). (4)
- GitHub Check: Analyzers
- GitHub Check: Style
- GitHub Check: Security Scan
- GitHub Check: Build and analyze
🔇 Additional comments (13)
BookWorm.sln.DotSettings (1)
25-25: LGTM - Proper dictionary addition for MCP integrationThe addition of "jsonrpc" to the user dictionary is appropriate given the MCP integration's use of JSON-RPC protocol. The format is consistent with existing entries.
src/Aspire/BookWorm.ServiceDefaults/ApiSpecification/OpenApi/OpenApiOptionsExtensions.cs (1)
8-8: LGTM - Appropriate visibility change for MCP integrationMaking
OpenApiOptionsExtensionspublic is a reasonable change to support the MCP integration's OpenAPI configuration needs. The class is well-designed with proper separation of concerns and follows .NET extension method patterns correctly.src/Aspire/BookWorm.AppHost/AppHost.cs (1)
240-241: LGTM - Consistent OpenAPI integration patternThe addition of
.WithOpenAPI(mcp)follows the established pattern for other API projects and properly integrates the MCP service into the application's OpenAPI infrastructure. The conditional execution withinIsRunModeis appropriate for development scenarios.src/Integrations/BookWorm.McpTools/Tools/Product.cs (1)
11-11: LGTM - Enhanced MCP tool metadataThe addition of the
Titleparameter improves API documentation and tooling discoverability. The title "Search BookWorm Catalog" is descriptive and accurately reflects the method's functionality.src/Services/Chat/BookWorm.Chat/Infrastructure/ChatStreaming/Extensions.cs (1)
35-35: Verify MCP version format consistency
- The only
Versionreference found is inExtensions.cs(ClientInfo.Version = "1.0"); no other MCP-related CS files use the previous “1.0.0” format.- Please confirm that switching from full SemVer to the simplified “1.0” aligns with your API versioning strategy and that all MCP components (NuGet metadata, docs, other services) follow the same convention.
- If full semantic versioning is still required, consider reverting to “1.0.0” throughout.
src/Integrations/BookWorm.McpTools/Prompts/Instruction.cs (1)
9-9: Excellent enhancement for OpenAPI documentation.Adding the
Titleproperty to theMcpServerPromptattribute improves the generated OpenAPI documentation by providing clearer identification of the system prompt. This aligns well with the overall OpenAPI enhancement goals of this PR.src/Integrations/BookWorm.McpTools/GlobalUsings.cs (1)
4-5: Good adherence to coding standards and supports new OpenAPI functionality.The added global using directives follow the preferred single-line format from the coding guidelines and appropriately support the new OpenAPI and extension functionality introduced in this PR. The namespaces are logically grouped and necessary for the enhanced MCP integration.
Also applies to: 8-9
src/Integrations/BookWorm.McpTools/Extensions/Extensions.cs (1)
44-62: OpenAPI configuration approach is architecturally sound.The
AddMcpOpenApimethod demonstrates good separation of concerns and proper OpenAPI configuration patterns. The approach of iterating through API versions and applying consistent configuration is excellent for enterprise applications.However, consider the BuildServiceProvider issue flagged above for a more robust implementation.
src/Integrations/BookWorm.McpTools/OpenApi/McpDocumentTransformer.cs (5)
1-8: LGTM! Clean namespace organization and appropriate using statements.The using statements are well-organized and the namespace follows the expected pattern for the integration layer.
10-12: Excellent use of modern C# features and Clean Architecture principles.The sealed class with primary constructor and dependency on
IHttpContextAccessorabstraction demonstrates proper enterprise patterns and performance optimization.
13-17: Proper interface implementation with async pattern.The method signature correctly implements
IOpenApiDocumentTransformerand maintains the async pattern for future extensibility.
93-94: Proper defensive programming with null-coalescing initialization.The null-coalescing assignment (
??= []) and path addition are correctly implemented, following defensive programming practices.
96-97: Correct async pattern implementation.Returning
Task.CompletedTaskis the appropriate pattern for synchronous implementations of async interfaces.
bdf424f to
1a4a0c3
Compare
|
Pull Request Description
Checklist
Summary by CodeRabbit
New Features
/mcpwith detailed schema and response descriptions.Improvements
Bug Fixes
/mcppath.Chores