fix: support allOf and oneOf in tool input forms - #1602
Closed
pablopupo wants to merge 1 commit into
Closed
Conversation
Tool schemas that compose properties with allOf rendered an empty form because ToolsTab and DynamicJsonForm only looked at top level properties. Merge allOf branches (properties, required, refs) before rendering, and render oneOf variant unions with a selector while keeping the existing const and title enum selects untouched. Fixes modelcontextprotocol#496
Member
|
Hi @pablopupo. As mentioned in the PR template, we are not accepting issues for anything but security and bug fixes for the v1 Inspector. In the new SDK, full JSON Schema will be supported, including allOf/oneOf. We are hard at work on V2 of the Inspector, which should be available very soon, and this will be supported. |
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
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
Tool input schemas that compose their properties with
allOfrendered an empty form, because bothToolsTabandDynamicJsonFormonly look at top-levelproperties.oneOfunions of full schemas rendered nothing at all. This makes both work.Type of Change
Changes Made
client/src/utils/schemaUtils.tsaddsmergeAllOf(), which flattensallOfbranches before a form renders. Properties are merged,requiredarrays are unioned,$refbranches resolve against the root schema, and circular ref chains terminate.client/src/components/ToolsTab.tsxmerges the input schema before iterating its properties, passesrequiredandoneOfthrough to nested forms (both were dropped when the schema object was rebuilt), and keys per-property form state by tool name so state cannot leak between tools that share a property name.client/src/components/DynamicJsonForm.tsxmergesallOfat the top level and per property, and renders a variant selector foroneOfmembers that are full schemas. The selector infers the initial variant from the current value, keeps nested selectors independent, and treatsproperties-without-typemembers as objects. Options carryingconstkeep the existing titled enum select from Add support for SEP-1330 Elicitation Enums #952 untouched.client/src/utils/jsonUtils.tsaddsallOftoJsonSchemaType.Related Issues
Fixes #496
Testing
Test Results and/or Instructions
22 new tests across
schemaUtils.test.ts,DynamicJsonForm.test.tsx, andToolsTab.test.tsx, including the exact schema shape from the issue. Full client suite passes (557 tests, 32 suites) and the Playwright e2e suite passes.To see the original bug, point the inspector at a server whose tool wraps
propertiesin a top-levelallOf(the schema in #496). On main the tool form renders no fields; on this branch the fields render with their required markers.Checklist
npm run prettier-fix)Breaking Changes
None.
Additional Context
resolveRefstill only recurses intoanyOf, so$refsupport insideoneOfbranches beyond what the variant selector resolves stays with #445.