Change type of Tool.InputSchema to be JsonElement #4
Conversation
| /// </summary> | ||
| [JsonPropertyName("inputSchema")] | ||
| public JsonSchema? InputSchema { get; set; } | ||
| public JsonElement InputSchema { get; set; } |
There was a problem hiding this comment.
Not all JsonElement are valid JSON schemas, and not all JSON schemas are valid root-level schemas per the MCP specification. We should try to insert baseline validation (e.g. using a wrapper type or validation on the setter) to make sure we conform.
There was a problem hiding this comment.
Roughly, this should be checking that the JsonElement is an object and also contains a "type" keyword that is set to "object". Everything else including the "properties" keyword is optional IIRC.
|
This seems to be causing hangs in the Ubuntu debug leg. |
There appear to be general reliability issues with our integration testing, although this is the first time I see a hang happening. |
eiriktsarpalis
force-pushed
the
fixschematype
branch
from
044a291 to
65a9f2e
Compare
eiriktsarpalis
force-pushed
the
fixschematype
branch
from
65a9f2e to
36333a0
Compare
|
The Ubuntu hand is happening on #9 as well, so whatever the issue is, it's already in main it seems. |
|
Adding validation to the property setter revealed a few bugs in the process :-) Merging since CI is borked right now. |
| ["description"] = tool.Description ?? string.Empty, | ||
| ["properties"] = tool.InputSchema?.Properties ?? [], | ||
| ["required"] = tool.InputSchema?.Required ?? [] | ||
| ["properties"] = new Dictionary<string, object?>(), |
There was a problem hiding this comment.
I just noticed this. Your changes are leaving the properties and required keywords empty. Was this an intentional change?
There was a problem hiding this comment.
I've updated the code so that it always just forwards the JsonElement in the Tool. It is now always guaranteed to be populated.
No description provided.