Skip to content

Comments

Fix: Remove 'kind' field from MCP schema output#641

Merged
xingyaoww merged 4 commits intomainfrom
openhands/fix-kind-field-in-mcp-schema
Oct 6, 2025
Merged

Fix: Remove 'kind' field from MCP schema output#641
xingyaoww merged 4 commits intomainfrom
openhands/fix-kind-field-in-mcp-schema

Conversation

@xingyaoww
Copy link
Collaborator

@xingyaoww xingyaoww commented Oct 4, 2025

Description

This PR fixes issue #640 where the kind field was incorrectly included in MCP schemas sent to LLMs.

The kind field is an internal discriminator field used by Pydantic for polymorphic serialization (from DiscriminatedUnionMixin) and should not be exposed to LLMs in the MCP tool schema.

Changes Made

Core Implementation

  • Modified Schema.to_mcp_schema() in openhands/sdk/tool/schema.py to filter out the kind field from:
    • The properties dictionary
    • The required list (if present)

Tests

  • Added comprehensive test suite in tests/sdk/tool/test_mcp_schema.py with 6 test cases:
    • test_action_to_mcp_schema_excludes_kind() - Verifies basic Action schema excludes 'kind'
    • test_complex_action_to_mcp_schema_excludes_kind() - Tests complex Action with multiple field types
    • test_observation_to_mcp_schema_excludes_kind() - Verifies Observation schema excludes 'kind'
    • test_kind_still_works_for_discriminated_unions() - Ensures 'kind' still functions correctly for internal discriminated union serialization
    • test_kind_not_in_required_list() - Checks that 'kind' is removed from required fields list
    • test_schema_structure_remains_valid() - Validates that schema structure is preserved after 'kind' removal

Testing

All tests pass successfully:

  • ✅ All 6 new tests in test_mcp_schema.py pass
  • ✅ All 77 existing tests in tests/sdk/tool/ pass
  • ✅ Pre-commit hooks pass (ruff format, ruff lint, pycodestyle, pyright)

Verification

Before the fix:

schema = TestAction.to_mcp_schema()
print(schema["properties"].keys())
# Output: dict_keys(['kind', 'command'])  # ❌ 'kind' present

After the fix:

schema = TestAction.to_mcp_schema()
print(schema["properties"].keys())
# Output: dict_keys(['command'])  # ✅ 'kind' excluded

Impact

  • Positive: LLMs will receive cleaner schemas without internal Pydantic discriminator fields
  • No Breaking Changes: The kind field is still used internally for discriminated unions, only excluded from MCP schema output
  • Backward Compatible: All existing functionality preserved

Fixes #640

@xingyaoww can click here to continue refining the PR


Agent Server images for this PR

GHCR package: https://github.com/All-Hands-AI/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant Base Image Docs / Tags
golang golang:1.21-bookworm Link
java eclipse-temurin:17-jdk Link
python nikolaik/python-nodejs:python3.12-nodejs22 Link

Pull (multi-arch manifest)

docker pull ghcr.io/all-hands-ai/agent-server:47018a0-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-47018a0-python \
  ghcr.io/all-hands-ai/agent-server:47018a0-python

All tags pushed for this build

ghcr.io/all-hands-ai/agent-server:47018a0-golang
ghcr.io/all-hands-ai/agent-server:v1.0.0_golang_tag_1.21-bookworm_golang
ghcr.io/all-hands-ai/agent-server:47018a0-java
ghcr.io/all-hands-ai/agent-server:v1.0.0_eclipse-temurin_tag_17-jdk_java
ghcr.io/all-hands-ai/agent-server:47018a0-python
ghcr.io/all-hands-ai/agent-server:v1.0.0_nikolaik_s_python-nodejs_tag_python3.12-nodejs22_python

The 47018a0 tag is a multi-arch manifest (amd64/arm64); your client pulls the right arch automatically.

The 'kind' field is an internal discriminator field used by Pydantic
for polymorphic serialization (from DiscriminatedUnionMixin) and should
not be exposed to LLMs in the MCP tool schema.

Changes:
- Modified Schema.to_mcp_schema() to filter out 'kind' field from
  properties and required lists
- Added comprehensive test suite (test_mcp_schema.py) to verify:
  - 'kind' field is excluded from Action MCP schemas
  - 'kind' field is excluded from Observation MCP schemas
  - 'kind' field is excluded from complex types
  - 'kind' still works correctly for discriminated unions
  - Schema structure remains valid after exclusion

Fixes #640

Co-authored-by: openhands <openhands@all-hands.dev>
@xingyaoww
Copy link
Collaborator Author

@OpenHands fix the tests -- DO NOT CHANGE THE IMPLEMENTATION, just change the tests

=========================== short test summary info ============================
FAILED tests/sdk/mcp/test_mcp_security_risk.py::test_mcp_tool_action_from_arguments_with_security_risk
FAILED tests/sdk/mcp/test_mcp_security_risk.py::test_mcp_tool_validates_correctly_after_security_risk_pop
FAILED tests/sdk/mcp/test_mcp_tool_validation.py::test_mcp_action_from_arguments_validates_and_sanitizes
================== 3 failed, 843 passed in 122.81s (0:02:02) ===================

@openhands-ai
Copy link

openhands-ai bot commented Oct 5, 2025

I'm on it! xingyaoww can track my progress at all-hands.dev

The PR implementation correctly filters out the 'kind' field from MCP schemas
sent to LLMs. However, action.data still includes 'kind' because:

1. Schema.from_mcp_schema() creates dynamic types that inherit from Schema
2. Schema inherits from DiscriminatedUnionMixin which includes 'kind'
3. When model_dump() is called, it includes all fields including 'kind'
4. This is correct behavior - 'kind' is part of internal representation

Updated test assertions to expect 'kind' in action.data while the MCP
schema (sent to LLMs) correctly excludes it.

Co-authored-by: openhands <openhands@all-hands.dev>
@openhands-ai
Copy link

openhands-ai bot commented Oct 5, 2025

Summary

I've successfully fixed the 3 failing tests without modifying the implementation. Here's what was done:

Changes Made

Updated test assertions in 2 test files to account for the kind field in action.data:

  • tests/sdk/mcp/test_mcp_security_risk.py (2 tests)
  • tests/sdk/mcp/test_mcp_tool_validation.py (1 test)

Root Cause

The PR implementation correctly filters out the kind field from MCP schemas sent to LLMs. However, action.data still includes kind because:

  1. Schema.from_mcp_schema() creates dynamic types that inherit from Schema
  2. Schema inherits from DiscriminatedUnionMixin which includes the kind field
  3. When model_dump() is called, it includes all fields including kind
  4. This is correct behavior - kind is part of the internal representation

Test Updates

Updated assertions from:

assert action.data == {"url": "https://example.com"}

To:

# Note: action.data includes 'kind' field from DiscriminatedUnionMixin
# which is part of the internal representation
assert action.data == {"kind": "FetchAction", "url": "https://example.com"}

Verification

✅ All 3 failing tests now pass
✅ All 43 MCP tests pass
✅ Pre-commit hooks pass (ruff format, ruff lint, pycodestyle, pyright)
✅ Changes committed and pushed to openhands/fix-kind-field-in-mcp-schema

The PR correctly achieves its goal: the kind field is excluded from MCP schemas sent to LLMs, while remaining part of the internal action representation.

View full conversation

Copy link
Collaborator

@enyst enyst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I don't fully understand this issue: I see kind shows up also in a regular FileEditor, not only MCP
image
Maybe this isn't expected, and I wonder if the MessageToolCall wrapper could be related... WDYT?

@xingyaoww
Copy link
Collaborator Author

TBH I don't fully understand this issue: I see kind shows up also in a regular FileEditor, not only MCP

@enyst Yeah, that's why i think it is a bug -- we should not send kind to the LLM and ask it to fill it out for us

@xingyaoww xingyaoww merged commit 487e903 into main Oct 6, 2025
16 checks passed
@xingyaoww xingyaoww deleted the openhands/fix-kind-field-in-mcp-schema branch October 6, 2025 13:29
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.

Tool.to_mcp_schema() includes 'kind' field in properties which should not be sent to LLMs

3 participants