Skip to content

Conversation

@4t145
Copy link
Collaborator

@4t145 4t145 commented May 28, 2025

  1. provide context for notification
  2. allow extract more info from tool call
  3. inject http request part for streamable http server

Motivation and Context

#235#195 (comment)

How Has This Been Tested?

Breaking Changes

Add a notification context parameter for notification handlers.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

1. provide context for notification
2. allow extract more info from tool call
3. inject http request part for streamable http server
@4t145 4t145 requested review from Copilot and jokemanfire May 28, 2025 18:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces additional context information in notification handling and tool call processing, while also updating the streamable HTTP server transport to inject HTTP request parts. Key changes include:

  • Adding a NotificationContext parameter to various notification handling functions in both client and server code paths.
  • Updating tool call context extraction logic with new FromToolCallContextPart implementations.
  • Injecting the HTTP request part into the JSON RPC messages in the streamable HTTP server.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/rmcp/tests/test_notification.rs Updated test notification handler to accept a new context param.
crates/rmcp/tests/common/handlers.rs Updated logging message handler signature with new context.
crates/rmcp/src/transport/streamable_http_server/tower.rs Injects HTTP request parts into JSON RPC messages.
crates/rmcp/src/service/tower.rs Added NotificationContext parameter to the tower service handler.
crates/rmcp/src/service/server.rs Builds and passes NotificationContext for handling notifications.
crates/rmcp/src/service.rs Updated Service and DynService traits to include NotificationContext.
crates/rmcp/src/model/extension.rs Changed new() to a const fn.
crates/rmcp/src/handler/server/tool.rs Provided new FromToolCallContextPart implementations.
crates/rmcp/src/handler/server.rs Updated ServerHandler trait functions to accept NotificationContext.
crates/rmcp/src/handler/client.rs Updated ClientHandler trait functions to accept NotificationContext.
Comments suppressed due to low confidence (1)

crates/rmcp/src/service/server.rs:213

  • [nitpick] Consider adding documentation or inline comments explaining the purpose and expected usage of NotificationContext, to improve maintainability for future developers.
let context = NotificationContext { meta: notification.get_meta().clone(), extensions: notification.extensions().clone(), peer: peer.clone(), };

@jokemanfire
Copy link
Collaborator

I think it is a new feature ,can you provide some docs?

@4t145 4t145 force-pushed the feat-more-context-info branch from dd9c3d3 to cefeb83 Compare May 29, 2025 03:37
@4t145
Copy link
Collaborator Author

4t145 commented May 29, 2025

@jokemanfire added some docs in README.md of rmcp crate.

@4t145 4t145 merged commit 969fad2 into modelcontextprotocol:main May 29, 2025
10 checks passed
@github-actions github-actions bot mentioned this pull request Jul 2, 2025
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.

2 participants