From d16e89df1ecfe0f92741dfcb565b9a30ed2f5e0a Mon Sep 17 00:00:00 2001 From: olaservo Date: Tue, 29 Apr 2025 21:32:21 -0700 Subject: [PATCH 1/7] Preserve original URL --- cli/src/transport.ts | 4 ++++ client/src/lib/hooks/useConnection.ts | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/cli/src/transport.ts b/cli/src/transport.ts index e693f246..5df44f30 100644 --- a/cli/src/transport.ts +++ b/cli/src/transport.ts @@ -15,7 +15,11 @@ export type TransportOptions = { function createSSETransport(options: TransportOptions): Transport { const baseUrl = new URL(options.url ?? ""); + // Create new URL while preserving the port from baseUrl const sseUrl = new URL("/sse", baseUrl); + if (baseUrl.port) { + sseUrl.port = baseUrl.port; + } return new SSEClientTransport(sseUrl); } diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 27bc11a2..98ae0447 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -280,6 +280,8 @@ export function useConnection({ return; } let mcpProxyServerUrl; + const originalUrl = new URL(sseUrl); + switch (transportType) { case "stdio": mcpProxyServerUrl = new URL(`${getMCPProxyAddress(config)}/stdio`); @@ -290,12 +292,12 @@ export function useConnection({ case "sse": mcpProxyServerUrl = new URL(`${getMCPProxyAddress(config)}/sse`); - mcpProxyServerUrl.searchParams.append("url", sseUrl); + mcpProxyServerUrl.searchParams.append("url", originalUrl.toString()); break; case "streamable-http": mcpProxyServerUrl = new URL(`${getMCPProxyAddress(config)}/mcp`); - mcpProxyServerUrl.searchParams.append("url", sseUrl); + mcpProxyServerUrl.searchParams.append("url", originalUrl.toString()); break; } (mcpProxyServerUrl as URL).searchParams.append( From 0838ffde38098ecd84699d94dfd767caa6af3766 Mon Sep 17 00:00:00 2001 From: olaservo Date: Wed, 30 Apr 2025 05:03:00 -0700 Subject: [PATCH 2/7] Update tests --- .../hooks/__tests__/useConnection.test.tsx | 88 ++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/client/src/lib/hooks/__tests__/useConnection.test.tsx b/client/src/lib/hooks/__tests__/useConnection.test.tsx index e191d6c0..debab46d 100644 --- a/client/src/lib/hooks/__tests__/useConnection.test.tsx +++ b/client/src/lib/hooks/__tests__/useConnection.test.tsx @@ -28,10 +28,18 @@ jest.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ })); jest.mock("@modelcontextprotocol/sdk/client/sse.js", () => ({ - SSEClientTransport: jest.fn(), + SSEClientTransport: jest.fn((url) => ({ + toString: () => url, + })), SseError: jest.fn(), })); +jest.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ + StreamableHTTPClientTransport: jest.fn((url) => ({ + toString: () => url, + })), +})); + jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({ auth: jest.fn().mockResolvedValue("AUTHORIZED"), })); @@ -163,4 +171,82 @@ describe("useConnection", () => { result.current.makeRequest(mockRequest, mockSchema), ).rejects.toThrow("MCP client not connected"); }); + + describe("URL Port Handling", () => { + const SSEClientTransport = jest.requireMock("@modelcontextprotocol/sdk/client/sse.js").SSEClientTransport; + const StreamableHTTPClientTransport = jest.requireMock("@modelcontextprotocol/sdk/client/streamableHttp.js").StreamableHTTPClientTransport; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + test("preserves HTTPS port number when connecting", async () => { + const props = { + ...defaultProps, + sseUrl: "https://example.com:8443/api", + transportType: "sse" as const, + }; + + const { result } = renderHook(() => useConnection(props)); + + await act(async () => { + await result.current.connect(); + }); + + const call = SSEClientTransport.mock.calls[0][0]; + expect(call.toString()).toContain("url=https%3A%2F%2Fexample.com%3A8443%2Fapi"); + }); + + test("preserves HTTP port number when connecting", async () => { + const props = { + ...defaultProps, + sseUrl: "http://localhost:3000/api", + transportType: "sse" as const, + }; + + const { result } = renderHook(() => useConnection(props)); + + await act(async () => { + await result.current.connect(); + }); + + const call = SSEClientTransport.mock.calls[0][0]; + expect(call.toString()).toContain("url=http%3A%2F%2Flocalhost%3A3000%2Fapi"); + }); + + test("uses default port for HTTPS when not specified", async () => { + const props = { + ...defaultProps, + sseUrl: "https://example.com/api", + transportType: "sse" as const, + }; + + const { result } = renderHook(() => useConnection(props)); + + await act(async () => { + await result.current.connect(); + }); + + const call = SSEClientTransport.mock.calls[0][0]; + expect(call.toString()).toContain("url=https%3A%2F%2Fexample.com%2Fapi"); + expect(call.toString()).not.toContain("%3A443"); + }); + + test("preserves port number in streamable-http transport", async () => { + const props = { + ...defaultProps, + sseUrl: "https://example.com:8443/api", + transportType: "streamable-http" as const, + }; + + const { result } = renderHook(() => useConnection(props)); + + await act(async () => { + await result.current.connect(); + }); + + const call = StreamableHTTPClientTransport.mock.calls[0][0]; + expect(call.toString()).toContain("url=https%3A%2F%2Fexample.com%3A8443%2Fapi"); + }); + }); }); From 7bf363cca9ac425a7b301580962bbbefada1b8f3 Mon Sep 17 00:00:00 2001 From: olaservo Date: Wed, 30 Apr 2025 05:05:23 -0700 Subject: [PATCH 3/7] Remove unnecessary comment --- cli/src/transport.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/src/transport.ts b/cli/src/transport.ts index 5df44f30..f3a1bec9 100644 --- a/cli/src/transport.ts +++ b/cli/src/transport.ts @@ -15,7 +15,6 @@ export type TransportOptions = { function createSSETransport(options: TransportOptions): Transport { const baseUrl = new URL(options.url ?? ""); - // Create new URL while preserving the port from baseUrl const sseUrl = new URL("/sse", baseUrl); if (baseUrl.port) { sseUrl.port = baseUrl.port; From fe827426ae748b3c5cc816d5c28bd3309e72a1a4 Mon Sep 17 00:00:00 2001 From: olaservo Date: Wed, 30 Apr 2025 05:16:00 -0700 Subject: [PATCH 4/7] Revert re-parsing url --- client/src/lib/hooks/useConnection.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 98ae0447..34388b8e 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -280,7 +280,6 @@ export function useConnection({ return; } let mcpProxyServerUrl; - const originalUrl = new URL(sseUrl); switch (transportType) { case "stdio": @@ -292,12 +291,12 @@ export function useConnection({ case "sse": mcpProxyServerUrl = new URL(`${getMCPProxyAddress(config)}/sse`); - mcpProxyServerUrl.searchParams.append("url", originalUrl.toString()); + mcpProxyServerUrl.searchParams.append("url", sseUrl); break; case "streamable-http": mcpProxyServerUrl = new URL(`${getMCPProxyAddress(config)}/mcp`); - mcpProxyServerUrl.searchParams.append("url", originalUrl.toString()); + mcpProxyServerUrl.searchParams.append("url", sseUrl); break; } (mcpProxyServerUrl as URL).searchParams.append( From f641aa047099eae4cd1d5b7d6c77b36dbffedf0a Mon Sep 17 00:00:00 2001 From: olaservo Date: Wed, 30 Apr 2025 05:16:42 -0700 Subject: [PATCH 5/7] Revert re-parsing url --- client/src/lib/hooks/useConnection.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 34388b8e..27bc11a2 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -280,7 +280,6 @@ export function useConnection({ return; } let mcpProxyServerUrl; - switch (transportType) { case "stdio": mcpProxyServerUrl = new URL(`${getMCPProxyAddress(config)}/stdio`); From 24e1aaefd170572031bb6b4c23c840395e8d27cb Mon Sep 17 00:00:00 2001 From: olaservo Date: Wed, 30 Apr 2025 06:09:35 -0700 Subject: [PATCH 6/7] Remove unnecessary explicit port setting --- cli/src/transport.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/cli/src/transport.ts b/cli/src/transport.ts index f3a1bec9..e693f246 100644 --- a/cli/src/transport.ts +++ b/cli/src/transport.ts @@ -16,9 +16,6 @@ export type TransportOptions = { function createSSETransport(options: TransportOptions): Transport { const baseUrl = new URL(options.url ?? ""); const sseUrl = new URL("/sse", baseUrl); - if (baseUrl.port) { - sseUrl.port = baseUrl.port; - } return new SSEClientTransport(sseUrl); } From 7611227fd4be86e9fb78342e8640740d17b5e009 Mon Sep 17 00:00:00 2001 From: olaservo Date: Wed, 30 Apr 2025 06:19:44 -0700 Subject: [PATCH 7/7] Fix formatting --- .../hooks/__tests__/useConnection.test.tsx | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/client/src/lib/hooks/__tests__/useConnection.test.tsx b/client/src/lib/hooks/__tests__/useConnection.test.tsx index debab46d..d1dae99d 100644 --- a/client/src/lib/hooks/__tests__/useConnection.test.tsx +++ b/client/src/lib/hooks/__tests__/useConnection.test.tsx @@ -173,8 +173,12 @@ describe("useConnection", () => { }); describe("URL Port Handling", () => { - const SSEClientTransport = jest.requireMock("@modelcontextprotocol/sdk/client/sse.js").SSEClientTransport; - const StreamableHTTPClientTransport = jest.requireMock("@modelcontextprotocol/sdk/client/streamableHttp.js").StreamableHTTPClientTransport; + const SSEClientTransport = jest.requireMock( + "@modelcontextprotocol/sdk/client/sse.js", + ).SSEClientTransport; + const StreamableHTTPClientTransport = jest.requireMock( + "@modelcontextprotocol/sdk/client/streamableHttp.js", + ).StreamableHTTPClientTransport; beforeEach(() => { jest.clearAllMocks(); @@ -194,7 +198,9 @@ describe("useConnection", () => { }); const call = SSEClientTransport.mock.calls[0][0]; - expect(call.toString()).toContain("url=https%3A%2F%2Fexample.com%3A8443%2Fapi"); + expect(call.toString()).toContain( + "url=https%3A%2F%2Fexample.com%3A8443%2Fapi", + ); }); test("preserves HTTP port number when connecting", async () => { @@ -211,7 +217,9 @@ describe("useConnection", () => { }); const call = SSEClientTransport.mock.calls[0][0]; - expect(call.toString()).toContain("url=http%3A%2F%2Flocalhost%3A3000%2Fapi"); + expect(call.toString()).toContain( + "url=http%3A%2F%2Flocalhost%3A3000%2Fapi", + ); }); test("uses default port for HTTPS when not specified", async () => { @@ -246,7 +254,9 @@ describe("useConnection", () => { }); const call = StreamableHTTPClientTransport.mock.calls[0][0]; - expect(call.toString()).toContain("url=https%3A%2F%2Fexample.com%3A8443%2Fapi"); + expect(call.toString()).toContain( + "url=https%3A%2F%2Fexample.com%3A8443%2Fapi", + ); }); }); });