-
Notifications
You must be signed in to change notification settings - Fork 8.2k
feat: pins MCP composer version and adds new setting to configure it #10163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Introduced mcp_composer_version attribute to specify version constraints for mcp-composer using PEP 440 syntax. - Implemented a field validator to ensure version strings have appropriate specifier prefixes, defaulting to '~=0.1.0.7' if none is provided. - Enhanced documentation for clarity on versioning behavior and validation logic.
…uction - Updated the command for starting the MCP Composer subprocess to include the mcp_composer_version from settings, ensuring the correct version is used. - Enhanced the installation function to reflect the same change, improving consistency across the codebase.
|
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 WalkthroughUpdates configure MCP Composer to use a versioned executable name derived from settings. Adds a new mcp_composer_version setting with validation, and adjusts both API and service layers to fetch settings and construct mcp-composer{version}. Adds a comment for secret handling in OAuth env mapping. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as mcp_projects.install_mcp_config
participant Settings as SettingsService
participant OS as System Process
Client->>API: install_mcp_config(use_mcp_composer=True)
API->>Settings: get_settings_service().settings
Settings-->>API: settings (mcp_composer_version)
API->>API: Build cmd "mcp-composer{version}"
API->>OS: Spawn composer process with env
OS-->>API: Process result
API-->>Client: Return outcome
sequenceDiagram
autonumber
participant Service as mcp_composer.service._start_project_composer_process
participant Settings as SettingsService
participant OS as System Process
Service->>Settings: get_settings_service().settings
Settings-->>Service: settings (mcp_composer_version)
Service->>Service: Build exec "mcp-composer{version}"
Note right of Service: OAuth env includes client_secret <!-- allowlist secret -->
Service->>OS: Start process with env
OS-->>Service: PID/handle
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 error, 2 warnings, 1 inconclusive)
✅ Passed checks (3 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/lfx/src/lfx/services/settings/base.py (2)
354-380: Consider moving thereimport to module level.The validator logic correctly handles bare versions and existing specifiers. However, importing
reinside the validator function (line 373) is slightly inefficient, even though validators only run at initialization time.Consider moving the import to the module level for better practice:
import asyncio import contextlib import json import os +import re from pathlib import Path from shutil import copy2 from typing import Any, LiteralThen remove the local import from the validator:
# If it's a bare version number, add ~= prefix # This regex matches version numbers like 0.1.0.7, 1.2.3, etc. - import re - if re.match(r"^\d+(\.\d+)*", value):
288-290: Approve code changes
- Default
~=0.1.0.7matches the latest PyPI release (0.1.0.7).- Optional: move the
import restatement to module scope to avoid repeated imports in the validator.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/backend/base/langflow/api/v1/mcp_projects.py(1 hunks)src/lfx/src/lfx/services/mcp_composer/service.py(2 hunks)src/lfx/src/lfx/services/settings/base.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{src/backend/**/*.py,tests/**/*.py,Makefile}
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
{src/backend/**/*.py,tests/**/*.py,Makefile}: Run make format_backend to format Python code before linting or committing changes
Run make lint to perform linting checks on backend Python code
Files:
src/backend/base/langflow/api/v1/mcp_projects.py
🧬 Code graph analysis (2)
src/backend/base/langflow/api/v1/mcp_projects.py (1)
src/backend/base/langflow/services/deps.py (1)
get_settings_service(111-124)
src/lfx/src/lfx/services/mcp_composer/service.py (1)
src/backend/base/langflow/services/deps.py (1)
get_settings_service(111-124)
⏰ 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). (18)
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 6/9
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 9/9
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 8/9
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 7/9
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 5/9
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 3/9
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 1/9
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 2/9
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 4/9
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 2
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 4
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 1
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
- GitHub Check: Run Backend Tests / Integration Tests - Python 3.10
- GitHub Check: Test Starter Templates
- GitHub Check: Update Starter Projects
- GitHub Check: Optimize new Python code in this PR
🔇 Additional comments (3)
src/backend/base/langflow/api/v1/mcp_projects.py (1)
632-635: LGTM! Versioned MCP Composer command construction.The implementation correctly fetches the configured version and constructs the MCP Composer command name. The resulting format (e.g.,
mcp-composer~=0.1.0.7) is valid for uvx and follows PEP 440 version specifier syntax.src/lfx/src/lfx/services/mcp_composer/service.py (2)
420-423: LGTM! Consistent versioned command construction.The implementation correctly mirrors the pattern used in
mcp_projects.py, ensuring consistency across the codebase.
451-451: LGTM! Security scanning directive added.The pragma comment is a standard directive for security scanning tools to prevent false positives when handling OAuth client secrets in environment variable mappings.
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #10163 +/- ##
==========================================
- Coverage 24.16% 23.41% -0.75%
==========================================
Files 1087 1075 -12
Lines 40072 39963 -109
Branches 5546 5530 -16
==========================================
- Hits 9682 9358 -324
- Misses 30219 30444 +225
+ Partials 171 161 -10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Introduced tests for the mcp_composer_version validator, covering various version formats and ensuring correct behavior for both valid and default cases. - Created a new test file for settings services, enhancing test coverage and documentation for the versioning logic.
|
lucaseduoli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!



Summary by CodeRabbit