feat(mcp): add Kmesh version tool - #1900
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @AslinDhurai! It looks like this is your first PR to kmesh-net/kmesh 🎉 |
There was a problem hiding this comment.
Pull request overview
Adds an initial MCP (Model Context Protocol) server to the Kmesh repo, exposing build/version metadata via a new kmesh_version tool, along with supporting dependencies and a basic unit test.
Changes:
- Introduced an MCP server binary (
mcp/server.go) registering akmesh_versiontool. - Added a version tool implementation (
mcp/tools/version.go) and unit test (mcp/tools/version_test.go) backed bypkg/version. - Updated Go module dependencies to include the official MCP Go SDK and related indirect deps.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| mcp/tools/version.go | Adds the MCP tool handler and output struct for version/build info. |
| mcp/tools/version_test.go | Adds unit tests validating the tool output against version.Get(). |
| mcp/server.go | Adds a runnable MCP server exposing the tool over HTTP. |
| go.mod | Adds MCP SDK dependency and updates golang.org/x/* versions. |
| go.sum | Records new/updated module checksums. |
Suppressed comments (1)
mcp/server.go:58
- Starting the HTTP server via
http.ListenAndServe(":8080", nil)leaves all server timeouts at their zero values. Prefer constructing anhttp.Serverwith explicit timeouts (similar to pkg/status/status_server.go) and callingListenAndServeon it.
log.Println("Kmesh MCP server listening on :8080")
if err := http.ListenAndServe(":8080", nil); err != nil {
log.Fatalf("MCP server stopped: %v", err)
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| type VersionOutput struct { | ||
| Version string `json:"version"` | ||
| Commit string `json:"commit"` | ||
| TreeState string `json:"treeState"` | ||
| BuildDate string `json:"buildDate"` |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
go.mod:154
github.com/modelcontextprotocol/go-sdkis directly imported by the new MCP code, but it appears in the large indirectrequireblock (without// indirect) and there is also an extra smallrequireblock added above for a few indirect deps. This suggestsgo.modis not in ago mod tidystate, which makes future dependency changes harder to review and can cause churn in later PRs.
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/moby/spdystream v0.5.0 // indirect
github.com/moby/term v0.5.0 // indirect
github.com/modelcontextprotocol/go-sdk v1.4.0
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
mcp/server.go:61
- This uses the global DefaultServeMux via http.Handle and sets http.Server.Handler to nil. For a standalone server this makes handler registration global and can lead to accidental handler reuse if other packages register routes; prefer a dedicated ServeMux and pass it explicitly to the server.
http.Handle("/mcp", transport)
log.Println("Kmesh MCP server listening on :8080")
srv := &http.Server{
Addr: ":8080",
go.mod:52
- go.mod adds a standalone
require (...)block containing only a few indirect dependencies. This makes futurego mod tidyoutput noisier and increases the chance of merge conflicts; it’s usually better to letgo mod tidyconsolidate indirect requirements into the existing indirect require block(s).
require (
github.com/google/jsonschema-go v0.4.2 // indirect
github.com/segmentio/asm v1.1.3 // indirect
github.com/segmentio/encoding v0.5.3 // indirect
github.com/yosida95/uritemplate/v3 v3.0.2 // indirect
)
| transport := &mcp.StreamableServerTransport{ | ||
| SessionID: "kmesh-mcp", | ||
| } | ||
|
|
||
| if _, err := server.Connect(context.Background(), transport, nil); err != nil { | ||
| log.Fatalf("failed to connect MCP server: %v", err) | ||
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (4)
mcp/tools/version.go:33
- VersionOutput duplicates pkg/version.Info but changes JSON field names (e.g.,
versionvsgitVersion). Reusing version.Info (via a type alias) keeps the MCP response schema consistent with the existing version endpoint and avoids schema drift.
type VersionOutput struct {
Version string `json:"version"`
Commit string `json:"commit"`
TreeState string `json:"treeState"`
BuildDate string `json:"buildDate"`
mcp/tools/version.go:48
- Once VersionOutput is a type alias of version.Info, the tool can return version.Get() directly instead of manually copying each field. This removes duplicate mapping code and keeps future additions to version.Info automatically reflected in the tool output.
v := version.Get()
return nil, VersionOutput{
Version: v.GitVersion,
Commit: v.GitCommit,
mcp/server.go:65
- The HTTP server is started without ReadTimeout/WriteTimeout/IdleTimeout and uses the default global ServeMux (via http.Handle + Handler:nil). This makes the server more vulnerable to slow clients and can lead to handler registration conflicts if more endpoints are added. Prefer a dedicated ServeMux and set full server timeouts (similar to pkg/status/status_server.go).
srv := &http.Server{
Addr: ":8080",
Handler: nil,
ReadHeaderTimeout: 5 * time.Second,
}
mcp/tools/version_test.go:57
- If VersionOutput is changed to alias pkg/version.Info (recommended for schema consistency), this test must assert against Info field names (GitVersion/GitCommit/GitTreeState/...) or it will not compile.
if output.Version != expected.GitVersion {
t.Errorf("version = %q, want %q", output.Version, expected.GitVersion)
}
if output.Commit != expected.GitCommit {
Codecov Report✅ All modified and coverable lines are covered by tests. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Signed-off-by: AslinDhurai <aslindhurai1@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: AslinDhurai <aslindhurai1@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: AslinDhurai <aslindhurai1@gmail.com>
Signed-off-by: AslinDhurai <aslindhurai1@gmail.com>
1453588 to
f290643
Compare
|
The goimports formatting issue in mcp/server.go has been fixed, and I verified the configured golangci-lint checks locally with no errors. I also noticed an issue while running make e2e-ipv6: the E2E script appears to have a logging-related problem around kmesh_daemon.log/pod log capture. This seems unrelated to the changes in this PR, but it may be affecting the E2E build/check. Could someone please rerun the failed CI job and confirm whether the E2E failure is related to the existing test infrastructure or the PR? Thanks! |
What type of PR is this?
/kind feature
Summary
This PR adds an initial MCP server implementation for Kmesh with a version information tool.
Changes
kmesh_versionMCP toolValidation
go test ./mcp/...go vet ./mcp/...go test -race ./mcp/...git diff --checkAll checks pass.
Mentorship
I am interested in contributing to Kmesh as a mentee through the mentorship program. I would be happy to continue working on MCP-related improvements and take guidance from the maintainers on suitable follow-up tasks.