Skip to content

Commit a90e4dd

Browse files
committed
fix(gemini): sanitize JSON-Schema metadata and inline $ref for Gemini
Gemini's generateContent uses an OpenAPI 3.0 schema subset and rejects standard JSON-Schema metadata ($defs, $ref, $schema, $id, $comment, $anchor, title). MCP servers like notion and supabase emit schemas that combine $defs+$ref to keep their tool surface compact, which crashed the Gemini call when forwarded verbatim. Replace the previous shallow gemini_compatible_schema (only mapped const→enum) with a real sanitizer: - Extract $defs / definitions from the root before recursion. - For each object node: - If it has a $ref, resolve it against the extracted defs map and recurse on the target. Unresolvable refs degrade to a permissive empty object. - Otherwise, strip metadata keys Gemini doesn't accept and recurse on each value. - Bound recursion at depth 24 so circular schemas (head -> Loop -> head -> …) terminate instead of overflowing the stack. Add 4 regression tests in src/provider/gemini_tests.rs covering: - ref inlining + metadata stripping + const→enum after inlining - legacy 'definitions' alias - unresolvable $ref → permissive empty object - circular schema does not recurse forever The skill-side hunks of upstream PR 1jehuang#162 (API name 'Skill'/'skill' → skill_manage routing + serde alias on `name`) are NOT applied here — local code already provides both via `canonical_tool_name` in src/tool/mod.rs:330 and #[serde(alias = "skill")] on SkillInput.name in src/tool/skill.rs:31. Closes #126
1 parent e1886bb commit a90e4dd

2 files changed

Lines changed: 182 additions & 3 deletions

File tree

src/provider/gemini.rs

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -921,22 +921,76 @@ pub(crate) fn build_tools(tools: &[ToolDefinition]) -> Option<Vec<GeminiTool>> {
921921
}
922922

923923
fn gemini_compatible_schema(schema: &Value) -> Value {
924+
// Gemini's generateContent uses an OpenAPI 3.0 schema subset and rejects
925+
// standard JSON-Schema metadata ($defs, $ref, $schema, $id, $comment, etc.).
926+
// MCP servers like notion and supabase emit schemas with $defs+$ref, which
927+
// would otherwise be forwarded verbatim and cause 400-class failures.
928+
//
929+
// Extract $defs/definitions from the root and recursively inline $ref
930+
// references while stripping metadata keywords Gemini doesn't accept.
931+
// See issue #126 / upstream PR #162.
932+
let defs = schema
933+
.as_object()
934+
.and_then(|m| m.get("$defs").or_else(|| m.get("definitions")))
935+
.and_then(|v| v.as_object())
936+
.cloned()
937+
.unwrap_or_default();
938+
sanitize_for_gemini(schema, &defs, 0)
939+
}
940+
941+
const GEMINI_SCHEMA_MAX_DEPTH: usize = 24;
942+
943+
fn sanitize_for_gemini(
944+
schema: &Value,
945+
defs: &serde_json::Map<String, Value>,
946+
depth: usize,
947+
) -> Value {
948+
if depth > GEMINI_SCHEMA_MAX_DEPTH {
949+
// Avoid blow-up on circular schemas — return permissive empty object.
950+
return Value::Object(serde_json::Map::new());
951+
}
924952
match schema {
925953
Value::Object(map) => {
954+
// Resolve $ref by replacing the entire object with the target definition.
955+
if let Some(ref_str) = map.get("$ref").and_then(|v| v.as_str()) {
956+
let name = ref_str
957+
.strip_prefix("#/$defs/")
958+
.or_else(|| ref_str.strip_prefix("#/definitions/"));
959+
if let Some(target_name) = name
960+
&& let Some(target) = defs.get(target_name)
961+
{
962+
return sanitize_for_gemini(target, defs, depth + 1);
963+
}
964+
// Unresolvable $ref → permissive empty object.
965+
return Value::Object(serde_json::Map::new());
966+
}
967+
926968
let mut out = serde_json::Map::new();
927969
for (key, value) in map {
970+
// Strip JSON-Schema metadata not supported by Gemini.
971+
if matches!(
972+
key.as_str(),
973+
"$defs" | "definitions" | "$schema" | "$id" | "$comment" | "$anchor" | "title"
974+
) {
975+
continue;
976+
}
928977
if key == "const" {
929978
out.insert(
930979
"enum".to_string(),
931-
Value::Array(vec![gemini_compatible_schema(value)]),
980+
Value::Array(vec![sanitize_for_gemini(value, defs, depth + 1)]),
932981
);
933982
} else {
934-
out.insert(key.clone(), gemini_compatible_schema(value));
983+
out.insert(key.clone(), sanitize_for_gemini(value, defs, depth + 1));
935984
}
936985
}
937986
Value::Object(out)
938987
}
939-
Value::Array(items) => Value::Array(items.iter().map(gemini_compatible_schema).collect()),
988+
Value::Array(items) => Value::Array(
989+
items
990+
.iter()
991+
.map(|i| sanitize_for_gemini(i, defs, depth + 1))
992+
.collect(),
993+
),
940994
_ => schema.clone(),
941995
}
942996
}

src/provider/gemini_tests.rs

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,3 +351,128 @@ fn parses_candidate_finish_message() {
351351
Some("Response blocked by safety filters")
352352
);
353353
}
354+
355+
// ---------------------------------------------------------------------------
356+
// Regression tests for issue #126 / upstream PR #162: Gemini's generateContent
357+
// rejects standard JSON-Schema metadata. MCP servers (notion, supabase, …)
358+
// emit schemas with $defs/$ref/$schema, which would otherwise crash the call.
359+
// `gemini_compatible_schema` must inline $ref targets and strip the metadata.
360+
// ---------------------------------------------------------------------------
361+
362+
#[test]
363+
fn gemini_schema_inlines_refs_and_strips_metadata() {
364+
let schema = serde_json::json!({
365+
"$schema": "https://json-schema.org/draft-07",
366+
"$id": "https://example.com/foo.json",
367+
"title": "Foo",
368+
"type": "object",
369+
"$defs": {
370+
"Inner": {
371+
"type": "object",
372+
"properties": {
373+
"kind": { "const": "leaf" }
374+
}
375+
}
376+
},
377+
"properties": {
378+
"inner": { "$ref": "#/$defs/Inner" }
379+
}
380+
});
381+
382+
let out = gemini_compatible_schema(&schema);
383+
let obj = out.as_object().expect("object root");
384+
385+
// Metadata keywords are stripped.
386+
assert!(!obj.contains_key("$schema"), "$schema should be stripped");
387+
assert!(!obj.contains_key("$id"), "$id should be stripped");
388+
assert!(!obj.contains_key("title"), "title should be stripped");
389+
assert!(
390+
!obj.contains_key("$defs"),
391+
"$defs should be stripped from root"
392+
);
393+
394+
// $ref is inlined — `inner` should now look like the Inner def.
395+
let inner = obj
396+
.get("properties")
397+
.and_then(|p| p.get("inner"))
398+
.and_then(|v| v.as_object())
399+
.expect("inner object");
400+
assert_eq!(inner.get("type").and_then(|v| v.as_str()), Some("object"));
401+
// const → enum conversion still happens after inlining.
402+
let kind = inner
403+
.get("properties")
404+
.and_then(|p| p.get("kind"))
405+
.and_then(|v| v.as_object())
406+
.expect("kind object");
407+
let enum_arr = kind.get("enum").and_then(|v| v.as_array()).expect("enum");
408+
assert_eq!(enum_arr.len(), 1);
409+
assert_eq!(enum_arr[0].as_str(), Some("leaf"));
410+
}
411+
412+
#[test]
413+
fn gemini_schema_resolves_definitions_alias_too() {
414+
// Older drafts use `definitions` instead of `$defs`. Both must work.
415+
let schema = serde_json::json!({
416+
"type": "object",
417+
"definitions": {
418+
"Bar": { "type": "string" }
419+
},
420+
"properties": {
421+
"bar": { "$ref": "#/definitions/Bar" }
422+
}
423+
});
424+
425+
let out = gemini_compatible_schema(&schema);
426+
let obj = out.as_object().expect("object root");
427+
assert!(!obj.contains_key("definitions"));
428+
let bar = obj
429+
.get("properties")
430+
.and_then(|p| p.get("bar"))
431+
.and_then(|v| v.as_object())
432+
.expect("bar object");
433+
assert_eq!(bar.get("type").and_then(|v| v.as_str()), Some("string"));
434+
}
435+
436+
#[test]
437+
fn gemini_schema_falls_back_to_empty_object_on_unresolved_ref() {
438+
// Unresolvable $ref must produce a permissive empty object, not a crash
439+
// and not a forwarded $ref Gemini would reject.
440+
let schema = serde_json::json!({
441+
"type": "object",
442+
"properties": {
443+
"ghost": { "$ref": "#/$defs/DoesNotExist" }
444+
}
445+
});
446+
let out = gemini_compatible_schema(&schema);
447+
let ghost = out
448+
.get("properties")
449+
.and_then(|p| p.get("ghost"))
450+
.expect("ghost");
451+
let ghost_obj = ghost.as_object().expect("ghost object");
452+
assert!(
453+
ghost_obj.is_empty(),
454+
"unresolved $ref should become an empty object, got {ghost:?}"
455+
);
456+
}
457+
458+
#[test]
459+
fn gemini_schema_does_not_recurse_forever_on_cycles() {
460+
// A self-referential schema must terminate (returns an empty object once
461+
// depth limit is exceeded) instead of overflowing the stack.
462+
let schema = serde_json::json!({
463+
"type": "object",
464+
"$defs": {
465+
"Loop": {
466+
"type": "object",
467+
"properties": {
468+
"next": { "$ref": "#/$defs/Loop" }
469+
}
470+
}
471+
},
472+
"properties": {
473+
"head": { "$ref": "#/$defs/Loop" }
474+
}
475+
});
476+
// Should not panic / overflow.
477+
let _ = gemini_compatible_schema(&schema);
478+
}

0 commit comments

Comments
 (0)