Skip to content
63 changes: 58 additions & 5 deletions crates/forge_app/src/dto/anthropic/transforms/enforce_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,20 @@ use forge_domain::Transformer;
use crate::dto::anthropic::{OutputFormat, Request};
use crate::utils::enforce_strict_schema;

/// Transformer that normalizes output_format schema to meet Anthropic API
/// requirements.
/// Transformer that normalizes JSON schemas in the Anthropic request to meet
/// API requirements.
///
/// Anthropic requires that all object types in JSON schemas must explicitly set
/// `additionalProperties: false`. This transformer recursively processes the
/// schema to add this requirement.
/// Two categories of schemas are processed:
///
/// 1. **`output_format` schema** — Anthropic requires all object types to
/// explicitly set `additionalProperties: false`.
///
/// 2. **Tool `input_schema`** — each tool's parameter schema is sanitized to
/// remove constructs that produce invalid JSON Schema, such as `null`
/// values in `enum` arrays that contradict the declared `type`. This is
/// essential when routing to Anthropic-compatible endpoints backed by
/// third-party models (e.g. Moonshot/Kimi) whose validators reject such
/// schemas.
///
/// # Example
///
Expand Down Expand Up @@ -54,6 +62,13 @@ impl Transformer for EnforceStrictObjectSchema {
}
}

// Normalize each tool's input schema. Non-strict mode keeps the
// `nullable` keyword for providers that support it while stripping
// invalid constructs (e.g. null in enum) that strict validators reject.
for tool in &mut request.tools {
enforce_strict_schema(&mut tool.input_schema, false);
}

request
}
}
Expand All @@ -64,6 +79,8 @@ mod tests {
use schemars::JsonSchema;
use serde::Deserialize;

use crate::dto::anthropic::ToolDefinition;

use super::*;

#[derive(Deserialize, JsonSchema)]
Expand Down Expand Up @@ -135,4 +152,40 @@ mod tests {

assert_eq!(actual.output_format, None);
}

#[test]
fn test_normalize_tool_schema_strips_null_from_enum() {
// Simulate a tool input schema with a nullable enum that contains
// null in the enum array — exactly what Moonshot/Kimi rejects.
let fixture = Request::default().max_tokens(1u64).tools(vec![ToolDefinition {
name: "fs_search".to_string(),
description: Some("Search files".to_string()),
cache_control: None,
input_schema: serde_json::json!({
"type": "object",
"properties": {
"output_mode": {
"type": "string",
"enum": ["content", "files_with_matches", "count", null],
"nullable": true
}
}
}),
}]);

let actual = EnforceStrictObjectSchema.transform(fixture);

let tool = &actual.tools[0];
let enum_values = tool
.input_schema
.pointer("/properties/output_mode/enum")
.and_then(|v| v.as_array())
.unwrap();

assert!(
!enum_values.contains(&serde_json::Value::Null),
"enum must not contain null after normalization"
);
assert_eq!(enum_values.len(), 3);
}
}
57 changes: 53 additions & 4 deletions crates/forge_app/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,25 @@ pub fn enforce_strict_schema(schema: &mut serde_json::Value, strict_mode: bool)

normalize_array_items(map, strict_mode);

// Always strip null from enum arrays when nullable is set,
// regardless of strict mode. A null enum value contradicts any
// non-null type declaration, producing invalid JSON Schema that
// strict validators (e.g. Moonshot/Kimi) reject. This also guards
// against external MCP schemas that may carry the same
// inconsistency, which are not processed by NormalizeNullable.
if map
.get("nullable")
.and_then(|v| v.as_bool())
.unwrap_or(false)
{
if let Some(serde_json::Value::Array(enum_values)) = map.get_mut("enum") {
enum_values.retain(|v| !v.is_null());
}
}

// In strict mode, convert the nullable keyword into an explicit
// anyOf union because OpenAI's Responses API does not support
// nullable. The enum array has already been cleaned above.
if strict_mode
&& map
.get("nullable")
Expand All @@ -530,10 +549,6 @@ pub fn enforce_strict_schema(schema: &mut serde_json::Value, strict_mode: bool)
{
map.remove("nullable");

if let Some(serde_json::Value::Array(enum_values)) = map.get_mut("enum") {
enum_values.retain(|v| !v.is_null());
}

let description = map.remove("description");
let non_null_branch = serde_json::Value::Object(std::mem::take(map));
let null_branch = serde_json::json!({"type": "null"});
Expand Down Expand Up @@ -1327,6 +1342,40 @@ mod tests {
assert!(schema["properties"]["output_mode"].get("anyOf").is_none());
}

#[test]
fn test_nullable_enum_null_stripped_in_non_strict_mode() {
// Schemas produced by external MCP tools may carry null in enum
// alongside nullable: true. Even in non-strict mode, null must be
// stripped so the schema stays valid for strict validators like
// Moonshot/Kimi.
let mut schema = json!({
"type": "object",
"properties": {
"output_mode": {
"nullable": true,
"type": "string",
"enum": ["content", "files_with_matches", "count", null]
}
}
});

enforce_strict_schema(&mut schema, false);

let enum_values = schema["properties"]["output_mode"]["enum"]
.as_array()
.unwrap();
assert!(
!enum_values.contains(&json!(null)),
"enum must not contain null in non-strict mode"
);
assert_eq!(enum_values.len(), 3);
// nullable marker should be preserved
assert_eq!(
schema["properties"]["output_mode"]["nullable"],
json!(true)
);
}

#[test]
fn test_schema_valued_additional_properties_is_normalized() {
let mut schema = json!({
Expand Down
8 changes: 5 additions & 3 deletions crates/forge_domain/src/tools/catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,7 @@ fn normalize_tool_name(name: &ToolName) -> ToolName {
impl ToolCatalog {
pub fn schema(&self) -> Schema {
use schemars::generate::SchemaSettings;
use schemars::transform::{AddNullable, Transform};
use schemars::transform::Transform;

let r#gen = SchemaSettings::default()
.with(|s| {
Expand Down Expand Up @@ -889,8 +889,10 @@ impl ToolCatalog {
ToolCatalog::TodoRead(_) => r#gen.into_root_schema_for::<TodoRead>(),
};

// Apply transform to add nullable property and remove null from type
AddNullable::default().transform(&mut schema);
// Normalize nullable schemas: add the "nullable": true marker and
// ensure enum arrays stay internally consistent (no null values that
// contradict the declared type).
crate::NormalizeNullable.transform(&mut schema);

schema
}
Expand Down
133 changes: 133 additions & 0 deletions crates/forge_domain/src/tools/definition/tool_definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use schemars::Schema;
use schemars::generate::SchemaGenerator;
use schemars::transform::{Transform, transform_subschemas};
use serde::{Deserialize, Serialize};
use serde_json::Value;

use crate::ToolName;

Expand All @@ -25,6 +26,60 @@ impl Transform for RemoveSchemaTitles {
}
}

/// A [`Transform`] that marks nullable schemas with `"nullable": true` while
/// keeping `enum` arrays internally consistent.
///
/// Schemars' built-in [`schemars::transform::AddNullable`] adds the
/// `"nullable": true` keyword and removes `"null"` from the `type` keyword,
/// but it does **not** remove `null` values from `enum` arrays. This leaves
/// schemas where the `enum` array contains `null` while `type` is e.g.
/// `"string"` — an invalid combination that strict JSON Schema validators
/// (such as Moonshot/Kimi) reject.
///
/// This transform delegates to [`AddNullable`](schemars::transform::AddNullable)
/// for the `type` keyword, then recursively strips `null` from every `enum`
/// array in a schema that carries `"nullable": true`. The `nullable` marker
/// alone fully conveys nullability in OpenAPI 3.0-style dialects, so `null`
/// in `enum` is redundant once the marker is present.
#[derive(Debug, Clone, Default)]
pub struct NormalizeNullable;

impl Transform for NormalizeNullable {
fn transform(&mut self, schema: &mut Schema) {
// Delegate type-keyword normalization (including subschema recursion)
// to schemars' AddNullable.
schemars::transform::AddNullable::default().transform(schema);

// Recursively clean up enum arrays left inconsistent by AddNullable.
remove_null_enum_values(schema);
transform_subschemas(&mut remove_null_enum_values, schema);
}
}

/// Removes `null` entries from the `enum` array of a schema that is marked
/// `nullable: true`. The `nullable` marker already conveys nullability, so
/// keeping `null` in `enum` is redundant and can make the schema invalid when
/// the `type` no longer includes `"null"`.
fn remove_null_enum_values(schema: &mut Schema) {
if let Some(map) = schema.as_object_mut() {
let is_nullable = map
.get("nullable")
.and_then(Value::as_bool)
.unwrap_or(false);

if is_nullable {
if let Some(Value::Array(enum_values)) = map.get_mut("enum") {
enum_values.retain(|v| !v.is_null());

// Drop the enum key if every value was null.
if enum_values.is_empty() {
map.remove("enum");
}
}
}
}
}

/// Returns a [`SchemaGenerator`] whose settings include [`RemoveSchemaTitles`]
/// as a registered transform.
///
Expand Down Expand Up @@ -155,4 +210,82 @@ mod tests {
None
);
}

#[test]
fn test_normalize_nullable_strips_null_from_enum() {
use schemars::json_schema;

// Simulate what schemars produces for Option<EnumWithCustomSchema>:
// type includes "null", and enum array contains null.
let mut schema = json_schema!({
"type": ["string", "null"],
"enum": ["content", "files_with_matches", "count", null]
});

NormalizeNullable.transform(&mut schema);

let actual = serde_json::to_value(&schema).unwrap();

// nullable marker should be present
assert_eq!(actual["nullable"], serde_json::Value::Bool(true));

// type should no longer contain "null"
assert_eq!(actual["type"], serde_json::json!("string"));

// enum must NOT contain null — this is the bug fix
let enum_values = actual["enum"].as_array().unwrap();
assert!(
!enum_values.contains(&serde_json::Value::Null),
"enum must not contain null after NormalizeNullable"
);
assert_eq!(enum_values.len(), 3);
}

#[test]
fn test_normalize_nullable_preserves_non_nullable_enum() {
use schemars::json_schema;

// A non-nullable enum should be left untouched.
let mut schema = json_schema!({
"type": "string",
"enum": ["a", "b", "c"]
});

let expected = serde_json::to_value(&schema).unwrap();

NormalizeNullable.transform(&mut schema);

let actual = serde_json::to_value(&schema).unwrap();
assert_eq!(actual, expected);
}

#[test]
fn test_normalize_nullable_recurses_into_properties() {
use schemars::json_schema;

// Nullable enum nested inside a properties object.
let mut schema = json_schema!({
"type": "object",
"properties": {
"mode": {
"type": ["string", "null"],
"enum": ["fast", "slow", null]
}
}
});

NormalizeNullable.transform(&mut schema);

let actual = serde_json::to_value(&schema).unwrap();
let mode_enum = actual
.pointer("/properties/mode/enum")
.and_then(|v| v.as_array())
.unwrap();

assert!(
!mode_enum.contains(&serde_json::Value::Null),
"nested enum must not contain null"
);
assert_eq!(mode_enum.len(), 2);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,7 @@ expression: tools
"enum": [
"content",
"files_with_matches",
"count",
null
"count"
],
"nullable": true
},
Expand Down
Loading