-
Notifications
You must be signed in to change notification settings - Fork 147
Fix MCP Inspector to support HTTPS endpoints #993
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
Conversation
Co-authored-by: aaronpowell <[email protected]>
Minimum allowed line rate is |
There was a problem hiding this 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 fixes a bug where the MCP Inspector integration fails when MCP servers expose only HTTPS endpoints. The fix enables HTTPS endpoint support while maintaining backward compatibility with HTTP-only servers.
- Updates endpoint resolution logic to prefer HTTPS over HTTP with a fallback chain
- Improves error messaging to reflect support for both HTTPS and HTTP endpoints
- Adds comprehensive test coverage for HTTPS-only, HTTP-only, and mixed endpoint scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/CommunityToolkit.Aspire.Hosting.McpInspector/McpInspectorResource.cs | Updated endpoint resolution to check for "https" first, then fall back to "http", following the pattern used in JavaScriptHostingExtensions.cs. Also updated error message to accurately reflect support for both endpoint types. |
| tests/CommunityToolkit.Aspire.Hosting.McpInspector.Tests/McpInspectorResourceBuilderExtensionsTests.cs | Added three new test methods to verify HTTPS endpoint support, HTTPS preference when both endpoints exist, and proper endpoint selection behavior across different configurations. |
| // Arrange | ||
| var appBuilder = DistributedApplication.CreateBuilder(); | ||
|
|
||
| // AddProject creates both http and https endpoints by default |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment states "AddProject creates both http and https endpoints by default", but this is inconsistent with the comment in the test above (line 450) which states "AddProject creates 'http' by default". This creates confusion about the actual behavior. Please verify the behavior and update comments consistently. Based on the code context, AddProject typically creates an HTTP endpoint by default, and HTTPS needs to be added explicitly with WithHttpsEndpoint().
| // AddProject creates both http and https endpoints by default | |
| // AddProject creates "http" endpoint by default |
| .WithHttpsEndpoint(name: "https"); | ||
|
|
||
| // Act | ||
| var inspector = appBuilder.AddMcpInspector("inspector") |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call to obsolete method AddMcpInspector.
| var inspector = appBuilder.AddMcpInspector("inspector") | |
| var inspector = appBuilder.AddMcpInspectorResource("inspector") |
| .WithHttpsEndpoint(name: "https"); | ||
|
|
||
| // Act | ||
| var inspector = appBuilder.AddMcpInspector("inspector") |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call to obsolete method AddMcpInspector.
| var mockServer = appBuilder.AddProject<Projects.CommunityToolkit_Aspire_Hosting_McpInspector_McpServer>("mcpServer"); | ||
|
|
||
| // Act | ||
| var inspector = appBuilder.AddMcpInspector("inspector") |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call to obsolete method AddMcpInspector.
| .WithHttpsEndpoint(name: "https"); | ||
|
|
||
| // Act | ||
| var inspector = appBuilder.AddMcpInspector("inspector") |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment to inspector is useless, since its value is never read.
| var inspector = appBuilder.AddMcpInspector("inspector") | |
| appBuilder.AddMcpInspector("inspector") |
| .WithHttpsEndpoint(name: "https"); | ||
|
|
||
| // Act | ||
| var inspector = appBuilder.AddMcpInspector("inspector") |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment to inspector is useless, since its value is never read.
| var inspector = appBuilder.AddMcpInspector("inspector") | |
| appBuilder.AddMcpInspector("inspector") |
| var mockServer = appBuilder.AddProject<Projects.CommunityToolkit_Aspire_Hosting_McpInspector_McpServer>("mcpServer"); | ||
|
|
||
| // Act | ||
| var inspector = appBuilder.AddMcpInspector("inspector") |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment to inspector is useless, since its value is never read.
| var inspector = appBuilder.AddMcpInspector("inspector") | |
| appBuilder.AddMcpInspector("inspector") |
| McpServerMetadata item = new( | ||
| mcpServer.Name, | ||
| mcpServer.GetEndpoint("http") ?? throw new InvalidOperationException($"The MCP server {mcpServer.Name} must have a 'http' endpoint defined."), | ||
| mcpServer.GetEndpoint("https") ?? mcpServer.GetEndpoint("http") ?? throw new InvalidOperationException($"The MCP server {mcpServer.Name} must have an 'https' or 'http' endpoint defined."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This never returns null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if there's no registered HTTPS endpoint though?
The MCP Inspector integration fails when MCP servers only expose HTTPS endpoints, throwing
InvalidOperationExceptionbecause it only checks for HTTP.Changes
McpInspectorResource.cs: Prefer HTTPS over HTTP when resolving endpoints
Tests: Added coverage for HTTPS-only, HTTP-only, and both endpoints scenarios
Follows the existing pattern in
JavaScriptHostingExtensions.csfor endpoint resolution.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.