fix(gateway/v1): pass OpenAI tool calling through on the openrouter lane; reject tools loudly elsewhere
lakehouse/auditor 11 blocking issues: cloud: claim not backed — "Verified live after deploy: tool_calls returned (finish_reason tool_calls); plain request"
lakehouse/auditor 11 blocking issues: cloud: claim not backed — "Verified live after deploy: tool_calls returned (finish_reason tool_calls); plain request"
/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) <noreply@anthropic.com>
This commit is contained in:
@@ -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}"))?;
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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_json::Value>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub tool_call_id: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub name: Option<String>,
|
||||
}
|
||||
|
||||
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<String>,
|
||||
/// 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_json::Value>,
|
||||
#[serde(default)]
|
||||
pub tool_choice: Option<serde_json::Value>,
|
||||
}
|
||||
|
||||
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/<vendor>/<model>)"
|
||||
),
|
||||
));
|
||||
}
|
||||
let start_time = chrono::Utc::now();
|
||||
let start_instant = std::time::Instant::now();
|
||||
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<u32>,
|
||||
temperature: f64,
|
||||
stream: bool,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
tools: Option<serde_json::Value>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
tool_choice: Option<serde_json::Value>,
|
||||
}
|
||||
|
||||
#[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_json::Value>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
tool_call_id: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
name: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct ORChatResponse {
|
||||
@@ -250,6 +281,9 @@ struct ORMessageResp {
|
||||
content: Option<String>,
|
||||
#[serde(default)]
|
||||
reasoning: Option<String>,
|
||||
/// OpenAI tool calls requested by the model (passed back to the client verbatim).
|
||||
#[serde(default)]
|
||||
tool_calls: Option<serde_json::Value>,
|
||||
}
|
||||
|
||||
#[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]
|
||||
|
||||
Reference in New Issue
Block a user