tools/ui: Respect configured MCP transport instead of always using Streamable HTTP#97
Open
joydolma wants to merge 1 commit into
Open
tools/ui: Respect configured MCP transport instead of always using Streamable HTTP#97joydolma wants to merge 1 commit into
joydolma wants to merge 1 commit into
Conversation
Collaborator
|
Thanks for the PR, will take a closer look. Is this somethign related to our fork? Or should it be more in the main llama.cpp. Does not seem to be touching our part of code (our main addition is mianly the kernels for 1-bit and ternary models, and few experiments features kv-cache 4-bit and dspark.) |
There was a problem hiding this comment.
Pull request overview
Updates MCP transport selection to support legacy SSE servers while retaining Streamable HTTP defaults.
Changes:
- Detects SSE transport from endpoint URLs.
- Creates the transport specified by the MCP configuration.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tools/ui/src/lib/utils/mcp.ts |
Adds SSE URL detection. |
tools/ui/src/lib/services/mcp.service.ts |
Selects configured SSE or Streamable HTTP transport. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+52
to
+53
| normalized.endsWith("/sse") || | ||
| normalized.includes("/sse?") |
Comment on lines
+422
to
+423
| // Respect the configured transport instead of always defaulting to Streamable HTTP. | ||
| switch (config.transport) { |
Comment on lines
+423
to
+424
| switch (config.transport) { | ||
| case MCPTransportType.SSE: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes MCP transport selection in the UI by respecting the configured
transport type instead of always defaulting to Streamable HTTP.
This restores compatibility with legacy SSE-based MCP servers while preserving
support for newer Streamable HTTP servers.
Changes
Motivation
Some MCP servers (including existing third-party implementations) still expose
the legacy SSE transport. The current behavior always prefers Streamable HTTP,
which prevents successful connections to those servers even when SSE is
explicitly configured.
This change allows the configured transport to be used while maintaining
compatibility with both transport implementations.
Compatibility
This change has been verified with:
The goal is to preserve compatibility with both transport implementations
without changing the default behavior for Streamable HTTP servers.
Testing
Tested successfully with:
Verified tool discovery and execution with both legacy SSE and Streamable HTTP
servers.