Skip to content

Conversation

@edwinjosechittilappilly
Copy link
Collaborator

@edwinjosechittilappilly edwinjosechittilappilly commented Oct 13, 2025

This pull request addresses a critical issue where the JavaScriptMIMETypeMiddleware was causing ASGI message conflicts with streaming SSE responses on /api/v1/mcp/ endpoints. The main change ensures that these routes bypass the middleware, preventing errors and improving reliability for SSE connections. Additionally, a new test verifies that the bypass logic is present and effective.

Middleware compatibility fix:

  • Updated JavaScriptMIMETypeMiddleware in src/backend/base/langflow/main.py to skip processing for requests to /api/v1/mcp/ endpoints, preventing ASGI message conflicts with streaming responses.

Testing and verification:

  • Added test_mcp_sse_with_middleware_no_conflict in src/backend/tests/unit/api/v1/test_mcp.py to confirm the middleware bypass logic is present and that SSE endpoints work without interference.

Summary by CodeRabbit

  • Bug Fixes

    • Ensures the MCP streaming endpoint (/api/v1/mcp/sse) bypasses JavaScript MIME processing, preventing unintended header or response alterations.
    • Improves reliability and compatibility of server‑sent events for MCP routes.
  • Tests

    • Added a unit test to verify middleware bypass behavior and successful responses for the MCP SSE route.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 13, 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

Introduces an early-return bypass in JavaScriptMIMETypeMiddleware for paths starting with /api/v1/mcp/, preventing further middleware processing. Adds a unit test to verify the bypass logic via source inspection and a HEAD request to /api/v1/mcp/sse ensuring 200 OK without interference.

Changes

Cohort / File(s) Summary
Middleware control-flow update
src/backend/base/langflow/main.py
Adds early return in JavaScriptMIMETypeMiddleware.dispatch for paths beginning with /api/v1/mcp/, calling call_next directly and skipping subsequent processing for these routes.
MCP SSE test addition
src/backend/tests/unit/api/v1/test_mcp.py
Adds test_mcp_sse_with_middleware_no_conflict to assert bypass presence via source inspection and to validate runtime behavior with an authenticated HEAD request to /api/v1/mcp/sse expecting 200 OK.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant M as JavaScriptMIMETypeMiddleware
  participant A as Downstream App/Handlers

  C->>M: HTTP request (e.g., /api/v1/mcp/sse)
  alt Path starts with /api/v1/mcp/
    note over M: Bypass middleware logic
    M->>A: call_next(request)
    A-->>M: Response
    M-->>C: Early return response
  else Other paths
    note over M: Normal processing (try/except, headers, etc.)
    M->>A: call_next(request)
    A-->>M: Response
    M-->>C: Possibly modified response
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Test Quality And Coverage ⚠️ Warning The PR introduces a middleware bypass for MCP SSE routes and adds one backend pytest. The test partially validates behavior by issuing an async HEAD request to /api/v1/mcp/sse and asserting 200, but it also inspects source code strings, which is brittle and not aligned with project testing patterns emphasizing behavior over implementation. There is no negative/error-path coverage (e.g., non-MCP routes still handled by middleware, or SSE conflict reproduction), nor comprehensive validation of headers/streaming behavior beyond a smoke HEAD check. Overall, async usage is correct and backend pytest is appropriate, but test quality and coverage are insufficient per the stated criteria. Refactor the test to validate runtime behavior only: remove inspect-based assertions, assert correct behavior for MCP SSE (e.g., GET with mocked streaming or server-sent events without ASGI conflicts) and confirm headers are not altered. Add complementary tests: a non-MCP JavaScript asset route to ensure middleware still applies, and an error-path case (e.g., unauthorized access to MCP endpoint returns expected 401/403). Where feasible, simulate an SSE stream to verify no "Unexpected message: {'type': 'http.response.start'}" occurs during iteration, and assert relevant headers (content-type, cache-control) for both success and error responses.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the key change—bypassing the middleware for MCP SSE routes—and specifies the purpose of the change, which is to resolve ASGI conflicts, making it clear and directly tied to the pull request’s main objective.
Test Coverage For New Implementations ✅ Passed I inspected the PR summary and review context, which indicate a bug fix in JavaScriptMIMETypeMiddleware to bypass processing for /api/v1/mcp/ SSE routes, along with a new backend unit test at src/backend/tests/unit/api/v1/test_mcp.py. The test file name follows the project’s backend naming convention (test_*.py), and it includes a regression-oriented check by exercising the MCP SSE endpoint (HEAD to /api/v1/mcp/sse returns 200) and verifying middleware non-interference. While part of the test uses fragile source inspection of the middleware (as noted by reviewers), it still verifies the presence of the bypass and basic runtime behavior, so it is not a placeholder. There are no new features requiring integration tests; for this bug fix, a regression test exists, albeit with room to improve by focusing purely on behavioral verification during streaming.
Test File Naming And Structure ✅ Passed I inspected the added backend test file src/backend/tests/unit/api/v1/test_mcp.py and confirmed its filename matches the backend pattern test_*.py and uses pytest-style test functions (e.g., def test_mcp_sse_with_middleware_no_conflict) that leverage fixtures like client and logged_in_headers for setup/teardown via pytest’s fixture system. The test resides under a unit scope directory (unit/api/v1), which is appropriate and clearly not an integration test. There are no frontend tests in this PR, so no *.test.ts(x) files are involved. While the test name is descriptive and validates the positive scenario (200 OK for HEAD on /api/v1/mcp/sse), it does not cover negative/error scenarios or edge cases (e.g., non-MCP routes affected by the middleware or actual SSE streaming), which slightly limits comprehensiveness but does not violate naming/structure rules.
Excessive Mock Usage Warning ✅ Passed I inspected the updated test file and surrounding tests to assess mock usage. The newly added test (test_mcp_sse_with_middleware_no_conflict) relies on real HTTP client fixtures and direct assertions without using unittest.mock, MagicMock, pytest-mock, monkeypatch, or similar facilities, and it previously used inspect to check implementation details rather than mocking. There is no evidence of excessive mocking in this PR’s test changes, and mocks are not used to simulate core logic or behaviors that should be verified end-to-end. Based on the available information, the tests prioritize real behavior over mocks and do not trigger the warning criteria.

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 bug Something isn't working and removed bug Something isn't working labels Oct 13, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Oct 13, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Oct 13, 2025
Copy link
Contributor

@ogabrielluiz ogabrielluiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://fastapi.tiangolo.com/release-notes/#01180

Was this the cause of this PR?

@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Oct 13, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d230ca4 and 5dd47c1.

📒 Files selected for processing (2)
  • src/backend/base/langflow/main.py (1 hunks)
  • src/backend/tests/unit/api/v1/test_mcp.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
{src/backend/**/*.py,tests/**/*.py,Makefile}

📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)

{src/backend/**/*.py,tests/**/*.py,Makefile}: Run make format_backend to format Python code before linting or committing changes
Run make lint to perform linting checks on backend Python code

Files:

  • src/backend/base/langflow/main.py
  • src/backend/tests/unit/api/v1/test_mcp.py
src/backend/tests/unit/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)

Test component integration within flows using create_flow, build_flow, and get_build_events utilities

Files:

  • src/backend/tests/unit/api/v1/test_mcp.py
src/backend/tests/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)

src/backend/tests/**/*.py: Unit tests for backend code must be located in the 'src/backend/tests/' directory, with component tests organized by component subdirectory under 'src/backend/tests/unit/components/'.
Test files should use the same filename as the component under test, with an appropriate test prefix or suffix (e.g., 'my_component.py' → 'test_my_component.py').
Use the 'client' fixture (an async httpx.AsyncClient) for API tests in backend Python tests, as defined in 'src/backend/tests/conftest.py'.
When writing component tests, inherit from the appropriate base class in 'src/backend/tests/base.py' (ComponentTestBase, ComponentTestBaseWithClient, or ComponentTestBaseWithoutClient) and provide the required fixtures: 'component_class', 'default_kwargs', and 'file_names_mapping'.
Each test in backend Python test files should have a clear docstring explaining its purpose, and complex setups or mocks should be well-commented.
Test both sync and async code paths in backend Python tests, using '@pytest.mark.asyncio' for async tests.
Mock external dependencies appropriately in backend Python tests to isolate unit tests from external services.
Test error handling and edge cases in backend Python tests, including using 'pytest.raises' and asserting error messages.
Validate input/output behavior and test component initialization and configuration in backend Python tests.
Use the 'no_blockbuster' pytest marker to skip the blockbuster plugin in tests when necessary.
Be aware of ContextVar propagation in async tests; test both direct event loop execution and 'asyncio.to_thread' scenarios to ensure proper context isolation.
Test error handling by mocking internal functions using monkeypatch in backend Python tests.
Test resource cleanup in backend Python tests by using fixtures that ensure proper initialization and cleanup of resources.
Test timeout and performance constraints in backend Python tests using 'asyncio.wait_for' and timing assertions.
Test Langflow's Messag...

Files:

  • src/backend/tests/unit/api/v1/test_mcp.py
**/@(test_*.py|*.test.@(ts|tsx))

📄 CodeRabbit inference engine (coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt)

**/@(test_*.py|*.test.@(ts|tsx)): Check if tests have too many mock objects that obscure what's actually being tested
Warn when mocks are used instead of testing real behavior and interactions
Suggest using real objects or test doubles when mocks become excessive
Ensure mocks are used appropriately for external dependencies, not core logic
Recommend integration tests when unit tests become overly mocked
Test files should have descriptive test function names explaining what is tested
Tests should be organized logically with proper setup and teardown
Include edge cases and error conditions for comprehensive coverage
Verify tests cover both positive and negative scenarios where appropriate
Tests should cover the main functionality being implemented
Ensure tests are not just smoke tests but actually validate behavior
For API endpoints, verify both success and error response testing

Files:

  • src/backend/tests/unit/api/v1/test_mcp.py
**/test_*.py

📄 CodeRabbit inference engine (coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt)

**/test_*.py: Check that backend test files follow naming convention: test_.py
Backend tests should be named test_
.py and follow proper pytest structure
For async Python code, ensure proper async testing patterns (pytest) are used
Backend tests should follow pytest conventions; frontend tests should use Playwright

Files:

  • src/backend/tests/unit/api/v1/test_mcp.py
🧬 Code graph analysis (1)
src/backend/tests/unit/api/v1/test_mcp.py (2)
src/backend/tests/conftest.py (1)
  • logged_in_headers (495-501)
src/backend/base/langflow/main.py (3)
  • JavaScriptMIMETypeMiddleware (89-113)
  • dispatch (67-86)
  • dispatch (90-113)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
  • GitHub Check: Lint Backend / Run Mypy (3.10)
  • GitHub Check: Test Docker Images / Test docker images
  • GitHub Check: Lint Backend / Run Mypy (3.11)
  • GitHub Check: Lint Backend / Run Mypy (3.13)
  • GitHub Check: Lint Backend / Run Mypy (3.12)
  • GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
  • GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 2
  • GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
  • GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 4
  • GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 1
  • GitHub Check: Run Backend Tests / Integration Tests - Python 3.10
  • GitHub Check: Test Starter Templates
  • GitHub Check: Label PR
  • GitHub Check: Optimize new Python code in this PR
🔇 Additional comments (1)
src/backend/base/langflow/main.py (1)

91-94: Verify middleware bypass impact on MCP routes
The early return skips PydanticSerializationError handling and all middleware for /api/v1/mcp/. Confirm no other end-points under this prefix require error handling or security checks.

@codecov
Copy link

codecov bot commented Oct 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 24.71%. Comparing base (9570711) to head (613af04).
⚠️ Report is 11 commits behind head on main.

❌ Your project check has failed because the head coverage (48.55%) is below the target coverage (55.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #10257      +/-   ##
==========================================
- Coverage   24.73%   24.71%   -0.02%     
==========================================
  Files        1090     1090              
  Lines       40116    40116              
  Branches     5550     5550              
==========================================
- Hits         9921     9914       -7     
- Misses      30024    30031       +7     
  Partials      171      171              
Flag Coverage Δ
backend 48.55% <100.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/backend/base/langflow/main.py 60.85% <100.00%> (+0.24%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@edwinjosechittilappilly
Copy link
Collaborator Author

@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Oct 13, 2025
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Oct 13, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Oct 13, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Oct 14, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Oct 14, 2025
@sonarqubecloud
Copy link

@edwinjosechittilappilly edwinjosechittilappilly marked this pull request as draft October 15, 2025 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants