-
Notifications
You must be signed in to change notification settings - Fork 60
feat: added mcp to http transport #100
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
Summary by CodeRabbit
WalkthroughThis update introduces comprehensive Model Context Protocol (MCP) integration to the Bifrost HTTP transport, including new configuration structures, an MCP tool execution API endpoint, and expanded documentation. Provider configurations are now nested under a "providers" key, and a new "mcp" section is added for MCP client settings. Documentation and configuration examples are updated throughout. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HTTPTransport
participant BifrostCore
participant MCPClient
User->>HTTPTransport: POST /v1/chat/completions (with prompt)
HTTPTransport->>BifrostCore: Process chat request
BifrostCore->>MCPClient: Discover available tools
BifrostCore-->>HTTPTransport: Response with tool call(s) if needed
HTTPTransport-->>User: Tool call(s) in response
User->>HTTPTransport: POST /v1/mcp/tool/execute (with tool call)
HTTPTransport->>BifrostCore: ExecuteMCPTool(tool call)
BifrostCore->>MCPClient: Execute tool
MCPClient-->>BifrostCore: Tool result
BifrostCore-->>HTTPTransport: Tool execution result
HTTPTransport-->>User: Tool result
User->>HTTPTransport: POST /v1/chat/completions (with tool result)
HTTPTransport->>BifrostCore: Continue conversation with tool result
BifrostCore-->>HTTPTransport: Assistant's final response
HTTPTransport-->>User: Final assistant message
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 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
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
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: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
transports/go.sum
is excluded by!**/*.sum
📒 Files selected for processing (15)
.gitignore
(1 hunks)README.md
(3 hunks)core/mcp.go
(1 hunks)docs/http-transport-api.md
(2 hunks)docs/logger.md
(1 hunks)docs/mcp.md
(6 hunks)docs/memory-management.md
(1 hunks)docs/openapi.json
(5 hunks)docs/providers.md
(2 hunks)tests/core-chatbot/main.go
(1 hunks)transports/README.md
(5 hunks)transports/bifrost-http/lib/config.go
(2 hunks)transports/bifrost-http/main.go
(5 hunks)transports/config.example.json
(1 hunks)transports/go.mod
(3 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
transports/bifrost-http/main.go
358-358: Error return value of (*encoding/json.Encoder).Encode
is not checked
(errcheck)
🪛 markdownlint-cli2 (0.17.2)
transports/README.md
309-309: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
334-334: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1
(MD029, ol-prefix)
🔇 Additional comments (30)
docs/logger.md (1)
40-42
: Formatting update looks good.
The code fences are correctly annotated withtext
and the log examples are properly indented for readability.Also applies to: 47-49
docs/providers.md (1)
219-227
: Verify support forenv.
prefix in config loader.
The JSON example uses"value": "env.OPENAI_API_KEY"
to denote an environment lookup. Confirm that the parser handles theenv.
prefix correctly and document the mechanism if needed.Also applies to: 229-233
docs/memory-management.md (1)
65-81
: LGTM! Clean configuration restructuring with weighted key distribution.The nested "providers" structure improves organization and the addition of the "weight" field enables weighted key distribution as mentioned in the PR objectives. The existing concurrency and buffer size configurations are preserved under the new structure.
README.md (4)
21-33
: Configuration structure updated correctly.The nested "providers" structure with weighted key distribution aligns with the broader configuration changes across the project.
121-121
: Weight field added correctly to Go example.The addition of
Weight: 1.0
in the Go code example maintains consistency with the JSON configuration format and implements the weighted key distribution feature.
211-211
: Feature description enhanced appropriately.The addition of "weighted distribution" accurately describes the new key management capability.
216-216
: MCP feature description improved.The addition of "and execution" better reflects the comprehensive MCP integration that includes both tool integration and execution capabilities.
transports/config.example.json (2)
2-146
: Excellent configuration restructuring with comprehensive provider support.The new nested "providers" structure significantly improves organization and maintainability. Key improvements:
- Clear separation between provider configs and MCP configs
- Consistent lowercased provider naming
- Preserved all existing configuration values
- Proper weight field addition for weighted key distribution
The structure supports all major providers (OpenAI, Anthropic, Bedrock, Cohere, Azure, Vertex) with appropriate configurations.
147-160
: MCP client configuration section properly structured.The new MCP section follows best practices:
- Clear separation from provider configs
- Supports multiple client configurations via array
- Includes all necessary fields: name, connection_type, stdio_config
- Environment variable support for MCP server configuration
This structure aligns with MCP protocol requirements and enables flexible tool integration.
transports/bifrost-http/main.go (4)
4-7
: Documentation updated accurately for new MCP endpoint.The addition of the
/v1/mcp/tool/execute
endpoint to the service description correctly reflects the new MCP capabilities.
154-154
: Configuration integration updated correctly.The use of
config.ProviderConfig
for the BaseAccount while passingconfig.MCPConfig
to the Bifrost client properly separates provider and MCP configurations.
192-192
: MCP configuration integration implemented properly.The MCPConfig is correctly passed to the Bifrost client initialization, enabling MCP functionality.
215-217
: New MCP endpoint route registered correctly.The route registration follows the established pattern and integrates cleanly with the existing router setup.
docs/http-transport-api.md (3)
7-12
: Base URL section enhanced with helpful MCP integration note.The addition of the MCP integration note with references to comprehensive setup guides provides users with clear next steps for implementing MCP functionality.
220-250
: Comprehensive MCP tool execution endpoint documentation.Excellent documentation that includes:
- Clear endpoint description and requirements
- Realistic request/response examples using Google search
- Proper JSON structure with all required fields
- Note about MCP configuration requirements
The documentation accurately reflects the endpoint implementation.
251-298
: Outstanding multi-turn workflow documentation.This section provides tremendous value by:
- Breaking down the complete workflow into clear steps
- Providing practical curl command examples
- Demonstrating real-world usage patterns
- Showing how tool results integrate back into conversations
This will significantly help developers understand and implement MCP tool workflows.
transports/README.md (3)
44-82
: Excellent configuration restructuring documentation.The new nested configuration structure under the "providers" key is well-documented with clear examples. The examples effectively demonstrate weighted key distribution and environment variable usage patterns.
130-160
: Strong MCP environment variable clarification.The distinction between regular Bifrost environment variables (with
env.
prefix) and MCP environment variables (without prefix) is clearly explained. This helps prevent common configuration mistakes.
264-394
: Comprehensive multi-turn conversation workflow.The detailed step-by-step example of multi-turn conversations with MCP tools is excellent. It clearly demonstrates the complete workflow from initial request through tool execution to final response, making it easy for developers to implement.
transports/bifrost-http/lib/config.go (2)
27-32
: Well-structured composite configuration design.The new
BifrostHTTPConfig
struct cleanly separates provider configurations from MCP configuration, providing a clear separation of concerns. The optional MCP configuration field allows for backward compatibility.
104-138
: Robust meta configuration handling with comprehensive provider support.The type-specific meta configuration handling for Azure, Bedrock, and Vertex providers is well-implemented with proper error handling and warning logs. The switch statement approach provides clear extensibility for additional providers.
docs/openapi.json (4)
5-6
: Appropriate version bump and enhanced description.The version increment to 1.1.2 and the updated description that mentions MCP integration appropriately reflects the new functionality. The description clearly communicates the new capabilities.
348-428
: Comprehensive MCP tool execution endpoint specification.The new
/v1/mcp/tool/execute
endpoint is well-specified with:
- Clear operation description and summary
- Comprehensive request/response examples for different tool types
- Proper error response references
- Detailed request and response schemas
The examples for Google Search and File Read tools provide practical guidance for implementation.
821-821
: Minor schema improvements for consistency.The changes to the
ToolCall
example ID and the explicitrequired
fields inFunctionCall
improve schema clarity and consistency.Also applies to: 836-836
1299-1306
: Good API organization with new MCP Tools tag.The addition of the "MCP Tools" tag with appropriate description helps organize the API documentation and makes the MCP functionality discoverable.
docs/mcp.md (5)
153-200
: Excellent HTTP transport configuration documentation.The HTTP transport configuration section provides clear, practical examples that align perfectly with the config changes in
transports/bifrost-http/lib/config.go
. The JSON structure matches the newBifrostHTTPConfig
format.
214-371
: Comprehensive API endpoint documentation with practical examples.The documentation of the two key endpoints (
/v1/chat/completions
and/v1/mcp/tool/execute
) is excellent. The multi-turn tool execution flow is clearly explained with complete curl examples, making it easy for developers to implement.
316-332
: Clear HTTP headers documentation for client filtering.The explanation of
X-MCP-Include-Clients
andX-MCP-Exclude-Clients
headers provides a clean alternative to context-based filtering for HTTP transport users, with practical examples.
574-581
: Good separation of concerns between Go package and HTTP transport usage.The clear distinction between Go package usage and HTTP transport usage, with appropriate cross-references, helps users understand which documentation applies to their use case.
765-770
: Improved code block formatting for better readability.The formatting changes to the text flow diagram improve readability and maintain consistency with the rest of the documentation.
2c4a2ab
to
7ab7659
Compare
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
♻️ Duplicate comments (6)
.gitignore (1)
7-7
: Overly broadprivate.*
ignore patternThis will match any file or directory beginning with
private.
throughout the repo. To limit this to the root, anchor the pattern:-private.* +/private.*transports/go.mod (1)
14-14
: Remove or conditionalize local replace directive.
Thereplace github.com/maximhq/bifrost/core => ../core
directive is useful for development but must be removed or gated by a build tag before publishing the module to avoid impacting downstream consumers.transports/bifrost-http/main.go (1)
330-342
: Error handling improvements address previous feedback.The enhanced error handling for JSON encoding properly captures and handles encoding errors, addressing the past review comment about unchecked error returns from
json.NewEncoder().Encode()
.transports/bifrost-http/lib/config.go (2)
78-81
: Add validation for empty provider configurations.This is the same validation concern raised in previous reviews. The code only checks for nil
ProviderConfig
but should also validate that at least one provider is configured.
86-91
: Consider simplifying the double marshaling/unmarshaling approach.This is the exact issue flagged in previous reviews. The double marshaling/unmarshaling approach adds unnecessary complexity and should be refactored using custom unmarshalers or direct decoding.
transports/README.md (1)
334-334
: Fix ordered list numbering for consistency.The markdown linter identifies inconsistent ordered list numbering. This is the same issue flagged in previous reviews.
-3. **Continue Conversation** (Add tool result and get final response): +1. **Continue Conversation** (Add tool result and get final response):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
transports/go.sum
is excluded by!**/*.sum
📒 Files selected for processing (15)
.gitignore
(1 hunks)README.md
(3 hunks)core/mcp.go
(1 hunks)docs/http-transport-api.md
(2 hunks)docs/logger.md
(1 hunks)docs/mcp.md
(6 hunks)docs/memory-management.md
(1 hunks)docs/openapi.json
(5 hunks)docs/providers.md
(2 hunks)tests/core-chatbot/main.go
(1 hunks)transports/README.md
(6 hunks)transports/bifrost-http/lib/config.go
(2 hunks)transports/bifrost-http/main.go
(5 hunks)transports/config.example.json
(1 hunks)transports/go.mod
(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
core/mcp.go (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#100
File: core/mcp.go:489-490
Timestamp: 2025-06-19T12:38:07.525Z
Learning: In the Bifrost MCP manager (core/mcp.go), the connectToMCPClient method is called during initialization/connection setup, not frequently during runtime. Logging operations like m.logger.Info inside critical sections in this context have negligible performance impact and don't require optimization for lock contention.
🪛 markdownlint-cli2 (0.17.2)
transports/README.md
284-284: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
334-334: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1
(MD029, ol-prefix)
🪛 LanguageTool
transports/README.md
[grammar] ~409-~409: The base form is expected after “to”.
Context: ...maximhq/bifrost/tree/main/plugins). And to attached these plugins to your HTTP transport, j...
(IN_TO_VBD)
🔇 Additional comments (28)
.gitignore (1)
6-6
: Correctly ignoring Python bytecode cachesThe pattern
**/__pycache__/**
ensures all__pycache__
directories are excluded. No changes required here.docs/logger.md (1)
40-50
: Documentation formatting updated appropriately.
The indentation andtext
language spec for the log examples improve readability without altering content or semantics.core/mcp.go (1)
489-490
: Skip log-precision micro-optimization.
Per retrieved learnings,connectToMCPClient
runs only at init time and logging inside this lock has negligible impact. No need to move the log outside the critical section.tests/core-chatbot/main.go (1)
242-249
: Pass required environment variable to MCP subprocess.
IncludingEnvs: []string{"SERPER_API_KEY"}
ensures the subprocess inherits the API key. This aligns with the updated MCP client configuration pattern and matches the documentation.README.md (3)
21-31
: Configuration structure successfully updated to support provider grouping.The JSON configuration restructuring under a "providers" key improves organization and aligns with the new MCP integration. The addition of the "weight" field enables weighted key distribution as mentioned in the enhanced feature list.
121-121
: Weight field addition aligns with enhanced key management.The addition of the Weight field in the Go example correctly reflects the new weighted key distribution capability mentioned in the feature list.
211-216
: Feature descriptions accurately reflect new capabilities.The enhanced descriptions for "Dynamic Key Management" and "MCP Integration" correctly highlight the new weighted distribution and tool execution capabilities introduced in this PR.
docs/memory-management.md (1)
65-81
: Configuration structure consistently updated across documentation.The memory management configuration example properly follows the new "providers" structure introduced across the codebase, maintaining all existing functionality while improving organization.
transports/config.example.json (2)
2-146
: Comprehensive configuration restructuring improves organization.The restructuring under the "providers" key significantly improves configuration organization while maintaining all existing provider settings. The consistent lowercasing of provider keys and proper nesting structure enhances readability and maintainability.
147-159
: MCP configuration section properly structured.The new MCP configuration section with client_configs array is well-structured and provides a clear example of stdio connection type configuration with command, arguments, and environment variables.
transports/bifrost-http/main.go (4)
4-7
: Package documentation accurately reflects new MCP endpoint.The documentation update correctly includes the new
/v1/mcp/tool/execute
endpoint alongside the existing text and chat completion endpoints.
154-192
: Configuration integration properly updated for new structure.The configuration loading correctly uses the new
BifrostHTTPConfig
structure and properly integrates the MCP configuration into the Bifrost client initialization.
215-217
: MCP tool execution endpoint properly implemented.The new endpoint is correctly registered and routes to the appropriate handler function for MCP tool execution.
345-368
: MCP tool execution handler correctly implemented.The
handleMCPToolExecution
function properly:
- Unmarshals the ToolCall request
- Converts the FastHTTP context to Bifrost context
- Calls the MCP execution method
- Handles errors and responses appropriately
- Includes proper JSON encoding error handling
docs/http-transport-api.md (2)
7-11
: Base URL section enhanced with MCP integration note.The addition of the code block for the base URL and the comprehensive MCP integration note with references to detailed guides improves usability and discoverability of MCP features.
220-297
: Comprehensive MCP documentation enhances API usability.The new MCP tool execution section provides:
- Clear endpoint documentation with request/response examples
- Detailed multi-turn workflow with step-by-step curl commands
- Proper integration with existing documentation structure
- References to additional MCP setup guides
This comprehensive documentation significantly improves the developer experience for MCP integration.
transports/bifrost-http/lib/config.go (3)
27-32
: LGTM: Well-structured configuration container.The new
BifrostHTTPConfig
struct appropriately separates provider and MCP configurations while maintaining clear JSON mappings. This aligns well with the updated configuration format.
66-66
: LGTM: Function signature updated appropriately.The updated return type to
*BifrostHTTPConfig
correctly reflects the new configuration structure that includes both provider and MCP configurations.
110-134
: LGTM: Provider-specific meta config handling.The switch-based approach for handling different provider meta configs (Azure, Bedrock, Vertex) is appropriate and extensible. The warning logs provide useful feedback when meta config parsing fails.
transports/README.md (2)
88-161
: Excellent comprehensive MCP configuration documentation.This section provides valuable guidance on MCP integration with clear configuration examples and important distinctions between regular Bifrost environment variables and MCP-specific environment variables. The examples cover both STDIO and HTTP connection types effectively.
264-394
: Outstanding multi-turn conversation workflow documentation.The step-by-step workflow clearly demonstrates how to use MCP tools in practice, from initial chat completion through tool execution to final response. The examples are comprehensive and the endpoint summary is particularly helpful.
docs/openapi.json (5)
5-6
: LGTM: API version and description appropriately updated.The version increment to 1.1.2 and the expanded description clearly communicate the new MCP integration capabilities. The description provides a good overview of the MCP functionality without being overly verbose.
348-428
: Excellent comprehensive MCP tool execution endpoint documentation.The new
/v1/mcp/tool/execute
endpoint is well-documented with:
- Clear operation description and requirements
- Realistic examples covering different tool types (Google Search, File Read)
- Proper error response definitions
- Appropriate request/response schemas
The examples demonstrate practical usage scenarios that align with the multi-turn workflow documented in the README.
821-821
: LGTM: ToolCall example ID updated for consistency.The change from
"call_123"
to"tool_123"
provides better semantic clarity for the tool call identifier example.
836-836
: LGTM: FunctionCall schema made more precise.Adding the
required
array to explicitly specify that both"name"
and"arguments"
are required properties improves the schema precision and API contract clarity.
1299-1306
: LGTM: New MCP Tools tag appropriately added.The new tag provides proper categorization for the MCP tool execution endpoint and includes a clear description. The refined "Monitoring" tag description is also an improvement.
docs/mcp.md (2)
153-153
: Approve insertion of HTTP Transport Usage section
The new “## HTTP Transport Usage” section is well-placed between Quick Start and Configuration.
157-157
: Verify external docs anchor exists
Ensure the target anchor#mcp-model-context-protocol-configuration
is present intransports/README.md
so the link resolves.#!/bin/bash # Check for the MCP config anchor in transports README rg -n "#mcp-model-context-protocol-configuration" transports/README.md
c83ae20
to
9b7d322
Compare
9b7d322
to
af88735
Compare
4161031
to
f8a1882
Compare
af88735
to
8a9fe55
Compare
8a9fe55
to
84625d8
Compare
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: 4
♻️ Duplicate comments (3)
docs/providers.md (1)
217-223
: JSON language annotation has been added.This addresses the previous nitpick about syntax highlighting in the provider example.
transports/bifrost-http/lib/config.go (2)
100-102
: Improved error handling addresses previous feedback.The fatal error handling for configuration unmarshaling failures with a clear, descriptive message properly addresses the previous review comment about masking important configuration issues.
85-91
: Double marshaling approach is functional for current needs.The double marshaling/unmarshaling approach, while not optimal, works correctly for the configuration loading use case and was previously discussed as acceptable for initialization time performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
.gitignore
(1 hunks)README.md
(3 hunks)core/mcp.go
(1 hunks)docs/http-transport-api.md
(2 hunks)docs/logger.md
(1 hunks)docs/mcp.md
(6 hunks)docs/memory-management.md
(1 hunks)docs/openapi.json
(5 hunks)docs/providers.md
(2 hunks)tests/core-chatbot/main.go
(1 hunks)transports/README.md
(5 hunks)transports/bifrost-http/lib/config.go
(2 hunks)transports/bifrost-http/main.go
(5 hunks)transports/config.example.json
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
core/mcp.go (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#100
File: core/mcp.go:489-490
Timestamp: 2025-06-19T12:38:07.525Z
Learning: In the Bifrost MCP manager (core/mcp.go), the connectToMCPClient method is called during initialization/connection setup, not frequently during runtime. Logging operations like m.logger.Info inside critical sections in this context have negligible performance impact and don't require optimization for lock contention.
🪛 golangci-lint (1.64.8)
transports/bifrost-http/main.go
190-190: unknown field MCPConfig in struct literal of type schemas.BifrostConfig
(typecheck)
353-353: client.ExecuteMCPTool undefined (type *bifrost.Bifrost has no field or method ExecuteMCPTool)
(typecheck)
🔇 Additional comments (36)
docs/logger.md (1)
40-43
: Formatting improvement looks good.Using the
text
language annotation and consistent indentation for log examples enhances readability without altering semantics.Also applies to: 47-50
transports/config.example.json (2)
2-146
: Provider configuration restructure is correct.The nested
providers
object with lowercase keys aligns with the updated parsing logic and improves clarity.
147-159
: MCP client configuration section is well-defined.The new
mcp.client_configs
block accurately demonstrates STDIO setup and environment variable usage, matching the code expectations.core/mcp.go (1)
489-490
: Added connection log is helpful.The new
m.logger.Info
call provides visibility into client connections and is acceptable inside the lock per initialization context.tests/core-chatbot/main.go (1)
247-247
: Environment variable inclusion for Serper is appropriate.Adding
Envs: []string{"SERPER_API_KEY"}
ensures the subprocess checks for the required key and aligns with the connection logic.README.md (3)
21-31
: Configuration structure update looks good!The restructuring with the "providers" wrapper and addition of the "weight" field aligns well with the broader MCP integration changes. This provides better organization and supports weighted key management.
115-115
: Weight field addition is consistent.The addition of the
Weight: 1.0
field in the Go example matches the JSON configuration format and supports the new weighted distribution feature mentioned in the feature list.
207-207
: Feature descriptions accurately reflect new capabilities.The updates to "Dynamic Key Management" to include "weighted distribution" and "MCP Integration" to include "and execution" correctly describe the enhanced functionality being introduced.
Also applies to: 212-212
docs/memory-management.md (1)
65-81
: Configuration update is consistent with the overall restructuring.The documentation correctly reflects the new nested "providers" structure and includes the weight field, maintaining consistency with other configuration examples across the project.
transports/bifrost-http/main.go (7)
4-7
: Documentation update accurately reflects new endpoint.The comment correctly documents the new
/v1/mcp/tool/execute
endpoint alongside the existing completion endpoints.
152-152
: Configuration update follows new structure.The change to use
config.ProviderConfig
correctly adapts to the newBifrostHTTPConfig
structure that separates provider and MCP configurations.
213-216
: MCP tool execution endpoint implementation looks correct.The endpoint properly handles JSON unmarshaling, context conversion, and error responses. The structure follows the same pattern as other endpoints.
328-331
: Excellent improvement to JSON encoding error handling.The error handling for JSON encoding failures has been properly implemented, addressing the previous static analysis issue. This ensures encoding errors are caught and appropriate HTTP error responses are returned.
Also applies to: 337-340
362-365
: Consistent error handling for MCP endpoint.The JSON encoding error handling in the MCP endpoint follows the same improved pattern as the completion endpoints, ensuring consistency across the codebase.
353-353
: Verify ExecuteMCPTool method availability in Bifrost client.Static analysis indicates that the
ExecuteMCPTool
method may not exist on the Bifrost client. Please ensure this method is implemented in the core before merging.#!/bin/bash # Description: Check if ExecuteMCPTool method exists in Bifrost struct # Expected: Should find the ExecuteMCPTool method definition ast-grep --pattern $'func ($_ *Bifrost) ExecuteMCPTool($$$) ($$$) { $$$ }'
190-190
: ```shell
#!/bin/bashLocate the core schemas file and verify MCPConfig in BifrostConfig
Step 1: Find the file defining BifrostConfig in core/schemas
file=$(rg -l "type BifrostConfig" --glob "core/schemas/*.go" 2>/dev/null | head -n1)
if [[ -z "$file" ]]; then
echo "❌ No BifrostConfig definition found under core/schemas."
exit 1
fiecho "✅ BifrostConfig found in: $file"
echo -e "\n--- Struct Definition (with context) ---"
rg -n -C3 "type BifrostConfig struct" "$file"echo -e "\n--- Checking for MCPConfig field ---"
rg -n "MCPConfig" "$file" || echo "❌ MCPConfig field not found in $file"</details> <details> <summary>docs/http-transport-api.md (3)</summary> `7-11`: **Improved base URL formatting with helpful MCP note.** The code block formatting for the base URL is cleaner, and the note about built-in MCP support provides valuable context for users. --- `220-249`: **Comprehensive MCP tool execution endpoint documentation.** The documentation for the new `/v1/mcp/tool/execute` endpoint is thorough, including proper request/response examples and a clear note about MCP configuration requirements. --- `251-297`: **Excellent multi-turn tool workflow documentation.** The step-by-step workflow clearly demonstrates how to integrate MCP tools into conversations. The curl examples are practical and help users understand the complete interaction pattern. </details> <details> <summary>transports/bifrost-http/lib/config.go (3)</summary> `27-32`: **Well-designed configuration structure for MCP integration.** The new `BifrostHTTPConfig` struct cleanly separates provider configurations from MCP configuration, making the structure more organized and extensible. --- `66-66`: **Function signature and validation updates are appropriate.** The updated function signature returning `*BifrostHTTPConfig` and the validation for the "providers" section correctly reflect the new configuration structure. Also applies to: 72-80 --- `141-142`: **Clean return of processed configuration.** The final assignment and return of the processed configuration structure properly maintains all the processed provider configurations and MCP settings. </details> <details> <summary>transports/README.md (4)</summary> `14-15`: **Update TOC entries for MCP sections** The Table of Contents now includes links for both MCP configuration and multi-turn workflows. Anchors (`#mcp-model-context-protocol-configuration` and `#multi-turn-conversations-with-mcp-tools`) correctly match the section headings. Also applies to: 21-21 --- `46-55`: **Illustrate new `providers` nesting in JSON example** The sample config correctly nests provider settings under `"providers"`. This aligns with the updated schema in `transports/bifrost-http/lib/config.go`. --- `88-127`: **Add MCP configuration section** The `MCP (Model Context Protocol) Configuration` section and its JSON example accurately reflect the new `mcp.client_configs` structure, including `stdio` and `http` connection types. --- `266-269`: **Document multi-turn conversations with MCP tools** The `Multi-Turn Conversations with MCP Tools` section clearly demonstrates the HTTP sequence and matches the TOC anchor. </details> <details> <summary>docs/openapi.json (5)</summary> `5-6`: **Bump API version and enrich description** The `version` has been updated to `1.1.2`, and the description now highlights MCP support. Ensure the version aligns with the actual release tag. --- `348-428`: **Add `/v1/mcp/tool/execute` endpoint** The new endpoint is well-defined with `ToolCall` request schema, `BifrostMessage` response, examples, and standard error responses. OperationId and tags follow conventions. --- `818-822`: **Update `ToolCall` schema example** The example ID `"tool_123"` matches the updated example in the spec. --- `835-837`: **Enforce required fields in `FunctionCall`** Marking both `name` and `arguments` as required improves validation. --- `1299-1302`: **Introduce "MCP Tools" tag** The new tag groups MCP-related endpoints. The Monitoring tag description is now more precise. Also applies to: 1305-1305 </details> <details> <summary>docs/mcp.md (5)</summary> `10-10`: **Add TOC entry for HTTP Transport Usage** The new link `[HTTP Transport Usage](#http-transport-usage)` matches the section header exactly. --- `153-155`: **Introduce HTTP Transport Usage section** Section header and intro correctly scope HTTP-specific MCP setup and usage. --- `157-161`: **Reference transports README for examples** Linking to `transports/README.md#mcp-model-context-protocol-configuration` directs users to detailed HTTP transport config. --- `264-265`: **Link to multi-turn examples** The pointer to `#multi-turn-conversations-with-mcp-tools` in the transports README aligns with the documented workflow. --- `321-325`: **Document HTTP headers for client filtering** Examples for `X-MCP-Include-Clients` and `X-MCP-Exclude-Clients` are clear and accurate. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
**/__pycache__/ | ||
**/__pycache__/** | ||
private.* | ||
private.* |
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.
🧹 Nitpick (assertive)
Refine the private.*
ignore pattern
The entry private.*
will match any file or directory starting with private.
at any depth. Anchor it to the repo root (e.g., /private.*
) or be explicit (e.g., /private.env
) to avoid unintentionally excluding other assets.
🤖 Prompt for AI Agents
In the .gitignore file at line 7, the pattern `private.*` is too broad and may
exclude unintended files or directories at any depth. Refine this pattern by
anchoring it to the repository root using `/private.*` or specify exact
filenames like `/private.env` to limit the ignore scope only to intended files
at the root level.
### HTTP API Endpoints with MCP Tools | ||
|
||
When MCP is configured, tools are automatically added to chat completion requests. The HTTP transport provides two key endpoints: | ||
|
||
- `POST /v1/chat/completions` - Chat with automatic tool discovery | ||
- `POST /v1/mcp/tool/execute` - Execute specific tool calls | ||
|
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.
🧹 Nitpick (assertive)
Detail HTTP API endpoints for MCP tools
Endpoints are correctly listed; consider linking to the OpenAPI spec (docs/openapi.json
) for in-depth parameter and schema details.
🤖 Prompt for AI Agents
In docs/mcp.md around lines 214 to 220, the HTTP API endpoints for MCP tools are
listed but lack direct references to detailed specifications. Enhance the
documentation by adding links to the OpenAPI specification file
(docs/openapi.json) for each endpoint, enabling readers to access comprehensive
parameter and schema details easily.
```json | ||
{ | ||
"providers": { | ||
"openai": { | ||
"keys": [ | ||
{ | ||
"value": "env.OPENAI_API_KEY", | ||
"models": ["gpt-4o-mini"], | ||
"weight": 1.0 | ||
} | ||
] | ||
} | ||
}, | ||
"mcp": { | ||
"client_configs": [ | ||
{ | ||
"name": "filesystem", | ||
"connection_type": "stdio", | ||
"stdio_config": { | ||
"command": "npx", | ||
"args": ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"], | ||
"envs": [] | ||
}, | ||
"tools_to_skip": ["rm", "delete"], | ||
"tools_to_execute": [] | ||
}, | ||
{ | ||
"name": "web-search", | ||
"connection_type": "http", | ||
"http_connection_string": "http://localhost:3001/mcp", | ||
"tools_to_skip": [], | ||
"tools_to_execute": [] | ||
} | ||
] | ||
} | ||
} | ||
``` |
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.
🧹 Nitpick (assertive)
Show JSON config for HTTP Transport MCP
The example accurately reflects the new providers
and mcp
structure. Consider annotating default values or required keys for clarity.
🤖 Prompt for AI Agents
In docs/mcp.md between lines 163 and 199, the JSON configuration example for
HTTP Transport MCP lacks annotations for default values and required keys. Add
inline comments or notes next to each key in the JSON snippet to indicate which
keys are required and which have default values, improving clarity for users
configuring the MCP.
```bash | ||
# Start Bifrost HTTP server with MCP configuration | ||
bifrost-http -config config.json -port 8080 -pool-size 300 | ||
|
||
# Or using Docker | ||
docker run -p 8080:8080 \ | ||
-v ./config.json:/app/config.json \ | ||
-e OPENAI_API_KEY \ | ||
bifrost-transports | ||
``` |
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.
Verify Docker image name
The docker run ... bifrost-transports
reference differs from maximhq/bifrost
used elsewhere. Confirm or correct the image name to avoid confusion.
🤖 Prompt for AI Agents
In docs/mcp.md around lines 203 to 212, the Docker image name used in the docker
run command is inconsistent with the rest of the documentation. Verify the
correct Docker image name, and update the command to use the consistent and
correct image name, such as replacing 'bifrost-transports' with
'maximhq/bifrost' if that is the intended image.
Enhanced Configuration Structure and MCP Integration
This PR introduces a more organized configuration structure and adds comprehensive Model Context Protocol (MCP) support for tool integration:
providers
object for better organization/v1/mcp/tool/execute
endpoint for tool calls.gitignore
to exclude private filesThe new configuration structure provides a cleaner separation between provider settings and MCP configuration, while the MCP integration enables AI models to discover and use external tools through a standardized protocol.