feat: enable streamable http server for osv#14
Conversation
add an env var that allows to switch mcp server mode, from sse to streamable http mode
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Adds an environment‐variable switch to start the MCP server in either SSE or streamable HTTP mode.
- Introduce
ServeHTTPStreaminpkg/mcp/server.gowith streamable‐HTTP transport and heartbeat - Enhance
handleQueryVulnerabilitiesBatchto validate argument structure - Wire
MCP_TRANSPORT_MODEincmd/server/main.goto select between SSE and stream modes and bump related dependencies
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/mcp/server.go | Added ServeHTTPStream, heartbeat config, and improved batch‐query args parsing |
| cmd/server/main.go | Added MCP_TRANSPORT_MODE env var and switch logic for transport modes |
| go.mod | Bumped mcp-go and spf13/cast versions |
Comments suppressed due to low confidence (4)
pkg/mcp/server.go:167
- [nitpick] Consider making the endpoint path configurable instead of hard-coding "/mcp", so it can be adjusted without code changes.
server.WithEndpointPath("/mcp"),
pkg/mcp/server.go:231
- [nitpick] The error message "Invalid arguments format" is quite generic; consider adding context, e.g. expected a map[string]interface{}.
if !ok {
pkg/mcp/server.go:164
- Update documentation (README or comments) to include the new "streamable HTTP" mode and describe how to configure it via MCP_TRANSPORT_MODE.
log.Printf("Starting OSV MCP server (Streamable HTTP) on %s", addr)
pkg/mcp/server.go:163
- There are no tests covering
ServeHTTPStream; consider adding unit or integration tests to verify its behavior and heartbeat functionality.
func (s *Server) ServeHTTPStream(addr string) error {
cmd/server/main.go
Outdated
| case "stream": | ||
| errChan <- mcpServer.ServeHTTPStream(*addr) | ||
| default: | ||
| log.Printf("Starting OSV MCP server (SSE) on %s", *addr) |
There was a problem hiding this comment.
[nitpick] Extract the mode string literals ("stream", "sse") into named constants to avoid typos and improve maintainability.
| case "stream": | |
| errChan <- mcpServer.ServeHTTPStream(*addr) | |
| default: | |
| log.Printf("Starting OSV MCP server (SSE) on %s", *addr) | |
| case ModeStream: | |
| errChan <- mcpServer.ServeHTTPStream(*addr) | |
| default: | |
| log.Printf("Starting OSV MCP server (%s) on %s", ModeSSE, *addr) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
9c26711 to
8cefde4
Compare
eleftherias
left a comment
There was a problem hiding this comment.
I left one non-blocking comment. Also should we document this option in the README or is it not ready for general usage yet?
cmd/server/main.go
Outdated
| case "stream": | ||
| errChan <- mcpServer.ServeHTTPStream(*addr) | ||
| default: | ||
| log.Printf("Starting OSV MCP server (SSE) on %s", *addr) |
There was a problem hiding this comment.
nit: This message gets printed twice because it's also printed in ServeSSE
add an env var that allows to switch mcp server mode, from sse to streamable http mode