Preserve unmodeled top-level tool schema keywords - #186
Merged
Conversation
ToolArgumentsSchema (backing Tool.InputSchema/OutputSchema) only modeled
$defs/type/properties/required/additionalProperties. A tools/list schema
using any other top-level JSON Schema keyword — oneOf, anyOf, allOf,
$ref, enum, const, patternProperties — was silently dropped on the
unmarshal step, so a schema like {"oneOf": [...]} was gutted before any
consumer saw it. Separately, MarshalJSON emitted "type": tas.Type
unconditionally, fabricating an invalid "type":"" for schemas that
legitimately omit a top-level type (e.g. a top-level oneOf).
- Add an Extra map[string]json.RawMessage catch-all: UnmarshalJSON
captures top-level keywords outside the modeled fields, MarshalJSON
re-emits them (modeled fields still win on any collision).
- Emit "type" only when non-empty, so a schema without a top-level type
no longer gains a spurious "type":"".
- Stop fabricating an empty properties/required for schemas that are not
object-typed and carry none of their own; object schemas keep the
historical always-present properties/required that clients rely on.
Fixes the vMCP tool-schema ingestion fidelity gap tracked in
stacklok/toolhive#5976.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WG3mSjVGWNc8nfgbkdd79
ChrisJBurns
approved these changes
Jul 24, 2026
3 tasks
JAORMX
added a commit
to stacklok/toolhive
that referenced
this pull request
Jul 25, 2026
The §12 tool-schema ingestion fidelity fix landed upstream in stacklok/toolhive-core#186 (ToolArgumentsSchema now preserves top-level JSON Schema keywords like oneOf/anyOf via an Extra catch-all and stops fabricating an empty "type"). Bump toolhive-core to pick it up and unskip the in-repo gate added in #5977, closing the ToolHive half of #5976. The dependency is pinned to the exact #186 commit rather than to toolhive-core main HEAD: HEAD also carries #185 (adopt go-sdk v1.7.0-pre.3), a 2026-07-28 groundwork change tracked separately under #5754 and out of scope here. Re-pin to a tagged release (v0.0.34+) once one is cut that includes #186. - go.mod/go.sum: toolhive-core v0.0.32 -> #186 commit (plus minor transitive aws-sdk patch bumps). - Remove the t.Skip on TestRegression_ToolSchemaFidelity_PreservesCompositors; it now passes, pinning backend->vMCP ingestion of a top-level oneOf and the absence of a fabricated "type":"". Claude-Session: https://claude.ai/code/session_016WG3mSjVGWNc8nfgbkdd79 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Summary
ToolArgumentsSchema(backingTool.InputSchema/Tool.OutputSchema) only models$defs/type/properties/required/additionalProperties. Any other top-level JSON Schema keyword —oneOf,anyOf,allOf,$ref,enum,const,patternProperties, … — was silently dropped during unmarshal, so atools/listschema like{"oneOf": [...]}was gutted before any consumer (e.g. a client ingesting a backend's advertised tools) ever saw it. Separately,MarshalJSONemitted"type": tas.Typeunconditionally, fabricating an invalid"type":""for schemas that legitimately omit a top-level type.This was found downstream in ToolHive, where vMCP ingests backend tool schemas: a backend tool using a top-level compositor had its schema replaced by a near-empty object. Tracked as stacklok/toolhive#5976.
What changed
Extra map[string]json.RawMessagecatch-all onToolArgumentsSchema:UnmarshalJSONcaptures any top-level keyword outside the modeled fields;MarshalJSONre-emits them. Modeled fields still take precedence on the (spec-wise impossible) chance of a key collision. This makes the typed schema lossless, so no downstream code has to change —MarshalJSON/ConvertToolInputSchemaround-trips preserve everything."type"only when non-empty, so a schema without a top-level type no longer gains a spurious"type":"".properties/requiredfor schemas that are not object-typed and carry none of their own (e.g. a top-leveloneOf). Object schemas keep the historical always-presentproperties/requiredthat clients rely on for no-argument tools, so the common path is byte-identical.Tests
mcpcompat/mcp/tools_schema_fidelity_test.go: table-driven round-trip covering top-leveloneOf/anyOf, type-lessproperties-only schemas, ordinary object schemas (unchanged), and object schemas carrying an extra compositor; plus a Tool-level ingestion round-trip../mcpcompat/mcp/...tests pass;task lintclean.replace: the ToolHive regression testTestRegression_ToolSchemaFidelity_PreservesCompositors(currently skipped, gated on this) flips green with this change.Notes for reviewers
Tool.MarshalJSON(InputSchemavsRawInputSchema) is untouched — this fix keeps everything in the typedInputSchemapath, soRawInputSchemais not involved.type, and (b) no fabricatedproperties/requiredon non-object schemas. Object schemas — including no-arg{"type":"object"}tools — are unaffected.toolhive-coreand unskips the gating test (Backend tool-schema ingestion drops top-level oneOf/anyOf/enum toolhive#5976).Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com