Fix MCP schema normalizer mangling parameter named 'definitions' into '$defs'

[PolyphonyRequiem]

Problem

_normalize_mcp_input_schema (in tools/mcp_tool.py) unconditionally renamed every dict key named definitions to $defs, in order to migrate draft-07 schemas to draft-2020. The recursion didn't distinguish schema keyword positions (where the rename is correct) from user parameter names nested inside properties (where the rename is wrong).

That produced schemas like:

"properties": {
  "$defs": { "type": "array", "items": {...} }
}

Anthropic's tool-input validator requires property keys to match ^[a-zA-Z0-9_.-]{1,64}$, so the whole tool list is rejected with HTTP 400 invalid_request_body the moment one offending tool is registered.

Repro

Azure DevOps MCP (@azure-devops/mcp) defines pipelines_get_builds with a Zod parameter literally named definitions:

server.tool(PIPELINE_TOOLS.pipelines_get_builds, "...", {
    project: z.string()...,
    definitions: z.array(z.coerce.number().min(1)).optional()...,
    // ...
})

After Hermes normalizes the schema, the live tool sent to Anthropic has properties.$defs — and every subsequent assistant turn 400s with:

tools.27.custom.input_schema.properties: Property keys should match
pattern '^[a-zA-Z0-9_.-]{1,64}$'

This breaks every Anthropic-compatible model (Claude direct, Claude via Copilot, etc.) for any user with the ADO MCP enabled, and likely affects a long tail of other Zod-based MCPs whose authors used definitions as a parameter name.

Fix

Two-part change, both in _normalize_mcp_input_schema:

1. Make _rewrite_local_refs boundary-aware. It now tracks whether the recursion is descending through a properties dict and skips the definitions → $defs rename for those keys. The schema-keyword rename still happens everywhere else (root, $defs.X, nested object schemas, etc.).

2. Add a defensive _sanitize_property_keys pass. Anything that still fails Anthropic's regex (e.g. $weird, has spaces, unicode, over-64-char keys from some other MCP we haven't seen yet) gets renamed in place, with the required array rewritten in lockstep and a warning logged that names the offending tool and key. Catches future converters before they reach the wire.

Tests

tests/tools/test_mcp_tool.py gains 4 new cases:

• test_property_named_definitions_is_not_renamed — the direct regression for the ADO bug.
• test_property_named_definitions_alongside_schema_definitions — the schema-keyword form still gets rewritten when both forms appear together (covers refs into $defs).
• test_invalid_property_keys_are_sanitized — $weird, has spaces get renamed; required is rewritten too.
• test_valid_property_keys_are_unchanged — sanitizer is a no-op on clean schemas (including kebab-case, dotted, snake_case).

All 242 tests in tests/tools/test_mcp_tool.py and tests/agent/test_moonshot_schema.py pass.

Live verification

Reproduced the bug end-to-end against real ADO MCP output (90 tools loaded):

• Before: mcp_ado_pipelines_get_builds (index 27): properties.$defs — fails regex.
• After: mcp_ado_pipelines_get_builds: properties.definitions (type=array, description preserved). All 121 tools in the live agent pass the Anthropic regex.

Notes for reviewers

• The sanitizer logs at warning level via the module's existing logger, so users see noisy MCPs without surprise. The warning includes the path, renamed key pair, and tool name so server authors can fix upstream.
• Renaming is deterministic and stable across runs (no random suffixes).
• Collisions (e.g. an MCP with both weird and $weird) get an integer suffix (_weird, _weird_1, …).
• Upstream @azure-devops/mcp would benefit from renaming the param to buildDefinitions or piping the result, but Hermes shouldn't break on the existing schema in the meantime.

[alt-glitch]

Duplicate of #30491, which fixes the same root cause — _normalize_mcp_input_schema unconditionally renaming definitions → $defs inside properties, producing property keys that fail Anthropic's ^[a-zA-Z0-9_.-]{1,64}$ validator and 400'ing the whole tool array.

This PR is a superset (adds a defensive _sanitize_property_keys pass for other invalid keys like $weird/spaces/over-64-char), so maintainers may prefer to consolidate. Flagging so the two aren't merged independently.

[PolyphonyRequiem]

Thanks for the cross-link @alt-glitch — confirmed, this and #30491 fix the same root cause in _normalize_mcp_input_schema (the blind definitions → `` rename descending into properties dicts). #30491 was opened first, so I'm happy to defer to it for the core fix.

If maintainers want to land #30491, I'll close this one and re-submit the defensive _sanitize_property_keys pass (the Anthropic-regex belt-and-suspenders that handles $weird, spaces, and >64-char keys from looser MCP servers) as a small follow-up PR on top — that's an independent concern and worth separating either way.

Either path works for me — just flagging here so reviewers can pick one and I'll rebase accordingly.

[teknium1]

Thanks for the clear reproduction and the careful regression coverage.

This is an automated hermes-sweeper review. The reported behavior is already implemented on current main:

• dd22c2f533 (salvaged through merged PR fix(mcp): preserve 'definitions' as a property name in tool schemas #56150) preserves names inside properties and patternProperties in tools/mcp_tool.py:4371-4379, while retaining schema-keyword and local-reference normalization.
• Current regression coverage in tests/tools/test_mcp_tool.py:328-409 verifies both a property literally named definitions and coexistence with the root schema definitions keyword.
• This matches the duplicate/superset discussion linking fix(mcp): preserve 'definitions' as a property name in tool schemas #30491; its core fix has shipped in release v2026.7.1.

Closing as implemented on main.