fix(mcp): fix validation, session isolation, and update_flow_from_spec parity - #12528
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR contains dependency version updates across multiple packages in component_index.json, alongside a significant refactoring of MCP server session management, flow validation, creation, and update operations, supported by comprehensive new test coverage. ChangesDependency Updates
MCP Server Flow Management
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 6 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-1.10.0 #12528 +/- ##
==================================================
+ Coverage 55.24% 55.90% +0.65%
==================================================
Files 2173 2186 +13
Lines 203469 207824 +4355
Branches 30709 32600 +1891
==================================================
+ Hits 112404 116181 +3777
- Misses 89781 90289 +508
- Partials 1284 1354 +70
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
@ogabrielluiz Seeing this in the backend tests: |
fa48d7e to
935b8cd
Compare
935b8cd to
db18fee
Compare
validate_flow already tracked how many nodes were expected, but the final success path only checked whether a failing event had been seen. An early end event could therefore return valid=true even when only part of the flow finished. This change treats that condition as invalid and records a flow-level error, while keeping the existing fast-fail behavior unchanged. Constraint: Keep the fix narrowly scoped to validate_flow rather than folding it into broader MCP/session work Rejected: Wait for PR langflow-ai#12528 to land | it does not close this exact success-path gap on current main Confidence: high Scope-risk: narrow Reversibility: clean Directive: If validate_flow semantics change again, preserve the invariant that success means every expected component completed Tested: cd src/lfx && uv sync && uv run pytest tests/unit/mcp/test_validate_flow.py -q Not-tested: End-to-end MCP validation against a live Langflow server Related: langflow-ai#12757
33fddc5 to
33b298a
Compare
Pull request was closed
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
create_flow_from_spec parses YAML config blocks, so numeric / boolean
inputs naturally land as int / float / bool primitives. The flat
configure_component path wrote those into the template as-is and the
downstream build then choked: e.g. ChatInput.input_value is a str-typed
MultilineInput, but YAML 'A.input_value: 42' delivered int 42 and the
Message constructor refused ("The message does not have the required
fields (text, sender, sender_name).") -> RuntimeError 'Flow validation
failed: Build error' rolled the whole spec back.
Adds _coerce_param_value that reads each field's declared 'type' from
the template and coerces str/int/float/bool primitives accordingly.
Containers, code, and unknown shapes pass through unchanged so existing
non-primitive call sites are unaffected. None passes through too --
clearing a field is a valid intent.
bool->str converts to lowercase 'true'/'false' to match the YAML spec
convention (str(True) would give 'True'/'False' capitalized).
Test test_create_flow_from_spec_coerces_numeric_config now passes; the
assertion was tightened to compare against the coerced string form
"42" since the coercion target for input_value is str.
Pre-existing test_create_flow_from_spec_with_tool_mode failure (the
Agent component requires a model + API key to build) is unrelated to
this change -- it fails the same way against the parent commit and is
left for a separate fix.
Agent requires a model + API key to build, but this test only verifies that URLComponent.component_as_tool -> Agent.tools wiring round-trips through the spec parser. Pass validate=False so the test no longer depends on a buildable graph.
Summary
create_flow_from_spec(validate=True)now actually waits for validation to complete instead of fire-and-forgetvalidate_flowfilters poll results by job_id so stale builds from prior runs are not picked upupdate_flow_from_specnow uses server-side tools (configure_component, connect_components) for feature parity with create_flow_from_spec, including dynamic template refresh, prompt variable creation, and component_as_tool auto-enable. Includes rollback on failure to restore original flow datalogin()only closes the current session's client, no longer disrupts other concurrent SSE sessions_set_clientinvalidates the shared registry when the client changes to prevent stale registries after server switchvalidate_flowreturns consistenterrorslist format so callers get proper error messagesSummary by CodeRabbit
Bug Fixes
Improvements
Dependencies