-
Notifications
You must be signed in to change notification settings - Fork 711
feat(mcp): add HTTP and Stdio client Roots feature
#620
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
Conversation
WalkthroughAdds end-to-end "roots" support: new MCP method and notification, client and server roots interfaces/handlers, transport integrations (stdio, streamable HTTP, in-process), examples, tests, and a .gitignore update. (≤50 words) Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 9
🧹 Nitpick comments (10)
examples/roots_server/main.go (1)
12-14: Add newline to Printf format string.The Printf call is missing a newline character, which may affect output formatting in the console.
func handleNotification(ctx context.Context, notification mcp.JSONRPCNotification) { - fmt.Printf("notification received: %v", notification.Notification.Method) + fmt.Printf("notification received: %v\n", notification.Notification.Method) }examples/roots_http_server/main.go (1)
12-14: Add newline to Printf format string.The Printf call is missing a newline character, which may affect output formatting in the console.
func handleNotification(ctx context.Context, notification mcp.JSONRPCNotification) { - fmt.Printf("notification received: %v", notification.Notification.Method) + fmt.Printf("notification received: %v\n", notification.Notification.Method) }examples/roots_http_client/main.go (2)
20-34: Consider using more realistic example paths.The hardcoded root URIs use "User" (should be "Users" on macOS) and "haxxx" as a placeholder username. For better clarity in documentation, consider using more conventional example paths.
func (h *MockRootsHandler) ListRoots(ctx context.Context, request mcp.ListRootsRequest) (*mcp.ListRootsResult, error) { result := &mcp.ListRootsResult{ Roots: []mcp.Root{ { Name: "app", - URI: "file:///User/haxxx/app", + URI: "file:///home/user/app", }, { Name: "test-project", - URI: "file:///User/haxxx/projects/test-project", + URI: "file:///home/user/projects/test-project", }, }, } return result, nil }
94-109: Consider using structured data for tool arguments.Line 97 sets
Argumentsas a JSON string, but the field type isanywhich can accept structured data. Using a map would be more idiomatic and type-safe.// call server tool request := mcp.CallToolRequest{} request.Params.Name = "roots" - request.Params.Arguments = "{\"testonly\": \"yes\"}" + request.Params.Arguments = map[string]any{ + "testonly": "yes", + } result, err := mcpClient.CallTool(ctx, request)server/stdio.go (3)
105-107: Update mutex comment to include roots mappendingMu also protects pendingRoots; update the comment for accuracy.
- pendingMu sync.RWMutex // protects pendingRequests and pendingElicitations + pendingMu sync.RWMutex // protects pendingRequests, pendingElicitations, and pendingRoots
121-126: Fix comment grammar and namingUse “a list roots request” and pluralize consistently.
-// rootsResponse represents a response to an list root request +// rootsResponse represents a response to a list roots request
246-305: ListRoots flow looks correct; minor nit about unused paramImplementation is sound (ID correlation, cleanup, ctx handling). The request parameter is currently unused; either document that the request carries no params per spec or rename to _ if desired.
server/roots_test.go (1)
107-164: Tests exercise happy-path and error-path wellMocks are minimal and clear; capability gating via WithRoots verified; table-driven test is good. Consider consistently using require/assert instead of t.Error/t.Errorf for uniformity.
client/transport/inprocess.go (1)
41-45: Add GoDoc for exported optionExported functions should have GoDoc starting with the identifier.
- func WithRootsHandler(handler server.RootsHandler) InProcessOption { +// WithRootsHandler configures an in‑process transport to handle roots requests. +func WithRootsHandler(handler server.RootsHandler) InProcessOption {examples/roots_client/main.go (1)
114-129: Prefer structured arguments over raw JSON stringsPass a map for Arguments to avoid double‑encoding and improve type safety.
- request := mcp.CallToolRequest{} - request.Params.Name = "roots" - request.Params.Arguments = "{\"testonly\": \"yes\"}" + request := mcp.CallToolRequest{} + request.Params.Name = "roots" + request.Params.Arguments = map[string]any{"testonly": "yes"}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.gitignore(1 hunks)client/client.go(6 hunks)client/roots.go(1 hunks)client/transport/inprocess.go(3 hunks)examples/roots_client/main.go(1 hunks)examples/roots_http_client/main.go(1 hunks)examples/roots_http_server/main.go(1 hunks)examples/roots_server/main.go(1 hunks)mcp/types.go(2 hunks)server/inprocess_session.go(5 hunks)server/roots.go(1 hunks)server/roots_test.go(1 hunks)server/server.go(3 hunks)server/session.go(1 hunks)server/stdio.go(6 hunks)server/streamable_http.go(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing
Files:
mcp/types.goserver/session.goserver/server.goserver/roots.goserver/roots_test.goexamples/roots_server/main.goexamples/roots_client/main.goserver/stdio.goclient/client.goserver/streamable_http.goexamples/roots_http_client/main.goclient/roots.goserver/inprocess_session.goexamples/roots_http_server/main.goclient/transport/inprocess.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Testing: use testify/assert and testify/require
Write table-driven tests using a tests := []struct{ name, ... } pattern
Go test files must end with _test.go
Files:
server/roots_test.go
🧠 Learnings (1)
📚 Learning: 2025-06-30T07:13:17.052Z
Learnt from: ezynda3
PR: mark3labs/mcp-go#461
File: server/sampling.go:22-26
Timestamp: 2025-06-30T07:13:17.052Z
Learning: In the mark3labs/mcp-go project, the MCPServer.capabilities field is a struct value (serverCapabilities), not a pointer, so it cannot be nil and doesn't require nil checking. Only pointer fields within the capabilities struct should be checked for nil.
Applied to files:
server/server.go
🧬 Code graph analysis (14)
server/session.go (1)
mcp/types.go (2)
ListRootsRequest(1150-1152)ListRootsResult(1157-1160)
server/server.go (1)
mcp/utils.go (1)
ToBoolPtr(949-951)
server/roots.go (2)
mcp/types.go (2)
ListRootsRequest(1150-1152)ListRootsResult(1157-1160)server/session.go (2)
ClientSessionFromContext(101-106)SessionWithRoots(75-79)
server/roots_test.go (6)
mcp/types.go (6)
JSONRPCNotification(339-342)ListRootsResult(1157-1160)ListRootsRequest(1150-1152)Request(175-178)MethodListRoots(64-64)Root(1163-1174)server/server.go (3)
NewMCPServer(345-373)WithRoots(331-335)ServerOption(31-31)mcp/utils.go (1)
ToBoolPtr(949-951)server/elicitation.go (1)
ErrNoActiveSession(12-12)server/roots.go (1)
ErrRootsNotSupported(14-14)server/session.go (1)
ClientSession(12-21)
examples/roots_server/main.go (4)
mcp/types.go (7)
JSONRPCNotification(339-342)Notification(186-189)MethodNotificationToolsListChanged(78-78)ListRootsRequest(1150-1152)Request(175-178)MethodListRoots(64-64)Content(960-962)server/server.go (3)
ServerOption(31-31)WithToolCapabilities(307-314)WithRoots(331-335)mcp/tools.go (5)
Tool(557-574)Description(875-879)ToolInputSchema(632-632)CallToolRequest(54-58)CallToolResult(40-51)server/stdio.go (1)
ServeStdio(857-877)
examples/roots_client/main.go (4)
mcp/types.go (10)
ListRootsRequest(1150-1152)ListRootsResult(1157-1160)Root(1163-1174)InitializeRequest(437-441)Params(184-184)InitializeParams(443-449)LATEST_PROTOCOL_VERSION(108-108)Implementation(527-530)ClientCapabilities(478-490)Content(960-962)client/transport/stdio.go (1)
NewStdio(91-97)client/client.go (1)
NewClient(79-89)mcp/tools.go (2)
ListToolsRequest(18-21)CallToolRequest(54-58)
server/stdio.go (2)
mcp/types.go (6)
ListRootsResult(1157-1160)ListRootsRequest(1150-1152)JSONRPC_VERSION(118-118)MethodListRoots(64-64)JSONRPCNotification(339-342)Result(253-257)server/session.go (1)
SessionWithRoots(75-79)
client/client.go (7)
server/inprocess_session.go (1)
RootsHandler(23-25)client/roots.go (1)
RootsHandler(11-17)client/transport/inprocess.go (1)
WithRootsHandler(41-45)mcp/types.go (9)
JSONRPCNotification(339-342)JSONRPC_VERSION(118-118)Notification(186-189)MethodNotificationToolsListChanged(78-78)MethodListRoots(64-64)JSONRPCRequest(331-336)JSONRPCResponse(345-349)ListRootsRequest(1150-1152)Request(175-178)client/transport/interface.go (2)
JSONRPCRequest(57-62)JSONRPCResponse(67-72)mcp/utils.go (1)
NewJSONRPCResultResponse(129-135)client/transport/utils.go (1)
NewJSONRPCResultResponse(20-26)
server/streamable_http.go (2)
mcp/types.go (6)
JSONRPCRequest(331-336)NewRequestId(266-268)Request(175-178)MethodListRoots(64-64)ListRootsRequest(1150-1152)ListRootsResult(1157-1160)server/session.go (1)
SessionWithRoots(75-79)
examples/roots_http_client/main.go (4)
mcp/types.go (10)
ListRootsRequest(1150-1152)ListRootsResult(1157-1160)Root(1163-1174)InitializeRequest(437-441)Params(184-184)InitializeParams(443-449)LATEST_PROTOCOL_VERSION(108-108)ClientCapabilities(478-490)Implementation(527-530)Content(960-962)client/transport/streamable_http.go (2)
NewStreamableHTTP(131-161)WithContinuousListening(32-36)client/client.go (1)
NewClient(79-89)mcp/tools.go (1)
CallToolRequest(54-58)
client/roots.go (1)
mcp/types.go (2)
ListRootsRequest(1150-1152)ListRootsResult(1157-1160)
server/inprocess_session.go (2)
mcp/types.go (2)
ListRootsRequest(1150-1152)ListRootsResult(1157-1160)server/session.go (1)
SessionWithRoots(75-79)
examples/roots_http_server/main.go (4)
mcp/types.go (7)
JSONRPCNotification(339-342)Notification(186-189)MethodNotificationToolsListChanged(78-78)ListRootsRequest(1150-1152)Request(175-178)MethodListRoots(64-64)Content(960-962)server/server.go (3)
ServerOption(31-31)WithToolCapabilities(307-314)WithRoots(331-335)mcp/tools.go (2)
Tool(557-574)CallToolResult(40-51)server/streamable_http.go (2)
StreamableHTTPOption(26-26)NewStreamableHTTPServer(165-181)
client/transport/inprocess.go (2)
server/inprocess_session.go (2)
RootsHandler(23-25)NewInProcessSessionWithHandlers(48-56)client/client.go (1)
WithRootsHandler(50-54)
🔇 Additional comments (22)
.gitignore (1)
8-8: LGTM!Adding
.vscode/launch.jsonto the ignore list is appropriate and follows best practices. This ensures VS Code debug configurations (which typically contain user-specific settings) are not accidentally committed to the repository. The change aligns with the existing pattern of ignoring IDE-specific files (e.g.,.idea).mcp/types.go (2)
62-64: LGTM!The constant definition follows MCP naming conventions and includes a proper specification reference.
522-523: LGTM!The Roots capability follows the established pattern for MCP server capabilities, using
*struct{}to indicate feature presence and proper JSON serialization.server/server.go (3)
186-186: LGTM!The roots field follows the established pattern for capability flags, using
*boolto distinguish between unset and false values.
330-335: LGTM!The
WithRoots()option follows the established pattern for enabling server capabilities and uses the utility functionmcp.ToBoolPtr()appropriately.
707-709: LGTM!The roots capability initialization follows the established pattern used for other capabilities, with proper nil checking before dereferencing.
client/roots.go (1)
9-17: LGTM!The
RootsHandlerinterface is well-defined with clear documentation. It follows Go conventions by acceptingcontext.Contextas the first parameter and provides helpful implementation guidance.server/session.go (1)
74-79: LGTM!The
SessionWithRootsinterface follows the established pattern for session extensions and properly embedsClientSessionwhile adding theListRootsmethod.examples/roots_http_client/main.go (1)
111-120: LGTM!The signal handling implementation follows standard Go patterns for graceful shutdown.
server/roots.go (2)
10-15: LGTM!The sentinel errors follow Go conventions with clear, descriptive messages and proper exported names for error checking with
errors.Is.
17-32: LGTM!The
RequestRootsmethod correctly:
- Retrieves the session from context
- Checks for session presence before use
- Uses type assertion to verify roots support
- Delegates to the appropriate session method
- Returns clear, actionable errors
The implementation follows established patterns in the codebase for similar request methods.
server/stdio.go (2)
595-599: Good: roots response short‑circuitingEarly return on handled roots responses mirrors existing flows and avoids double processing.
770-830: Robust response routing; aligns with JSON‑RPC parsingParsing by id and non‑blocking send prevents handler stalls; error wrapping is clear.
client/transport/inprocess.go (1)
75-78: Session wiring LGTMCreating/registering a session when rootsHandler is present mirrors other handlers and is correct.
client/client.go (3)
189-195: Correct capability exposureDeclaring roots capability with listChanged is per spec.
514-516: ListRoots request handler wired correctlyRoutes roots/list to handler; clean, matches other handlers.
578-607: Transport handler solid; no params requiredValidates handler presence, wraps result in JSON‑RPC response.
examples/roots_client/main.go (1)
109-113: Ensure RootListChanges uses the roots notificationThis relies on client.RootListChanges sending notifications/roots/list_changed (see fix in client.go).
server/roots_test.go (1)
210-219: ****The claim that
fakeSessionis "not declared" is false.fakeSessionis properly defined inserver/server_test.go(line 1802) and implements theClientSessioninterface. BothfakeSessionandmockBasicRootsSessionare valid, visible types within the package; the code compiles and runs without undefined-type errors.While
mockBasicRootsSession(defined inroots_test.go) could be a stylistic preference for semantic clarity in that file's scope, the undefined-type blocker claim is incorrect and does not reflect the actual state of the codebase.Likely an incorrect or invalid review comment.
server/streamable_http.go (3)
555-568: LGTM!The roots request handling in the GET streaming path correctly follows the established pattern for sampling and elicitation requests.
904-909: LGTM!The
rootsRequestItemtype and associated channel infrastructure correctly follow the established patterns for bidirectional request/response communication over HTTP transport.Also applies to: 925-925, 940-940
1149-1149: LGTM!The interface compliance assertion correctly ensures
streamableHttpSessionimplementsSessionWithRoots.
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @yuehaii. * #620 (comment) The following files were modified: * `client/client.go` * `client/transport/inprocess.go` * `examples/roots_client/main.go` * `examples/roots_http_client/main.go` * `examples/roots_http_server/main.go` * `examples/roots_server/main.go` * `server/inprocess_session.go` * `server/server.go` * `server/streamable_http.go`
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.
Actionable comments posted: 2
♻️ Duplicate comments (2)
examples/roots_server/main.go (1)
30-33: Fix comment grammar and remove unexplained sampling.
- Clarify the comment.
- EnableSampling() seems unrelated to roots; remove it or explain why it’s needed here. This was flagged previously.
- // Add list root list change notification + // Register roots list-change notification handler mcpServer.AddNotificationHandler(mcp.MethodNotificationRootsListChanged, handleNotification) - mcpServer.EnableSampling() + // Sampling not required for roots example; enable only if demonstrating sampling + // mcpServer.EnableSampling()client/client.go (1)
484-503: Add GoDoc for exported method.Public functions require GoDoc starting with the identifier name.
+// RootListChanges sends a roots list-changed notification to the server. func (c *Client) RootListChanges( ctx context.Context, ) error {
🧹 Nitpick comments (9)
examples/roots_server/main.go (2)
12-15: Log notifications with newline (or use log package).fmt.Printf without newline can produce messy output. Prefer log.Printf or add “\n”.
-func handleNotification(ctx context.Context, notification mcp.JSONRPCNotification) { - fmt.Printf("notification received: %v", notification.Notification.Method) -} +func handleNotification(ctx context.Context, notification mcp.JSONRPCNotification) { + log.Printf("notification received: %s", notification.Notification.Method) +}
51-63: Prefer tool-level errors (IsError) over returning JSON-RPC errors.Returning a CallToolResult with IsError communicates failures at the tool layer and avoids surfacing a transport error to the caller.
- if result, err := mcpServer.RequestRoots(ctx, rootRequest); err == nil { + if result, err := mcpServer.RequestRoots(ctx, rootRequest); err == nil { return &mcp.CallToolResult{ Content: []mcp.Content{ mcp.TextContent{ Type: "text", Text: fmt.Sprintf("Root list: %v", result.Roots), }, }, }, nil - - } else { - return nil, err + } else { + return &mcp.CallToolResult{ + Content: []mcp.Content{ + mcp.TextContent{ + Type: "text", + Text: fmt.Sprintf("Fail to list roots: %v", err), + }, + }, + IsError: true, + }, nil }examples/roots_http_client/main.go (4)
16-18: Tighten doc: this isn’t an LLM handler.Clarify that MockRootsHandler provides workspace roots, not LLM access.
-// MockRootsHandler implements client.RootsHandler for demonstration. -// In a real implementation, this would integrate with an actual LLM API. +// MockRootsHandler implements client.RootsHandler for demonstration. +// In a real implementation, this would enumerate workspace/project roots.
36-39: Fix comment grammar for main description.-// main starts a mock MCP roots client that communicates with a MCP server over http. -// client will call server tool to get the root list. in server tool hander, it will send the list root request to client. -// shuts down the client gracefully on SIGINT or SIGTERM. +// main starts a mock MCP roots client over HTTP. +// The server tool triggers a roots/list request on the client. +// The client shuts down gracefully on SIGINT or SIGTERM.
97-101: Pass structured arguments instead of a raw JSON string.Use a map to match the tool’s input schema.
- request := mcp.CallToolRequest{} - request.Params.Name = "roots" - request.Params.Arguments = "{\"testonly\": \"yes\"}" + request := mcp.CallToolRequest{} + request.Params.Name = "roots" + request.Params.Arguments = map[string]any{"testonly": "yes"}
111-112: Add newline when printing result.- fmt.Printf("client call tool result: %s", resultStr) + fmt.Printf("client call tool result: %s\n", resultStr)examples/roots_client/main.go (3)
16-18: Clarify handler purpose; not an LLM.-// MockRootsHandler implements client.RootsHandler for demonstration. -// In a real implementation, this would integrate with an actual LLM API. +// MockRootsHandler implements client.RootsHandler for demonstration. +// In a real implementation, this would enumerate workspace/project roots.
115-120: Use structured arguments instead of a raw JSON string.- request := mcp.CallToolRequest{} - request.Params.Name = "roots" - request.Params.Arguments = "{\"testonly\": \"yes\"}" + request := mcp.CallToolRequest{} + request.Params.Name = "roots" + request.Params.Arguments = map[string]any{"testonly": "yes"}
129-130: Add newline when printing result.- fmt.Printf("client call tool result: %s", resultStr) + fmt.Printf("client call tool result: %s\n", resultStr)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
client/client.go(6 hunks)examples/roots_client/main.go(1 hunks)examples/roots_http_client/main.go(1 hunks)examples/roots_http_server/main.go(1 hunks)examples/roots_server/main.go(1 hunks)mcp/types.go(3 hunks)server/inprocess_session.go(5 hunks)server/server.go(3 hunks)server/streamable_http.go(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- server/server.go
- examples/roots_http_server/main.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing
Files:
examples/roots_http_client/main.goclient/client.goserver/streamable_http.goexamples/roots_client/main.goserver/inprocess_session.gomcp/types.goexamples/roots_server/main.go
🧬 Code graph analysis (6)
examples/roots_http_client/main.go (4)
mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)ClientCapabilities(482-494)Implementation(531-534)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/streamable_http.go (2)
NewStreamableHTTP(131-161)WithContinuousListening(32-36)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (1)
CallToolRequest(54-58)
client/client.go (7)
server/inprocess_session.go (1)
RootsHandler(24-26)client/roots.go (1)
RootsHandler(11-17)client/transport/inprocess.go (1)
WithRootsHandler(41-45)mcp/types.go (9)
JSONRPCNotification(343-346)JSONRPC_VERSION(122-122)Notification(190-193)MethodNotificationRootsListChanged(82-82)MethodListRoots(64-64)JSONRPCRequest(335-340)JSONRPCResponse(349-353)ListRootsRequest(1154-1156)Request(179-182)client/transport/interface.go (2)
JSONRPCRequest(57-62)JSONRPCResponse(67-72)mcp/utils.go (1)
NewJSONRPCResultResponse(129-135)client/transport/utils.go (1)
NewJSONRPCResultResponse(20-26)
server/streamable_http.go (2)
mcp/types.go (6)
JSONRPCRequest(335-340)NewRequestId(270-272)Request(179-182)MethodListRoots(64-64)ListRootsRequest(1154-1156)ListRootsResult(1161-1164)server/session.go (1)
SessionWithRoots(75-79)
examples/roots_client/main.go (4)
mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)Implementation(531-534)ClientCapabilities(482-494)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/stdio.go (1)
NewStdio(91-97)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (2)
ListToolsRequest(18-21)CallToolRequest(54-58)
server/inprocess_session.go (3)
mcp/types.go (2)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)client/elicitation.go (1)
ElicitationHandler(11-19)server/session.go (1)
SessionWithRoots(75-79)
examples/roots_server/main.go (4)
mcp/types.go (7)
JSONRPCNotification(343-346)Notification(190-193)MethodNotificationRootsListChanged(82-82)ListRootsRequest(1154-1156)Content(964-966)TextContent(970-977)TextContent(979-979)server/server.go (4)
ServerOption(31-31)WithToolCapabilities(307-314)WithRoots(331-335)NewMCPServer(345-373)mcp/tools.go (6)
Tool(557-574)Description(875-879)ToolInputSchema(632-632)Properties(1141-1145)CallToolRequest(54-58)CallToolResult(40-51)server/stdio.go (1)
ServeStdio(857-877)
🔇 Additional comments (11)
server/streamable_http.go (6)
555-568: LGTM! Roots request handling follows established pattern.The implementation correctly constructs and sends list roots requests via SSE, mirroring the sampling and elicitation request flows. The JSON-RPC request structure is properly formed according to the MCP specification.
904-909: Well-designed type definition.The
rootsRequestItemtype correctly follows the established pattern for bidirectional request/response flows. ReusingsamplingResponseItemfor the response channel is appropriate since all three request types (sampling, elicitation, roots) share the same response envelope structure.
925-925: LGTM!Field addition is consistent with the existing session structure for bidirectional communication support.
940-940: LGTM!Channel initialization follows the established pattern with an appropriate buffer size.
1057-1101: Excellent implementation with proper documentation and error handling.The
ListRootsmethod correctly implements the SessionWithRoots interface for HTTP transport:
- GoDoc comment properly describes the method's purpose (addresses past review feedback)
- Thread-safe implementation using atomic operations and sync.Map
- Proper context cancellation handling at both send and receive points
- Error messages are clear and actionable
- Follows the established pattern from RequestSampling and RequestElicitation
The reuse of
samplingRequestsmap for tracking all bidirectional requests (sampling, elicitation, roots) is intentional and correct since they share a common requestID space and response structure.
1150-1150: LGTM!Compile-time interface assertion ensures type safety and catches implementation issues at build time.
server/inprocess_session.go (5)
23-26: Well-documented interface definition.The
RootsHandlerinterface is properly defined with clear GoDoc documentation (addresses past review feedback). The design is consistent withSamplingHandlerandElicitationHandler, following the established pattern for client-side request handlers.
37-37: LGTM!Field addition is consistent with the existing handler fields and accessed thread-safely via RLock.
49-56: LGTM! Constructor properly extended.The
NewInProcessSessionWithHandlersfunction correctly initializes the newrootsHandlerfield, maintaining consistency with the existing handler initialization pattern.
138-150: Excellent implementation with proper documentation and thread safety.The
ListRootsmethod correctly implements the SessionWithRoots interface:
- GoDoc comment clearly describes the method and its error behavior (addresses past review feedback)
- Thread-safe handler access using RLock/RUnlock
- Appropriate error handling for missing handler
- Follows the exact pattern of
RequestSamplingandRequestElicitation- Adheres to all coding guidelines (context as first parameter, proper error messages)
164-164: LGTM!Compile-time interface assertion ensures type safety and is properly placed with the other interface compliance checks.
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
examples/roots_server/main.go (1)
18-20: Fix outdated comment (references ToolsListChanged and sampling, which aren’t used).Update to reflect RootsListChanged and remove “enables sampling.”
Apply:
-// It registers a handler for ToolsListChanged notifications, enables sampling, and adds a "roots" tool -// that requests and returns the current root list. The program serves the MCP server over stdio and +// It registers a handler for RootsListChanged notifications and adds a "roots" tool +// that requests and returns the current roots list. The program serves the MCP server over stdio and // logs a fatal error if the server fails to start.
🧹 Nitpick comments (8)
examples/roots_server/main.go (2)
37-46: Avoid requiring an unused tool parameter.“testonly” is required but never read. Make it optional or remove the property to reduce friction.
Apply:
mcpServer.AddTool(mcp.Tool{ Name: "roots", Description: "list root result", InputSchema: mcp.ToolInputSchema{ - Type: "object", - Properties: map[string]any{ - "testonly": map[string]any{ - "type": "string", - "description": "is this test only?", - }, - }, - Required: []string{"testonly"}, + Type: "object", }, }, func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
47-70: Return structured content alongside text for better interoperability.Include StructuredContent with the actual roots array; keep text for readability.
Apply:
- return &mcp.CallToolResult{ - Content: []mcp.Content{ - mcp.TextContent{ - Type: "text", - Text: fmt.Sprintf("Root list: %v", result.Roots), - }, - }, - }, nil + return &mcp.CallToolResult{ + Content: []mcp.Content{ + mcp.TextContent{ + Type: "text", + Text: fmt.Sprintf("Root list: %v", result.Roots), + }, + }, + StructuredContent: map[string]any{ + "roots": result.Roots, + }, + }, nilexamples/roots_http_client/main.go (4)
24-31: Avoid hardcoded user-specific file:// URIs.Build roots from the current user’s home directory for portability.
Apply:
-import ( +import ( "context" "fmt" "log" "os" "os/signal" "syscall" + "path/filepath" + "net/url" @@ func (h *MockRootsHandler) ListRoots(ctx context.Context, request mcp.ListRootsRequest) (*mcp.ListRootsResult, error) { - result := &mcp.ListRootsResult{ - Roots: []mcp.Root{ - { - Name: "app", - URI: "file:///User/haxxx/app", - }, - { - Name: "test-project", - URI: "file:///User/haxxx/projects/test-project", - }, - }, - } + home, _ := os.UserHomeDir() + app := filepath.ToSlash(filepath.Join(home, "app")) + proj := filepath.ToSlash(filepath.Join(home, "projects", "test-project")) + result := &mcp.ListRootsResult{ + Roots: []mcp.Root{ + { Name: "app", URI: (&url.URL{Scheme: "file", Path: "/" + app}).String() }, + { Name: "test-project", URI: (&url.URL{Scheme: "file", Path: "/" + proj}).String() }, + }, + } return result, nil }
59-65: Ensure client.Close is called to release resources.Close the client in addition to the transport.
Apply:
ctx := context.Background() err = mcpClient.Start(ctx) if err != nil { log.Fatalf("Failed to start client: %v", err) } + defer func() { + if cerr := mcpClient.Close(); cerr != nil { + log.Printf("Error closing client: %v", cerr) + } + }()
97-112: Check CallTool IsError to surface tool-level failures.The server may return IsError=true without an RPC error.
Apply:
result, err := mcpClient.CallTool(ctx, request) if err != nil { log.Fatalf("failed to call tool roots: %v", err) - } else if len(result.Content) > 0 { + } else if result.IsError { + log.Printf("tool reported error") + } else if len(result.Content) > 0 {
115-124: Optional: cancelable context on signal.Use a cancelable context so in-flight calls can observe shutdown.
Apply:
- sigChan := make(chan os.Signal, 1) - signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM) - select { - case <-ctx.Done(): - log.Println("Client context cancelled") - case <-sigChan: - log.Println("Received shutdown signal") - } + sigChan := make(chan os.Signal, 1) + signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM) + ctx, cancel := context.WithCancel(ctx) + defer cancel() + go func() { <-sigChan; log.Println("Received shutdown signal"); cancel() }() + <-ctx.Done()examples/roots_client/main.go (2)
24-31: Avoid hardcoded user-specific file:// URIs.Mirror the HTTP example: compute from os.UserHomeDir and construct file:// URLs.
I can provide a ready-to-apply diff similar to the HTTP client upon request. Based on learnings.
116-130: Check CallTool IsError to surface tool-level failures.Same rationale as the HTTP client.
Apply:
result, err := mcpClient.CallTool(ctx, request) if err != nil { log.Fatalf("failed to call tool roots: %v", err) - } else if len(result.Content) > 0 { + } else if result.IsError { + log.Printf("tool reported error") + } else if len(result.Content) > 0 {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
client/client.go(6 hunks)examples/roots_client/main.go(1 hunks)examples/roots_http_client/main.go(1 hunks)examples/roots_server/main.go(1 hunks)mcp/types.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- mcp/types.go
- client/client.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing
Files:
examples/roots_http_client/main.goexamples/roots_server/main.goexamples/roots_client/main.go
🧬 Code graph analysis (3)
examples/roots_http_client/main.go (4)
mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)ClientCapabilities(482-494)Implementation(531-534)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/streamable_http.go (2)
NewStreamableHTTP(131-161)WithContinuousListening(32-36)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (1)
CallToolRequest(54-58)
examples/roots_server/main.go (4)
mcp/types.go (7)
JSONRPCNotification(343-346)Notification(190-193)MethodNotificationRootsListChanged(82-82)ListRootsRequest(1154-1156)Content(964-966)TextContent(970-977)TextContent(979-979)server/server.go (4)
ServerOption(31-31)WithToolCapabilities(307-314)WithRoots(331-335)NewMCPServer(345-373)mcp/tools.go (5)
Tool(557-574)Description(875-879)ToolInputSchema(632-632)CallToolRequest(54-58)CallToolResult(40-51)server/stdio.go (1)
ServeStdio(857-877)
examples/roots_client/main.go (5)
examples/roots_http_client/main.go (1)
MockRootsHandler(18-18)mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)Implementation(531-534)ClientCapabilities(482-494)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/stdio.go (1)
NewStdio(91-97)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (2)
ListToolsRequest(18-21)CallToolRequest(54-58)
🔇 Additional comments (2)
examples/roots_server/main.go (1)
30-31: Correct notification constant for roots list changes.Registration uses mcp.MethodNotificationRootsListChanged. Looks good.
examples/roots_http_client/main.go (1)
16-19: GoDoc for exported type is present and clear.No action needed.
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/roots_client/main.go(1 hunks)examples/roots_http_client/main.go(1 hunks)examples/roots_server/main.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing
Files:
examples/roots_server/main.goexamples/roots_client/main.goexamples/roots_http_client/main.go
🧬 Code graph analysis (3)
examples/roots_server/main.go (4)
mcp/types.go (7)
JSONRPCNotification(343-346)Notification(190-193)MethodNotificationRootsListChanged(82-82)ListRootsRequest(1154-1156)Content(964-966)TextContent(970-977)TextContent(979-979)server/server.go (4)
ServerOption(31-31)WithToolCapabilities(307-314)WithRoots(331-335)NewMCPServer(345-373)mcp/tools.go (5)
Tool(557-574)Description(875-879)ToolInputSchema(632-632)CallToolRequest(54-58)CallToolResult(40-51)server/stdio.go (1)
ServeStdio(857-877)
examples/roots_client/main.go (5)
examples/roots_http_client/main.go (1)
MockRootsHandler(20-20)mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)Implementation(531-534)ClientCapabilities(482-494)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/stdio.go (1)
NewStdio(91-97)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (2)
ListToolsRequest(18-21)CallToolRequest(54-58)
examples/roots_http_client/main.go (4)
mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)ClientCapabilities(482-494)Implementation(531-534)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/streamable_http.go (2)
NewStreamableHTTP(131-161)WithContinuousListening(32-36)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (1)
CallToolRequest(54-58)
🔇 Additional comments (3)
examples/roots_server/main.go (1)
21-78: LGTM! All previous issues have been resolved.The server implementation correctly:
- Registers the appropriate roots notification handler
- Handles errors according to MCP specification (returning
IsError: truewith nil error)- Simplifies the
ListRootsRequestconstruction without redundant field assignmentsThe example demonstrates roots capability effectively.
examples/roots_http_client/main.go (1)
44-138: Well-structured HTTP client example.The main function demonstrates proper:
- Resource management with
deferfor cleanup- Error handling throughout the initialization and tool call flow
- Graceful shutdown with signal handling and context cancellation
- Blocking wait to keep the client running
examples/roots_client/main.go (1)
90-150: LGTM! Previous issues have been resolved.The code now correctly:
- Uses the client-appropriate name
"roots-stdio-client"(line 95)- Comments accurately reference the Roots capability (line 99)
- Blocks properly with a
selectstatement (lines 145-150) to keep the client running until shutdownThe stdio client example demonstrates the roots feature effectively with proper resource management and graceful shutdown.
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
examples/roots_client/main.go (1)
22-43: Fix URI construction for Unix paths.Identical issue as in
examples/roots_http_client/main.go: on Unix,filepath.Join(home, "app")returns an absolute path like/home/user/app, so prepending/creates//home/user/app, resulting in malformed file URIs with four slashes.Apply this diff:
func (h *MockRootsHandler) ListRoots(ctx context.Context, request mcp.ListRootsRequest) (*mcp.ListRootsResult, error) { home, err := os.UserHomeDir() if err != nil { log.Printf("Warning: failed to get home directory: %v", err) home = "/tmp" // fallback for demonstration } app := filepath.ToSlash(filepath.Join(home, "app")) proj := filepath.ToSlash(filepath.Join(home, "projects", "test-project")) result := &mcp.ListRootsResult{ Roots: []mcp.Root{ { Name: "app", - URI: (&url.URL{Scheme: "file", Path: "/" + app}).String(), + URI: (&url.URL{Scheme: "file", Path: app}).String(), }, { Name: "test-project", - URI: (&url.URL{Scheme: "file", Path: "/" + proj}).String(), + URI: (&url.URL{Scheme: "file", Path: proj}).String(), }, }, } return result, nil }
🧹 Nitpick comments (1)
examples/roots_client/main.go (1)
148-154: Simplify shutdown logic—second select case is unreachable.The goroutine at lines 81–85 consumes the signal from
sigChanand callscancel(), so the select at line 149 will always fire on<-ctx.Done(). The second case<-sigChanis unreachable because the signal has already been consumed by the goroutine.Simplify by removing the unreachable case:
// Keep running until cancelled by signal - select { - case <-ctx.Done(): - log.Println("Client context cancelled") - case <-sigChan: - log.Println("Received shutdown signal") - } + <-ctx.Done() + log.Println("Client context cancelled")Alternatively, if you want to handle the signal directly in the select, remove the goroutine and use:
- // Create a context that cancels on signal - ctx, cancel := context.WithCancel(ctx) - go func() { - <-sigChan - log.Println("Received shutdown signal, closing client...") - cancel() - }() - - // Move defer after error checking defer func() { if err := mcpClient.Close(); err != nil { log.Printf("Error closing client: %v", err) } }() @@ - // Keep running until cancelled by signal select { case <-ctx.Done(): log.Println("Client context cancelled") case <-sigChan: log.Println("Received shutdown signal") + cancel() // Optional: cancel context for cleanup }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/roots_client/main.go(1 hunks)examples/roots_http_client/main.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing
Files:
examples/roots_client/main.goexamples/roots_http_client/main.go
🧬 Code graph analysis (2)
examples/roots_client/main.go (5)
examples/roots_http_client/main.go (1)
MockRootsHandler(20-20)mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)Implementation(531-534)ClientCapabilities(482-494)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/stdio.go (1)
NewStdio(91-97)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (2)
ListToolsRequest(18-21)CallToolRequest(54-58)
examples/roots_http_client/main.go (5)
examples/roots_client/main.go (1)
MockRootsHandler(20-20)mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)ClientCapabilities(482-494)Implementation(531-534)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/streamable_http.go (2)
NewStreamableHTTP(131-161)WithContinuousListening(32-36)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (1)
CallToolRequest(54-58)
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
examples/roots_http_client/main.go (1)
22-43: Fix URI construction for Unix paths.The file URI construction still has the issue previously flagged: on Unix systems,
filepath.Join(home, "app")returns an absolute path like/home/user/app, and prepending another/results in a double leading slash (//home/user/app), producing a malformed file URI (file:////home/user/app) instead of the correctfile:///home/user/app.Remove the extra
/prefix from lines 34 and 38:{ Name: "app", - URI: (&url.URL{Scheme: "file", Path: "/" + app}).String(), + URI: (&url.URL{Scheme: "file", Path: app}).String(), }, { Name: "test-project", - URI: (&url.URL{Scheme: "file", Path: "/" + proj}).String(), + URI: (&url.URL{Scheme: "file", Path: proj}).String(), },
🧹 Nitpick comments (1)
examples/roots_http_client/main.go (1)
111-128: Consider using struct literal for request construction.The request is constructed by creating a zero value and then setting fields individually. For clarity and idiomatic Go, consider using a struct literal.
- request := mcp.CallToolRequest{} - request.Params.Name = "roots" - request.Params.Arguments = map[string]any{"testonly": "yes"} + request := mcp.CallToolRequest{ + Params: mcp.CallToolParams{ + Name: "roots", + Arguments: map[string]any{"testonly": "yes"}, + }, + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/roots_client/main.go(1 hunks)examples/roots_http_client/main.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/roots_client/main.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing
Files:
examples/roots_http_client/main.go
🧬 Code graph analysis (1)
examples/roots_http_client/main.go (4)
mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)ClientCapabilities(482-494)Implementation(531-534)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/streamable_http.go (2)
NewStreamableHTTP(131-161)WithContinuousListening(32-36)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (1)
CallToolRequest(54-58)
🔇 Additional comments (6)
examples/roots_http_client/main.go (6)
1-16: LGTM!Package declaration and imports follow Go conventions correctly, with standard library imports preceding third-party packages.
18-20: LGTM!Type definition follows Go naming conventions with proper GoDoc comment.
48-60: LGTM!Transport creation with proper error handling and cleanup via defer.
62-78: LGTM!Client creation and startup with proper context usage and cleanup handling.
99-109: LGTM!Appropriate logging and notification handling with non-fatal error logging for the root change notification.
80-97: Example is misleading about automatic Roots capability—show where rootsHandler is configured.The Roots capability is indeed automatically added by the client when
rootsHandleris set (verified inclient.go:190-195), but the example doesn't show how the rootsHandler was configured on the client. This creates confusion: readers may assume the Roots capability is automatically added unconditionally, when it only happens ifWithRootsHandler()was called during client creation. Either show the client initialization withWithRootsHandler()in the example, or make the comment more explicit: "Roots capability will be automatically added if a roots handler was configured on this client."
|
made below verification locally after fixing the test&lint errors: _ITCNSHG-XN0045:mcp-go ha***$ go test -coverprofile=coverage.txt -covermode=atomic $(go list ./... | grep -v '/examples/' | grep -v '/testdata' | grep -v '/mcptest' | grep -v '/server/internal/gen') ITCNSHG-XN0045:mcp-go ha***$ go test ./... -race ITCNSHG-XN0045:mcp-go ha***$ golangci-lint config path |
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.
Actionable comments posted: 3
🧹 Nitpick comments (4)
examples/roots_http_client/main.go (4)
3-16: Import strings for fileURI helper.If you adopt the helper above, add strings to the imports.
import ( "context" "fmt" "log" "net/url" "os" "os/signal" "path/filepath" + "strings" "syscall"
22-23: Add GoDoc for exported method.Exported method ListRoots should have a comment starting with its name.
- func (h *MockRootsHandler) ListRoots(ctx context.Context, request mcp.ListRootsRequest) (*mcp.ListRootsResult, error) { + // ListRoots implements client.RootsHandler by returning example workspace roots. + func (h *MockRootsHandler) ListRoots(ctx context.Context, request mcp.ListRootsRequest) (*mcp.ListRootsResult, error) {As per coding guidelines.
125-130: Be robust to pointer/value mcp.TextContent.Depending on decoding, content may be mcp.TextContent or *mcp.TextContent. Handle both.
- for _, content := range result.Content { - if textContent, ok := content.(mcp.TextContent); ok { - resultStr += fmt.Sprintf("%s\n", textContent.Text) - } - } + for _, content := range result.Content { + switch tc := content.(type) { + case mcp.TextContent: + resultStr += fmt.Sprintf("%s\n", tc.Text) + case *mcp.TextContent: + resultStr += fmt.Sprintf("%s\n", tc.Text) + } + }
134-146: Simplify shutdown with signal.NotifyContext.Cleaner, fewer goroutines, auto-signal stop.
- sigChan := make(chan os.Signal, 1) - signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM) - - shutdownCtx, cancel := context.WithCancel(ctx) - defer cancel() - - go func() { - <-sigChan - log.Println("Received shutdown signal") - cancel() - }() - <-shutdownCtx.Done() + waitCtx, stop := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) + defer stop() + <-waitCtx.Done() + log.Println("Received shutdown signal")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/roots_http_client/main.go(1 hunks)examples/roots_http_server/main.go(1 hunks)examples/roots_server/main.go(1 hunks)server/roots_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/roots_http_server/main.go
- server/roots_test.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing
Files:
examples/roots_http_client/main.goexamples/roots_server/main.go
🧬 Code graph analysis (2)
examples/roots_http_client/main.go (4)
mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)ClientCapabilities(482-494)Implementation(531-534)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/streamable_http.go (2)
NewStreamableHTTP(131-161)WithContinuousListening(32-36)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (2)
CallToolRequest(54-58)CallToolParams(60-64)
examples/roots_server/main.go (4)
mcp/types.go (6)
JSONRPCNotification(343-346)MethodNotificationRootsListChanged(82-82)ListRootsRequest(1154-1156)Content(964-966)TextContent(970-977)TextContent(979-979)server/server.go (4)
ServerOption(31-31)WithToolCapabilities(307-314)WithRoots(331-335)NewMCPServer(345-373)mcp/tools.go (4)
Tool(557-574)Properties(1141-1145)CallToolRequest(54-58)CallToolResult(40-51)server/stdio.go (1)
ServeStdio(857-877)
🔇 Additional comments (6)
examples/roots_server/main.go (4)
1-10: LGTM: Package and imports are well-organized.The import order follows Go conventions (standard library first, then third-party packages), and all necessary packages are included.
12-15: LGTM: Notification handler is correctly implemented.The function follows Go conventions with a descriptive GoDoc comment and proper signature with context as the first parameter.
17-31: LGTM: Server setup and notification handler registration are correct.The server is properly configured with roots capability, and the notification handler is registered for the correct roots list-changed event.
75-79: LGTM: Server startup and error handling are correct.The stdio server is properly started with appropriate fatal error logging if startup fails.
examples/roots_http_client/main.go (2)
106-110: Method name verified as correct.The client method is named
RootListChangesas called in the example code. Verification confirms this is the correct method exposed by the client.
80-92: Roots capability injection verified—no issues found.The comment at lines 80-92 in
examples/roots_http_client/main.gois accurate. WhenWithRootsHandleris called, the client automatically injects the roots capability duringInitialize(). The implementation inclient/client.golines 190–195 confirms that ifc.rootsHandler != nil, it addscapabilities.RootswithListChanged: true, preventing any protocol mismatch.
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
examples/roots_server/main.go (1)
34-68: Consider clarifying the tool description.The tool description states "Returns the current list of roots from the server," but the implementation calls
mcpServer.RequestRoots(), which requests roots from the connected client (per MCP spec, servers request roots from clients).Consider updating the description to: "Requests and returns the current list of roots from the connected client."
Apply this diff:
mcpServer.AddTool(mcp.Tool{ Name: "roots", - Description: "Returns the current list of roots from the server", + Description: "Requests and returns the current list of roots from the connected client", InputSchema: mcp.ToolInputSchema{Note: The tool will only work when a client with roots capability is connected. This is acceptable for an example demonstrating the roots request flow.
examples/roots_http_client/main.go (1)
122-127: Unused argument in tool call.The client sends
"testonly": "yes"as an argument (line 125), but the server example (examples/roots_server/main.go lines 37-40) defines an emptyInputSchema.Properties. This argument will be ignored by the server.While not incorrect (clients can send additional arguments), this mismatch between the two examples may confuse users trying to understand the roots feature.
Consider either:
- Removing the unused argument from the client example, or
- Adding a comment explaining that the argument is intentionally unused for demonstration purposes.
Apply this diff for option 1:
request := mcp.CallToolRequest{ Params: mcp.CallToolParams{ Name: "roots", - Arguments: map[string]any{"testonly": "yes"}, + Arguments: map[string]any{}, }, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/roots_client/main.go(1 hunks)examples/roots_http_client/main.go(1 hunks)examples/roots_server/main.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/roots_client/main.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing
Files:
examples/roots_http_client/main.goexamples/roots_server/main.go
🧬 Code graph analysis (2)
examples/roots_http_client/main.go (4)
mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)ClientCapabilities(482-494)Implementation(531-534)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/streamable_http.go (2)
NewStreamableHTTP(131-161)WithContinuousListening(32-36)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (2)
CallToolRequest(54-58)CallToolParams(60-64)
examples/roots_server/main.go (4)
mcp/types.go (6)
JSONRPCNotification(343-346)MethodNotificationRootsListChanged(82-82)ListRootsRequest(1154-1156)Content(964-966)TextContent(970-977)TextContent(979-979)server/server.go (4)
ServerOption(31-31)WithToolCapabilities(307-314)WithRoots(331-335)NewMCPServer(345-373)mcp/tools.go (6)
Tool(557-574)Description(875-879)ToolInputSchema(632-632)Properties(1141-1145)CallToolRequest(54-58)CallToolResult(40-51)server/stdio.go (1)
ServeStdio(857-877)
🔇 Additional comments (8)
examples/roots_server/main.go (3)
1-15: LGTM!The imports are properly ordered (standard library first, then third-party), and the
handleNotificationfunction is simple and well-documented.
17-31: LGTM!The server setup correctly enables roots capability and registers the appropriate notification handler for
MethodNotificationRootsListChanged. The past issue regarding comment/handler mismatch has been resolved.
70-74: LGTM!The server startup is straightforward with appropriate error handling using
log.Fatalf.examples/roots_http_client/main.go (5)
1-21: LGTM!The imports are properly ordered per Go conventions, and the
MockRootsHandlertype has an appropriate GoDoc comment.
23-45: LGTM!The
ListRootsimplementation properly handles home directory errors with a fallback and uses thefileURIhelper for correct URI construction. The past issues regarding Unix and Windows path handling have been resolved.
47-54: LGTM!The
fileURIhelper correctly handles both Unix and Windows absolute paths, ensuring proper file:// URI formatting. This addresses the past issue regarding Windows URI construction.
56-88: LGTM!The transport and client setup is correct. The past issue regarding potential double-close of the transport has been resolved—only
mcpClient.Close()is deferred, which properly manages the transport lifecycle.
90-151: LGTM on session initialization and shutdown handling!The MCP session initialization is properly structured with correct protocol version and client info. The shutdown handling using
signal.NotifyContextis clean and avoids the context shadowing issue from earlier reviews.
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
examples/roots_http_client/main.go (2)
47-54: Consider making fileURI a standalone function.The
fileURIhelper doesn't use any receiver state, so it would be more idiomatic to define it as a standalone function rather than a method.-// fileURI returns a file:// URI for both Unix and Windows absolute paths. -func (h *MockRootsHandler) fileURI(p string) string { +// fileURI returns a file:// URI for both Unix and Windows absolute paths. +func fileURI(p string) string { p = filepath.ToSlash(p) if !strings.HasPrefix(p, "/") { // e.g., "C:/Users/..." on Windows p = "/" + p } return (&url.URL{Scheme: "file", Path: p}).String() }And update the calls in
ListRoots:{ Name: "app", - URI: h.fileURI(app), + URI: fileURI(app), }, { Name: "test-project", - URI: h.fileURI(proj), + URI: fileURI(proj), },
134-143: Type switch is defensively handling both value and pointer types.The type switch handles both
mcp.TextContentand*mcp.TextContent. Based on the protocol definition,TextContenthas a value receiver forisContent(), so theContentslice should containmcp.TextContentvalues, not pointers. The pointer case is defensive but likely redundant.For production code, you could simplify to just handle the value type:
for _, content := range result.Content { switch tc := content.(type) { case mcp.TextContent: resultStr += fmt.Sprintf("%s\n", tc.Text) - case *mcp.TextContent: - resultStr += fmt.Sprintf("%s\n", tc.Text) } }However, for example/demonstration code, the defensive approach is reasonable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/roots_client/main.go(1 hunks)examples/roots_http_client/main.go(1 hunks)examples/roots_server/main.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/roots_client/main.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing
Files:
examples/roots_http_client/main.goexamples/roots_server/main.go
🧬 Code graph analysis (2)
examples/roots_http_client/main.go (4)
mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)ClientCapabilities(482-494)Implementation(531-534)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/streamable_http.go (2)
NewStreamableHTTP(131-161)WithContinuousListening(32-36)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (2)
CallToolRequest(54-58)CallToolParams(60-64)
examples/roots_server/main.go (4)
mcp/types.go (6)
JSONRPCNotification(343-346)MethodNotificationRootsListChanged(82-82)ListRootsRequest(1154-1156)Content(964-966)TextContent(970-977)TextContent(979-979)server/server.go (4)
ServerOption(31-31)WithToolCapabilities(307-314)WithRoots(331-335)NewMCPServer(345-373)mcp/tools.go (6)
Tool(557-574)Description(875-879)ToolInputSchema(632-632)Properties(1141-1145)CallToolRequest(54-58)CallToolResult(40-51)server/stdio.go (1)
ServeStdio(857-877)
🔇 Additional comments (11)
examples/roots_server/main.go (6)
1-10: LGTM: Package declaration and imports are correct.Import ordering follows Go conventions with standard library packages first, then local packages.
12-15: LGTM: Notification handler follows conventions.The function correctly accepts context as the first parameter and includes a clear GoDoc comment.
17-28: LGTM: Server initialization is correct.The server is properly configured with tool and roots capabilities, and the main function has a clear descriptive comment.
30-31: LGTM: Notification handler registration is correct.The handler is properly registered for
RootsListChangednotifications with an accurate comment.
33-68: LGTM: Tool registration demonstrates the roots feature correctly.The tool implementation properly:
- Constructs an empty
ListRootsRequestwithout manual field manipulation- Handles errors using the MCP-compliant pattern (returning
IsError: truewithnilerror)- Provides both text content and structured content for backwards compatibility
- Uses correct grammar in error messages
All issues from previous reviews have been successfully resolved.
70-73: LGTM: Server startup follows the standard pattern.The stdio server is correctly initialized and fatal errors are appropriately logged.
examples/roots_http_client/main.go (5)
1-17: LGTM: Imports properly ordered.The imports follow the project's guidelines with standard library packages first, followed by local packages.
19-45: LGTM: MockRootsHandler implementation is correct.The implementation properly:
- Documents the type and method with GoDoc comments starting with the identifier name
- Accepts
context.Contextas the first parameter per guidelines- Handles errors gracefully with logging and fallback
- Uses the helper method to construct portable file:// URIs
60-88: LGTM: Client initialization and resource management are correct.The client setup properly:
- Creates the transport with appropriate options
- Configures the client with the roots handler using
WithRootsHandler- Defers cleanup with error logging
- Uses
log.Fatalffor fatal errors, which is appropriate for a main function
146-151: LGTM: Graceful shutdown pattern is clean and idiomatic.The use of
signal.NotifyContextis the recommended approach for graceful shutdown in modern Go. The deferredstop()call ensures proper cleanup of signal notification.
90-111: No issues found—the comment is accurate.Verification confirms that the
MockRootsHandleris properly configured viaWithRootsHandlerat line 75 beforeInitializeis called. The client'sInitializemethod automatically adds the roots capability when aRootsHandleris configured, making the comment on line 95 correct.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/roots_client/main.go (1)
19-26: Consider adding a brief comment to the fileURI helper.While not required for unexported functions, a one-line comment explaining that this helper creates cross-platform file:// URIs would improve readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/roots_client/main.go(1 hunks)examples/roots_http_client/main.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/roots_http_client/main.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing
Files:
examples/roots_client/main.go
🧬 Code graph analysis (1)
examples/roots_client/main.go (4)
mcp/types.go (12)
ListRootsRequest(1154-1156)ListRootsResult(1161-1164)Root(1167-1178)InitializeRequest(441-445)Params(188-188)InitializeParams(447-453)LATEST_PROTOCOL_VERSION(112-112)Implementation(531-534)ClientCapabilities(482-494)Content(964-966)TextContent(970-977)TextContent(979-979)client/transport/stdio.go (1)
NewStdio(91-97)client/client.go (2)
NewClient(80-90)WithRootsHandler(51-55)mcp/tools.go (2)
ListToolsRequest(18-21)CallToolRequest(54-58)
🔇 Additional comments (5)
examples/roots_client/main.go (5)
1-17: Imports are well-organized and follow conventions.The import order correctly groups standard library packages first, followed by local project packages, consistent with goimports formatting.
28-54: MockRootsHandler implementation looks solid.The type and method follow Go naming conventions with proper GoDoc comments. Error handling for
os.UserHomeDir()has been correctly addressed with a warning log and reasonable fallback, resolving the previous review concern.
62-103: Client setup and lifecycle management are well-implemented.The main function correctly:
- Handles CLI arguments with clear usage message
- Creates stdio transport and wires up the MockRootsHandler via
WithRootsHandler- Starts the client with proper error handling
- Sets up graceful shutdown with signal handling and context cancellation
- Defers cleanup after confirming successful startup
The context flow is appropriate:
Start()uses the base context, while subsequent operations use the cancellable context created at line 91.
105-133: Client initialization and server interaction look correct.The code properly:
- Initializes with the latest protocol version
- Uses the corrected client name "roots-stdio-client" (previously addressed)
- Includes an accurate comment about Roots capability auto-registration (previously addressed)
- Lists and logs available tools with appropriate error handling
135-162: Tool invocation and notification handling are appropriate.The example demonstrates:
- Calling the "roots" tool with test arguments
- Extracting and displaying text content from results
- Sending a
RootListChangesnotification (with non-fatal error logging)- Blocking on
ctx.Done()to keep the client running until signaled (previously addressed)All operations use proper error handling suitable for demonstration code.
|
Hello @pottekkat @rwjblue-glean @dugenkui03 @ezynda3 , hope you are doing good. I am checking the "Generate coverage report" issue. And I don't know who maintain this action, hope you can help me on this. This issue caused by "gh pr comment 620 --body-file=.github/outputs/coverage-comment.md" command and raised "GraphQL: Resource not accessible by integration (addComment)" error. It indicates the action did not has write permission to update comment into pull request. I forked the mcp-go repository. And in the "fgrosse/go-coverage-report" action, it use 'pull_request' to trigger the action. According to official document: "for forked repositories, but typically you can't grant write access", GITHUB_TOKEN only has read permission when trigged by 'pull_request' event for forked repositories. And if the action been trigged by 'pull_request_target' event, the GITHUB_TOKEN has write permission for forked repositories. There is a document for reference: "When a workflow is triggered by the pull_request_target event, the GITHUB_TOKEN is granted read/write repository permission, even when it is triggered from a public fork.". From "fgrosse/go-coverage-report" repository README, this action will only update comment to pull request, not checkout code for execution. It should be safe to use 'pull_request_target' event. You may consider replacing 'pull_request' with 'pull_request_target' in following code. So that the action will have write permission to update PR comment from pubic forked repositories. Please let me know if you have other suggestion to solve this issue. Or maybe we can bypass this action checking. Thanks. |
Description
Implemented the client
Rootsfeature for both HTTP and Sdio transport.https://modelcontextprotocol.io/specification/2025-06-18/client/roots
roots_http_server,roots_http_client,roots_serverandroots_clientexamples have been added to demonstrate how to use clientRootsfeature. Below are steps to use the examples:cd examples/roots_http_server; go build -o server; ./server; \
cd examples/roots_http_client; go build -o client; ./client
cd examples/roots_server; go build -o server; \
cd examples/roots_client; go build -o client; ./client ../roots_server/server
Type of Change
Checklist
MCP Spec Compliance
Additional Information
Summary by CodeRabbit
New Features
Client
Transport & Server
Examples
Tests
Chores