feat(cli): add initial mcp subcommand in cli to start mcp server#145
feat(cli): add initial mcp subcommand in cli to start mcp server#145Adamkadaban wants to merge 6 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new MCP (Model Context Protocol) feature to the OpenLabs CLI, enabling AI assistants to interact with OpenLabs services through standardized protocol tools. The implementation includes server functionality, command-line interface, and comprehensive tool definitions for managing ranges, blueprints, and cloud credentials.
- Adds
mcpcommand withstart,status, andtoolssubcommands for managing the MCP server - Implements MCP server with support for both stdio and SSE transport modes
- Provides 14 tools covering range management, blueprint operations, cloud credentials, and authentication
- Enhances logging with new status indicator functions and table formatting improvements
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cli/cmd/mcp/*.go | New MCP command structure with start, status, and tools subcommands |
| cli/internal/mcp/*.go | Core MCP server implementation, tool definitions, and request handlers |
| cli/internal/output/table.go | Enhanced table formatting with MCP-specific display function |
| cli/internal/logger/logger.go | Added status logging functions with consistent indicators |
| cli/go.mod | Updated dependencies for MCP functionality |
| cli/cmd/root.go | Integration of MCP commands into CLI structure |
Comments suppressed due to low confidence (2)
cli/internal/output/table.go:247
- [nitpick] The function name
getStructFieldValueis ambiguous. Consider renaming toextractStructFieldByNameorgetFieldValueByNameto better indicate it searches for fields by name.
func getStructFieldValue(val reflect.Value, fieldName string) string {
cli/internal/mcp/server.go:53
- [nitpick] The function name
reloadConfigIfChangeddoesn't clearly indicate what constitutes a 'change'. Consider renaming toreloadConfigIfTokenChangedto be more specific about what triggers the reload.
func (s *Server) reloadConfigIfChanged() bool {
…ainability Consider placing these into markdown files and embedding them at compile-time
|
I should probably change the prompt to say creds don't go through an llm, instead of saying it won't go over the protocol. Probably more accurate + relevant should do the same for cloud secrets. maybe consider making prompts shorter to save context Also need to make Docs. TODO Also need to look into sse vs streamable http for transport |
…users know sending credentials to llm is potentially insecure
| return nil | ||
| } | ||
|
|
||
| func (s *Server) handleListRanges(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { |
There was a problem hiding this comment.
I am not sure if this is Go convention, but why would we want to have an error as a return type here if we only ever return nil for the error in this function?
| return mcp.NewToolResultError("Missing or invalid aws_secret_key parameter"), nil | ||
| } | ||
|
|
||
| err = s.client.UpdateAWSSecrets(accessKey, secretKey) |
There was a problem hiding this comment.
This probably will need to be updated because of the secret's endpoint changes.
| return mcp.NewToolResultError("Missing or invalid azure_subscription_id parameter"), nil | ||
| } | ||
|
|
||
| err = s.client.UpdateAzureSecrets(clientID, clientSecret, tenantID, subscriptionID) |
There was a problem hiding this comment.
This will also probably need to be updated to reflect the updated secrets endpoint.
alexchristy
left a comment
alexchristy
left a comment
There was a problem hiding this comment.
Looks good! It was interesting to see how the MCP server works and how the data and prompts are built. This was also a good refresher on go.
| s.mcpServer, | ||
| server.WithSSEEndpoint("/sse"), | ||
| server.WithMessageEndpoint("/message"), | ||
| server.WithBaseURL(fmt.Sprintf("http://localhost%s", port)), |
There was a problem hiding this comment.
Would this break if addr = '127.0.0.1:9000'?
Checklist
Description
This pull request introduces a new "MCP" (Model Context Protocol) feature to the CLI, enabling users to manage an MCP server for AI assistant integration. It adds commands for starting the server, checking its status, and listing available tools, along with the necessary backend support for these operations. Below are the most important changes grouped by theme:
New MCP Command and Subcommands
mcpcommand with subcommands:start,status, andtools. These commands allow users to start the MCP server, check its status, and list available tools, respectively. (cli/cmd/mcp/mcp.go,cli/cmd/mcp/start.go,cli/cmd/mcp/status.go,cli/cmd/mcp/tools.go) [1] [2] [3] [4]MCP Server Implementation
Serverstruct incli/internal/mcp/server.goto manage the MCP server lifecycle and provide functionality for both "stdio" and "SSE" transport modes. It includes methods for registering tools, handling configuration changes, and providing detailed instructions to the server. (cli/internal/mcp/server.go)Integration with CLI Framework
mcpcommand into the CLI by adding it to the root command and ensuring it shares the global configuration. (cli/cmd/root.go) [1] [2] [3]Logging Enhancements
Success,Failure,Notice) incli/internal/logger/logger.goto provide consistent and user-friendly status indicators for CLI operations. (cli/internal/logger/logger.go)Dependency Updates
go.modto include new dependencies required for the MCP server, such asgithub.com/mark3labs/mcp-goandgithub.com/spf13/cast. (cli/go.mod) [1] [2]Examples in Claude Code
Examples in VSCode
Fixes: #141