Skip to content

Conversation

@tninesling
Copy link

Adds blanket implementations for Box<T> and Arc<T> when T implements ClientHandler or ServerHandler.

Motivation and Context

When building an MCP service, we may want to wrap it in a smart pointer before passing it into the tower service constructor. In my case, I want to spawn a background thread to do some work using some session state as requests are processed. However, introducing the Arc results in a compilation error:

use std::sync::Arc;
use rmcp::ServerHandler;
use rmcp::transport::StreamableHttpService;

struct MyService;

impl ServerHandler for MyService {}

fn start_server() {
    let my_service = Arc::new(MyService);

    // This gives "the trait bound `Arc<MyService>: ServerHandler` is not satisfied"
    let http_service = StreamableHttpService::new(
        move || Ok(my_service), 
        Default::default(),
        Default::default(),
    );
    // ...
}

How Has This Been Tested?

Added integration tests and confirmed this fixes the compilation error in my project.

Breaking Changes

I believe adding blanket implementations of this kind is typically considered a breaking change, although it should be a no-op for most users.

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

I omitted blanket implementations for &T and &mut T due to the 'static bound on the traits, and I omitted Rc<T> due to the Sync bound.

@github-actions github-actions bot added T-test Testing related changes T-core Core library changes T-handler Handler implementation changes labels Jan 7, 2026
@alexhancock
Copy link
Collaborator

@tninesling Nice! A good idea

I had a play with macros to see if it could dedupe some of the volume

Let me know what you think of 0b54219

It's somewhat vibecoded so let me know if it does the job and if you like it

@tninesling
Copy link
Author

Let me know what you think of 0b54219

Thanks for taking a look @alexhancock. As long as those tests pass, it fixes the issue I ran into. Although, the macros in that commit add a lot of indirection that make it hard to read. I agree with the direction of deduplicating the implementations though. I added some simpler macros in a493962, which are slightly more verbose, but hopefully easier to read and maintain.

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

Labels

T-core Core library changes T-handler Handler implementation changes T-test Testing related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants