Skip to content

fix(mcp): keep tool schemas available across SDK field names - #918

Open
breedx wants to merge 2 commits into
mpfaffenberger:mainfrom
breedx:fix/upstream-mcp-schema-fields
Open

fix(mcp): keep tool schemas available across SDK field names#918
breedx wants to merge 2 commits into
mpfaffenberger:mainfrom
breedx:fix/upstream-mcp-schema-fields

Conversation

@breedx

@breedx breedx commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

MCP tools can lose schema-based argument conversion when a tool definition exposes input_schema instead of inputSchema. Support both field names so tool calls keep their expected argument types without requiring an SDK upgrade.

Problem

Live schema lookup and cached introspection currently read only the camelCase field. Snake-case-only definitions lose their schema, and definitions with a deprecated camelCase alias still access that alias even when the preferred field exists.

Change

  • Share a snake-case-first accessor between live argument-coercion lookup and cached tool introspection.
  • Read inputSchema only when input_schema is absent or None.
  • Preserve an explicitly empty modern schema instead of substituting a conflicting legacy schema.
  • Replace an auto-vivifying mock in the existing partial-call fixture with explicit attributes.

The accessor does not change connector entry, tool execution policy, or lifecycle behavior. It does not depend on the open MCP capability refactor (#834); that refactor also touches managed_server.py and should retain the shared accessor.

Validation

Tested on Linux / Python 3.13.13 using the unchanged upstream lock, based on 1d25d696. The installed MCP SDK is 1.27.1; its real Tool objects verify legacy compatibility. Modern-field shapes use explicit test doubles, including a deprecated alias that raises if touched.

  • New regression file on unpatched base: 5 failed, 5 passed.
  • Coverage includes modern-only, legacy-only, None fallback, conflicting fields, empty schemas, absent schemas and real SDK Tool instances. Expectations are explicit rather than calculated using the production fallback algorithm.
  • python -m pytest -q -o addopts= tests/mcp/test_schema_field_compatibility.py tests/mcp/test_managed_server.py tests/mcp/test_tool_arg_coercion.py: 106 passed.
  • python -m pytest -q -o addopts= tests/mcp: 483 passed, 6 skipped, no warnings.
  • Ruff lint, format check and git diff --check pass for the changed files.

Tests ran with disposable HOME/XDG directories, no inherited credentials, and socket connect/DNS/bind blocked. All six skips explicitly cover search functionality that is not implemented; none hides a schema regression.

Compatibility and limits

No dependency, package-version or public configuration changes. No live connector or installed SDK-v2 qualification is claimed; this patch tests the two field contracts and the currently pinned SDK.

@breedx

breedx commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

The focused fix remains independently testable, but full CI is red on an existing completion assertion: a quoted ~/... insertion is checked as if it were unquoted. I reproduced the same failure on the unpatched base; the correction is already proposed separately in #916, so this PR stays narrowly scoped.

Details:

  • Quality and Windows encoding checks passed.
  • The macOS test job finished with 1 failed, 7,818 passed, 14 skipped. Its sole failure is tests/test_completions_and_small_modules.py::TestFilePathCompleterMissedLines::test_tilde_prefix_display.
  • On unpatched 1d25d696, python -m pytest -q -o addopts= tests/test_completions_and_small_modules.py::TestFilePathCompleterMissedLines::test_tilde_prefix_display reproduces the same assertion: the insertion is "'~/.../test.txt'", so .startswith("~") is false.
  • This PR changes neither the completion implementation nor that test. fix(completion): apostrophes in prose no longer hide later @file refs #916 updates the assertion to decode the quoted insertion before checking the tilde path.
  • CI's job label says Python 3.13, but its test environment/log paths show Python 3.14.7. The local focused/control results in the PR body were run on Python 3.13.13; these are different environments.

No failing checks or warnings were suppressed. The full CI run also reports deprecation/resource/coroutine warnings beyond the locally reproduced stream warnings; I am not claiming that all of those have independent base controls. No merge-ready or all-green CI claim is made. Once the separate completion correction lands, these unchanged fixes can be requalified on the updated base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant