-
Notifications
You must be signed in to change notification settings - Fork 8.2k
fix: Convert object strings to object for MCP tools #10575
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
base: main
Are you sure you want to change the base?
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 WalkthroughAdds automatic JSON string parsing for fields annotated as Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–12 minutes
Pre-merge checks and finishing touchesImportant Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 warnings, 2 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 |
|
https://github.com/search?q=repo%3Amongodb-js%2Fmongodb-mcp-server%20object(%7B%7D)&type=code reveals the open-ended |
4930efa to
381cb5e
Compare
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lfx/src/lfx/base/mcp/util.py(1 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). (24)
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 13/13
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 9/13
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 12/13
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 11/13
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 7/13
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 10/13
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 5/13
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 4/13
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 6/13
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 8/13
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 1/13
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 3/13
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 2/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: Lint Backend / Run Mypy (3.11)
- 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 2
- GitHub Check: Run Backend Tests / LFX Tests - Python 3.10
- GitHub Check: Lint Backend / Run Mypy (3.12)
- GitHub Check: Run Backend Tests / Integration Tests - Python 3.10
- GitHub Check: Run Frontend Unit Tests / Frontend Jest Unit Tests
- GitHub Check: Test Starter Templates
src/lfx/src/lfx/base/mcp/util.py
Outdated
| for arg in provided_args.items(): | ||
| if "AnonModel" in str(arg_schema.model_fields[arg].annotation) and isinstance(provided_args[arg], str): | ||
| provided_args[arg] = json.loads(provided_args[arg]) |
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.
Critical bug: incorrect loop variable unpacking causes runtime failure.
Line 319 iterates over provided_args.items(), which returns (key, value) tuples. However, lines 320-321 treat arg as if it's the key string, when it's actually a tuple. This will cause a TypeError or KeyError immediately when the code executes.
Additionally, there's no error handling for json.loads(), which can raise JSONDecodeError on invalid JSON input.
Apply this diff to fix the bug and add error handling:
- for arg in provided_args.items():
- if "AnonModel" in str(arg_schema.model_fields[arg].annotation) and isinstance(provided_args[arg], str):
- provided_args[arg] = json.loads(provided_args[arg])
+ for arg_name, arg_value in provided_args.items():
+ # Skip if field doesn't exist in schema
+ if arg_name not in arg_schema.model_fields:
+ continue
+ # Parse JSON strings for AnonModel fields
+ if isinstance(arg_value, str) and "AnonModel" in str(arg_schema.model_fields[arg_name].annotation):
+ try:
+ provided_args[arg_name] = json.loads(arg_value)
+ except json.JSONDecodeError as e:
+ # Let validation handle the error with better context
+ logger.warning(f"Failed to parse JSON for field '{arg_name}': {e}")Key fixes:
- Unpack tuple into
arg_nameandarg_value - Check field existence before accessing
model_fields - Reorder checks:
isinstancebefore field access for efficiency - Add try-except for
JSONDecodeErrorwith logging
🤖 Prompt for AI Agents
In src/lfx/src/lfx/base/mcp/util.py around lines 319 to 321, the loop
incorrectly iterates over provided_args.items() as a single variable causing
tuple misuse; change the loop to unpack each item into arg_name and arg_value,
first check isinstance(arg_value, str) before touching arg_schema.model_fields,
verify arg_name exists in arg_schema.model_fields before using it, and if the
field annotation string contains "AnonModel" attempt to json.loads(arg_value)
inside a try/except catching json.JSONDecodeError and log the error (and leave
the original value unchanged on failure).
3a17981 to
d67789b
Compare
db1625c to
2d413da
Compare
|
It can be related to #9881 (flow does not transfer to MCP deep dicts) |
afe8762 to
1496bc3
Compare
fe6d657 to
040d249
Compare
When using an MCP tool directly as a component, rather than via an
agent tool call, provided arguments arrive as strings rather than
objects so the dynamically generated AnonModel pydantic validation
fails. This is observed when using the mongodb-mcp-server with the
"find" tool call when setting filter, projection, or sort fields.
mongodb-mcp-server exposes the schema for these fields only as {},
since there is no fixed schema for these dynamic objects. The error
will look like:
`Error in build_output: Invalid input: 3 validation errors for InputSchema filter Input should be a valid dictionary or instance of AnonModel0 [type=model_type, input_value='', input_type=str] ...`
8a27c3d to
f391916
Compare
When using an MCP tool directly as a component, rather than via an agent tool call, provided arguments arrive as strings rather than objects so the dynamically generated AnonModel pydantic validation fails. This is observed when using the mongodb-mcp-server with the "find" tool call when setting filter, projection, or sort fields. mongodb-mcp-server exposes the schema for these fields only as {}, since there is no fixed schema for these dynamic objects. The error will look like:
Error in build_output: Invalid input: 3 validation errors for InputSchema filter Input should be a valid dictionary or instance of AnonModel0 [type=model_type, input_value='', input_type=str] ...Summary by CodeRabbit