mirror of
https://github.com/zed-industries/zed.git
synced 2026-07-09 16:00:35 +00:00
Fix MCP tools with $ref/$defs being silently rejected (#60165)
Closes #60162. MCP servers whose tool `inputSchema` uses `$ref`/`$defs` (e.g., Notion MCP v2.x, and any server using Zod/Pydantic-generated schemas) are silently rejected with: ``` ERROR Schema cannot be made compatible because it contains "$ref" ``` The affected tools are dropped from the agent panel — the user never sees them and there is no user-visible error. ## Root cause `adapt_to_json_schema_subset` rejects any schema containing `$ref` via `UNSUPPORTED_KEYS`: ```rust const UNSUPPORTED_KEYS: [&str; 4] = ["if", "then", "else", "$ref"]; ``` This is hit by every provider that uses `JsonSchemaSubset` format (Google Gemini, xAI Grok, OpenAI-compatible proxies, Vercel AI Gateway, Copilot Chat for Google/xAI vendors, OpenRouter for gemini/grok models). Providers using `JsonSchema` (Anthropic direct, OpenAI direct) don't hit this check — `$ref` is passed through to the API, which may or may not handle it correctly. This is not an edge case. Every modern MCP server using Zod (TypeScript), Pydantic (Python), or JSON Schema with shared definitions generates `$ref`/`$defs` in tool schemas. ## Fix Add a `resolve_refs` step in `adapt_schema_to_format` that dereferences all `$ref` pointers using the document's own `$defs` (or legacy `definitions`) map, making the schema self-contained before format-specific processing. Applied at the entry point so both `JsonSchema` and `JsonSchemaSubset` formats benefit. **Scope note:** previously `JsonSchema` providers (Anthropic direct, OpenAI direct) received the raw `$ref`/`$defs` and were expected to handle it themselves — which most do not. After this change, both paths receive a self-contained schema with refs inlined. This is intentional: it fixes the same root cause for both paths and avoids provider-specific behavior divergence. Supported `$ref` forms: - `#/$defs/<name>` (JSON Schema draft 2019-09+) - `#/definitions/<name>` (draft 4-7 legacy) Edge cases: - **Nested `$ref`** (definition references another definition): resolved recursively. - **Sibling properties alongside `$ref`** (e.g. `{ "$ref": "...", "description": "..." }`, legal under draft 2019-09+): merged onto the resolved definition, with siblings overriding the definition's keys. - **Cyclic references** (A → B → A, or self-referential schemas like a Tree node): replaced with an empty schema `{}` ("any JSON value"). The tool still works, just without type info for that recursive field. - **Unsupported `$ref` forms** (e.g., external URLs): returns an error with a clear message. - **Missing definition target**: returns an error naming the missing ref. ## Testing Added 10 unit tests in `crates/language_model_core/src/tool_schema.rs` that cover the patterns produced by Zod/Pydantic-generated MCP schemas: - `test_refs_are_resolved_via_adapt_schema_to_format` — basic `$ref` → `$defs` resolution - `test_refs_in_defs_are_resolved` — nested `$ref` (definition references another definition) - `test_refs_in_array_items_are_resolved` — `$ref` inside `array.items` - `test_legacy_definitions_prefix_is_supported` — old `#/definitions/<name>` prefix - `test_schema_without_defs_is_unchanged` — schemas with no `$defs` are unaffected - `test_refs_fail_for_unsupported_prefix` — external URL refs error clearly - `test_refs_fail_for_missing_definition` — missing target errors clearly - `test_cyclic_refs_are_replaced_with_empty_schema` — A → B → A cycle replaced with `{}` - `test_self_referential_ref_is_replaced_with_empty_schema` — Tree-like self-ref replaced with `{}` - `test_ref_sibling_properties_are_preserved` — sibling properties alongside `$ref` are merged onto the resolved definition Existing tests (which call `adapt_to_json_schema_subset` and `preprocess_json_schema` directly) are unaffected — the fix is additive at the `adapt_schema_to_format` level. ## Disclosure I used an LLM to help draft the implementation and tests. I reviewed and understand the change — it adds one new function (`resolve_refs`) with two helpers (`parse_ref`, `resolve_refs_recursive`), plus unit tests. Release Notes: - Fixed MCP tools with `$ref`/`$defs` in their `inputSchema` being silently rejected by providers using the JSON Schema Subset format (Google Gemini, xAI Grok, OpenAI-compatible proxies, etc.). Tools from servers like Notion MCP v2.x, and any server using Zod or Pydantic-generated schemas, now work correctly. --------- Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de> Co-authored-by: Bennet Bo Fenner <bennet@zed.dev>
This commit is contained in:
parent
001bda1a46
commit
a956add0b6
3 changed files with 400 additions and 0 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -9792,6 +9792,7 @@ dependencies = [
|
|||
"http_client",
|
||||
"log",
|
||||
"partial-json-fixer",
|
||||
"pretty_assertions",
|
||||
"schemars 1.0.4",
|
||||
"serde",
|
||||
"serde_json",
|
||||
|
|
|
|||
|
|
@ -26,3 +26,6 @@ serde.workspace = true
|
|||
serde_json.workspace = true
|
||||
strum.workspace = true
|
||||
thiserror.workspace = true
|
||||
|
||||
[dev-dependencies]
|
||||
pretty_assertions.workspace = true
|
||||
|
|
|
|||
|
|
@ -82,6 +82,8 @@ pub fn adapt_schema_to_format(
|
|||
obj.remove("description");
|
||||
}
|
||||
|
||||
resolve_refs(json)?;
|
||||
|
||||
match format {
|
||||
LanguageModelToolSchemaFormat::JsonSchema => preprocess_json_schema(json),
|
||||
LanguageModelToolSchemaFormat::JsonSchemaSubset => adapt_to_json_schema_subset(json),
|
||||
|
|
@ -106,6 +108,98 @@ fn preprocess_json_schema(json: &mut Value) -> Result<()> {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// Inlines same-document `$ref`s from `$defs`/`definitions` and removes those.
|
||||
fn resolve_refs(json: &mut Value) -> Result<()> {
|
||||
let Some(root_obj) = json.as_object_mut() else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
let defs = root_obj.remove("$defs");
|
||||
let legacy_defs = root_obj.remove("definitions");
|
||||
if defs.is_none() && legacy_defs.is_none() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
resolve_refs_recursive(json, defs.as_ref(), legacy_defs.as_ref(), &mut Vec::new())
|
||||
}
|
||||
|
||||
fn resolve_refs_recursive(
|
||||
value: &mut Value,
|
||||
defs: Option<&Value>,
|
||||
legacy_defs: Option<&Value>,
|
||||
visiting: &mut Vec<String>,
|
||||
) -> Result<()> {
|
||||
match value {
|
||||
Value::Object(obj) => {
|
||||
if let Some(ref_str) = obj.get("$ref").and_then(|v| v.as_str()) {
|
||||
// Guard against cycles (A -> B -> A, or self-referential
|
||||
// schemas like a Tree node whose children are Trees)
|
||||
if visiting.iter().any(|v| v == ref_str) {
|
||||
*obj = Map::new();
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let (defs_key, name) = parse_ref(ref_str)?;
|
||||
let defs_for_key = match defs_key {
|
||||
"$defs" => defs,
|
||||
"definitions" => legacy_defs,
|
||||
_ => None,
|
||||
};
|
||||
let Some(def) = defs_for_key.and_then(|defs| defs.get(name)) else {
|
||||
anyhow::bail!("$ref target not found in {defs_key}: {ref_str}");
|
||||
};
|
||||
|
||||
let ref_owned = ref_str.to_string();
|
||||
|
||||
// Inline the referenced definition into the current object.
|
||||
let mut resolved = def.clone();
|
||||
if let Value::Object(resolved_obj) = &mut resolved {
|
||||
for (key, val) in obj.iter() {
|
||||
if key != "$ref" {
|
||||
resolved_obj.insert(key.clone(), val.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
*value = resolved;
|
||||
|
||||
visiting.push(ref_owned);
|
||||
let result = resolve_refs_recursive(value, defs, legacy_defs, visiting);
|
||||
visiting.pop();
|
||||
return result;
|
||||
}
|
||||
|
||||
let keys: Vec<String> = obj.keys().cloned().collect();
|
||||
for key in keys {
|
||||
if let Some(child) = obj.get_mut(&key) {
|
||||
resolve_refs_recursive(child, defs, legacy_defs, visiting)?;
|
||||
}
|
||||
}
|
||||
}
|
||||
Value::Array(arr) => {
|
||||
for item in arr.iter_mut() {
|
||||
resolve_refs_recursive(item, defs, legacy_defs, visiting)?;
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Parses a same-document `$ref` like `#/$defs/Foo` or `#/definitions/Foo`.
|
||||
/// Returns `(defs_key, name)` where `defs_key` is the top-level key the
|
||||
/// definition was looked up under, and `name` is the definition name.
|
||||
fn parse_ref(ref_str: &str) -> Result<(&'static str, &str)> {
|
||||
if let Some(name) = ref_str.strip_prefix("#/$defs/") {
|
||||
return Ok(("$defs", name));
|
||||
}
|
||||
if let Some(name) = ref_str.strip_prefix("#/definitions/") {
|
||||
return Ok(("definitions", name));
|
||||
}
|
||||
anyhow::bail!(
|
||||
"Unsupported $ref format (only `#/$defs/<name>` and `#/definitions/<name>` are supported): {ref_str}"
|
||||
);
|
||||
}
|
||||
|
||||
fn adapt_to_json_schema_subset(json: &mut Value) -> Result<()> {
|
||||
if let Value::Object(obj) = json {
|
||||
const UNSUPPORTED_KEYS: [&str; 4] = ["if", "then", "else", "$ref"];
|
||||
|
|
@ -305,6 +399,7 @@ fn collapse_nullable_only_any_of(obj: &mut Map<String, Value>) {
|
|||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
fn obj(value: Value) -> Map<String, Value> {
|
||||
|
|
@ -787,6 +882,307 @@ mod tests {
|
|||
assert!(adapt_to_json_schema_subset(&mut json).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_refs_are_resolved_via_adapt_schema_to_format() {
|
||||
let mut json = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"parent": {
|
||||
"$ref": "#/$defs/pageParent"
|
||||
},
|
||||
"title": {
|
||||
"type": "string",
|
||||
"description": "Page title"
|
||||
}
|
||||
},
|
||||
"required": ["parent"],
|
||||
"$defs": {
|
||||
"pageParent": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"type": {
|
||||
"type": "string",
|
||||
"description": "Parent type"
|
||||
}
|
||||
},
|
||||
"required": ["type"]
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
adapt_schema_to_format(&mut json, LanguageModelToolSchemaFormat::JsonSchemaSubset).unwrap();
|
||||
|
||||
let expected = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"parent": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"type": {
|
||||
"type": "string",
|
||||
"description": "Parent type"
|
||||
}
|
||||
},
|
||||
"required": ["type"]
|
||||
},
|
||||
"title": {
|
||||
"type": "string",
|
||||
"description": "Page title"
|
||||
}
|
||||
},
|
||||
"required": ["parent"],
|
||||
});
|
||||
assert_eq!(json, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_refs_fail_for_unsupported_prefix() {
|
||||
let mut json = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"child": {
|
||||
"$ref": "https://example.com/schema.json#/User"
|
||||
}
|
||||
},
|
||||
"$defs": {
|
||||
"User": { "type": "string" }
|
||||
}
|
||||
});
|
||||
|
||||
assert!(
|
||||
adapt_schema_to_format(&mut json, LanguageModelToolSchemaFormat::JsonSchemaSubset)
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_refs_fail_for_missing_definition() {
|
||||
let mut json = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"child": {
|
||||
"$ref": "#/$defs/NonExistent"
|
||||
}
|
||||
},
|
||||
"$defs": {
|
||||
"Existing": { "type": "string" }
|
||||
}
|
||||
});
|
||||
|
||||
assert!(
|
||||
adapt_schema_to_format(&mut json, LanguageModelToolSchemaFormat::JsonSchemaSubset)
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_refs_in_defs_are_resolved() {
|
||||
// A definition that itself references another definition.
|
||||
let mut json = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"parent": {
|
||||
"$ref": "#/$defs/pageParent"
|
||||
}
|
||||
},
|
||||
"$defs": {
|
||||
"pageParent": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"database_id": {
|
||||
"$ref": "#/$defs/databaseId"
|
||||
}
|
||||
}
|
||||
},
|
||||
"databaseId": {
|
||||
"type": "string",
|
||||
"description": "A database ID"
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
adapt_schema_to_format(&mut json, LanguageModelToolSchemaFormat::JsonSchemaSubset).unwrap();
|
||||
|
||||
// The nested $ref in pageParent -> databaseId should be resolved.
|
||||
assert_eq!(
|
||||
json,
|
||||
json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"parent": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"database_id": {
|
||||
"type": "string",
|
||||
"description": "A database ID"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_refs_resolve_when_both_defs_and_definitions_exist() {
|
||||
let mut json = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"modern": {
|
||||
"$ref": "#/$defs/Modern"
|
||||
},
|
||||
"legacy": {
|
||||
"$ref": "#/definitions/Legacy"
|
||||
}
|
||||
},
|
||||
"$defs": {
|
||||
"Modern": {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"definitions": {
|
||||
"Legacy": {
|
||||
"type": "number"
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
adapt_schema_to_format(&mut json, LanguageModelToolSchemaFormat::JsonSchemaSubset).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
json,
|
||||
json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"modern": {
|
||||
"type": "string"
|
||||
},
|
||||
"legacy": {
|
||||
"type": "number"
|
||||
}
|
||||
}
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_refs_in_array_items_are_resolved() {
|
||||
let mut json = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"items": {
|
||||
"type": "array",
|
||||
"items": {
|
||||
"$ref": "#/$defs/itemDef"
|
||||
}
|
||||
}
|
||||
},
|
||||
"$defs": {
|
||||
"itemDef": {
|
||||
"type": "string",
|
||||
"description": "An item"
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
adapt_schema_to_format(&mut json, LanguageModelToolSchemaFormat::JsonSchemaSubset).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
json,
|
||||
json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"items": {
|
||||
"type": "array",
|
||||
"items": {
|
||||
"type": "string",
|
||||
"description": "An item"
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_self_referential_ref_is_replaced_with_empty_schema() {
|
||||
// A common pattern: a Tree node with children of the same type.
|
||||
let mut json = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"root": { "$ref": "#/$defs/Tree" }
|
||||
},
|
||||
"$defs": {
|
||||
"Tree": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"value": { "type": "string" },
|
||||
"children": {
|
||||
"type": "array",
|
||||
"items": { "$ref": "#/$defs/Tree" }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
adapt_schema_to_format(&mut json, LanguageModelToolSchemaFormat::JsonSchemaSubset)
|
||||
.expect("self-referential $ref should not error");
|
||||
|
||||
assert_eq!(
|
||||
json,
|
||||
json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"root": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"value": { "type": "string" },
|
||||
"children": {
|
||||
"type": "array",
|
||||
"items": {}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_ref_sibling_properties_are_preserved() {
|
||||
// JSON Schema draft 2019-09+ allows sibling properties alongside
|
||||
// `$ref`. They must be merged into the resolved definition rather than
|
||||
// discarded, with siblings overriding the definition's keys.
|
||||
let mut json = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"child": {
|
||||
"$ref": "#/$defs/childDef",
|
||||
"description": "Local description overrides def"
|
||||
}
|
||||
},
|
||||
"$defs": {
|
||||
"childDef": {
|
||||
"type": "string",
|
||||
"description": "Def description",
|
||||
"minLength": 1
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
adapt_schema_to_format(&mut json, LanguageModelToolSchemaFormat::JsonSchemaSubset).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
json["properties"]["child"],
|
||||
json!({
|
||||
"type": "string",
|
||||
"description": "Local description overrides def",
|
||||
"minLength": 1
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_preprocess_json_schema_adds_additional_properties() {
|
||||
let mut json = json!({
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue