-
Notifications
You must be signed in to change notification settings - Fork 160
Feat/mcp serve cli #2596
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
base: main
Are you sure you want to change the base?
Feat/mcp serve cli #2596
Conversation
Implements issue golemcloud#1926 - MCP Server for Golem CLI - Add --serve and --serve-port flags to enable MCP server mode - Expose all CLI leaf commands as MCP tools - Expose golem.yaml manifests as MCP resources - Use rmcp crate with HTTP Streamable transport - Add E2E tests for MCP functionality - Update README with MCP usage documentation
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 implements MCP (Model Context Protocol) server functionality for the Golem CLI, enabling AI agents to interact with Golem programmatically through HTTP-based JSON-RPC communication.
Changes:
- Added new
mcpmodule exposing CLI commands as MCP tools and golem.yaml manifests as MCP resources - Added comprehensive E2E tests for MCP server functionality
- Updated dependencies to include
rmcpandaxumcrates - Added MCP usage documentation to README
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/golem-cli/src/mcp.rs | New MCP server implementation exposing CLI commands as tools and manifests as resources |
| cli/golem-cli/tests/mcp_e2e.rs | Comprehensive E2E tests for MCP server functionality |
| cli/golem-cli/src/lib.rs | Added mcp module to public exports |
| cli/golem-cli/Cargo.toml | Added rmcp and axum dependencies |
| README.md | Added MCP server mode documentation with usage examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Start the MCP server on the specified port | ||
| /// | ||
| /// This function blocks until the server is shut down (Ctrl+C). | ||
| /// | ||
| /// # Arguments | ||
| /// * `port` - Port to listen on | ||
| /// | ||
| /// # Example | ||
| /// ```no_run | ||
| /// # use golem_cli::mcp::run_mcp_server; | ||
| /// # tokio::runtime::Runtime::new().unwrap().block_on(async { | ||
| /// run_mcp_server(1232).await.unwrap(); | ||
| /// # }); | ||
| /// ``` |
Copilot
AI
Jan 15, 2026
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.
The run_mcp_server function is publicly exported but there's no visible integration with the CLI's command-line argument parsing. The PR description mentions --serve and --serve-port flags, but these flags don't appear to be defined in the command structure (GolemCliCommand or GolemCliGlobalFlags). Without these flags being added to the CLI argument parser, this function cannot be invoked by users as documented. You need to either: (1) add the --serve and --serve-port flags to the command structure and wire them up in the command handler, or (2) update the documentation to clarify the actual invocation method.
| /// Start the MCP server on the specified port | |
| /// | |
| /// This function blocks until the server is shut down (Ctrl+C). | |
| /// | |
| /// # Arguments | |
| /// * `port` - Port to listen on | |
| /// | |
| /// # Example | |
| /// ```no_run | |
| /// # use golem_cli::mcp::run_mcp_server; | |
| /// # tokio::runtime::Runtime::new().unwrap().block_on(async { | |
| /// run_mcp_server(1232).await.unwrap(); | |
| /// # }); | |
| /// ``` | |
| /// Start the MCP server on the specified port. | |
| /// | |
| /// This is a **library entrypoint** used by callers that want to expose the | |
| /// Golem CLI via the MCP protocol. Calling this function does **not** add any | |
| /// command-line flags (such as `--serve` or `--serve-port`) on its own; it is | |
| /// up to the embedding binary or CLI layer to define such flags and call | |
| /// `run_mcp_server` with the appropriate port value. | |
| /// | |
| /// This function blocks until the server is shut down (Ctrl+C or SIGTERM). | |
| /// | |
| /// # Arguments | |
| /// * `port` - Port to listen on | |
| /// | |
| /// # Examples | |
| /// | |
| /// Programmatic use from Rust: | |
| /// | |
| /// ```no_run | |
| /// # use golem_cli::mcp::run_mcp_server; | |
| /// # tokio::runtime::Runtime::new().unwrap().block_on(async { | |
| /// run_mcp_server(1232).await.unwrap(); | |
| /// # }); | |
| /// ``` | |
| /// | |
| /// A CLI wrapper (defined elsewhere) can parse flags such as `--serve` and | |
| /// `--serve-port` and delegate to this function. This module does not define | |
| /// or register those flags directly; it only provides the server runner. |
|
|
||
| let result = client | ||
| .call_tool(name, serde_json::json!({"args": ["--help"]})) | ||
| .expect(&format!("Failed to call tool: {}", name)); |
Copilot
AI
Jan 15, 2026
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.
Using expect with a formatted string is inefficient because the string is allocated even when the Result is Ok. Replace with unwrap_or_else or use expect with a static string and include the error context in the panic message using the error's Display implementation.
|
|
||
| let result = client | ||
| .read_resource(uri) | ||
| .expect(&format!("Failed to read resource: {}", uri)); |
Copilot
AI
Jan 15, 2026
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.
Using expect with a formatted string is inefficient because the string is allocated even when the Result is Ok. Replace with unwrap_or_else or use expect with a static string and include the error context in the panic message using the error's Display implementation.
| .expect(&format!("Failed to read resource: {}", uri)); | |
| .unwrap_or_else(|e| panic!("Failed to read resource {}: {}", uri, e)); |
| # Call a tool (e.g., get help) | ||
| curl -X POST http://127.0.0.1:1232/mcp \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"component","arguments":{"args":["--help"]}}}' |
Copilot
AI
Jan 15, 2026
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.
The example shows calling a tool named 'component', but based on the tool naming pattern in mcp.rs (line 162: format!(\"{}.{}\", prefix, path.join(\".\"))), the actual tool name would be something like 'golem-cli.component' (with the command_name prefix). This example may not work as shown and could confuse users. Either update the example to use the correct tool name format, or clarify in the documentation what the actual tool naming convention is.
| # Call a tool (e.g., get help) | |
| curl -X POST http://127.0.0.1:1232/mcp \ | |
| -H "Content-Type: application/json" \ | |
| -d '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"component","arguments":{"args":["--help"]}}}' | |
| # Call a tool (e.g., get help for the `component` command) | |
| curl -X POST http://127.0.0.1:1232/mcp \ | |
| -H "Content-Type: application/json" \ | |
| -d '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"golem-cli.component","arguments":{"args":["--help"]}}}' |
|
|
||
| /// Output from CLI tool execution | ||
| /// | ||
| /// # HUMAN-VALIDATED 2025-01-15 |
Copilot
AI
Jan 15, 2026
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.
The comment '# HUMAN-VALIDATED 2025-01-15' appears to be a marker for AI-assisted code review or validation. This type of internal process comment should not be included in production code. Remove this comment as it provides no value to future code maintainers and may confuse readers.
| /// # HUMAN-VALIDATED 2025-01-15 |
| let server = GolemMcpServer::new()?; | ||
| let addr: SocketAddr = ([127, 0, 0, 1], port).into(); | ||
|
|
||
| eprintln!("{} running MCP Server at port {}", command_name(), port); |
Copilot
AI
Jan 15, 2026
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.
Using eprintln! for informational server startup messages is unconventional. Informational messages typically go to stdout (println!), while stderr (eprintln!) is reserved for errors and warnings. Consider using println! instead, or better yet, use the info! logging macro which is already imported and used elsewhere in this module (line 481).
| eprintln!("{} running MCP Server at port {}", command_name(), port); | |
| info!("{} running MCP Server at port {}", command_name(), port); |
Summary
Implements #1926 - MCP Server for Golem CLI
This PR adds an MCP (Model Context Protocol) server mode to the Golem CLI, enabling AI agents and tools to interact with Golem programmatically.
Changes
--serveand--serve-portflags to enable MCP server modegolem.yamlmanifests as MCP resourcesrmcpcrate with HTTP Streamable transportUsage