Skip to content

Conversation

yuehaii
Copy link

@yuehaii yuehaii commented Oct 21, 2025

Description

Implemented the client Roots feature for both HTTP and Sdio transport.

  • Mcp client Roots feature protocol definition:
    https://modelcontextprotocol.io/specification/2025-06-18/client/roots
  • Example Usage: The roots_http_server, roots_http_client, roots_server and roots_client examples have been added to demonstrate how to use client Roots feature. Below are steps to use the examples:
    1. for Http client roots verification:
      cd examples/roots_http_server; go build -o server; ./server; \
      cd examples/roots_http_client; go build -o client; ./client
    2. for Stdio client roots verification:
      cd examples/roots_server; go build -o server; \
      cd examples/roots_client; go build -o client; ./client ../roots_server/server

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • MCP spec compatibility implementation
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring (no functional changes)
  • Performance improvement
  • Tests only (no functional changes)
  • Other (please describe):

Checklist

  • My code follows the code style of this project
  • I have performed a self-review of my own code
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the documentation accordingly

MCP Spec Compliance

  • This PR implements a feature defined in the MCP specification
  • Link to relevant spec section: roots
  • Implementation follows the specification exactly

Additional Information

Summary by CodeRabbit

  • New Features

    • Added a Roots capability: clients may advertise roots, servers can request a client's root list, and RootsListChanged notifications can be emitted.
  • Client

    • Client API to provide a Roots handler and emit root-list change notifications.
  • Transport & Server

    • Roots request/response supported across stdio, HTTP streaming, and in-process transports; server option to enable Roots capability.
  • Examples

    • New example programs demonstrating roots usage with mock handlers.
  • Tests

    • Unit tests covering success, empty, unsupported, and no-session cases.
  • Chores

    • Updated .gitignore to exclude local editor launch configs.

Copy link
Contributor

coderabbitai bot commented Oct 21, 2025

Warning

Rate limit exceeded

@yuehaii has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 40 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 77875f8 and 2a1fa2e.

📒 Files selected for processing (1)
  • examples/roots_http_client/main.go (1 hunks)

Walkthrough

Adds end-to-end "roots" support: new MCP method and notification, client and server interfaces/handlers, transport integrations (stdio, HTTP, in-process), examples, tests, and a .gitignore update. (≤50 words)

Changes

Cohort / File(s) Summary
Protocol types
mcp/types.go
Adds MethodListRoots and MethodNotificationRootsListChanged; adds Roots to ServerCapabilities; removes Header from ListRootsRequest.
Server API & options
server/server.go, server/session.go
Adds WithRoots() server option and new SessionWithRoots interface exposing ListRoots.
Server request surface
server/roots.go
Adds RequestRoots implementation and sentinel errors ErrNoClientSession, ErrRootsNotSupported.
Server in-process session
server/inprocess_session.go
Adds server RootsHandler interface, rootsHandler field on InProcessSession, ListRoots method, and extends NewInProcessSessionWithHandlers.
Server stdio transport
server/stdio.go
Adds pending roots tracking, response routing, stdioSession.ListRoots, and implements SessionWithRoots for stdio sessions.
Server streamable HTTP transport
server/streamable_http.go
Adds SSE/streaming support for ListRoots with rootsRequestItem, per-request response channels, rootsRequestChan, and streamableHttpSession.ListRoots.
Server tests
server/roots_test.go
Adds unit tests for no-session, unsupported-session, success (populated/empty), and error cases for RequestRoots.
Client core
client/client.go, client/roots.go
Adds client RootsHandler interface, RootsHandler field on Client, WithRootsHandler option, transport-level handler for MethodListRoots, and Client.RootListChanges notification method.
Client in-process transport
client/transport/inprocess.go
Adds rootsHandler field, WithRootsHandler option, and updates session creation to pass the roots handler.
Client examples
examples/roots_client/main.go, examples/roots_http_client/main.go
Adds example clients with MockRootsHandler, startup/init flow, RootListChanges usage, and tool call demos.
Server examples
examples/roots_server/main.go, examples/roots_http_server/main.go
Adds example MCP servers enabling roots capability, tools invoking RequestRoots, and notification handlers.
Misc
.gitignore
Adds .vscode/launch.json to ignore rules.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

type: enhancement, area: mcp spec

Suggested reviewers

  • pottekkat
  • rwjblue-glean
  • dugenkui03
  • cryo-zd

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "feat(mcp): add HTTP and Stdio client Roots feature" directly corresponds to the main changes in the changeset. The summary confirms that this PR implements client-side Roots feature support across multiple transport types (HTTP and Stdio), adding interfaces, handlers, and example implementations. The title uses conventional commit format, is concise at 52 characters, and clearly identifies the primary change as the addition of Roots feature support for specific transports. A reviewer scanning history would understand this PR introduces client Roots capability across HTTP and Stdio transport implementations.
Description Check ✅ Passed The PR description addresses all required template sections. The Description section explains what was implemented (client Roots feature for HTTP and Stdio) and provides a link to the relevant MCP specification along with clear usage examples for both HTTP and Stdio transports. The Type of Change section correctly selects "New feature" and "MCP spec compatibility implementation." The Checklist confirms code style compliance, self-review, tests added, and documentation updated. The MCP Spec Compliance section is fully completed with the specification link and a claim that the implementation follows the specification exactly. While the description contains a minor typo ("Sdio" instead of "Stdio"), the content is substantive and covers the essential information needed for reviewers.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 Arguments as a JSON string, but the field type is any which 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 map

pendingMu 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 naming

Use “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 param

Implementation 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 well

Mocks 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 option

Exported 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 strings

Pass 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

📥 Commits

Reviewing files that changed from the base of the PR and between f60537b and b300907.

📒 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.go
  • server/session.go
  • server/server.go
  • server/roots.go
  • server/roots_test.go
  • examples/roots_server/main.go
  • examples/roots_client/main.go
  • server/stdio.go
  • client/client.go
  • server/streamable_http.go
  • examples/roots_http_client/main.go
  • client/roots.go
  • server/inprocess_session.go
  • examples/roots_http_server/main.go
  • client/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.json to 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 *bool to distinguish between unset and false values.


330-335: LGTM!

The WithRoots() option follows the established pattern for enabling server capabilities and uses the utility function mcp.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 RootsHandler interface is well-defined with clear documentation. It follows Go conventions by accepting context.Context as the first parameter and provides helpful implementation guidance.

server/session.go (1)

74-79: LGTM!

The SessionWithRoots interface follows the established pattern for session extensions and properly embeds ClientSession while adding the ListRoots method.

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 RequestRoots method 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‑circuiting

Early return on handled roots responses mirrors existing flows and avoids double processing.


770-830: Robust response routing; aligns with JSON‑RPC parsing

Parsing by id and non‑blocking send prevents handler stalls; error wrapping is clear.

client/transport/inprocess.go (1)

75-78: Session wiring LGTM

Creating/registering a session when rootsHandler is present mirrors other handlers and is correct.

client/client.go (3)

189-195: Correct capability exposure

Declaring roots capability with listChanged is per spec.


514-516: ListRoots request handler wired correctly

Routes roots/list to handler; clean, matches other handlers.


578-607: Transport handler solid; no params required

Validates handler presence, wraps result in JSON‑RPC response.

examples/roots_client/main.go (1)

109-113: Ensure RootListChanges uses the roots notification

This relies on client.RootListChanges sending notifications/roots/list_changed (see fix in client.go).

server/roots_test.go (1)

210-219: ****

The claim that fakeSession is "not declared" is false. fakeSession is properly defined in server/server_test.go (line 1802) and implements the ClientSession interface. Both fakeSession and mockBasicRootsSession are valid, visible types within the package; the code compiles and runs without undefined-type errors.

While mockBasicRootsSession (defined in roots_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 rootsRequestItem type 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 streamableHttpSession implements SessionWithRoots.

@yuehaii
Copy link
Author

yuehaii commented Oct 22, 2025

@coderabbitai generate docstrings

Copy link
Contributor

coderabbitai bot commented Oct 22, 2025

✅ Actions performed

Initiated docstring generation; will generate only if new commits exist.

Copy link
Contributor

coderabbitai bot commented Oct 22, 2025

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #621

coderabbitai bot added a commit that referenced this pull request Oct 22, 2025
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`
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between b300907 and 896a4c2.

📒 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.go
  • client/client.go
  • server/streamable_http.go
  • examples/roots_client/main.go
  • server/inprocess_session.go
  • mcp/types.go
  • examples/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 rootsRequestItem type correctly follows the established pattern for bidirectional request/response flows. Reusing samplingResponseItem for 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 ListRoots method 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 samplingRequests map 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 RootsHandler interface is properly defined with clear GoDoc documentation (addresses past review feedback). The design is consistent with SamplingHandler and ElicitationHandler, 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 NewInProcessSessionWithHandlers function correctly initializes the new rootsHandler field, maintaining consistency with the existing handler initialization pattern.


138-150: Excellent implementation with proper documentation and thread safety.

The ListRoots method 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 RequestSampling and RequestElicitation
  • 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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,
+        },
+      }, nil
examples/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

📥 Commits

Reviewing files that changed from the base of the PR and between 896a4c2 and fadfa70.

📒 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.go
  • examples/roots_server/main.go
  • examples/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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between fadfa70 and 96b2183.

📒 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.go
  • examples/roots_client/main.go
  • examples/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: true with nil error)
  • Simplifies the ListRootsRequest construction without redundant field assignments

The 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 defer for 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 select statement (lines 145-150) to keep the client running until shutdown

The stdio client example demonstrates the roots feature effectively with proper resource management and graceful shutdown.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 sigChan and calls cancel(), so the select at line 149 will always fire on <-ctx.Done(). The second case <-sigChan is 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

📥 Commits

Reviewing files that changed from the base of the PR and between 96b2183 and 7d2188b.

📒 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.go
  • examples/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)

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 correct file:///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

📥 Commits

Reviewing files that changed from the base of the PR and between 7d2188b and 77875f8.

📒 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 rootsHandler is set (verified in client.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 if WithRootsHandler() was called during client creation. Either show the client initialization with WithRootsHandler() in the example, or make the comment more explicit: "Roots capability will be automatically added if a roots handler was configured on this client."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant