Add tool capabilities to serverCapabilities

[EdmondFrank]

The addition of tool capabilities in a structured manner enhances the modularity and flexibility of the server's capabilities management.

Summary by CodeRabbit

• New Features

  • Enhanced tool management capabilities now enable dynamic configuration of tool operations.
  • Users benefit from improved adaptability and streamlined handling of tool-related changes, ensuring a more responsive and customizable system experience.
  • New capability options allow for more granular control over server functionalities.

• Bug Fixes

  • Adjusted behavior in resource subscription capabilities to reflect the latest changes in tool management.

• Tests

  • Added new test cases to validate the behavior of tool capabilities and their impact on server operations.
  • Introduced a test case for specific capabilities, enhancing the testing framework for various configurations.

[coderabbitai]

Walkthrough

The changes introduce a new structure to manage tool-related capabilities. A toolCapabilities structure with a boolean field listChanged is added and integrated into the existing server capabilities via a new field. Additionally, a configuration function is provided to initialize this new capability within the server's setup. Modifications are also made to the server's message handling and the associated tests to accommodate these new capabilities.

Changes

File	Change Summary
server/server.go	- Added toolCapabilities structure with a listChanged boolean field. - Integrated this structure into serverCapabilities as a new pointer field tools. - Introduced WithToolCapabilities function to configure the new tool capabilities. - Updated NewMCPServer to initialize tools with listChanged set to false. - Modified HandleMessage logic to check listChanged before processing requests. - Updated error handling to use listChanged instead of nil checks.
server/server_test.go	- Added new test case "Specific capabilities" in TestMCPServer_Capabilities to test various combinations of capabilities. - Updated TestMCPServer_Capabilities to include assertions for listChanged properties. - Modified TestMCPServer_HandleMethodsWithoutCapabilities to include options []ServerOption for capability configuration in tests.
client/sse_test.go	- Added server.WithToolCapabilities(true) in TestSSEMCPClient to enable tool capabilities during server initialization.

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 220066c and 7a43195.

📒 Files selected for processing (3)

• client/sse_test.go (1 hunks)
• server/server.go (8 hunks)
• server/server_test.go (9 hunks)

🔇 Additional comments (14)

server/server.go (7)

141-161: Well-structured approach for capabilities management.

The addition of toolCapabilities follows the existing pattern for resource and prompt capabilities, maintaining consistency in the codebase architecture.

---

182-189: Consistent capabilities configuration function.

The WithToolCapabilities function properly follows the established pattern of other capability configuration functions.

---

213-218: Default initialization follows requested pattern.

This implementation correctly follows the previous feedback from ezynda3 who requested that capabilities should be empty by default rather than forcing implementation.

---

545-545: Consistent integration with initialize response.

The tools capability is correctly included in the initialize response, following the same pattern as other capabilities.

---

328-328:

✅ Verification successful

Verify the conditional check is working correctly.

The implementation changed from checking if s.capabilities.resources is nil to checking the listChanged boolean. Ensure this doesn't change the existing behavior, as it's now checking a specific field rather than the existence of the capability.

Let's verify this change maintains the expected behavior:

---

🏁 Script executed:

#!/bin/bash
# Description: Checking resource capability handling before and after the change

# Find the original condition pattern for resources
echo "Previous capability check pattern for resources:"
rg "if s.capabilities.resources == nil" server/server.go

# Find the new condition pattern for resources
echo "New capability check pattern for resources:"
rg "if !?s.capabilities.resources.listChanged" server/server.go

# Check the initialization of capabilities to understand the default values
echo "Default capabilities initialization:"
rg "capabilities: serverCapabilities" server/server.go --context 5

# Check the resource capability test cases
echo "Resource capability test assertions:"
rg "assert.*Resources.ListChanged" server/server_test.go

Length of output: 1436

---

Conditional Check Verification on s.capabilities.resources is Consistent

• The default initialization in server/server.go guarantees that s.capabilities.resources is always non-nil, so checking listChanged is valid.
• The new condition (if !s.capabilities.resources.listChanged {) correctly differentiates the intended states, with test cases in server/server_test.go confirming both true and false scenarios.
• No unintended behavior differences were observed with this change.

---

362-362:

✅ Verification successful

Verify the conditional check is working correctly.

Similar to the resources check, this implementation changed from checking if s.capabilities.prompts is nil to checking the listChanged boolean. Ensure this doesn't change the existing behavior.

---

🏁 Script executed:

#!/bin/bash
# Description: Checking prompt capability handling before and after the change

# Find the original condition pattern for prompts
echo "Previous capability check pattern for prompts:"
rg "if s.capabilities.prompts == nil" server/server.go

# Find the new condition pattern for prompts
echo "New capability check pattern for prompts:"
rg "if !?s.capabilities.prompts.listChanged" server/server.go

Length of output: 432

---

Conditional Check for List Change Verified

The new condition (if !s.capabilities.prompts.listChanged {) is present as expected, and the earlier nil-check for s.capabilities.prompts remains in the file. This indicates that the implementation assumes a non-nil prompts object before evaluating the listChanged flag, preserving the original behavior.

---

396-396:

✅ Verification successful

Check the condition logic for tool capabilities.

The condition !s.capabilities.tools.listChanged || len(s.tools) == 0 will reject tool calls if either listChanged is false OR there are no tools. Make sure this is the intended behavior and not an AND condition instead.

---

🏁 Script executed:

#!/bin/bash
# Description: Check for consistency in the error condition logic across capability checks

# Find all capability check conditions for method handlers
echo "All capability check conditions for method handlers:"
rg "if.*capabilities.*listChanged|if len\(s\.tools\)" server/server.go

# Find where tools are checked in the code
echo "Tool-specific capability checks:"
rg "if.*tools.*listChanged" server/server.go

Length of output: 546

---

Tool Capability Condition Check: Condition Is Correct If Both Requirements Must Be Met

After reviewing the capability checks across the code, the logic in:

if !s.capabilities.tools.listChanged || len(s.tools) == 0 {

appears intentional. It rejects tool calls when either the tool list has not been updated (i.e. s.capabilities.tools.listChanged is false) or there are no tools available. This effectively ensures that a tool call is allowed only when both conditions are met—which is consistent with the requirement that the tool list should be up-to-date and non-empty for processing.

• Consistency Check: Other capability checks (e.g., for resources and prompts) similarly verify the update state via listChanged, but tools include an extra validation to ensure availability.
• Logical Equivalence: The rejection condition as written (using OR) corresponds to allowing tool calls only if s.capabilities.tools.listChanged is true and len(s.tools) > 0.

If the design intent is indeed to enforce that both the list is updated and that tools exist before processing calls, then the current OR logic is appropriate.

client/sse_test.go (1)

19-19: Correctly enables tool capabilities for testing.

The addition of server.WithToolCapabilities(true) properly enables tool capabilities for the test, which is necessary since the test includes tool functionalities.

server/server_test.go (6)

45-49: Corrected test expectations for default capabilities.

The assertions have been updated to correctly expect false for the default capability values, which aligns with the implementation in server/server.go.

---

58-58: Tool capabilities now included in tests.

The "All capabilities" test now properly includes tool capabilities, maintaining consistency with the implementation.

---

91-124: Comprehensive test case for specific capability combinations.

This new test case validates various combinations of capabilities, which is important for ensuring the server correctly handles different capability configurations.

---

659-660: Improved test structure with explicit capability configuration.

The test now explicitly configures capabilities for each test case, which makes tests more precise and less dependent on default values. This is a good practice for test clarity.

---

697-697: Enhanced test flexibility with option parameters.

The test now allows passing server options for each test case, making it more flexible and allowing for testing of different capability configurations independently.

---

115-116:

❓ Verification inconclusive

Ensure test matches implementation intention.

The test expects Resources.Subscribe to be true and Resources.ListChanged to be false, which matches the server configuration set at line 93. Confirm this is the intended behavior.

---

🏁 Script executed:

#!/bin/bash
# Description: Verify consistency between test assertions and implementation

# Find how WithResourceCapabilities is configured with true/false options
echo "WithResourceCapabilities() usage in tests:"
rg "WithResourceCapabilities\((true|false), (true|false)\)" server/server_test.go

# Find where the resource capabilities are tested
echo "Resource capability assertions:"
rg "assert\..*Resources\.Subscribe.*\n.*assert\..*Resources\.ListChanged" server/server_test.go

Length of output: 790

---

Test Assertion Configuration Review:
The assertions at lines 115–116 in server/server_test.go verify that initResult.Capabilities.Resources.Subscribe is true and initResult.Capabilities.Resources.ListChanged is false. Our scan of the file shows that there are several invocations of WithResourceCapabilities, including one with the parameters (true, false), which aligns with the expected configuration set at line 93.

• Action Required: Please confirm that the specific test case using these assertions is indeed initialized via the correct call to WithResourceCapabilities(true, false), as other invocations (e.g., (true, true) or (false, false)) are also present in the file.

✨ Finishing Touches

• 📝 Generate Docstrings

---

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[ezynda3]

Thanks for this! Can you please add a simple test for this?

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

server/server_test.go (1)

57-58: Add a test to verify integrated tool capabilities behavior.

Consider enhancing the tests to verify that the WithToolCapabilities(false) option correctly disables tool capabilities in the server behavior, not just in the initialization response. This would provide more complete coverage of the new feature.

 {
     name: "Specific capabilities",
     options: []ServerOption{
         WithResourceCapabilities(true, false),
         WithPromptCapabilities(true),
         WithToolCapabilities(false),
         WithLogging(),
     },
     validate: func(t *testing.T, response mcp.JSONRPCMessage) {
+        // Verify initialization response
         resp, ok := response.(mcp.JSONRPCResponse)
         assert.True(t, ok)
 
         initResult, ok := resp.Result.(mcp.InitializeResult)
         assert.True(t, ok)
 
         // ... existing assertions ...
 
         assert.NotNil(t, initResult.Capabilities.Tools)
         assert.False(t, initResult.Capabilities.Tools.ListChanged)
         
         assert.NotNil(t, initResult.Capabilities.Logging)
+        
+        // Also verify that the server will reject tool calls
+        toolCallMessage := `{
+            "jsonrpc": "2.0",
+            "id": 2,
+            "method": "tools/call",
+            "params": {
+                "name": "test-tool"
+            }
+        }`
+        
+        server := response.(*mcp.JSONRPCResponse).Result.(*mcp.InitializeResult).server
+        toolResponse := server.HandleMessage(context.Background(), []byte(toolCallMessage))
+        errorResp, ok := toolResponse.(mcp.JSONRPCError)
+        assert.True(t, ok)
+        assert.Equal(t, mcp.METHOD_NOT_FOUND, errorResp.Error.Code)
     },
 },

Also applies to: 94-96

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9595774 and 220066c.

📒 Files selected for processing (2)

• server/server.go (8 hunks)
• server/server_test.go (7 hunks)

🔇 Additional comments (13)

server/server.go (7)

141-141: Implementation of tool capabilities in serverCapabilities structure.

The addition of the tools field to the serverCapabilities structure properly integrates tool capabilities into the server's capability management system.

---

158-161: Well-designed capability structure for tools.

The toolCapabilities structure follows the established pattern used for other capabilities (resourceCapabilities, promptCapabilities) with the listChanged boolean field that indicates whether the list of tools can change during the server's lifetime.

---

182-189: Consistent implementation of tool capabilities configuration function.

The WithToolCapabilities function follows the same pattern as other capability configuration functions, making the API consistent and intuitive.

---

214-217: Proper default initialization of capabilities.

The default initialization of capabilities now includes tools with listChanged set to true by default, which maintains backward compatibility while enabling the new feature.

---

328-329: Consistent capability checking for resources and prompts.

The code now checks the listChanged boolean field instead of checking for nil pointers, which is more consistent with how capabilities are structured.

Also applies to: 362-363

---

396-397: Enhanced conditional check for tool capability.

The condition now checks both !s.capabilities.tools.listChanged and len(s.tools) == 0 before rejecting tool call requests, ensuring that tools are only available when both conditions are satisfied. This is more robust than the previous implementation.

---

545-546: Proper integration with server capability reporting.

The tool capabilities are correctly exposed in the initialize response, allowing clients to discover what tool capabilities are supported by the server.

server/server_test.go (6)

58-59: Added test coverage for tool capabilities.

The test now includes validation for the tool capabilities, ensuring that the new feature is properly tested.

---

91-124: Comprehensive test case for specific capability combinations.

This new test case ensures that various combinations of capabilities are correctly initialized and reported, which improves test coverage significantly.

---

644-645: Enhanced test structure for capability configuration.

The test now accepts capability configuration options, allowing for more targeted testing of how the server handles different capability configurations.

Also applies to: 657-659

---

672-674: Consistent capability configuration across test cases.

Each test case now specifies its required capabilities explicitly, which makes the tests more self-contained and clearer about their prerequisites.

Also applies to: 687-689

---

696-697: Updated server initialization with capability options.

The server initialization now uses the capability options specified in each test case, ensuring that the server is properly configured for the test scenario.

---

77-78:

❓ Verification inconclusive

Changed resource subscription behavior assertion.

This change from assert.False to assert.True represents a change in expected behavior for resource subscription capabilities. Make sure this change is intentional and consistent with the actual implementation.

---

🏁 Script executed:

#!/bin/bash
# Verify that the resource subscription behavior in the code matches the updated test expectation

# Check the default initialization in server.go
echo "Checking default initialization in server.go:"
rg -A 5 -n "capabilities: serverCapabilities" server/server.go

# Check the WithResourceCapabilities function
echo "Checking WithResourceCapabilities function:"
rg -A 10 -n "WithResourceCapabilities" server/server.go

Length of output: 1062

---

Action Required: Verify Resource Subscription Configuration Override

The test now asserts that the resource subscription capability is enabled (true), which deviates from the default initialization in server/server.go where subscribe is set to false. We verified that the WithResourceCapabilities function correctly accepts a parameter to override this default. Please confirm that the server instance in the test is intentionally constructed with this override (e.g., by using WithResourceCapabilities(true, ...)) and that no unintentional behavior occurs in pathways that rely on the default initialization.

[EdmondFrank]

@ezynda3 Hi, I have added the tests according to your suggestions and optimized and adjusted the relevant code. Here are the go test results from my local machine. Looking forward to your feedback.

[ezynda3]

@EdmondFrank added a comment