feat: Add SSL verification option to MCP tools and clients#10269
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a configurable SSL verification flag (verify_ssl) across MCP tooling: new BoolInput in MCPToolsComponent, propagation into server configs, updates to MCPStreamableHttpClient connection methods and session factories to use SSL-aware httpx clients, starter project JSON updated accordingly, and unit tests added to validate propagation and defaults. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as MCPToolsComponent
participant Cfg as server_config
participant Cli as MCPStreamableHttpClient
participant Util as create_mcp_http_client_with_ssl_option
participant Net as httpx.AsyncClient
UI->>Cfg: update_tool_list() ensures verify_ssl present
UI->>Cli: connect_to_server(url, headers, sse_read_timeout, verify_ssl)
Note over Cli: Store verify_ssl in connection params
alt Streamable HTTP or SSE
Cli->>Util: create client (headers, timeout, auth, verify_ssl)
Util-->>Cli: factory returning SSL-configured client
Cli->>Net: open session with verify_ssl
Net-->>Cli: session established
end
Cli-->>UI: tools loaded
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (4 passed)
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. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR adds SSL certificate verification as an optional feature for MCP (Model Context Protocol) tools and clients. It enables users to disable SSL verification when working with development/testing environments that use self-signed certificates while maintaining security by defaulting to enabled SSL verification.
- Adds a new
verify_sslparameter to MCP components and HTTP clients - Updates MCP connection logic to respect the SSL verification setting
- Provides comprehensive test coverage for the new SSL verification functionality
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/lfx/src/lfx/components/agents/mcp_component.py | Adds verify_ssl input field and passes it through server configuration |
| src/lfx/src/lfx/base/mcp/util.py | Implements SSL verification option in HTTP clients and session management |
| src/backend/tests/unit/base/mcp/test_mcp_util.py | Adds comprehensive test coverage for SSL verification functionality |
| src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json | Updates project configuration to include the new verify_ssl field |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| session_id: Unique identifier for this session | ||
| connection_params: Connection parameters including URL, headers, timeouts | ||
| connection_params: Connection parameters including URL, headers, timeouts, verify_ssl | ||
| preferred_transport: If set to "sse", skip Streamable HTTP and go directly to SSE |
There was a problem hiding this comment.
The docstring comment is incomplete and should be properly formatted. It appears to be a fragment that should be part of a structured docstring.
| # Store connection parameters for later use in run_tool | ||
| # Include SSE read timeout for fallback | ||
| # Include SSE read timeout for fallback and SSL verification option |
There was a problem hiding this comment.
[nitpick] The comment should be more descriptive and explain what the connection parameters dictionary contains, not just list the additions.
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
|
✅ Component index has been automatically updated due to changes in New Index: |
github-actions
Bot
added
enhancement
|
✅ Component index has been automatically updated due to changes in New Index: |
github-actions
Bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/lfx/src/lfx/base/mcp/util.py (2)
1297-1310: Expose timeout_seconds on connect_to_server to avoid API regressionconnect_to_server no longer exposes timeout_seconds but _connect_to_server still supports it. If callers previously set it, this change breaks that. Recommend adding it back and passing through.
@@ - async def connect_to_server( - self, - url: str, - headers: dict[str, str] | None = None, - sse_read_timeout_seconds: int = 30, - verify_ssl: bool = True, - ) -> list[StructuredTool]: + async def connect_to_server( + self, + url: str, + headers: dict[str, str] | None = None, + timeout_seconds: int = 30, + sse_read_timeout_seconds: int = 30, + verify_ssl: bool = True, + ) -> list[StructuredTool]: @@ - self._connect_to_server( - url, headers, sse_read_timeout_seconds=sse_read_timeout_seconds, verify_ssl=verify_ssl - ), + self._connect_to_server( + url, + headers, + timeout_seconds=timeout_seconds, + sse_read_timeout_seconds=sse_read_timeout_seconds, + verify_ssl=verify_ssl, + ),
1348-1353: Use verify_ssl when sending DELETE on disconnect_terminate_remote_session creates a fresh AsyncClient with default verify=True. For self‑signed setups (verify_ssl=False), this will consistently fail. Respect the stored verify_ssl.
- async with httpx.AsyncClient(timeout=5.0) as client: + verify_ssl = self._connection_params.get("verify_ssl", True) + async with httpx.AsyncClient(timeout=5.0, verify=verify_ssl) as client: await client.delete(url, headers=headers)src/backend/tests/unit/base/mcp/test_mcp_util.py (1)
1627-1714: Close AsyncClient instances; make tests async and use async withThese tests construct httpx.AsyncClient but never close it, risking resource leaks and warnings. Convert to async tests and use async with (or await client.aclose()).
Example fixes:
@@ - def test_create_mcp_http_client_with_ssl_option_default(self): + @pytest.mark.asyncio + async def test_create_mcp_http_client_with_ssl_option_default(self): """Test that the custom httpx client factory defaults to SSL verification enabled.""" from lfx.base.mcp.util import create_mcp_http_client_with_ssl_option + import httpx - - client = create_mcp_http_client_with_ssl_option() - - # Verify that SSL verification is enabled by default using _transport - # httpx stores verify in the transport config, not as a direct attribute - assert client._transport is not None or client.is_closed - # Just verify the client was created successfully with default settings - assert isinstance(client, type(client)) + async with create_mcp_http_client_with_ssl_option() as client: + # Sanity: client created successfully + assert isinstance(client, httpx.AsyncClient) @@ - def test_create_mcp_http_client_with_ssl_disabled(self): + @pytest.mark.asyncio + async def test_create_mcp_http_client_with_ssl_disabled(self): """Test creating httpx client with SSL verification disabled.""" from lfx.base.mcp.util import create_mcp_http_client_with_ssl_option - - # The key test is that this doesn't raise an exception - client = create_mcp_http_client_with_ssl_option(verify_ssl=False) - - # Verify client was created successfully - assert isinstance(client, type(client)) + import httpx + async with create_mcp_http_client_with_ssl_option(verify_ssl=False) as client: + assert isinstance(client, httpx.AsyncClient) @@ - def test_create_mcp_http_client_with_ssl_enabled_explicit(self): + @pytest.mark.asyncio + async def test_create_mcp_http_client_with_ssl_enabled_explicit(self): """Test creating httpx client with SSL verification explicitly enabled.""" from lfx.base.mcp.util import create_mcp_http_client_with_ssl_option - - client = create_mcp_http_client_with_ssl_option(verify_ssl=True) - - # Verify client was created successfully - assert isinstance(client, type(client)) + import httpx + async with create_mcp_http_client_with_ssl_option(verify_ssl=True) as client: + assert isinstance(client, httpx.AsyncClient) @@ - def test_create_mcp_http_client_with_ssl_and_headers(self): + @pytest.mark.asyncio + async def test_create_mcp_http_client_with_ssl_and_headers(self): """Test creating httpx client with SSL option and custom headers.""" from lfx.base.mcp.util import create_mcp_http_client_with_ssl_option @@ - client = create_mcp_http_client_with_ssl_option(headers=test_headers, verify_ssl=False) - - # Verify headers are set correctly - assert client.headers["Authorization"] == "Bearer token123" - assert client.headers["X-API-Key"] == "secret" + async with create_mcp_http_client_with_ssl_option(headers=test_headers, verify_ssl=False) as client: + assert client.headers["Authorization"] == "Bearer token123" + assert client.headers["X-API-Key"] == "secret" @@ - def test_create_mcp_http_client_with_ssl_and_timeout(self): + @pytest.mark.asyncio + async def test_create_mcp_http_client_with_ssl_and_timeout(self): @@ - client = create_mcp_http_client_with_ssl_option(timeout=custom_timeout, verify_ssl=False) - - # Verify timeout is set correctly - httpx.Timeout has read attribute - assert client.timeout.read == 120.0 + async with create_mcp_http_client_with_ssl_option(timeout=custom_timeout, verify_ssl=False) as client: + assert client.timeout.read == 120.0 @@ - def test_create_mcp_http_client_with_ssl_and_auth(self): + @pytest.mark.asyncio + async def test_create_mcp_http_client_with_ssl_and_auth(self): @@ - client = create_mcp_http_client_with_ssl_option(auth=auth, verify_ssl=False) - - # Verify auth is set correctly - assert client.auth == auth + async with create_mcp_http_client_with_ssl_option(auth=auth, verify_ssl=False) as client: + assert client.auth == auth @@ - def test_create_mcp_http_client_all_options(self): + @pytest.mark.asyncio + async def test_create_mcp_http_client_all_options(self): @@ - client = create_mcp_http_client_with_ssl_option( - headers=test_headers, timeout=custom_timeout, auth=auth, verify_ssl=False - ) - - # Verify all options are set correctly - assert client.headers["Authorization"] == "Bearer token123" - assert client.timeout == custom_timeout - assert client.auth == auth + async with create_mcp_http_client_with_ssl_option( + headers=test_headers, timeout=custom_timeout, auth=auth, verify_ssl=False + ) as client: + assert client.headers["Authorization"] == "Bearer token123" + assert client.timeout == custom_timeout + assert client.auth == authBased on learnings
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json(3 hunks)src/backend/tests/unit/base/mcp/test_mcp_util.py(2 hunks)src/lfx/src/lfx/base/mcp/util.py(9 hunks)src/lfx/src/lfx/components/agents/mcp_component.py(3 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/tests/unit/base/mcp/test_mcp_util.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/base/mcp/test_mcp_util.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/base/mcp/test_mcp_util.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/base/mcp/test_mcp_util.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/base/mcp/test_mcp_util.py
🧬 Code graph analysis (2)
src/lfx/src/lfx/components/agents/mcp_component.py (1)
src/lfx/src/lfx/inputs/inputs.py (1)
BoolInput(414-426)
src/backend/tests/unit/base/mcp/test_mcp_util.py (1)
src/lfx/src/lfx/base/mcp/util.py (6)
create_mcp_http_client_with_ssl_option(68-104)MCPStreamableHttpClient(1213-1483)connect_to_server(1048-1052)connect_to_server(1296-1309)MCPSessionManager(438-997)update_tools(1491-1636)
🔇 Additional comments (7)
src/lfx/src/lfx/components/agents/mcp_component.py (3)
64-64: LGTM! Correct addition to default_keys.Adding "verify_ssl" to
default_keysensures this setting persists when switching tools and isn't cleared byremove_non_default_keys(). This is consistent with other persistent settings like "use_cache".
90-99: LGTM! Well-designed SSL verification input.The verify_ssl input is properly configured:
- Secure by default (
value=True)- Clear documentation warning users to disable only for development/testing
- Appropriately marked as advanced
- Follows the same pattern as the
use_cacheinput above
224-227: verify_ssl is consumed in the HTTP client factory
Theverify_sslflag flows fromserver_configintocreate_mcp_http_client_with_ssl_option, where it sets the HTTPX client'sverifyoption.src/lfx/src/lfx/base/mcp/util.py (2)
68-105: httpx client factory with SSL toggle looks goodUses httpx verify flag correctly; sane defaults and options. No issues.
Based on learnings
1538-1540: verify_ssl propagation into update_tools is correctverify_ssl read from server_config and passed to client. Looks good.
src/backend/tests/unit/base/mcp/test_mcp_util.py (2)
1750-1772: Assert verify_ssl persistence is good; no issuesThese cover False/True paths and ensure connection params carry verify_ssl. Looks good.
Also applies to: 1774-1804
1834-1912: update_tools verify_ssl propagation test is validCaptures verify_ssl argument; also tests default True. Solid coverage.
| # Get verify_ssl option from connection params, default to True | ||
| verify_ssl = connection_params.get("verify_ssl", True) | ||
|
|
||
| # Create custom httpx client factory with SSL verification option | ||
| def custom_httpx_factory( | ||
| headers: dict[str, str] | None = None, | ||
| timeout: httpx.Timeout | None = None, | ||
| auth: httpx.Auth | None = None, | ||
| ) -> httpx.AsyncClient: | ||
| return create_mcp_http_client_with_ssl_option( | ||
| headers=headers, timeout=timeout, auth=auth, verify_ssl=verify_ssl | ||
| ) | ||
|
|
There was a problem hiding this comment.
Honor verify_ssl across session reuse (include in server key)
You pass verify_ssl to the httpx factory (great), but MCPSessionManager’s streamable_http server key doesn’t include verify_ssl. A session created with verify_ssl=False can be reused when verify_ssl=True is requested, silently weakening TLS guarantees. Include verify_ssl in the server key to prevent insecure reuse.
Apply:
@@
- elif transport_type == "streamable_http" and (
- isinstance(connection_params, dict) and "url" in connection_params
- ):
- # Include URL and headers for uniqueness
- url = connection_params["url"]
- headers = str(sorted((connection_params.get("headers", {})).items()))
- key_input = f"{url}|{headers}"
- return f"streamable_http_{hash(key_input)}"
+ elif transport_type == "streamable_http" and (
+ isinstance(connection_params, dict) and "url" in connection_params
+ ):
+ # Include URL, headers, and SSL verify for uniqueness
+ url = connection_params["url"]
+ headers = str(sorted((connection_params.get("headers", {})).items()))
+ verify_ssl = connection_params.get("verify_ssl", True)
+ key_input = f"{url}|{headers}|verify_ssl={verify_ssl}"
+ return f"streamable_http_{hash(key_input)}"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/lfx/src/lfx/base/mcp/util.py around lines 733 to 745, the verify_ssl flag
is passed into the custom_httpx_factory but is not included in the
MCPSessionManager’s streamable_http server key; this can allow a session created
with verify_ssl=False to be reused for verify_ssl=True. Fix by adding the
normalized verify_ssl value (e.g., bool(verify_ssl)) to the server key
construction where streamable_http keys are built (include it in the tuple/dict
that identifies the server), and ensure any lookup or storage of sessions uses
this augmented key so sessions with different verify_ssl settings are not mixed.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lfx/src/lfx/base/mcp/util.py (3)
518-525: Includeverify_sslin the server key for streamable HTTP sessionsCurrently, sessions are keyed by URL+headers only. Reusing a session created with
verify_ssl=Falsewhen a caller requestsverify_ssl=True(or vice‑versa) can cause confusing behavior and weakens the isolation of TLS settings.- # Include URL and headers for uniqueness + # Include URL, headers and TLS verification mode for uniqueness url = connection_params["url"] headers = str(sorted((connection_params.get("headers", {})).items())) - key_input = f"{url}|{headers}" + verify = connection_params.get("verify_ssl", True) + key_input = f"{url}|{headers}|verify={verify}" return f"streamable_http_{hash(key_input)}"
1269-1281: Persistverify_sslchanges on subsequent connectsWhen reconnecting with different
verify_ssl, the value isn’t updated if_connection_paramsalready exist. This can lead to stale settings.- # Store connection parameters for later use in run_tool - # Include SSE read timeout for fallback and SSL verification option + # Store connection parameters for later use in run_tool + # Include SSE read timeout for fallback and SSL verification option self._connection_params = { "url": url, "headers": validated_headers, "timeout_seconds": timeout_seconds, "sse_read_timeout_seconds": sse_read_timeout_seconds, "verify_ssl": verify_ssl, } - elif headers: - self._connection_params["headers"] = validated_headers + else: + if headers is not None: + self._connection_params["headers"] = validated_headers + # Always refresh verify_ssl to reflect caller intent + self._connection_params["verify_ssl"] = verify_ssl
1348-1352: Honorverify_sslduring remote session terminationThe best‑effort DELETE uses a default
httpx.AsyncClientwith verification enabled, which will fail against self‑signed endpoints when the connection was established withverify_ssl=False.- try: - async with httpx.AsyncClient(timeout=5.0) as client: - await client.delete(url, headers=headers) + try: + verify = bool(self._connection_params.get("verify_ssl", True)) + async with create_mcp_http_client_with_ssl_option(timeout=httpx.Timeout(5.0), verify_ssl=verify) as client: + await client.delete(url, headers=headers)
🧹 Nitpick comments (2)
src/backend/tests/unit/base/mcp/test_mcp_util.py (1)
1630-1660: Optional: close AsyncClient instances in tests to avoid resource warningsThese tests create httpx.AsyncClient instances but never close them. Consider making them async and using
async withfor clean teardown.- def test_create_mcp_http_client_with_ssl_option_default(self): + @pytest.mark.asyncio + async def test_create_mcp_http_client_with_ssl_option_default(self): """Test that the custom httpx client factory defaults to SSL verification enabled.""" - from lfx.base.mcp.util import create_mcp_http_client_with_ssl_option - - client = create_mcp_http_client_with_ssl_option() - - # Verify that SSL verification is enabled by default using _transport - # httpx stores verify in the transport config, not as a direct attribute - assert client._transport is not None or client.is_closed - # Just verify the client was created successfully with default settings - assert isinstance(client, type(client)) + import httpx + from lfx.base.mcp.util import create_mcp_http_client_with_ssl_option + async with create_mcp_http_client_with_ssl_option() as client: + assert isinstance(client, httpx.AsyncClient)Apply the same pattern to the other
create_mcp_http_client_*tests for consistency.src/lfx/src/lfx/base/mcp/util.py (1)
1296-1309: Optional: exposetimeout_secondsinconnect_to_serverfor parityPublic
connect_to_serverno longer acceptstimeout_seconds. Consider adding it back (keyword‑only) to let callers tune initial connection timeouts without relying solely on global settings.Example:
- async def connect_to_server( - self, - url: str, - headers: dict[str, str] | None = None, - sse_read_timeout_seconds: int = 30, - verify_ssl: bool = True, - ) -> list[StructuredTool]: + async def connect_to_server( + self, + url: str, + headers: dict[str, str] | None = None, + *, + timeout_seconds: int = 30, + sse_read_timeout_seconds: int = 30, + verify_ssl: bool = True, + ) -> list[StructuredTool]: """Connect to MCP server using Streamable HTTP with SSE fallback transport (SDK style).""" return await asyncio.wait_for( - self._connect_to_server( - url, headers, sse_read_timeout_seconds=sse_read_timeout_seconds, verify_ssl=verify_ssl - ), + self._connect_to_server( + url, + headers, + timeout_seconds=timeout_seconds, + sse_read_timeout_seconds=sse_read_timeout_seconds, + verify_ssl=verify_ssl, + ), timeout=get_settings_service().settings.mcp_server_timeout, )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json(3 hunks)src/backend/tests/unit/base/mcp/test_mcp_util.py(2 hunks)src/lfx/src/lfx/base/mcp/util.py(9 hunks)src/lfx/src/lfx/components/agents/mcp_component.py(3 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/tests/unit/base/mcp/test_mcp_util.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/base/mcp/test_mcp_util.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/base/mcp/test_mcp_util.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/base/mcp/test_mcp_util.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/base/mcp/test_mcp_util.py
🧬 Code graph analysis (2)
src/lfx/src/lfx/components/agents/mcp_component.py (1)
src/lfx/src/lfx/inputs/inputs.py (1)
BoolInput(414-426)
src/backend/tests/unit/base/mcp/test_mcp_util.py (3)
src/lfx/src/lfx/base/mcp/util.py (7)
create_mcp_http_client_with_ssl_option(68-104)MCPStreamableHttpClient(1213-1483)connect_to_server(1048-1052)connect_to_server(1296-1309)MCPSessionManager(438-997)cleanup_all(935-972)update_tools(1491-1636)src/backend/tests/unit/components/agents/test_mcp_component_cache.py (1)
mock_tool(41-49)src/backend/tests/integration/components/mcp/test_mcp_memory_leak.py (2)
list_tools(334-343)list_tools(357-358)
🪛 GitHub Actions: Ruff Style Check
src/backend/tests/unit/base/mcp/test_mcp_util.py
[error] 423-423: Ruff check failed: I001 Import block is un-sorted or un-formatted.
🪛 GitHub Check: Ruff Style Check (3.13)
src/backend/tests/unit/base/mcp/test_mcp_util.py
[failure] 1888-1888: Ruff (ARG001)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1888:37: ARG001 Unused function argument: headers
[failure] 1888-1888: Ruff (ARG001)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1888:32: ARG001 Unused function argument: url
[failure] 1869-1871: Ruff (S110)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1869:9: S110 try-except-pass detected, consider logging the exception
[failure] 1863-1871: Ruff (SIM105)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1863:9: SIM105 Use contextlib.suppress(Exception) instead of try-except-pass
[failure] 1845-1845: Ruff (FBT002)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1845:80: FBT002 Boolean default positional argument in function definition
[failure] 1845-1845: Ruff (ARG001)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1845:51: ARG001 Unused function argument: sse_read_timeout_seconds
[failure] 1842-1842: Ruff (F841)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1842:9: F841 Local variable original_connect is assigned to but never used
[failure] 1704-1704: Ruff (S106)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1704:53: S106 Possible hardcoded password assigned to argument: "password"
[failure] 1690-1690: Ruff (S106)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1690:49: S106 Possible hardcoded password assigned to argument: "password"
⏰ 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). (16)
- GitHub Check: Lint Backend / Run Mypy (3.11)
- GitHub Check: Run Frontend Tests / Determine Test Suites and Shard Distribution
- GitHub Check: Lint Backend / Run Mypy (3.10)
- GitHub Check: Lint Backend / Run Mypy (3.12)
- GitHub Check: Lint Backend / Run Mypy (3.13)
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 4
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 2
- 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: Optimize new Python code in this PR
- GitHub Check: Update Starter Projects
- GitHub Check: test-starter-projects
- GitHub Check: update-index
🔇 Additional comments (3)
src/lfx/src/lfx/base/mcp/util.py (3)
68-105: Factory for SSL‑configurable AsyncClient looks goodUses httpx
verifycorrectly, sets sensible defaults, and cleanly passes headers/auth/timeout.If
httpxversion varies across environments, confirmverifystill accepts aboolas in 0.28.x (Based on learnings).
710-745: Good integration: factory wired into session creation and respectsverify_sslPulling
verify_sslfromconnection_paramsand injecting viahttpx_client_factoryis clean and extensible.
1537-1540:update_toolscorrectly propagatesverify_sslReading
verify_sslfromserver_configwith default True and passing to the client aligns with the new option.
|
|
||
| from pydantic import ValidationError | ||
|
|
||
| # Create a JSON schema with snake_case field names |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix import block ordering to satisfy Ruff I001
Within the function, group/sort imports correctly (third‑party before first‑party) and remove the blank line.
- from lfx.schema.json_schema import create_input_schema_from_json_schema
-
- from pydantic import ValidationError
+ from pydantic import ValidationError
+ from lfx.schema.json_schema import create_input_schema_from_json_schema📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from lfx.schema.json_schema import create_input_schema_from_json_schema | |
| from pydantic import ValidationError | |
| from pydantic import ValidationError | |
| from lfx.schema.json_schema import create_input_schema_from_json_schema |
🧰 Tools
🪛 GitHub Actions: Ruff Style Check
[error] 423-423: Ruff check failed: I001 Import block is un-sorted or un-formatted.
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 423-425: Ruff (I001)
src/backend/tests/unit/base/mcp/test_mcp_util.py:423:9: I001 Import block is un-sorted or un-formatted
🤖 Prompt for AI Agents
In src/backend/tests/unit/base/mcp/test_mcp_util.py around lines 423 to 426, the
in-function import block currently has a blank line and places the first‑party
import before a third‑party import; reorder the imports so third‑party imports
(e.g., from pydantic import ValidationError) come before first‑party imports
(e.g., from lfx.schema.json_schema import create_input_schema_from_json_schema),
remove the extraneous blank line, and ensure the imports are grouped and sorted
to satisfy Ruff I001.
| client = create_mcp_http_client_with_ssl_option(auth=auth, verify_ssl=False) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Silence hardcoded password warnings in tests (S106)
These are benign in unit tests. Add noqa to avoid failing the style check.
- auth = httpx.BasicAuth(username="user", password="pass")
+ auth = httpx.BasicAuth(username="user", password="pass") # noqa: S106- auth = httpx.BasicAuth(username="testuser", password="testpass")
+ auth = httpx.BasicAuth(username="testuser", password="testpass") # noqa: S106Also applies to: 1704-1705
🧰 Tools
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 1690-1690: Ruff (S106)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1690:49: S106 Possible hardcoded password assigned to argument: "password"
🤖 Prompt for AI Agents
In src/backend/tests/unit/base/mcp/test_mcp_util.py around lines 1690-1691 (and
also lines 1704-1705), the test uses hardcoded credentials which trigger the
S106 style check; silence these benign test warnings by appending a noqa S106
directive to those lines (e.g., add "# noqa: S106" at the end of the BasicAuth
assignment lines) so the linter ignores the hardcoded password in tests.
|
|
||
| async def mock_connect(url, headers=None, sse_read_timeout_seconds=30, verify_ssl=True): | ||
| connect_called_with["url"] = url | ||
| connect_called_with["headers"] = headers | ||
| connect_called_with["verify_ssl"] = verify_ssl | ||
| # Return empty tool list | ||
| return [] | ||
|
|
||
| mock_client.connect_to_server = mock_connect | ||
| mock_client._connected = False | ||
|
|
||
| # Test server config with verify_ssl=False |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused variable and fix boolean-trap/unused-args in mock connect_to_server
- Remove
original_connect(F841). - Make
verify_sslkeyword‑only (FBT002). - Prefix unused parameters with
_to silence ARG001.
- original_connect = mock_client.connect_to_server
connect_called_with = {}
- async def mock_connect(url, headers=None, sse_read_timeout_seconds=30, verify_ssl=True):
- connect_called_with["url"] = url
- connect_called_with["headers"] = headers
- connect_called_with["verify_ssl"] = verify_ssl
- # Return empty tool list
- return []
+ async def mock_connect(_url, _headers=None, *, _sse_read_timeout_seconds=30, verify_ssl: bool = True):
+ connect_called_with["verify_ssl"] = verify_ssl
+ return []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| original_connect = mock_client.connect_to_server | |
| connect_called_with = {} | |
| async def mock_connect(url, headers=None, sse_read_timeout_seconds=30, verify_ssl=True): | |
| connect_called_with["url"] = url | |
| connect_called_with["headers"] = headers | |
| connect_called_with["verify_ssl"] = verify_ssl | |
| # Return empty tool list | |
| return [] | |
| mock_client.connect_to_server = mock_connect | |
| mock_client._connected = False | |
| connect_called_with = {} | |
| async def mock_connect(_url, _headers=None, *, _sse_read_timeout_seconds=30, verify_ssl: bool = True): | |
| connect_called_with["verify_ssl"] = verify_ssl | |
| return [] | |
| mock_client.connect_to_server = mock_connect | |
| mock_client._connected = False |
🧰 Tools
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 1845-1845: Ruff (FBT002)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1845:80: FBT002 Boolean default positional argument in function definition
[failure] 1845-1845: Ruff (ARG001)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1845:51: ARG001 Unused function argument: sse_read_timeout_seconds
[failure] 1842-1842: Ruff (F841)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1842:9: F841 Local variable original_connect is assigned to but never used
🤖 Prompt for AI Agents
In src/backend/tests/unit/base/mcp/test_mcp_util.py around lines 1842-1853,
remove the unused variable original_connect, and update the mock_connect
signature to make verify_ssl keyword-only and silence unused-arg warnings by
prefixing unused parameters with underscores; e.g., change parameters to url, *,
_headers=None, _sse_read_timeout_seconds=30, verify_ssl=True (or equivalent) and
update any uses that record those values (connect_called_with["headers"] =
_headers, etc.).
| server_name="test_server", | ||
| server_config=server_config, | ||
| mcp_streamable_http_client=mock_client, | ||
| ) | ||
| except Exception: | ||
| # May fail due to incomplete mocking, but we can still check if connect was called correctly | ||
| pass | ||
|
|
||
| # Verify that connect_to_server was called with verify_ssl=False |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace try/except/pass with contextlib.suppress and avoid swallowing silently
Use contextlib.suppress to satisfy SIM105/S110 while keeping the test intent.
- try:
- await update_tools(
- server_name="test_server",
- server_config=server_config,
- mcp_streamable_http_client=mock_client,
- )
- except Exception:
- # May fail due to incomplete mocking, but we can still check if connect was called correctly
- pass
+ import contextlib
+ with contextlib.suppress(Exception):
+ await update_tools(
+ server_name="test_server",
+ server_config=server_config,
+ mcp_streamable_http_client=mock_client,
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| await update_tools( | |
| server_name="test_server", | |
| server_config=server_config, | |
| mcp_streamable_http_client=mock_client, | |
| ) | |
| except Exception: | |
| # May fail due to incomplete mocking, but we can still check if connect was called correctly | |
| pass | |
| import contextlib | |
| with contextlib.suppress(Exception): | |
| await update_tools( | |
| server_name="test_server", | |
| server_config=server_config, | |
| mcp_streamable_http_client=mock_client, | |
| ) |
🧰 Tools
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 1869-1871: Ruff (S110)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1869:9: S110 try-except-pass detected, consider logging the exception
[failure] 1863-1871: Ruff (SIM105)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1863:9: SIM105 Use contextlib.suppress(Exception) instead of try-except-pass
🤖 Prompt for AI Agents
In src/backend/tests/unit/base/mcp/test_mcp_util.py around lines 1863 to 1871,
replace the try/except/pass that swallows all exceptions with
contextlib.suppress to satisfy linters: import contextlib (or from contextlib
import suppress) at the top if missing, then wrap the await update_tools(...)
call in a contextlib.suppress(Exception): block so the test preserves the
original intent (ignore exceptions from incomplete mocking) without using a bare
except/pass.
| return [] | ||
|
|
||
| mock_client.connect_to_server = mock_connect | ||
| mock_client._connected = False |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix unused arguments and boolean‑trap in second mock connect_to_server
Make boolean arg keyword‑only and prefix unused params.
- async def mock_connect(url, headers=None, sse_read_timeout_seconds=30, verify_ssl=True):
- connect_called_with["verify_ssl"] = verify_ssl
- return []
+ async def mock_connect(_url, _headers=None, *, _sse_read_timeout_seconds=30, verify_ssl: bool = True):
+ connect_called_with["verify_ssl"] = verify_ssl
+ return []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def mock_connect(url, headers=None, sse_read_timeout_seconds=30, verify_ssl=True): | |
| connect_called_with["verify_ssl"] = verify_ssl | |
| return [] | |
| async def mock_connect(_url, _headers=None, *, _sse_read_timeout_seconds=30, verify_ssl: bool = True): | |
| connect_called_with["verify_ssl"] = verify_ssl | |
| return [] |
🧰 Tools
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 1888-1888: Ruff (ARG001)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1888:37: ARG001 Unused function argument: headers
[failure] 1888-1888: Ruff (ARG001)
src/backend/tests/unit/base/mcp/test_mcp_util.py:1888:32: ARG001 Unused function argument: url
🤖 Prompt for AI Agents
In src/backend/tests/unit/base/mcp/test_mcp_util.py around lines 1888-1891, the
mock connect function accepts unused positional params and a boolean that can be
accidentally passed positionally; make unused parameters name-prefixed with an
underscore (e.g. _url, _headers) and make boolean args keyword-only by adding a
bare * before them (so verify_ssl must be passed by name), then update the body
to use verify_ssl to record connect_called_with["verify_ssl"] and return the
same stub value; apply the same rename/keyword-only pattern to the other mock
connect_to_server in the same test file to avoid boolean-trap and
unused-argument warnings.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lfx/src/lfx/base/mcp/util.py (1)
1348-1350: Honor verify_ssl on remote session DELETEWhen verify_ssl=False, DELETE may fail to connect to self-signed endpoints. Use the stored flag.
- async with httpx.AsyncClient(timeout=5.0) as client: + async with httpx.AsyncClient( + timeout=5.0, + verify=bool(self._connection_params.get("verify_ssl", True)), + ) as client: await client.delete(url, headers=headers)
🧹 Nitpick comments (4)
src/backend/tests/unit/base/mcp/test_mcp_util.py (2)
1629-1713: Strengthen httpx client factory tests; avoid private attrs and resource leaks
- Accessing client._transport is relying on httpx internals; brittle.
- isinstance(client, type(client)) asserts nothing.
- These tests instantiate AsyncClient without closing it (ResourceWarning risk).
Patch httpx.AsyncClient and assert kwargs instead; no real client needed:
@@ - client = create_mcp_http_client_with_ssl_option() - - # Verify that SSL verification is enabled by default using _transport - # httpx stores verify in the transport config, not as a direct attribute - assert client._transport is not None or client.is_closed - # Just verify the client was created successfully with default settings - assert isinstance(client, type(client)) + with patch("lfx.base.mcp.util.httpx.AsyncClient") as mock_client: + create_mcp_http_client_with_ssl_option() + mock_client.assert_called_once() + assert mock_client.call_args.kwargs.get("verify") is True @@ - client = create_mcp_http_client_with_ssl_option(verify_ssl=False) - - # Verify client was created successfully - assert isinstance(client, type(client)) + with patch("lfx.base.mcp.util.httpx.AsyncClient") as mock_client: + create_mcp_http_client_with_ssl_option(verify_ssl=False) + mock_client.assert_called_once() + assert mock_client.call_args.kwargs.get("verify") is False @@ - client = create_mcp_http_client_with_ssl_option(headers=test_headers, verify_ssl=False) - - # Verify headers are set correctly - assert client.headers["Authorization"] == "Bearer token123" - assert client.headers["X-API-Key"] == "secret" + with patch("lfx.base.mcp.util.httpx.AsyncClient") as mock_client: + create_mcp_http_client_with_ssl_option(headers=test_headers, verify_ssl=False) + called_headers = mock_client.call_args.kwargs.get("headers") + assert called_headers == test_headers @@ - client = create_mcp_http_client_with_ssl_option(timeout=custom_timeout, verify_ssl=False) - - # Verify timeout is set correctly - httpx.Timeout has read attribute - assert client.timeout.read == 120.0 + with patch("lfx.base.mcp.util.httpx.AsyncClient") as mock_client: + create_mcp_http_client_with_ssl_option(timeout=custom_timeout, verify_ssl=False) + assert mock_client.call_args.kwargs.get("timeout") == custom_timeout @@ - client = create_mcp_http_client_with_ssl_option(auth=auth, verify_ssl=False) - - # Verify auth is set correctly - assert client.auth == auth + with patch("lfx.base.mcp.util.httpx.AsyncClient") as mock_client: + create_mcp_http_client_with_ssl_option(auth=auth, verify_ssl=False) + assert mock_client.call_args.kwargs.get("auth") == auth @@ - client = create_mcp_http_client_with_ssl_option( - headers=test_headers, timeout=custom_timeout, auth=auth, verify_ssl=False - ) - - # Verify all options are set correctly - assert client.headers["Authorization"] == "Bearer token123" - assert client.timeout == custom_timeout - assert client.auth == auth + with patch("lfx.base.mcp.util.httpx.AsyncClient") as mock_client: + create_mcp_http_client_with_ssl_option( + headers=test_headers, timeout=custom_timeout, auth=auth, verify_ssl=False + ) + kwargs = mock_client.call_args.kwargs + assert kwargs.get("headers") == test_headers + assert kwargs.get("timeout") == custom_timeout + assert kwargs.get("auth") == auth
1833-1911: Good coverage for update_tools verify_ssl behavior
- Verifies False is passed through when specified.
- Verifies default to True when omitted.
Consider also asserting headers normalization with _process_headers in a follow-up.
src/lfx/src/lfx/base/mcp/util.py (2)
68-105: httpx AsyncClient factory with verify flag — solidClear API, sensible defaults, and explicit verify wiring. Consider allowing explicit connect/read/write timeouts later if needed.
710-745: Guard httpx_client_factory kwarg for MCP SDK compatibilityPassing httpx_client_factory assumes the installed MCP SDK supports it. To avoid runtime TypeError on older versions, gate on the callable’s signature.
@@ - def custom_httpx_factory( + def custom_httpx_factory( headers: dict[str, str] | None = None, timeout: httpx.Timeout | None = None, auth: httpx.Auth | None = None, ) -> httpx.AsyncClient: return create_mcp_http_client_with_ssl_option( headers=headers, timeout=timeout, auth=auth, verify_ssl=verify_ssl ) @@ - async with streamablehttp_client( - url=connection_params["url"], - headers=connection_params["headers"], - timeout=connection_params["timeout_seconds"], - httpx_client_factory=custom_httpx_factory, - ) as (read, write, _): + import inspect as _ins + _kwargs = dict( + url=connection_params["url"], + headers=connection_params["headers"], + timeout=connection_params["timeout_seconds"], + ) + if "httpx_client_factory" in _ins.signature(streamablehttp_client).parameters: + _kwargs["httpx_client_factory"] = custom_httpx_factory + async with streamablehttp_client(**_kwargs) as (read, write, _): @@ - async with sse_client( - connection_params["url"], - connection_params["headers"], - connection_params["timeout_seconds"], - sse_read_timeout, - httpx_client_factory=custom_httpx_factory, - ) as (read, write): + import inspect as _ins + _args = [ + connection_params["url"], + connection_params["headers"], + connection_params["timeout_seconds"], + sse_read_timeout, + ] + _kwargs = {} + if "httpx_client_factory" in _ins.signature(sse_client).parameters: + _kwargs["httpx_client_factory"] = custom_httpx_factory + async with sse_client(*_args, **_kwargs) as (read, write):Please confirm your MCP SDK version exposes httpx_client_factory on streamablehttp_client and sse_client. If yes, the guard is optional; if not, it prevents a crash. Based on learnings
Also applies to: 756-761, 796-802
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json(3 hunks)src/backend/tests/unit/base/mcp/test_mcp_util.py(1 hunks)src/lfx/src/lfx/base/mcp/util.py(9 hunks)src/lfx/src/lfx/components/agents/mcp_component.py(3 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/tests/unit/base/mcp/test_mcp_util.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/base/mcp/test_mcp_util.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/base/mcp/test_mcp_util.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/base/mcp/test_mcp_util.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/base/mcp/test_mcp_util.py
🧬 Code graph analysis (2)
src/lfx/src/lfx/components/agents/mcp_component.py (1)
src/lfx/src/lfx/inputs/inputs.py (1)
BoolInput(414-426)
src/backend/tests/unit/base/mcp/test_mcp_util.py (1)
src/lfx/src/lfx/base/mcp/util.py (6)
create_mcp_http_client_with_ssl_option(68-104)MCPStreamableHttpClient(1213-1483)connect_to_server(1048-1052)connect_to_server(1296-1309)MCPSessionManager(438-997)update_tools(1491-1636)
🔇 Additional comments (3)
src/backend/tests/unit/base/mcp/test_mcp_util.py (1)
1741-1771: Nice propagation check for verify_ssl in connect_to_serverGood validation of connection parameters and storage of verify_ssl in _connection_params.
src/lfx/src/lfx/components/agents/mcp_component.py (1)
64-65: Expose and propagate verify_ssl — LGTM
- default_keys includes verify_ssl so it won’t be stripped from build_config.
- BoolInput is clear, advanced, defaults True with a good warning.
- update_tool_list sets server_config["verify_ssl"] when missing.
No issues spotted.
Also applies to: 90-99, 224-228
src/lfx/src/lfx/base/mcp/util.py (1)
1248-1278: verify_ssl propagation through connection params and update_tools — LGTM
- _connect_to_server stores verify_ssl.
- connect_to_server plumbs it through.
- update_tools passes verify_ssl from server_config.
Looks correct and consistent with tests.
Also applies to: 1296-1309, 1538-1540
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
edwinjosechittilappilly
removed
the
lgtm
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
Updated component_index.json to include new Notion integration components: AddContentToPage, NotionDatabaseProperties, NotionListPages, NotionPageContent, NotionPageCreator, NotionPageUpdate, and NotionSearch. These components provide functionality for interacting with Notion databases and pages, including creating, updating, listing, searching, and retrieving content.
@coderabbitai
Summary by CodeRabbit
New Features
Improvements
Tests