Skip to content

Conversation

@lucaseduoli
Copy link
Collaborator

@lucaseduoli lucaseduoli commented Oct 7, 2025

This pull request refactors the MCP server connection logic to replace the "SSE" (Server-Sent Events) transport type with a more general "Streamable HTTP" (or "HTTP") transport type throughout both the frontend and backend codebases. The changes include renaming variables, updating validation logic, and refactoring session management to support the new transport type, as well as improving HTTP endpoint validation.

Frontend updates:

  • All references to "SSE" in the AddMcpServerModal component are replaced with "HTTP/SSE", including state variables, form fields, and tab labels. [1] [2] [3] [4] [5] [6] [7]

Backend updates:

Transport type and session management refactor:

  • The backend replaces all "SSE" transport type logic with "StreamableHTTP", but with a fallback to "SSE", including parameter validation, session key generation, and session creation. The session creation logic now uses a new streamablehttp_client instead of an SSE client. [1] [2] [3] [4] [5] [6] [7] [8] [9]

Frontend: HTTP replaces SSE in server modal

  • All "SSE" references in AddMcpServerModal are renamed to "HTTP/SSE", including state, tab labels, and form fields. [1] [2] [3] [4] [5] [6] [7]

Backend: Transport type and session management

  • Replaces "SSE" with "StreamableHTTP" in parameter validation, session key generation, and session creation logic, using a new streamablehttp_client. [1] [2] [3] [4] [5] [6] [7] [8] [9]

Backend: HTTP endpoint validation

  • Improves HTTP endpoint validation to use HEAD requests, handle common status codes gracefully, and only fail on clear client errors (401, 403). [1] [2]

Summary by CodeRabbit

  • New Features

    • Added Streamable HTTP transport for MCP servers with automatic fallback to SSE and per‑server transport caching.
    • Updated Add MCP Server modal to use HTTP fields/labels and defaults.
    • Improved session handling, timeouts, and clearer connection error messages.
  • Tests

    • Expanded end-to-end coverage for HTTP/SSE connection flows, tool loading, and interactions.
  • Chores

    • Updated development dependencies.

@lucaseduoli lucaseduoli self-assigned this Oct 7, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 7, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Replaces SSE-based MCP transport with a Streamable HTTP client across backend utilities, agents, starter project payload, frontend modal, and end-to-end tests. Updates session/transport selection, state, and UI/test identifiers accordingly. Adds dev dependency "@types/node". Adjusts method signatures and caching to support Streamable HTTP with SSE fallback.

Changes

Cohort / File(s) Summary of Changes
Dev tooling
package.json
Added devDependency: @types/node@^24.7.0.
Backend MCP transport/utilities
src/lfx/src/lfx/base/mcp/util.py, src/lfx/src/lfx/components/agents/mcp_component.py
Introduced MCPStreamableHttpClient replacing MCPSseClient. Expanded transport modes to include Streamable_HTTP with SSE fallback and caching. Updated session creation, connection, tool updating, and method signatures (e.g., sse_read_timeout_seconds). Agent component now holds streamable_http_client and passes it through update/build flows.
Starter project payload
src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json
In embedded MCPToolsComponent, swapped MCPSseClient for MCPStreamableHttpClient, renamed attribute to streamable_http_client, updated imports/usages, and revised code hash.
Frontend modal (Add MCP Server)
src/frontend/src/modals/addMcpServerModal/index.tsx
Renamed SSE state/labels/logic to HTTP equivalents (e.g., sseNamehttpName). Default type now "HTTP". Submission, validation, and reset paths updated to HTTP terminology and payload fields.
Frontend E2E tests
src/frontend/tests/extended/features/mcp-server.spec.ts
Updated selectors and flows from SSE to HTTP (e.g., sse-tabhttp-tab). Added scenarios for Streamable HTTP and SSE fallback, including server spin-up, connection, tool loading, and echo tool validation. Enhanced error-handling paths.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant UI as Frontend UI (Add MCP Server)
  participant LFX as LFX Agent/Orchestrator
  participant Util as MCP Util
  participant HTTP as MCPStreamableHttpClient
  participant SSE as (fallback) SSE Client
  participant STDIO as MCPStdioClient

  User->>UI: Select type=HTTP, enter name/url/headers/env
  UI->>LFX: Submit server config (HTTP)
  LFX->>Util: update_tools(config)
  Util->>HTTP: connect_to_server(url, headers, sse_read_timeout_seconds)
  alt Streamable HTTP succeeds
    HTTP-->>Util: session established
    Util-->>LFX: tools list + transport_used=Streamable_HTTP
  else Streamable HTTP fails
    Util->>SSE: connect_to_server(url, headers, read_timeout)
    alt SSE succeeds
      SSE-->>Util: session established
      Util-->>LFX: tools list + transport_used=SSE
    else Both fail
      Util-->>LFX: error
      LFX-->>UI: surface error
    end
  end
  Note over Util,LFX: Cache transport preference per server

  User->>UI: Run tool
  UI->>LFX: execute(tool, args)
  LFX->>Util: get_session(server)
  Util-->>LFX: session + transport_used
  LFX->>HTTP: run(tool, args) Note right of LFX: if transport_used=HTTP
  LFX->>SSE: run(tool, args) Note right of LFX: if transport_used=SSE
  LFX-->>UI: result
Loading
sequenceDiagram
  autonumber
  participant Agent as MCPToolsComponent (Agent)
  participant Util as MCP Util
  participant HTTP as MCPStreamableHttpClient
  participant STDIO as MCPStdioClient

  Agent->>Util: update_tools(mcp_streamable_http_client=HTTP, mcp_stdio_client=STDIO, config)
  Util->>HTTP: ensure session context
  Util-->>Agent: updated tool registry
  Agent->>Agent: build_output() uses session context from HTTP/STDIO
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested labels

refactor, size:XXL, lgtm

Suggested reviewers

  • mfortman11
  • ogabrielluiz
  • jordanrfrazier

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 3 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Test Coverage For New Implementations ❓ Inconclusive I inspected the repository for tests added or updated alongside the Streamable HTTP replacement. The frontend includes updated and expanded end-to-end coverage in src/frontend/tests/extended/features/mcp-server.spec.ts that exercises the new HTTP/SSE flow (selectors renamed to http-, transport selection, tool loading, and invocation). However, I could not find backend Python tests (test_.py) that target the new MCPStreamableHttpClient or the updated session/transport selection logic in src/lfx/src/lfx/base/mcp/util.py. The project’s frontend convention appears to use *.spec.ts rather than *.test.ts, and the updated tests follow that existing convention and meaningfully test the new functionality; still, there is a gap in backend unit/integration tests for the newly introduced transport behavior and API changes. Add backend tests following the project’s Python convention (test_*.py) that cover MCPStreamableHttpClient connection behavior, transport fallback from Streamable HTTP to SSE, session caching/keys, and updated validation paths in util.py; include at least one regression-style test for the Python fixes highlighted in review (timeout handling, unused variable, and broad exception fallback) if applicable. Keep the existing frontend *.spec.ts E2E tests but consider adding a focused unit/integration test around the AddMcpServerModal logic for HTTP/SSE field handling; if the project expects *.test.ts, align naming, otherwise keep *.spec.ts consistent with the repo. Once backend tests are added, this check can pass.
Test Quality And Coverage ❓ Inconclusive I inspected the repository’s tests and focused on the areas touched by this PR. The frontend Playwright suite includes updated scenarios in src/frontend/tests/extended/features/mcp-server.spec.ts that exercise the new HTTP/SSE flow, but they currently spawn external servers via npx and rely on fixed sleeps, which are flaky and don’t deterministically validate readiness or error paths; previous review comments also flag this. I found no new or adapted backend pytest coverage for the substantial async/session-management changes in src/lfx/src/lfx/base/mcp/util.py (transport selection, fallback to SSE, timeouts, caching), nor tests validating both success and error responses for the HTTP validation/connection logic. Overall, while there is some end-to-end coverage on the frontend, it’s brittle and lacks robust behavior assertions and negative cases, and backend changes appear untested. Add deterministic frontend tests by pre-installing the server fixture, probing readiness (e.g., polling a health/HEAD endpoint), and asserting concrete behaviors and error states without fixed sleeps; avoid npx in CI. For backend, add pytest-based async tests covering get_session, transport selection (HTTP first, fallback to SSE), timeout handling, caching of successful transports, and both success and error paths for URL validation and connect_to_server. Include negative tests for 401/403 and unreachable endpoints, and ensure tests follow the project’s pytest/Playwright conventions.
Test File Naming And Structure ❓ Inconclusive I inspected the repository for test files and found frontend Playwright tests under src/frontend/tests, including src/frontend/tests/extended/features/mcp-server.spec.ts, which uses the .spec.ts naming rather than the requested .test.ts/tsx pattern. The file appears to be an end-to-end/integration-style test (server process spawning, UI interaction), and it sits in an integration-appropriate path (extended/features), but it is not clearly labeled as integration beyond directory naming. Backend pytest-style tests named test_.py were not evident in the provided changes; without broader repository context, I cannot confirm backend test presence or structure. Within the updated frontend tests, there is setup/teardown logic implied (terminating spawned servers), but prior review comments flagged flakiness and ad-hoc readiness waits, suggesting setup/teardown and negative-path coverage could be improved. Given the explicit naming requirement (*.test.ts/tsx) and limited confirmation of backend pytest structure, the check cannot conclusively pass. Rename frontend test files from *.spec.ts to .test.ts/tsx for consistency with the stated pattern, and add an explicit marker or directory README clarifying that extended/features contains integration tests. Ensure robust setup/teardown by replacing fixed sleeps with readiness probes and guaranteed cleanup. If backend tests exist, confirm they follow test_.py with pytest conventions; if not, add or document them. Expand tests to include clear negative/error scenarios and edge cases alongside the positive paths.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely summarizes the primary change of replacing the SSE transport with a Streamable HTTP client in the MCP client, matching the PR’s main focus without unnecessary detail.
Excessive Mock Usage Warning ✅ Passed I scanned the repository’s test files to identify heavy mock usage across frontend and backend tests, focusing on common mocking patterns (jest.mock/vi.mock, spies, nock/msw, Python unittest.mock/patch). The updated E2E tests in src/frontend/tests/extended/features/mcp-server.spec.ts do not rely on mocks; they exercise real UI flows and even start external processes, which suggests integration-style testing rather than mock-heavy unit tests. No widespread or excessive mocking patterns were found elsewhere in the test suite that would obscure real behavior or replace core logic with mocks. The existing approach aligns with using real interactions for core behavior and reserving mocks for true external dependencies. Given the findings, there is no indication of excessive mock usage in this PR.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the enhancement New feature or request label Oct 7, 2025
@lucaseduoli lucaseduoli force-pushed the feat/streamable_http branch from a875248 to f869703 Compare October 7, 2025 14:57
@github-actions github-actions bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 7, 2025
@github-actions github-actions bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 7, 2025
@github-actions github-actions bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 7, 2025
@github-actions github-actions bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 7, 2025
@github-actions github-actions bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 7, 2025
@github-actions github-actions bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 7, 2025
@github-actions github-actions bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 7, 2025
@github-actions github-actions bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 7, 2025
@github-actions github-actions bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 7, 2025
@github-actions github-actions bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 7, 2025
@github-actions github-actions bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 7, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 7, 2025

@lucaseduoli lucaseduoli dismissed stale reviews from ogabrielluiz and edwinjosechittilappilly October 7, 2025 21:04

Outdated

@lucaseduoli lucaseduoli added this pull request to the merge queue Oct 7, 2025
Merged via the queue into main with commit 99fc2cf Oct 7, 2025
75 of 78 checks passed
@lucaseduoli lucaseduoli deleted the feat/streamable_http branch October 7, 2025 21:35
@alissonpadua
Copy link

helllo @lucaseduoli !! Did this PR also update the docker image on the hub ? Just pushed, but I still see the old version (without the streamable http support)

Thank you!

@alissonpadua
Copy link

helllo @lucaseduoli !! Did this PR also update the docker image on the hub ? Just pushed, but I still see the old version (without the streamable http support)

Thank you!

nvm, I see it was not released yet :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants