Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/services/mcp/McpHub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,9 @@ export class McpHub {
}
} else if (configInjected.type === "streamable-http") {
// Streamable HTTP connection
transport = new StreamableHTTPClientTransport(new URL(configInjected.url), {
// Normalize URL by removing trailing slashes to avoid 405 errors
const normalizedUrl = configInjected.url.replace(/\/+$/, "")
transport = new StreamableHTTPClientTransport(new URL(normalizedUrl), {
requestInit: {
headers: configInjected.headers,
},
Expand All @@ -769,6 +771,8 @@ export class McpHub {
}
} else if (configInjected.type === "sse") {
// SSE connection
// Normalize URL by removing trailing slashes for consistency
const normalizedUrl = configInjected.url.replace(/\/+$/, "")
const sseOptions = {
requestInit: {
headers: configInjected.headers,
Expand All @@ -787,7 +791,7 @@ export class McpHub {
},
}
global.EventSource = ReconnectingEventSource
transport = new SSEClientTransport(new URL(configInjected.url), {
transport = new SSEClientTransport(new URL(normalizedUrl), {
...sseOptions,
eventSourceInit: reconnectingEventSourceOptions,
})
Expand Down Expand Up @@ -1378,8 +1382,8 @@ export class McpHub {
await this.deleteConnection(serverName, serverSource)
// Re-add as a disabled connection
// Re-read config from file to get updated disabled state
const updatedConfig = await this.readServerConfigFromFile(serverName, serverSource)
await this.connectToServer(serverName, updatedConfig, serverSource)
const updatedConfig = await this.readServerConfigFromFile(serverName, serverSource)
await this.connectToServer(serverName, updatedConfig, serverSource)
} else if (!disabled && connection.server.status === "disconnected") {
// If enabling a disabled server, connect it
// Re-read config from file to get updated disabled state
Expand Down
223 changes: 223 additions & 0 deletions src/services/mcp/__tests__/McpHub.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2147,4 +2147,227 @@ describe("McpHub", () => {
)
})
})

describe("URL normalization", () => {
let StdioClientTransport: ReturnType<typeof vi.fn>
let StreamableHTTPClientTransport: ReturnType<typeof vi.fn>
let SSEClientTransport: ReturnType<typeof vi.fn>
let Client: ReturnType<typeof vi.fn>

beforeEach(async () => {
// Reset mocks
vi.clearAllMocks()

// Get references to the mocked constructors
const stdioModule = await import("@modelcontextprotocol/sdk/client/stdio.js")
const clientModule = await import("@modelcontextprotocol/sdk/client/index.js")
StdioClientTransport = stdioModule.StdioClientTransport as ReturnType<typeof vi.fn>
Client = clientModule.Client as ReturnType<typeof vi.fn>

// Mock StreamableHTTPClientTransport
vi.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
StreamableHTTPClientTransport: vi.fn(),
}))
const streamableModule = await import("@modelcontextprotocol/sdk/client/streamableHttp.js")
StreamableHTTPClientTransport = streamableModule.StreamableHTTPClientTransport as ReturnType<typeof vi.fn>

// Mock SSEClientTransport
vi.mock("@modelcontextprotocol/sdk/client/sse.js", () => ({
SSEClientTransport: vi.fn(),
}))
const sseModule = await import("@modelcontextprotocol/sdk/client/sse.js")
SSEClientTransport = sseModule.SSEClientTransport as ReturnType<typeof vi.fn>
})

it("should remove trailing slash from streamable-http URLs", async () => {
// Mock StreamableHTTPClientTransport
const mockTransport = {
start: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
onerror: null,
onclose: null,
}

StreamableHTTPClientTransport.mockImplementation((url: URL, options: any) => {
// Verify that the trailing slash is removed
expect(url.toString()).toBe("https://api.githubcopilot.com/mcp")
return mockTransport
})

// Mock Client
Client.mockImplementation(() => ({
connect: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
getInstructions: vi.fn().mockReturnValue("test instructions"),
request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }),
}))

// Create a new McpHub instance
const mcpHub = new McpHub(mockProvider as ClineProvider)

// Mock the config file read with URL containing trailing slash
vi.mocked(fs.readFile).mockResolvedValue(
JSON.stringify({
mcpServers: {
github: {
type: "streamable-http",
url: "https://api.githubcopilot.com/mcp/",
headers: {
Authorization: "Bearer GITHUB_PAT",
},
},
},
}),
)

// Initialize servers (this will trigger connectToServer)
await mcpHub["initializeGlobalMcpServers"]()

// Verify StreamableHTTPClientTransport was called with normalized URL
expect(StreamableHTTPClientTransport).toHaveBeenCalledWith(expect.any(URL), expect.any(Object))
const urlArg = StreamableHTTPClientTransport.mock.calls[0][0]
expect(urlArg.toString()).toBe("https://api.githubcopilot.com/mcp")
})

it("should remove multiple trailing slashes from streamable-http URLs", async () => {
// Mock StreamableHTTPClientTransport
const mockTransport = {
start: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
onerror: null,
onclose: null,
}

StreamableHTTPClientTransport.mockImplementation((url: URL, options: any) => {
// Verify that multiple trailing slashes are removed
expect(url.toString()).toBe("https://api.example.com/endpoint")
return mockTransport
})

// Mock Client
Client.mockImplementation(() => ({
connect: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
getInstructions: vi.fn().mockReturnValue("test instructions"),
request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }),
}))

// Create a new McpHub instance
const mcpHub = new McpHub(mockProvider as ClineProvider)

// Mock the config file read with URL containing multiple trailing slashes
vi.mocked(fs.readFile).mockResolvedValue(
JSON.stringify({
mcpServers: {
"test-server": {
type: "streamable-http",
url: "https://api.example.com/endpoint///",
},
},
}),
)

// Initialize servers
await mcpHub["initializeGlobalMcpServers"]()

// Verify StreamableHTTPClientTransport was called with normalized URL
expect(StreamableHTTPClientTransport).toHaveBeenCalledWith(expect.any(URL), expect.any(Object))
const urlArg = StreamableHTTPClientTransport.mock.calls[0][0]
expect(urlArg.toString()).toBe("https://api.example.com/endpoint")
})

it("should remove trailing slash from SSE URLs", async () => {
// Mock SSEClientTransport
const mockTransport = {
start: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
onerror: null,
onclose: null,
}

SSEClientTransport.mockImplementation((url: URL, options: any) => {
// Verify that the trailing slash is removed
expect(url.toString()).toBe("https://sse.example.com/events")
return mockTransport
})

// Mock Client
Client.mockImplementation(() => ({
connect: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
getInstructions: vi.fn().mockReturnValue("test instructions"),
request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }),
}))

// Create a new McpHub instance
const mcpHub = new McpHub(mockProvider as ClineProvider)

// Mock the config file read with SSE URL containing trailing slash
vi.mocked(fs.readFile).mockResolvedValue(
JSON.stringify({
mcpServers: {
"sse-server": {
type: "sse",
url: "https://sse.example.com/events/",
},
},
}),
)

// Initialize servers
await mcpHub["initializeGlobalMcpServers"]()

// Verify SSEClientTransport was called with normalized URL
expect(SSEClientTransport).toHaveBeenCalledWith(expect.any(URL), expect.any(Object))
const urlArg = SSEClientTransport.mock.calls[0][0]
expect(urlArg.toString()).toBe("https://sse.example.com/events")
})

it("should handle URLs without trailing slashes correctly", async () => {
// Mock StreamableHTTPClientTransport
const mockTransport = {
start: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
onerror: null,
onclose: null,
}

StreamableHTTPClientTransport.mockImplementation((url: URL, options: any) => {
// Verify that URL without trailing slash remains unchanged
expect(url.toString()).toBe("https://api.example.com/path")
return mockTransport
})

// Mock Client
Client.mockImplementation(() => ({
connect: vi.fn().mockResolvedValue(undefined),
close: vi.fn().mockResolvedValue(undefined),
getInstructions: vi.fn().mockReturnValue("test instructions"),
request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }),
}))

// Create a new McpHub instance
const mcpHub = new McpHub(mockProvider as ClineProvider)

// Mock the config file read with URL without trailing slash
vi.mocked(fs.readFile).mockResolvedValue(
JSON.stringify({
mcpServers: {
"no-slash-server": {
type: "streamable-http",
url: "https://api.example.com/path",
},
},
}),
)

// Initialize servers
await mcpHub["initializeGlobalMcpServers"]()

// Verify URL remains unchanged
expect(StreamableHTTPClientTransport).toHaveBeenCalledWith(expect.any(URL), expect.any(Object))
const urlArg = StreamableHTTPClientTransport.mock.calls[0][0]
expect(urlArg.toString()).toBe("https://api.example.com/path")
})
})
})