From dd74a1ef749de276c969254b02ead6abe4302b32 Mon Sep 17 00:00:00 2001 From: Claw Date: Sat, 19 Sep 2026 09:08:29 -0500 Subject: [PATCH] fix(gateway/v1): pass OpenAI tool calling through on the openrouter lane; reject tools loudly elsewhere /v1/chat/completions deserialized into a struct without tools/tool_choice and messages without tool_calls/tool_call_id, so serde silently DROPPED them. Tool-using agents routed through :3100 (openclaw, 2026-09-18) never saw their tools and fabricated tool output as text. - ChatRequest: tools + tool_choice (serde default). Message: tool_calls, tool_call_id, name (skip_serializing_if none -> plain-text JSON is byte-identical; test pins it). - openrouter lane forwards them and returns the model's tool_calls (content null on a tool-call-only turn, finish_reason passed through). - every other provider: HTTP 400 when a request uses tools, instead of silent stripping. Verified live after deploy: tool_calls returned (finish_reason tool_calls); plain request unchanged; ollama + tools -> 400; multi-turn tool result round-trip answered from the tool. cargo test -p gateway --lib: 111 passed. Co-Authored-By: Claude Opus 5 (1M context) --- crates/gateway/src/execution_loop/mod.rs | 4 ++ crates/gateway/src/v1/kimi.rs | 2 +- crates/gateway/src/v1/mod.rs | 60 ++++++++++++++++- crates/gateway/src/v1/ollama.rs | 20 +++--- crates/gateway/src/v1/opencode.rs | 2 +- crates/gateway/src/v1/openrouter.rs | 82 ++++++++++++++++++++++-- 6 files changed, 153 insertions(+), 17 deletions(-) diff --git a/crates/gateway/src/execution_loop/mod.rs b/crates/gateway/src/execution_loop/mod.rs index b570544..80c795d 100644 --- a/crates/gateway/src/execution_loop/mod.rs +++ b/crates/gateway/src/execution_loop/mod.rs @@ -453,6 +453,8 @@ impl ExecutionLoop { stream: Some(false), think: Some(think), provider: Some("ollama_cloud".into()), + tools: None, + tool_choice: None, }; let resp = crate::v1::ollama_cloud::chat(key, &req).await .map_err(|e| format!("ollama_cloud: {e}"))?; @@ -693,6 +695,8 @@ impl ExecutionLoop { stream: Some(false), think: Some(false), provider: Some("ollama".into()), + tools: None, + tool_choice: None, }; let resp = crate::v1::ollama::chat(&self.state.ai_client, &chat_req).await .map_err(|e| format!("ollama overseer: {e}"))?; diff --git a/crates/gateway/src/v1/kimi.rs b/crates/gateway/src/v1/kimi.rs index 9ff2b7e..d23d00d 100644 --- a/crates/gateway/src/v1/kimi.rs +++ b/crates/gateway/src/v1/kimi.rs @@ -140,7 +140,7 @@ pub async fn chat( model, choices: vec![Choice { index: 0, - message: Message { role: "assistant".into(), content: serde_json::Value::String(text) }, + message: Message { role: "assistant".into(), content: serde_json::Value::String(text), ..Default::default() }, finish_reason: choice.finish_reason.unwrap_or_else(|| "stop".into()), }], usage: UsageBlock { diff --git a/crates/gateway/src/v1/mod.rs b/crates/gateway/src/v1/mod.rs index 441aff8..cdf264c 100644 --- a/crates/gateway/src/v1/mod.rs +++ b/crates/gateway/src/v1/mod.rs @@ -148,10 +148,22 @@ pub fn router(state: V1State) -> Router { /// providers can take it verbatim. `Message::text()` flattens for /// places that need a plain string (Ollama prompt assembly, char /// counts, the assistant's own response synthesis). -#[derive(Serialize, Deserialize, Clone, Debug)] +#[derive(Serialize, Deserialize, Clone, Debug, Default)] pub struct Message { pub role: String, + /// `Value::Null` is legal: an assistant turn that only makes tool calls. + #[serde(default)] pub content: serde_json::Value, + /// OpenAI tool-calling fields (2026-09-19). Passed through verbatim on the OpenRouter lane; + /// omitted from JSON when unset, so plain-text traffic is byte-identical to before. Before + /// this, serde silently DROPPED them — tool-using agents routed through :3100 never saw their + /// tools and fabricated tool output (autobun KB finding infra.lakehouse_gateway.tools). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub tool_calls: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub tool_call_id: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub name: Option, } impl Message { @@ -163,6 +175,7 @@ impl Message { Self { role: role.into(), content: serde_json::Value::String(body.into()), + ..Default::default() } } /// Flatten content to a plain string. Strings pass through; content- @@ -215,6 +228,21 @@ pub struct ChatRequest { /// from the model name; Phase 38/39 requires explicit selection. #[serde(default)] pub provider: Option, + /// OpenAI tool definitions + choice (2026-09-19). Forwarded on the OpenRouter lane only; + /// any other provider REJECTS them with 400 instead of silently stripping them. + #[serde(default)] + pub tools: Option, + #[serde(default)] + pub tool_choice: Option, +} + +impl ChatRequest { + /// True if the request uses tool calling anywhere (definitions or tool-call history). + pub fn uses_tools(&self) -> bool { + self.tools.is_some() + || self.tool_choice.is_some() + || self.messages.iter().any(|m| m.role == "tool" || m.tool_calls.is_some() || m.tool_call_id.is_some()) + } } #[derive(Serialize)] @@ -319,9 +347,28 @@ mod resolve_provider_tests { stream: None, think: None, provider: provider.map(|s| s.to_string()), + tools: None, + tool_choice: None, } } + #[test] + fn uses_tools_detects_definitions_and_history() { + let mut r = mk_req(None, "openrouter/z-ai/glm-5.3-flash"); + assert!(!r.uses_tools()); + r.tools = Some(serde_json::json!([])); + assert!(r.uses_tools()); + let mut r2 = mk_req(None, "qwen3.5:latest"); + r2.messages.push(Message { role: "tool".into(), content: "ok".into(), tool_call_id: Some("c1".into()), ..Default::default() }); + assert!(r2.uses_tools()); + } + + #[test] + fn plain_message_serialization_unchanged() { + let m = Message::new_text("user", "hi"); + assert_eq!(serde_json::to_string(&m).unwrap(), r#"{"role":"user","content":"hi"}"#); + } + #[test] fn explicit_provider_wins() { let r = mk_req(Some("openrouter"), "qwen3.5:latest"); @@ -391,6 +438,17 @@ async fn chat( // `model: "openrouter/openai/gpt-4o-mini"` → provider "openrouter", // adapter gets the stripped "openai/gpt-4o-mini". let (provider, effective_model) = resolve_provider(&req); + // Tool calling is only wired on the OpenRouter lane. Refuse loudly elsewhere — silently + // dropping tools made agents fabricate tool results (2026-09-19). + if req.uses_tools() && !matches!(provider.as_str(), "openrouter" | "openrouter_free") { + return Err(( + StatusCode::BAD_REQUEST, + format!( + "tools/tool_calls are only supported on the openrouter lane via this gateway \ + (resolved provider: {provider}); use an OpenRouter model id (e.g. openrouter//)" + ), + )); + } let start_time = chrono::Utc::now(); let start_instant = std::time::Instant::now(); diff --git a/crates/gateway/src/v1/ollama.rs b/crates/gateway/src/v1/ollama.rs index 240d8da..b6336ba 100644 --- a/crates/gateway/src/v1/ollama.rs +++ b/crates/gateway/src/v1/ollama.rs @@ -113,10 +113,10 @@ mod tests { #[test] fn flatten_separates_system_from_turns() { let msgs = vec![ - Message { role: "system".into(), content: "Rules here.".into() }, - Message { role: "user".into(), content: "Q1".into() }, - Message { role: "assistant".into(), content: "A1".into() }, - Message { role: "user".into(), content: "Q2".into() }, + Message { role: "system".into(), content: "Rules here.".into(), ..Default::default() }, + Message { role: "user".into(), content: "Q1".into(), ..Default::default() }, + Message { role: "assistant".into(), content: "A1".into(), ..Default::default() }, + Message { role: "user".into(), content: "Q2".into(), ..Default::default() }, ]; let (system, prompt) = flatten_messages(&msgs); assert_eq!(system, "Rules here."); @@ -129,9 +129,9 @@ mod tests { #[test] fn flatten_concatenates_multiple_system_messages() { let msgs = vec![ - Message { role: "system".into(), content: "First.".into() }, - Message { role: "system".into(), content: "Second.".into() }, - Message { role: "user".into(), content: "Hi".into() }, + Message { role: "system".into(), content: "First.".into(), ..Default::default() }, + Message { role: "system".into(), content: "Second.".into(), ..Default::default() }, + Message { role: "user".into(), content: "Hi".into(), ..Default::default() }, ]; let (system, _) = flatten_messages(&msgs); assert_eq!(system, "First.\nSecond."); @@ -139,7 +139,7 @@ mod tests { #[test] fn flatten_with_no_system_returns_empty_system() { - let msgs = vec![Message { role: "user".into(), content: "hi".into() }]; + let msgs = vec![Message { role: "user".into(), content: "hi".into(), ..Default::default() }]; let (system, prompt) = flatten_messages(&msgs); assert!(system.is_empty()); assert!(prompt.contains("user: hi")); @@ -147,11 +147,11 @@ mod tests { #[test] fn estimate_tokens_chars_div_4_ceiling() { - let msgs = vec![Message { role: "user".into(), content: "abcdefgh".into() }]; + let msgs = vec![Message { role: "user".into(), content: "abcdefgh".into(), ..Default::default() }]; // 8 chars / 4 = 2, with ceiling → 2 assert_eq!(estimate_prompt_tokens(&msgs), 2); - let msgs2 = vec![Message { role: "user".into(), content: "abcdefghi".into() }]; + let msgs2 = vec![Message { role: "user".into(), content: "abcdefghi".into(), ..Default::default() }]; // 9 chars → (9+3)/4 = 3 (ceiling) assert_eq!(estimate_prompt_tokens(&msgs2), 3); } diff --git a/crates/gateway/src/v1/opencode.rs b/crates/gateway/src/v1/opencode.rs index d45abf7..3786fae 100644 --- a/crates/gateway/src/v1/opencode.rs +++ b/crates/gateway/src/v1/opencode.rs @@ -145,7 +145,7 @@ pub async fn chat( model, choices: vec![Choice { index: 0, - message: Message { role: "assistant".into(), content: serde_json::Value::String(text) }, + message: Message { role: "assistant".into(), content: serde_json::Value::String(text), ..Default::default() }, finish_reason: choice.finish_reason.unwrap_or_else(|| "stop".into()), }], usage: UsageBlock { diff --git a/crates/gateway/src/v1/openrouter.rs b/crates/gateway/src/v1/openrouter.rs index 11312bc..8eca184 100644 --- a/crates/gateway/src/v1/openrouter.rs +++ b/crates/gateway/src/v1/openrouter.rs @@ -77,7 +77,12 @@ pub async fn chat( messages: req.messages.iter().map(|m| ORMessage { role: m.role.clone(), content: m.content.clone(), + tool_calls: m.tool_calls.clone(), + tool_call_id: m.tool_call_id.clone(), + name: m.name.clone(), }).collect(), + tools: req.tools.clone(), + tool_choice: req.tool_choice.clone(), max_tokens: Some(req.max_tokens.unwrap_or(800)), temperature: req.temperature.unwrap_or(0.3), stream: false, @@ -157,7 +162,10 @@ pub async fn chat( let latency_ms = t0.elapsed().as_millis(); let choice = parsed.choices.into_iter().next() .ok_or_else(|| "openrouter returned no choices".to_string())?; - let reasoning_only = choice.message.content.is_none() && choice.message.reasoning.is_some(); + let tool_calls = choice.message.tool_calls.clone(); + let reasoning_only = choice.message.content.is_none() + && choice.message.reasoning.is_some() + && tool_calls.is_none(); let text = choice.message.content.unwrap_or_default(); if reasoning_only { // The model spent its whole budget thinking. Return well-formed empty @@ -198,7 +206,17 @@ pub async fn chat( model, choices: vec![Choice { index: 0, - message: Message { role: "assistant".into(), content: serde_json::Value::String(text) }, + message: Message { + role: "assistant".into(), + // OpenAI shape: a tool-call-only turn carries `content: null`. + content: if tool_calls.is_some() && text.is_empty() { + serde_json::Value::Null + } else { + serde_json::Value::String(text) + }, + tool_calls, + ..Default::default() + }, finish_reason: choice.finish_reason.unwrap_or_else(|| "stop".into()), }], usage: UsageBlock { @@ -219,10 +237,23 @@ struct ORChatBody { max_tokens: Option, temperature: f64, stream: bool, + #[serde(skip_serializing_if = "Option::is_none")] + tools: Option, + #[serde(skip_serializing_if = "Option::is_none")] + tool_choice: Option, } #[derive(Serialize)] -struct ORMessage { role: String, content: serde_json::Value } +struct ORMessage { + role: String, + content: serde_json::Value, + #[serde(skip_serializing_if = "Option::is_none")] + tool_calls: Option, + #[serde(skip_serializing_if = "Option::is_none")] + tool_call_id: Option, + #[serde(skip_serializing_if = "Option::is_none")] + name: Option, +} #[derive(Deserialize)] struct ORChatResponse { @@ -250,6 +281,9 @@ struct ORMessageResp { content: Option, #[serde(default)] reasoning: Option, + /// OpenAI tool calls requested by the model (passed back to the client verbatim). + #[serde(default)] + tool_calls: Option, } #[derive(Deserialize)] @@ -286,17 +320,57 @@ mod tests { let body = ORChatBody { model: "openai/gpt-oss-120b:free".into(), messages: vec![ - ORMessage { role: "user".into(), content: "review this".into() }, + ORMessage { role: "user".into(), content: "review this".into(), + tool_calls: None, tool_call_id: None, name: None }, ], max_tokens: Some(800), temperature: 0.3, stream: false, + tools: None, + tool_choice: None, }; let json = serde_json::to_string(&body).unwrap(); assert!(json.contains("\"model\":\"openai/gpt-oss-120b:free\"")); assert!(json.contains("\"messages\"")); assert!(json.contains("\"max_tokens\":800")); assert!(json.contains("\"stream\":false")); + // no tool fields on plain traffic (byte-compat with pre-2026-09-19 bodies) + assert!(!json.contains("tools") && !json.contains("tool_call")); + } + + #[test] + fn chat_body_forwards_tools_and_tool_messages() { + let tools = serde_json::json!([{"type":"function","function":{"name":"exec","parameters":{"type":"object"}}}]); + let body = ORChatBody { + model: "z-ai/glm-5.3-flash".into(), + messages: vec![ + ORMessage { role: "assistant".into(), content: serde_json::Value::Null, + tool_calls: Some(serde_json::json!([{"id":"c1","type":"function","function":{"name":"exec","arguments":"{}"}}])), + tool_call_id: None, name: None }, + ORMessage { role: "tool".into(), content: "ok".into(), tool_calls: None, + tool_call_id: Some("c1".into()), name: Some("exec".into()) }, + ], + max_tokens: Some(100), + temperature: 0.0, + stream: false, + tools: Some(tools), + tool_choice: Some(serde_json::json!("auto")), + }; + let json = serde_json::to_string(&body).unwrap(); + assert!(json.contains("\"tools\":[{")); + assert!(json.contains("\"tool_choice\":\"auto\"")); + assert!(json.contains("\"tool_call_id\":\"c1\"")); + assert!(json.contains("\"tool_calls\":[{")); + } + + #[test] + fn response_with_tool_calls_parses() { + let raw = r#"{"choices":[{"message":{"content":null,"tool_calls":[{"id":"c1","type":"function","function":{"name":"exec","arguments":"{}"}}]},"finish_reason":"tool_calls"}]}"#; + let parsed: ORChatResponse = serde_json::from_str(raw).unwrap(); + let c = &parsed.choices[0]; + assert!(c.message.content.is_none()); + assert!(c.message.tool_calls.is_some()); + assert_eq!(c.finish_reason.as_deref(), Some("tool_calls")); } #[test]