Skip to content

Comments

feat: expose StreamableHttpHandler for MVC controller support#1311

Open
jtarquino wants to merge 1 commit intomodelcontextprotocol:mainfrom
jtarquino:feature/mvc-controller-support
Open

feat: expose StreamableHttpHandler for MVC controller support#1311
jtarquino wants to merge 1 commit intomodelcontextprotocol:mainfrom
jtarquino:feature/mvc-controller-support

Conversation

@jtarquino
Copy link

Motivation and Context

The ModelContextProtocol.AspNetCore package currently only exposes MCP HTTP endpoints via MapMcp() — a minimal API extension method. Users whose applications rely exclusively on traditional MVC controllers (AddControllers() + MapControllers()) cannot integrate MCP into their ASP.NET Core apps without also adopting minimal APIs. This change makes StreamableHttpHandler public so users can inject it into their own controllers and delegate the POST/GET/DELETE requests directly.

How Has This Been Tested?

  • 3 new integration tests (McpControllerTests) covering connect, tool call, and tool listing via an MVC controller — all passing on net8.0, net9.0, and net10.0.
  • Full existing test suite passes (1,565 core + 272 AspNetCore tests, 0 failures).
  • Manual end-to-end test: launched the sample server and successfully executed initialize, notifications/initialized, tools/list, and tools/call (echo) via HTTP POST against the MVC controller endpoint.

Breaking Changes

None. This is an additive change — an internal class was made public. No existing public APIs were modified.

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

  • StreamableHttpHandler was converted from a primary constructor to a regular constructor taking IServiceProvider to avoid exposing internal types (StatefulSessionManager, StreamableHttpSession) in the public API surface.
  • The constructor is marked [EditorBrowsable(EditorBrowsableState.Never)] since it's intended for DI resolution only.
  • A new sample project (samples/AspNetCoreMcpControllerServer/) demonstrates the pattern.
  • The README includes a "Using with MVC Controllers" section.

Make StreamableHttpHandler public so it can be injected into traditional
ASP.NET Core MVC controllers, enabling MCP server scenarios without
minimal APIs.

Changes:
- StreamableHttpHandler: internal -> public, constructor takes IServiceProvider
- Updated WithHttpTransport() XML docs to mention controller injection
- Added 'Using with MVC Controllers' section to AspNetCore README
- New sample: AspNetCoreMcpControllerServer
- New integration tests: McpControllerTests (connect, call tool, list tools)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Contributor

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

@mikekistler @stephentoub Do you have any opinions on introducing a public API like the following for use in custom endpoints like controller actions?

namespace ModelContextProtocol.AspNetCore;

public interface IStreamableHttpHandler
{
    Task HandlePostRequestAsync(HttpContext context);
    Task HandleGetRequestAsync(HttpContext context);
    Task HandleDeletetRequestAsync(HttpContext context);
}

// Or just

public interface IStreamableHttpHandler
{
    Task HandleRequest(HttpContext);
}

// Or maybe instead of registering it as a singleton, we could have a factory

public static class McpRequestDelegateFactory
{
    public RequestDelegate Create(McpServerOptions? serverOptions = null,
                                  HttpServerTransportOptions? transportOptions = null,
                                  IServiceProvider? applicationServices = null);
}

I was hesitant to do this right after MCP's transition from "HTTP+SSE" to "Streamable HTTP" as the web-based transport, because I wasn't sure if a new kind of transport would cause us to want to create a completely different kind of interface.

I'm leaning towards my last proposal with the factory. I think having a way to create a RequestDelegate from McpServerOptions makes a lot of sense though. It's very general, so long as you can handle the routing without any input from the SDK. It reminds me a bit of RequestDelegateFactory.Create

"2025-11-25",
];
[EditorBrowsable(EditorBrowsableState.Never)]
public StreamableHttpHandler(IServiceProvider serviceProvider)
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand not wanting to add a public constructor with a bunch of parameters, but this forces StremableHttpHandler to use the service locator pattern which is generally considered bad practice. Not that constructing a singleton is a perf hotspot, but it's generally better to give the DI/IoC container more detailed knowledge of the service dependencies.

If we decide to make something like this public, I would sooner introduce an IStreamableHttpHandler interface and leave the implementation internal.

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