Python: Fix MCP tool schema normalization for zero-argument tools missing 'properties' key

[giles17]

Motivation and Context

Some MCP servers (e.g. matlab-mcp-core-server) declare zero-argument tools with inputSchema={"type": "object"} but omit the "properties" key. When these schemas are forwarded to the OpenAI API (e.g. via LM Studio's /v1/responses), the API rejects them with a 400 error because it requires "properties" on object-type schemas.

Fixes #4540

Description

The root cause is that MCPTool.load_tools() passed tool.inputSchema directly to FunctionTool without validating that the schema contained a "properties" key. The fix normalizes the inputSchema during tool loading: if the schema has "type": "object" but no "properties" key, an empty "properties": {} dict is injected. A regression test verifies that zero-argument tools get the missing key added while normal tools with existing properties are left unchanged.

Contribution Checklist

• The code builds clean without any errors or warnings
• The PR follows the Contribution Guidelines
• All unit tests pass, and I have added new tests where possible
• Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

---

Note: PR autogenerated by giles17's agent

[giles17]

Automated Code Review

Reviewers: 4 | Confidence: 92%

✓ Correctness

The fix is correct and well-targeted. It normalizes MCP tool inputSchema at ingestion time by ensuring zero-argument tools (which may omit "properties") get an empty "properties" dict injected. The dict(tool.inputSchema) shallow copy on line 908 is appropriate since the only mutation is adding a top-level key. The conditional guard on line 909 correctly checks both type == "object" and the absence of properties. The test covers both the zero-arg and normal-arg cases and validates the fix end-to-end through load_tools(). No correctness issues found.

✓ Security Reliability

The fix correctly normalizes MCP tool inputSchema by injecting an empty properties dict for zero-argument object schemas, preventing OpenAI API 400 errors. The shallow copy via dict(tool.inputSchema) is safe here because tool.inputSchema values are JSON-primitive types (strings, dicts, lists) and the only mutation is adding a top-level key. The condition guards (type == 'object' and 'properties' not in) are correct and narrow. The test covers both zero-arg and normal-arg tools. No security, reliability, or resource-leak concerns.

✓ Test Coverage

The test covers the primary fix (zero-arg tool with missing 'properties') and verifies the no-op path (tool with existing properties is unchanged). Assertions are meaningful — they check specific dict keys and values, not just 'no exception'. However, two edge cases in the normalization logic are untested: (1) a schema with no 'type' key at all (e.g. inputSchema={}), and (2) a non-object type schema (e.g. {"type": "string"}), both of which should NOT have 'properties' injected. These are guarded by the conditional in the production code but verifying them in tests would strengthen confidence. Additionally, the test does not verify that the original tool.inputSchema dict is not mutated by the normalization (the code does dict(tool.inputSchema) to shallow-copy, but there's no assertion confirming this). These are minor gaps — the core behavior is well-tested.

✓ Design Approach

The fix correctly implements the agreed Option C: normalizing inputSchema at MCP ingestion time by injecting an empty "properties" dict when an object-type schema omits it. The placement in load_tools is the right layer — upstream of all clients and serialization paths — and the shallow dict() copy prevents mutation of the original MCP object. The guard condition (type == "object" and "properties" not in schema) is correct and precise. The test covers both the zero-arg (normalized) and normal (unchanged) cases. No design issues found.

Suggestions

• Consider also normalizing schemas where type is missing but the intent is clearly an empty object (e.g., inputSchema={} with no type key), and add a defensive guard for tool.inputSchema being None—e.g., input_schema = dict(tool.inputSchema or {})—to protect against non-compliant MCP servers.
• Add test cases for negative paths: a non-object type schema (e.g. inputSchema={"type": "string"}) and an empty schema (inputSchema={}) to confirm properties is NOT injected, verifying the guard clause works correctly.
• Add an assertion that the original tool.inputSchema dict is not mutated after load_tools() to verify the shallow copy via dict() is working as intended.

---

Automated review by giles17's agents

[markwallace-microsoft]

Python Test Coverage Report •

File	Stmts	Miss	Cover	Missing
packages/core/agent_framework
_mcp.py	527	73	86%	96–97, 107–112, 123, 128, 171, 180, 192–193, 244, 253, 316, 324, 383, 496, 531, 544, 546–549, 568–569, 582–585, 587–588, 592, 649, 684, 686, 690–691, 693–694, 748, 763, 781, 826, 958, 971–976, 1000, 1059–1060, 1066–1068, 1087, 1112–1113, 1117–1121, 1138–1142, 1289
TOTAL	27104	3221	88%

Python Unit Test Overview

Tests	Skipped	Failures	Errors	Time
5265	20 💤	0 ❌	0 🔥	1m 28s ⏱️

[Copilot]

Pull request overview

This PR fixes interoperability with OpenAI-compatible Responses API backends when MCP servers expose zero-argument tools whose inputSchema is { "type": "object" } (missing the required "properties" key). It normalizes the schema during MCP tool loading and adds a regression test to prevent future regressions.

Changes:

• Normalize MCP tool inputSchema in MCPTool.load_tools() by injecting an empty "properties": {} for object schemas missing the key.
• Add a unit test covering both the zero-argument schema normalization case and a “normal” schema that already has properties.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
python/packages/core/agent_framework/_mcp.py	Adds inputSchema normalization during MCP tool loading to satisfy OpenAI schema requirements.
python/packages/core/tests/core/test_mcp.py	Adds a regression test ensuring missing "properties" is injected for zero-arg MCP tools.

---

You can also share your feedback on Copilot code review. Take the survey.

[giles17]

Automated Code Review

Reviewers: 4 | Confidence: 92%

✗ Correctness

The fix correctly normalizes MCP inputSchema at ingestion time by ensuring zero-argument tool schemas (with type 'object' but no 'properties' key) get an empty 'properties' dict injected. The dict(tool.inputSchema or {}) defensive guard against None is good. The non-mutation assertions in the test are valid because dict() creates a shallow copy. However, the test is missing coverage for the None inputSchema case that the or {} guard was specifically added to handle.

✓ Security Reliability

The fix correctly normalizes MCP tool schemas at ingestion to ensure zero-argument tools include an empty 'properties' dict. The dict() copy prevents mutation of the original inputSchema, and the or {} defensively handles a None inputSchema. The test coverage is thorough. One minor gap: the or {} guard for inputSchema=None was added but has no corresponding test case (only {} is tested). No blocking security or reliability issues found.

✗ Test Coverage

The test improvements are solid: edge cases for non-object schemas, empty schemas, and no-mutation assertions are all well covered. However, the code diff introduces a new defensive or {} guard for inputSchema=None (line 908 of _mcp.py), but there is no test exercising that None path. Since inputSchema is typed as dict[str, Any] (non-optional) in the MCP library, this guard is unlikely to fire in practice, but since it was explicitly added it should have a corresponding test to prevent regressions.

✓ Design Approach

The fix correctly implements the agreed Option C: normalizing inputSchema at MCP ingestion in _mcp.py before passing it to FunctionTool, using dict() for a shallow copy to avoid mutating the original schema. The approach is well-targeted — it fixes the root cause (MCP servers emitting {"type": "object"} without "properties") in the one place where raw external schemas enter the framework, protecting all downstream consumers (both LLM clients, serialization, etc.) without touching shared infrastructure. The tests are thorough and correctly verify the mutation-safety invariant. One minor observation: tool.inputSchema is typed dict[str, Any] (non-optional) in the MCP library, so the or {} fallback guards against a case the type system says cannot occur. This is harmless extra defensiveness against hypothetically malformed MCP servers, but the or {} could be slightly misleading to future readers who wonder why a non-nullable field needs a None guard.

Flagged Issues

• The or {} fallback in dict(tool.inputSchema or {}) (line 908 of _mcp.py) is the only behavioral change from the original dict(tool.inputSchema), but there is no test case where inputSchema is None. This new defensive branch has zero coverage and should be tested to prevent regressions.

Suggestions

• Add a test tool with inputSchema=None to verify the or {} fallback produces a function with an empty parameters dict and does not raise. Note: since types.Tool types inputSchema as dict[str, Any], you may need a MagicMock or cast to simulate a None value from a non-conforming MCP server.
• The or {} in dict(tool.inputSchema or {}) is dead code per the MCP type contract. If keeping the guard for robustness against misbehaving servers, add a short inline comment explaining that intent to prevent future confusion. Alternatively, remove or {} and rely on the type contract.

---

Automated review by giles17's agents

[giles17]

Automated Code Review

Reviewers: 4 | Confidence: 88%

✓ Correctness

The diff is minimal and correct. It adds two comment lines documenting the None inputSchema guard (which was already handled by the existing or {} on line 910) and adds a well-structured test case validating that behavior. The dict(tool.inputSchema or {}) expression correctly converts None to an empty dict, the MagicMock is appropriately used since types.Tool wouldn't accept None for inputSchema, and the test assertions are accurate. No correctness issues found.

✓ Security Reliability

This is a minimal defensive fix at the MCP ingestion layer. The dict(tool.inputSchema or {}) guard correctly handles non-conforming MCP servers that send inputSchema=None. The existing normalization for zero-argument tools (adding properties: {} to object schemas) is preserved. The test coverage is appropriate, including the MagicMock workaround for the None case since types.Tool itself would reject None. No security or reliability concerns — the change makes the trust boundary with external MCP servers more robust by defending against unexpected None input.

✓ Test Coverage

The diff adds a defensive guard for inputSchema=None from non-conforming MCP servers and includes a corresponding test case. Test coverage is adequate for the None case and prior edge cases remain well-covered. However, the test for the None schema asserts none_params == {} which verifies the _mcp.py ingestion path, but does not verify the downstream behavior that actually caused the original bug (e.g., that additionalProperties: false and properties: {} would appear when the Responses client builds tools). Additionally, there is a missing edge case for inputSchema={"type": "object", "properties": null} — a plausible malformed server response — which would cause "properties" not in input_schema to be False (the key exists), skipping the fix and later failing at the API level.

✓ Design Approach

The diff correctly implements the agreed Option C fix — normalizing inputSchema at MCP ingestion in _mcp.py before passing it to FunctionTool. The core fix (dict(tool.inputSchema or {}) + injecting "properties": {} for object-typed schemas without one) is well-placed and targeted. A bonus guard for None inputSchema is added, which is sensible. One minor design concern: when inputSchema is None, the fix normalizes to {} (an untyped empty schema), but semantically None most likely means "no parameters" — the same as {"type": "object"}. The current handling does not inject "properties" in this case (since there is no "type": "object" key), so if an API provider also rejects a schema-less {}, the guard is incomplete. This is a low-probability edge case for an already non-conforming server, so it is not blocking, but is worth a conscious decision. Everything else looks correct: mutation safety (copy via dict()), non-object schemas left untouched, and the test covers all key cases.

Suggestions

• When inputSchema is None, consider normalizing all the way to {"type": "object", "properties": {}} instead of {}, so that the "properties" injection logic on the next line fires consistently and strict API providers also receive a valid typed schema. Relatedly, the downstream _responses_client.py line 492 (params["additionalProperties"] = False) would then avoid producing a semantically odd {"additionalProperties": false} for the empty-dict case — though OpenAI appears to tolerate it today.
• Consider adding an end-to-end assertion (or integration-style test) that verifies the None-schema tool produces a valid payload after the Responses client processes it (i.e., with additionalProperties: False and properties: {}), not just that _mcp.py returns {}. Without this, the test covers normalization at ingestion but does not confirm the original bug symptom (OpenAI 400 error) is prevented.
• Consider adding an edge-case test for inputSchema={"type": "object", "properties": null} — a non-conforming server could send properties as null rather than omitting it. The current guard "properties" not in input_schema would not catch this since the key exists, and downstream consumers would see properties: null which may also be rejected by the API.

---

Automated review by giles17's agents