Skip to content

feat: Add cache toggle to MCPToolsComponent and tests#9931

Merged
edwinjosechittilappilly merged 12 commits into
mainfrom
mcp-tool-cache-control
Sep 23, 2025
Merged

feat: Add cache toggle to MCPToolsComponent and tests#9931
edwinjosechittilappilly merged 12 commits into
mainfrom
mcp-tool-cache-control

Conversation

@edwinjosechittilappilly
Copy link
Copy Markdown
Collaborator

@edwinjosechittilappilly edwinjosechittilappilly commented Sep 19, 2025

Introduces a 'use_cache' BoolInput to MCPToolsComponent, allowing users to enable or disable caching of MCP tools for performance or freshness. Updates the component logic to respect the cache toggle and adds comprehensive unit tests covering cache enabled, disabled, toggle, and edge cases.

Summary by CodeRabbit

  • New Features

    • Added an optional "Use Cache" toggle to MCP Tools for faster, per-server tool loading; Nvidia Remix starter exposes the new toggle (advanced, off by default).
  • Bug Fixes

    • Improved error handling and automatic cleanup for corrupted cache entries.
    • Avoids unnecessary reloads when tool data is already cached.
  • Tests

    • Added comprehensive tests covering cache enable/disable, edge cases, persistence, and integration with build outputs.

Introduces a 'use_cache' BoolInput to MCPToolsComponent, allowing users to enable or disable caching of MCP tools for performance or freshness. Updates the component logic to respect the cache toggle and adds comprehensive unit tests covering cache enabled, disabled, toggle, and edge cases.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 19, 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

Adds a configurable per-server caching toggle (use_cache) to MCPToolsComponent, implements guarded shared-cache read/write and corruption handling in update_tool_list/update_build_config, updates the Nvidia Remix starter JSON to expose the field, and adds unit tests covering cache behaviors and edge cases.

Changes

Cohort / File(s) Summary
MCPTools component caching
src/lfx/src/lfx/components/agents/mcp_component.py
Adds BoolInput use_cache to public inputs and default_keys; implements guarded shared-cache reads/writes in update_tool_list and update_build_config, handles corrupted cache entries, and gates cache behavior by use_cache.
Starter project schema update
src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json
Adds use_cache to the MCP Tools component public inputs/template and updates default key visibility in the starter project JSON.
Unit tests for caching
src/backend/tests/unit/components/agents/test_mcp_component_cache.py
New async unit tests validating cache enable/disable flows, persistence across component instances, corrupted cache cleanup, build-config integration, and error scenarios using mocks and shared cache helpers.

Sequence Diagram(s)

sequenceDiagram
  actor User
  participant UI as UI / Starter JSON
  participant MCP as MCPToolsComponent
  participant Cache as SharedCache (servers[server_name])
  participant API as DB/API (update_tools)

  User->>UI: select server, toggle use_cache
  UI->>MCP: call update/build with {server_name, use_cache}

  alt use_cache = true
    MCP->>Cache: safe_get(server_name)
    alt cache hit & valid
      Cache-->>MCP: {tools, tool_names, tool_cache, config}
      MCP-->>UI: return cached options/config
    else cache miss or corrupted
      MCP->>API: fetch tools
      API-->>MCP: tools, tool_names, tool_cache, config
      MCP->>Cache: safe_set(server_name, data)
      MCP-->>UI: return fetched options/config
    end
  else use_cache = false
    MCP->>API: fetch tools
    API-->>MCP: tools, tool_names, tool_cache, config
    MCP-->>UI: return fetched options/config
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

size:XXL, lgtm

Suggested reviewers

  • erichare
  • lucaseduoli
  • jordanrfrazier

Pre-merge checks and finishing touches

✅ 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 succinctly and accurately summarizes the primary change in the PR — adding a cache toggle to MCPToolsComponent — and also notes the accompanying tests; it is concise, specific, and directly reflects the changes described in the diff and PR objectives.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

@edwinjosechittilappilly
Copy link
Copy Markdown
Collaborator Author

@coderabbitai review the pr

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 19, 2025

@edwinjosechittilappilly I'll review the PR changes for you, focusing on the cache toggle implementation for MCPToolsComponent and the associated tests.


🧠 Learnings used
Learnt from: edwinjosechittilappilly
PR: langflow-ai/langflow#8504
File: src/backend/base/langflow/initial_setup/starter_projects/Image Sentiment Analysis.json:391-393
Timestamp: 2025-06-12T15:25:01.072Z
Learning: The repository owner prefers CodeRabbit not to review or comment on JSON files because they are autogenerated.
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Sep 19, 2025
@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Sep 19, 2025
Copy link
Copy Markdown
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: 4

🧹 Nitpick comments (9)
src/lfx/src/lfx/components/agents/mcp_component.py (3)

147-151: Bypass cache when incoming server config differs from cached config.

Currently, any present cache wins even if a new config is provided, which can serve stale tools. Compare configs and treat as miss if they differ.

Apply this diff:

@@
-        if use_cache:
-            servers_cache = safe_cache_get(self._shared_component_cache, "servers", {})
-            cached = servers_cache.get(server_name) if isinstance(servers_cache, dict) else None
+        if use_cache:
+            servers_cache = safe_cache_get(self._shared_component_cache, "servers", {})
+            cached = servers_cache.get(server_name) if isinstance(servers_cache, dict) else None
+            # If caller supplied a config, ensure we don't reuse a mismatched cache
+            if cached is not None and server_config_from_value is not None:
+                try:
+                    if cached.get("config") != server_config_from_value:
+                        cached = None  # force refresh on config change
+                except AttributeError:
+                    cached = None

Also applies to: 153-167


298-361: Honor use_cache toggle when populating tool options via cache.

Logic is sound. Mirror the backend default fix here to avoid accidental cache reads.

Apply this diff (consistent default):

-                    use_cache = getattr(self, "use_cache", True)
+                    use_cache = getattr(self, "use_cache", False)

Also applies to: 320-337


21-27: Class-level mutable defaults can leak state across instances.

schema_inputs, tools, and _tool_cache are class attributes. Prefer instance attributes to avoid cross-instance bleed, especially with caching.

Apply this diff:

 class MCPToolsComponent(ComponentWithCache):
-    schema_inputs: list = []
-    tools: list[StructuredTool] = []
+    schema_inputs: list
+    tools: list[StructuredTool]
     _not_load_actions: bool = False
-    _tool_cache: dict = {}
+    _tool_cache: dict
@@
     def __init__(self, **data) -> None:
         super().__init__(**data)
+        self.schema_inputs = []
+        self.tools = []
+        self._tool_cache = {}
         # Initialize cache keys to avoid CacheMiss when accessing them
         self._ensure_cache_structure()
src/backend/tests/unit/components/agents/test_mcp_component_cache.py (5)

67-71: Use user_id (not _user_id) to satisfy component checks.

update_tool_list validates self.user_id. Setting _user_id may not populate the expected property.

Apply this diff:

-        component_instance._user_id = str(uuid4())  # Required for server fetching
+        # Required for server fetching
+        setattr(component_instance, "user_id", str(uuid4()))

76-81: Rename/make assertion match intent.

The test name says “enabled by default” but it asserts False. Rename for clarity.

Apply this diff:

-    async def test_cache_enabled_by_default(self, component_class, default_kwargs):
-        """Test that cache is enabled by default."""
+    async def test_use_cache_disabled_by_default(self, component_class, default_kwargs):
+        """Test that use_cache is disabled by default."""
         component = await self.component_setup(component_class, default_kwargs)
-        assert getattr(component, "use_cache", False) is False  # Updated to match actual default
+        assert getattr(component, "use_cache", False) is False

509-520: Patch session_scope as an async context manager to avoid masking failures.

Currently patching session_scope without async context manager semantics can raise TypeError and unintentionally satisfy the test. Make the mock an async CM.

Apply this diff:

-        with (
-            patch("langflow.api.v2.mcp.get_server") as mock_get_server,
-            patch("lfx.services.deps.session_scope"),
-            patch("langflow.services.database.models.user.crud.get_user_by_id") as mock_get_user,
-        ):
+        with (
+            patch("langflow.api.v2.mcp.get_server") as mock_get_server,
+            patch("lfx.services.deps.session_scope") as mock_session_scope,
+            patch("langflow.services.database.models.user.crud.get_user_by_id") as mock_get_user,
+        ):
             # Simulate server error
             mock_get_server.side_effect = Exception("Server connection failed")
             mock_get_user.return_value = MagicMock()
+            # Async context manager for `async with session_scope() as db:`
+            mock_cm = AsyncMock()
+            mock_cm.__aenter__.return_value = MagicMock()
+            mock_session_scope.return_value = mock_cm

362-395: Exercise corrupted cache handling via component API (not manual).

You validate TypeError on raw cache access. Add an assertion that update_tool_list clears bad entries automatically, matching code behavior.

Append to this test:

@@
-        # Verify cache is now cleared
+        # Verify cache is now cleared
         servers_cache = safe_cache_get(component._shared_component_cache, "servers", {})
         assert server_name not in servers_cache
+
+        # Reinsert corrupted entry and invoke component to auto-heal
+        safe_cache_set(component._shared_component_cache, "servers", {server_name: "invalid_data"})
+        tools, _ = await component.update_tool_list(server_name)
+        assert tools == [] or isinstance(tools, list)
+        # The corrupted entry should be removed by the component
+        servers_cache = safe_cache_get(component._shared_component_cache, "servers", {})
+        assert server_name not in servers_cache

163-196: These tests validate toggle logic but don’t execute component paths.

They’re fine as logic checks; consider one end-to-end call per branch to guard against refactors.

Also applies to: 198-226

src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json (1)

2448-2448: Scope cached server entries to user/session to avoid cross-user leakage.

servers cache stores server_config; ensure entries are partitioned by user_id/session or avoid persisting sensitive fields. Consider keying as f"{user_id}:{server_name}" and/or redacting secrets.

Would you like a follow-up patch that namespaces the servers cache by user_id and strips sensitive headers before caching?

📜 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 0b67b6f and c13f51a.

📒 Files selected for processing (3)
  • src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json (3 hunks)
  • src/backend/tests/unit/components/agents/test_mcp_component_cache.py (1 hunks)
  • src/lfx/src/lfx/components/agents/mcp_component.py (6 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
src/backend/tests/unit/components/**/*.py

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

src/backend/tests/unit/components/**/*.py: Mirror the component directory structure for unit tests in src/backend/tests/unit/components/
Use ComponentTestBaseWithClient or ComponentTestBaseWithoutClient as base classes for component unit tests
Provide file_names_mapping for backward compatibility in component tests
Create comprehensive unit tests for all new components

Files:

  • src/backend/tests/unit/components/agents/test_mcp_component_cache.py
{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/components/agents/test_mcp_component_cache.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/components/agents/test_mcp_component_cache.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/components/agents/test_mcp_component_cache.py
src/backend/**/*component*.py

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

In your Python component class, set the icon attribute to a string matching the frontend icon mapping exactly (case-sensitive).

Files:

  • src/backend/tests/unit/components/agents/test_mcp_component_cache.py
src/backend/**/components/**/*.py

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

In your Python component class, set the icon attribute to a string matching the frontend icon mapping exactly (case-sensitive).

Files:

  • src/backend/tests/unit/components/agents/test_mcp_component_cache.py
🧠 Learnings (2)
📚 Learning: 2025-07-18T18:25:54.486Z
Learnt from: CR
PR: langflow-ai/langflow#0
File: .cursor/rules/backend_development.mdc:0-0
Timestamp: 2025-07-18T18:25:54.486Z
Learning: Applies to src/backend/tests/unit/components/**/*.py : Create comprehensive unit tests for all new components

Applied to files:

  • src/backend/tests/unit/components/agents/test_mcp_component_cache.py
📚 Learning: 2025-07-21T14:16:14.125Z
Learnt from: CR
PR: langflow-ai/langflow#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-21T14:16:14.125Z
Learning: Applies to src/backend/tests/**/*.py : Test component configuration updates in backend Python tests by asserting correct updates to build configuration objects.

Applied to files:

  • src/backend/tests/unit/components/agents/test_mcp_component_cache.py
🧬 Code graph analysis (1)
src/backend/tests/unit/components/agents/test_mcp_component_cache.py (2)
src/lfx/src/lfx/components/agents/mcp_component.py (4)
  • MCPToolsComponent (21-543)
  • update_tool_list (129-237)
  • update_build_config (239-381)
  • build_output (485-520)
src/backend/tests/base.py (1)
  • ComponentTestBaseWithoutClient (166-167)
⏰ 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). (56)
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 38/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 39/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 28/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 37/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 34/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 40/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 36/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 31/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 32/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 33/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 35/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 30/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 26/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 29/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 27/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 25/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 22/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 24/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 23/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 14/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 19/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 21/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 5/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 18/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 17/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 15/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 20/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 10/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 16/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 11/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 9/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 13/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 8/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 12/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 7/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 6/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 4/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 2/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 1/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 3/40
  • GitHub Check: Lint Backend / Run Mypy (3.11)
  • GitHub Check: Lint Backend / Run Mypy (3.12)
  • GitHub Check: Lint Backend / Run Mypy (3.10)
  • GitHub Check: Lint Backend / Run Mypy (3.13)
  • 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 2
  • GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 1
  • GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
  • 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: Run Ruff Check and Format
  • GitHub Check: Update Starter Projects
  • GitHub Check: Ruff Style Check (3.13)
  • GitHub Check: test-starter-projects
🔇 Additional comments (11)
src/lfx/src/lfx/components/agents/mcp_component.py (3)

13-13: Import looks good.

BoolInput import is correct and scoped to the new toggle.


153-166: Corrupted cache handling: good defensive read + cleanup.

Catching TypeError/KeyError/AttributeError and clearing the entry avoids repeated failures.


213-227: Conditional cache writes and safe updates look solid.

Only writing when enabled and guarding the shared dict is correct.

If multiple components share _shared_component_cache across processes, confirm the underlying cache is process-safe; otherwise document single-process scope.

src/backend/tests/unit/components/agents/test_mcp_component_cache.py (7)

22-30: Verify/file_names_mapping: ensure backward‑compat mapping is not required.

Empty mapping may be fine; if the component moved across versions, add VersionComponentMapping entries to keep tests stable across Langflow versions.


142-157: Good: asserts cache short‑circuit without calling update_tools.

Solid isolation by pre-populating cache and verifying no external calls.


431-462: Nice: build_config respects cached options.

Good coverage for dropdown population from cache when enabled.


467-502: Solid: build_output path with cached exec tool is well-isolated.

Mocking coroutine + inputs avoids external calls; DataFrame assertion is appropriate.


231-280: Useful toggle transition coverage.

Good state transition assertions across enable/disable.

Also applies to: 282-329


334-346: Edge cases for empty/None server names are covered.

Good to see early-return behavior validated.

Also applies to: 348-361


397-427: Cache persistence across instances is validated.

Confirms shared component cache behavior. Good.

src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json (1)

4926-4943: LGTM: Use Cache input wiring is correct and matches behavior.

Public input name, help text, and default False are consistent with the component logic and tests.

Comment on lines +56 to 57
"use_cache",
]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Align use_cache defaults across UI and backend (prevent surprising behavior).

UI default is False, but getattr defaults to True in backend. This can silently enable caching when the attribute is missing (e.g., programmatic use, legacy flows). Make backend default False too.

Apply this diff:

@@
-        BoolInput(
+        BoolInput(
             name="use_cache",
             display_name="Use Cache",
             info="Enable caching of MCP tools to improve performance. Disable to always fetch fresh tools.",
-            value=False,
+            value=False,  # UI default: disabled
             advanced=True,
         ),
@@
-        use_cache = getattr(self, "use_cache", True)
+        use_cache = getattr(self, "use_cache", False)
@@
-                    use_cache = getattr(self, "use_cache", True)
+                    use_cache = getattr(self, "use_cache", False)

Also applies to: 72-78, 144-145, 322-325

🤖 Prompt for AI Agents
In src/lfx/src/lfx/components/agents/mcp_component.py around lines 56-57, 72-78,
144-145, and 322-325 the backend uses getattr(..., True) for "use_cache" which
defaults to True and conflicts with the UI default False; change those getattr
calls to default to False (i.e., getattr(obj, "use_cache", False)) so missing
attributes do not enable caching, and update any related variable assignments or
conditional checks accordingly to use the new False default.

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 23.42%. Comparing base (0b67b6f) to head (549c564).
⚠️ Report is 2 commits behind head on main.

❌ Your project status has failed because the head coverage (46.01%) 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    #9931      +/-   ##
==========================================
+ Coverage   23.25%   23.42%   +0.17%     
==========================================
  Files        1091     1091              
  Lines       39839    39839              
  Branches     5530     5530              
==========================================
+ Hits         9266     9334      +68     
+ Misses      30402    30334      -68     
  Partials      171      171              
Flag Coverage Δ
backend 46.01% <ø> (+0.45%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 6 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.

@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Sep 19, 2025
Copy link
Copy Markdown
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: 0

♻️ Duplicate comments (4)
src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json (4)

2431-2448: Add tools_metadata to default_keys to avoid KeyError after cleanup.

remove_non_default_keys() drops tools_metadata, but update_build_config reads build_config["tools_metadata"]["show"] multiple times. Preserve it.

 class MCPToolsComponent(ComponentWithCache):
@@
-    default_keys: list[str] = [
+    default_keys: list[str] = [
         "code",
         "_type",
         "tool_mode",
         "tool_placeholder",
         "mcp_server",
         "tool",
+        "tools_metadata",
         "use_cache",
     ]

2431-2448: Guard missing tools_metadata reads.

Avoid direct indexing to prevent crashes when tools_metadata is absent or pruned.

-                except (TimeoutError, asyncio.TimeoutError) as e:
+                except (TimeoutError, asyncio.TimeoutError) as e:
                     msg = f"Timeout updating tool list: {e!s}"
                     await logger.aexception(msg)
-                    if not build_config["tools_metadata"]["show"]:
+                    if not build_config.get("tools_metadata", {}).get("show", False):
                         build_config["tool"]["show"] = True
@@
-                        except ValueError:
-                            if not build_config["tools_metadata"]["show"]:
+                        except ValueError:
+                            if not build_config.get("tools_metadata", {}).get("show", False):
                                 build_config["tool"]["show"] = True
@@
-                is_in_tool_mode = build_config["tools_metadata"]["show"]
+                is_in_tool_mode = build_config.get("tools_metadata", {}).get("show", False)

2431-2448: Honor UI default: make use_cache fallback False.

BoolInput defaults to False; code falls back to True in two places. Align behavior.

-        use_cache = getattr(self, "use_cache", True)
+        use_cache = getattr(self, "use_cache", False)

Apply in:

  • update_tool_list(...)
  • update_build_config(...) branch for "mcp_server" before using servers cache.

2431-2448: Harden build_output against stale cache and missing tool args.

Direct indexing can raise KeyError when cache is stale or tools changed. Fail gracefully.

-                exec_tool = self._tool_cache[self.tool]
-                tool_args = self.get_inputs_for_all_tools(self.tools)[self.tool]
+                exec_tool = self._tool_cache.get(self.tool)
+                if not exec_tool:
+                    return DataFrame(data=[{"error": f"Tool '{self.tool}' not found in cache. Refresh tools or disable cache."}])
+                tool_args_map = self.get_inputs_for_all_tools(self.tools)
+                tool_args = tool_args_map.get(self.tool, [])
🧹 Nitpick comments (3)
src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json (3)

2431-2448: Confirm use of UUID sentinel for tool.value.

Setting build_config["tool"]["value"] = uuid.uuid4() (type UUID) while the input is str may leak through to persistence/UI. If this is only a re-render sentinel and never serialized, OK; otherwise consider "".


2373-2380: Expose use_cache in MCP node field_order (UI consistency).

Field appears in template but is missing from field_order, which can lead to non-deterministic placement.

-              "field_order": [
+              "field_order": [
                 "mode",
                 "command",
                 "env",
                 "sse_url",
                 "headers_input",
+                "use_cache",
                 "tool",
                 "tool_placeholder"
               ],

2585-2592: Fix typo in tools_metadata entry name.

"remix__async_app_docs_endpoint_asyncapi_docs_g" likely should end with "_get" to match the actual tool; otherwise metadata won’t apply.

-                    "name": "remix__async_app_docs_endpoint_asyncapi_docs_g",
+                    "name": "remix__async_app_docs_endpoint_asyncapi_docs_get",
📜 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 c13f51a and 105970d.

📒 Files selected for processing (1)
  • src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json (3 hunks)
⏰ 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). (3)
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 32/40
  • GitHub Check: Run Frontend Tests / Playwright Tests - Shard 21/40
  • GitHub Check: Test Starter Templates
🔇 Additional comments (1)
src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json (1)

4926-4943: LGTM: use_cache BoolInput added with safe default.

Input wiring and default value False look correct and match the PR intent.

Please confirm tests cover toggling use_cache and cache-corruption recovery paths for both stdio and SSE clients.

Comment thread src/lfx/src/lfx/components/agents/mcp_component.py Outdated
Renamed 'Use Cache' to 'Use Cached Server' and clarified the info text to indicate caching applies to both MCP Server and tools, improving user understanding of the option.
@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Sep 22, 2025
@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Sep 22, 2025
@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Sep 22, 2025
Comment thread src/lfx/src/lfx/components/agents/mcp_component.py Outdated
Comment thread src/lfx/src/lfx/components/agents/mcp_component.py Outdated
Comment thread src/lfx/src/lfx/components/agents/mcp_component.py Outdated
@edwinjosechittilappilly edwinjosechittilappilly added this pull request to the merge queue Sep 22, 2025
@github-actions github-actions Bot added the lgtm This PR has been approved by a maintainer label Sep 22, 2025
@edwinjosechittilappilly edwinjosechittilappilly removed this pull request from the merge queue due to a manual request Sep 22, 2025
@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Sep 23, 2025
@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Sep 23, 2025
@sonarqubecloud
Copy link
Copy Markdown

@edwinjosechittilappilly edwinjosechittilappilly added this pull request to the merge queue Sep 23, 2025
Merged via the queue into main with commit ca3425e Sep 23, 2025
75 of 76 checks passed
@edwinjosechittilappilly edwinjosechittilappilly deleted the mcp-tool-cache-control branch September 23, 2025 04:47
dzbanek717 pushed a commit to dzbanek717/langflow that referenced this pull request Oct 1, 2025
* Add cache toggle to MCPToolsComponent and tests

Introduces a 'use_cache' BoolInput to MCPToolsComponent, allowing users to enable or disable caching of MCP tools for performance or freshness. Updates the component logic to respect the cache toggle and adds comprehensive unit tests covering cache enabled, disabled, toggle, and edge cases.

* Update test_mcp_component_cache.py

* consolidate tests

* [autofix.ci] apply automated fixes

* Update cache option labels in MCPToolsComponent

Renamed 'Use Cache' to 'Use Cached Server' and clarified the info text to indicate caching applies to both MCP Server and tools, improving user understanding of the option.

* Update mcp_component.py

* [autofix.ci] apply automated fixes

* Update Nvidia Remix.json

* log updates

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants