Skip to content
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

Refactor MCP Client/Server API for proper reactive programming compliance #48

Open
tzolov opened this issue Jan 4, 2025 · 1 comment
Assignees
Labels
Milestone

Comments

@tzolov
Copy link
Contributor

tzolov commented Jan 4, 2025

Problem

The current implementation of McpAsyncServer and McpAsyncClient mixes blocking and non-blocking operations, which violates reactive programming principles. Specifically:

  1. McpAsyncServer uses blocking collections and consumers: List<ToolRegistration> tools, Map<String, ResourceRegistration> resources, List<McpSchema.ResourceTemplate> resourceTemplates, Map<String, PromptRegistration> prompts, List<Consumer<List<McpSchema.Root>>> rootsChangeConsumers.
  2. McpAsyncClient uses blocking collections and consumers: Map<String, Root> roots,List<Consumer<List<McpSchema.Tool>>> toolsChangeConsumers,List<Consumer<List<McpSchema.Resource>>> resourcesChangeConsumers,List<Consumer<List<McpSchema.Prompt>>> promptsChangeConsumers,List<Consumer<McpSchema.LoggingMessageNotification>> loggingConsumers,Function<CreateMessageRequest, CreateMessageResult> samplingHandler
  3. The builder pattern doesn't properly separate sync/async concerns, leading to potential blocking operations in async contexts.

Proposed Solution

  1. Split the builder pattern into distinct sync and async implementations:
// Async-specific builder
McpServer.builder().async()  // Returns AsyncMcpServerSpec
    .tool(Mono<ToolRegistration>)
    .resource(Mono<ResourceRegistration>)
    .prompt(Mono<PromptRegistration>)
    .build();

// Sync-specific builder
McpServer.builder().sync()   // Returns SyncMcpServerSpec
    .tool(Supplier<ToolRegistration>)
    .resource(Supplier<ResourceRegistration>)
    .prompt(Supplier<PromptRegistration>)
    .build();
  1. Make async API purely reactive:
  • Replace blocking collections with reactive types
  • Use Flux/Mono for all async operations
  • Remove direct use of blocking operations
  1. Implement sync-to-async bridging:
  • Sync client delegates to async instance
  • Use appropriate scheduler strategies:
    • subscribeOn(boundedElastic) for user-provided blocking code
    • publishOn(boundedElastic) for consuming reactive streams in blocking manner

Implementation Details

  1. New AsyncMcpServerSpec interface:
public interface AsyncMcpServerSpec {
    AsyncMcpServerSpec tool(Mono<ToolRegistration>);
    AsyncMcpServerSpec resource(Mono<ResourceRegistration>);
    AsyncMcpServerSpec prompt(Mono<PromptRegistration>);
    AsyncMcpServerSpec rootsChangeListener(Flux<List<McpSchema.Root>>);
    McpAsyncServer build();
}
  1. New SyncMcpServerSpec interface:
public interface SyncMcpServerSpec {
    SyncMcpServerSpec tool(Supplier<ToolRegistration>);
    SyncMcpServerSpec resource(Supplier<ResourceRegistration>);
    SyncMcpServerSpec prompt(Supplier<PromptRegistration>);
    SyncMcpServerSpec rootsChangeListener(Consumer<List<McpSchema.Root>>);
    McpSyncServer build();
}
  1. Bridging example in sync client:
public class McpSyncServer {
    private final McpAsyncServer asyncServer;
    
    public void addTool(Supplier<ToolRegistration> toolSupplier) {
        Mono.fromSupplier(toolSupplier)
            .subscribeOn(Schedulers.boundedElastic())
            .flatMap(asyncClient::addTool)
            .block();
    }
    
    public List<Tool> listTools() {
        return asyncServer.listTools()
            .publishOn(Schedulers.boundedElastic())
            .block();
    }
}
public class McpSyncClient {
    private final McpAsyncClient asyncClient;
        
    public List<Tool> listTools() {
        return asyncClient.listTools()
            .publishOn(Schedulers.boundedElastic())
            .block();
    }
}
@tzolov tzolov added this to the 0.5.0 milestone Jan 4, 2025
@tzolov tzolov added client server bug Something isn't working sync async labels Jan 4, 2025
@ThomasVitale
Copy link
Contributor

ThomasVitale commented Jan 4, 2025

It would be great if there was a complete distinction between standard implementation and reactive implementation, so to make it possible for non-reactive applications not to have any reactive dependency, and rely on Java own features for the async behaviour (using virtual threads). And then have the reactive implementation fully Reactor-based. Similar to other Spring projects (Spring Data, Spring Security, Spring Web...).

tzolov added a commit that referenced this issue Jan 4, 2025
- Execute tool calls, resource reads and prompt handling in a non-blocking
  manner using Schedulers.boundedElastic(). This prevents blocking operations
  from impacting server responsiveness.
- Added integration tests to verify non-blocking behavior with tools that
  make HTTP calls to external services.

Related to #48
This is  a temp patch until #48 is resolved properly.
tzolov added a commit that referenced this issue Jan 4, 2025
- Execute tool calls, resource reads and prompt handling in a non-blocking
  manner using Schedulers.boundedElastic(). This prevents blocking operations
  from impacting server responsiveness.
- Added integration tests to verify non-blocking behavior with tools that
  make HTTP calls to external services.

Related to #48
This is  a temp patch until #48 is resolved properly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants