Support for listening on localhost with http - #1
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds HTTP server support to mcpshell, enabling it to serve MCP protocol over HTTP/SSE instead of just stdio. This allows integration with clients like Amazon Q and Claude Desktop that prefer HTTP-based communication.
- Adds HTTP server mode with
--httpand--portflags to the mcp command - Implements HTTP endpoint at
/ssethat handles MCP protocol requests via POST - Special handling for the
initializemethod to return proper server capabilities
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| pkg/server/server.go | Adds HTTP server functionality with StartHTTP method and handleMCPHTTP handler |
| cmd/mcp.go | Adds HTTP flags and conditional startup logic to use HTTP mode when requested |
| docs/usage.md | Documents the new HTTP/SSE mode flags and usage example |
Comments suppressed due to low confidence (1)
pkg/server/server.go:658
- The protocol version "2025-03-26" appears to be from the future (beyond January 2025 knowledge cutoff). Please verify this is a valid MCP protocol version or use a known valid version.
protocolVersion = "2025-03-26" // fallback, should always be present
| return | ||
| } | ||
|
|
||
| s.logger.Info("Request body: %s", string(body)) |
There was a problem hiding this comment.
Logging the full request body may expose sensitive information in logs. Consider logging only a summary or sanitized version of the request for security purposes.
| w.Header().Set("Content-Type", "application/json") | ||
| w.WriteHeader(http.StatusOK) | ||
| respBytes, _ := json.Marshal(resp) | ||
| s.logger.Info("Response: %s", string(respBytes)) |
There was a problem hiding this comment.
Logging the full response may expose sensitive information in logs. Consider logging only a summary or sanitized version of the response for security purposes.
| s.logger.Info("Response: %s", string(respBytes)) | |
| result := resp["result"].(map[string]interface{}) | |
| serverInfo := result["serverInfo"].(map[string]interface{}) | |
| sessionID := result["sessionId"] | |
| protocolVersion := result["protocolVersion"] | |
| s.logger.Info("Response summary: server=%s version=%s sessionId=%v protocolVersion=%v", serverInfo["name"], serverInfo["version"], sessionID, protocolVersion) |
| default: | ||
| respBytes, _ = json.Marshal(v) | ||
| } | ||
| s.logger.Info("Response: %s", string(respBytes)) |
There was a problem hiding this comment.
Logging the full response may expose sensitive information in logs. Consider logging only a summary or sanitized version of the response for security purposes.
| s.logger.Info("Response: %s", string(respBytes)) | |
| s.logger.Info("Response sent to %s: type=%T, length=%d bytes", r.RemoteAddr, resp, len(respBytes)) |
| http.HandleFunc("/sse", s.handleMCPHTTP) | ||
| addr := fmt.Sprintf(":%d", port) | ||
| s.logger.Info("Listening on http://localhost%s/sse", addr) | ||
| fmt.Printf("MCP HTTP server listening on http://localhost%s/sse\n", addr) |
There was a problem hiding this comment.
The endpoint is named '/sse' but the implementation is a standard HTTP POST handler, not Server-Sent Events. Consider renaming to '/mcp' or implementing actual SSE if that's the intended protocol.
| http.HandleFunc("/sse", s.handleMCPHTTP) | |
| addr := fmt.Sprintf(":%d", port) | |
| s.logger.Info("Listening on http://localhost%s/sse", addr) | |
| fmt.Printf("MCP HTTP server listening on http://localhost%s/sse\n", addr) | |
| http.HandleFunc("/mcp", s.handleMCPHTTP) | |
| addr := fmt.Sprintf(":%d", port) | |
| s.logger.Info("Listening on http://localhost%s/mcp", addr) | |
| fmt.Printf("MCP HTTP server listening on http://localhost%s/mcp\n", addr) |
| w.WriteHeader(http.StatusOK) | ||
| respBytes, _ := json.Marshal(resp) |
There was a problem hiding this comment.
Error from json.Marshal is being ignored. Consider handling the marshaling error to ensure robust error handling.
| w.WriteHeader(http.StatusOK) | |
| respBytes, _ := json.Marshal(resp) | |
| respBytes, err := json.Marshal(resp) | |
| if err != nil { | |
| s.logger.Error("Failed to marshal response: %v", err) | |
| http.Error(w, `{"error":"Internal server error"}`, http.StatusInternalServerError) | |
| return | |
| } | |
| w.WriteHeader(http.StatusOK) |
| case string: | ||
| respBytes = []byte(v) | ||
| default: | ||
| respBytes, _ = json.Marshal(v) |
There was a problem hiding this comment.
Error from json.Marshal is being ignored. Consider handling the marshaling error to ensure robust error handling.
| respBytes, _ = json.Marshal(v) | |
| var err error | |
| respBytes, err = json.Marshal(v) | |
| if err != nil { | |
| http.Error(w, "Failed to marshal response", http.StatusInternalServerError) | |
| s.logger.Error("Failed to marshal response for %s: %v", r.RemoteAddr, err) | |
| return | |
| } |
This change enables mcpshell to listen on localhost on a specified port for http connections. Mode and port are specified with the --http and --port options when the command mode is "mcp". This enables integration with amazonq and claude desktop.