Conversation
The bridge read `tool.inputSchema` and `result.isError`. Those are MCP's
JSON names, but the `mcp` Python SDK exposes `input_schema` / `is_error`
as the attributes and keeps the camelCase spellings as pydantic aliases.
Against mcp 2.x both reads therefore returned the default, silently:
- every registered MCP tool was advertised to the model with an empty
parameter schema, so a tool that needs arguments looked like it took
none — verified against a real stdio server, where `echo(message)`
came through as `{"type": "object", "properties": {}}`
- every failed call looked successful, handing the model a failure
message formatted as ordinary output
Neither failed loudly, and the tests couldn't catch either: their mocks
used the same spelling as the code. `_mcp_field()` now reads the first
present of several names (attribute, then key), and the tests gained a
`_sdk_tool` factory shaped like the SDK's own model. Both new tests fail
against the previous code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens the MCP bridge against SDK field-name/alias differences (camelCase wire JSON vs snake_case SDK attributes) and adds regression tests to prevent silent schema/error-loss.
Changes:
- Added
_mcp_fieldhelper to read MCP fields via multiple spellings (attribute-first, then dict key). - Updated tool registration and tool-call error detection to use
_mcp_fieldforinputSchema/input_schemaandisError/is_error. - Added tests covering SDK-shaped tool objects and call results.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/test_mcp_bridge.py | Adds regression tests and SDK-shaped mocks to validate schema passthrough and error detection under snake_case attributes. |
| prompture/integrations/mcp_bridge.py | Introduces _mcp_field and uses it to support both camelCase and snake_case MCP field spellings. |
Comment on lines
+79
to
+83
| if isinstance(obj, dict): | ||
| for name in names: | ||
| if obj.get(name) is not None: | ||
| return obj[name] | ||
| return default |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bridge read
tool.inputSchemaandresult.isError. Those are MCP's JSON names, but themcpPython SDK exposesinput_schema/is_erroras the attributes and keeps the camelCase spellings as pydantic aliases. Against mcp 2.x both reads therefore returned the default, silently:echo(message)came through as{"type": "object", "properties": {}}Neither failed loudly, and the tests couldn't catch either: their mocks used the same spelling as the code.
_mcp_field()now reads the first present of several names (attribute, then key), and the tests gained a_sdk_toolfactory shaped like the SDK's own model. Both new tests fail against the previous code.