fix: correctly pass headers in mcp stdio connections#11804
fix: correctly pass headers in mcp stdio connections#11804jordanrfrazier merged 10 commits intomainfrom
Conversation
* Correctly parse dicts from tweaks * Add test * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
* Fix dict handling of different formats * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) * [autofix.ci] apply automated fixes (attempt 3/3) * cmp index * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
|
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 Use the checkbox below for a quick retry:
WalkthroughThis pull request extends dict-type field handling in apply_tweaks logic across both backend and LFX packages, with special unwrapping for {"value": ...} wrappers. The parameter handler is updated to detect and convert lists with "key"/"value" patterns into flat dicts. MCP utilities are enhanced with improved command parsing using shlex and logic to inject headers into stdio commands with intelligent placement. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 ❌ Your patch status has failed because the patch coverage (14.58%) 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 #11804 +/- ##
==========================================
- Coverage 35.21% 35.19% -0.02%
==========================================
Files 1521 1521
Lines 72967 73008 +41
Branches 10938 10951 +13
==========================================
+ Hits 25695 25698 +3
- Misses 45876 45914 +38
Partials 1396 1396
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/backend/base/langflow/processing/process.py`:
- Around line 161-168: The current unwrapping logic in process.py (inside the
field_type == "dict" branch where template_data[tweak_name]["value"] is set)
naively unwraps any single-key dict {"value": ...}, which will incorrectly
transform a user-intended dict that legitimately has "value" as a key; update
the logic to only unwrap when the inner value is clearly a template-wrapped
scalar/sequence (e.g., if tweak_value == {"value": v} and v is not a dict —
i.e., isinstance(v, (str, int, float, list, bool, None)) — or when a specific
template marker/flag is present), otherwise preserve the original dict intact;
change the code around tweak_value/tweak_name assignment to test the type of
tweak_value["value"] (or check for an explicit marker) before assigning
template_data[tweak_name]["value"] = tweak_value["value"], otherwise assign the
whole tweak_value.
In `@src/lfx/src/lfx/base/mcp/util.py`:
- Around line 1591-1612: The positional-arg heuristic is fragile: instead of
checking not args[-1].startswith("-"), scan the args list treating tokens that
start with "-" as flags and skipping their immediate value (so flag+value are
kept together) to locate the true last positional argument index; then insert
extra_args before that positional token (or append if none). Update the logic
around the args handling block that builds extra_args and the branch that
currently checks args and not args[-1].startswith("-") (the code that mutates
args and later calls shlex.join) to compute a safe insert index by iterating
over args and skipping values after flags before performing the insertion.
| elif field_type == "dict" and isinstance(tweak_value, dict): | ||
| # Dict fields: set the dict directly as the value. | ||
| # If the tweak is wrapped in {"value": <actual>}, unwrap it | ||
| # to support the template-format style (e.g. from UI exports). | ||
| if len(tweak_value) == 1 and "value" in tweak_value: | ||
| template_data[tweak_name]["value"] = tweak_value["value"] | ||
| else: | ||
| template_data[tweak_name]["value"] = tweak_value |
There was a problem hiding this comment.
Ambiguous unwrapping of {"value": ...} for dict fields.
If a user legitimately wants to set a dict-type field to {"value": "some_actual_data"} (where "value" is a real key in their data), this heuristic will unwrap it and assign "some_actual_data" instead. This edge case may be unlikely for headers but could surprise users with generic dict fields.
Consider documenting this trade-off or adding a more explicit signal (e.g., checking if the inner value is a list of key/value pairs) to disambiguate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/backend/base/langflow/processing/process.py` around lines 161 - 168, The
current unwrapping logic in process.py (inside the field_type == "dict" branch
where template_data[tweak_name]["value"] is set) naively unwraps any single-key
dict {"value": ...}, which will incorrectly transform a user-intended dict that
legitimately has "value" as a key; update the logic to only unwrap when the
inner value is clearly a template-wrapped scalar/sequence (e.g., if tweak_value
== {"value": v} and v is not a dict — i.e., isinstance(v, (str, int, float,
list, bool, None)) — or when a specific template marker/flag is present),
otherwise preserve the original dict intact; change the code around
tweak_value/tweak_name assignment to test the type of tweak_value["value"] (or
check for an explicit marker) before assigning
template_data[tweak_name]["value"] = tweak_value["value"], otherwise assign the
whole tweak_value.
| args = list(server_config.get("args", [])) | ||
| env = server_config.get("env", {}) | ||
| full_command = " ".join([command, *args]) | ||
| # For stdio mode, inject component headers as --headers CLI args. | ||
| # This enables passing headers through proxy tools like mcp-proxy | ||
| # that forward them to the upstream HTTP server. | ||
| if headers: | ||
| extra_args = [] | ||
| for key, value in headers.items(): | ||
| extra_args.extend(["--headers", key, str(value)]) | ||
| if "--headers" in args: | ||
| # Insert before the existing --headers flag so all header | ||
| # flags are grouped together | ||
| idx = args.index("--headers") | ||
| for i, arg in enumerate(extra_args): | ||
| args.insert(idx + i, arg) | ||
| elif args and not args[-1].startswith("-"): | ||
| # No existing --headers flag; insert before the last positional arg | ||
| # (typically the URL in mcp-proxy commands) | ||
| args = args[:-1] + extra_args + [args[-1]] | ||
| else: | ||
| args.extend(extra_args) | ||
| full_command = shlex.join([command, *args]) |
There was a problem hiding this comment.
Heuristic for positional arg detection at line 1606 is fragile.
The check not args[-1].startswith("-") assumes the last non-flag arg is a positional argument (like a URL). However, flag values such as "streamablehttp" (from --transport streamablehttp) also don't start with "-". If the args list ends with a flag value rather than a positional URL, headers would be injected in the wrong position, breaking the preceding flag.
This works for the known mcp-proxy pattern (... <URL> at the end), but a more robust approach could inspect pairs of flag + value to avoid misidentifying flag values as positional args.
Example of the failure case
# args = ["mcp-proxy", "--transport", "streamablehttp"]
# Last arg "streamablehttp" doesn't start with "-"
# Code would insert headers before "streamablehttp", producing:
# ["mcp-proxy", "--transport", "--headers", "auth", "token", "streamablehttp"]
# This breaks the --transport flag🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lfx/src/lfx/base/mcp/util.py` around lines 1591 - 1612, The
positional-arg heuristic is fragile: instead of checking not
args[-1].startswith("-"), scan the args list treating tokens that start with "-"
as flags and skipping their immediate value (so flag+value are kept together) to
locate the true last positional argument index; then insert extra_args before
that positional token (or append if none). Update the logic around the args
handling block that builds extra_args and the branch that currently checks args
and not args[-1].startswith("-") (the code that mutates args and later calls
shlex.join) to compute a safe insert index by iterating over args and skipping
values after flags before performing the insertion.
Cherry-picked from release branch to build a nightly with this fix.
ceda88f
6e6ace1
4ff340d
Summary by CodeRabbit
Release Notes
New Features
Improvements