Skip to content

Commit e247318

Browse files
committed
fix: preserve Codex MCP namespaces through translation
1 parent 58f355a commit e247318

5 files changed

Lines changed: 409 additions & 19 deletions

File tree

crates/libsy-llm-client/src/client.rs

Lines changed: 172 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,7 @@ impl TranslatingLlmClient {
463463
model: Option<&str>,
464464
wire_format: WireFormat,
465465
) -> Result<RawResponse> {
466+
let response_namespaces = responses_tool_namespaces(&raw_http_request, wire_format);
466467
let llm_request = decode_request(wire_format, &raw_http_request)
467468
.map_err(|error| LlmClientError::RequestTranslation(error.to_string()))?;
468469
// The model that serves the call — the rewrite target when the caller pinned
@@ -490,14 +491,25 @@ impl TranslatingLlmClient {
490491

491492
match response.llm_response {
492493
LlmResponse::Agg(agg) => {
493-
let body =
494+
let mut body =
494495
encode_aggregated_response(&agg, wire_format, served_model.as_deref())
495496
.map_err(|error| LlmClientError::ResponseTranslation(error.to_string()))?;
497+
restore_responses_tool_namespaces(&mut body, &response_namespaces);
496498
Ok(RawResponse::Buffered(body))
497499
}
498500
LlmResponse::Stream(chunks) => {
499501
let events = encode_stream(chunks, wire_format, served_model)?;
500-
Ok(RawResponse::Stream(events))
502+
if response_namespaces.is_empty() {
503+
Ok(RawResponse::Stream(events))
504+
} else {
505+
let events = events.map(move |event| {
506+
event.map(|mut value| {
507+
restore_responses_tool_namespaces(&mut value, &response_namespaces);
508+
value
509+
})
510+
});
511+
Ok(RawResponse::Stream(Box::pin(events)))
512+
}
501513
}
502514
}
503515
}
@@ -792,6 +804,100 @@ fn is_reserved_header(name: &str) -> bool {
792804
.any(|reserved| name.eq_ignore_ascii_case(reserved))
793805
}
794806

807+
// Returns the MCP namespace for every unambiguous function exposed by an
808+
// OpenAI Responses request. Codex wraps MCP functions in a non-standard
809+
// ``namespace`` tool, while OpenAI-compatible upstreams accept only its child
810+
// function tools. Keeping this lookup at the raw proxy boundary lets the
811+
// response reintroduce the namespace Codex needs for MCP dispatch.
812+
fn responses_tool_namespaces(body: &Value, wire_format: WireFormat) -> HashMap<String, String> {
813+
if wire_format != WireFormat::OpenAiResponses {
814+
return HashMap::new();
815+
}
816+
let Some(tools) = body.get("tools").and_then(Value::as_array) else {
817+
return HashMap::new();
818+
};
819+
let mut namespaces = HashMap::new();
820+
collect_responses_tool_namespaces(tools, None, &mut namespaces);
821+
namespaces
822+
.into_iter()
823+
.filter_map(|(name, namespace)| namespace.map(|namespace| (name, namespace)))
824+
.collect()
825+
}
826+
827+
// A None entry marks a duplicate tool name under different MCP namespaces. A
828+
// plain function call cannot disambiguate those safely, so it remains flat.
829+
fn collect_responses_tool_namespaces(
830+
tools: &[Value],
831+
parent_namespace: Option<&str>,
832+
namespaces: &mut HashMap<String, Option<String>>,
833+
) {
834+
for tool in tools {
835+
let Some(tool) = tool.as_object() else {
836+
continue;
837+
};
838+
if tool.get("type").and_then(Value::as_str) == Some("namespace") {
839+
let namespace = tool.get("name").and_then(Value::as_str);
840+
if let Some(children) = tool.get("tools").and_then(Value::as_array) {
841+
collect_responses_tool_namespaces(children, namespace, namespaces);
842+
}
843+
continue;
844+
}
845+
let Some(namespace) = parent_namespace else {
846+
continue;
847+
};
848+
let name = tool
849+
.get("function")
850+
.and_then(Value::as_object)
851+
.and_then(|function| function.get("name"))
852+
.or_else(|| tool.get("name"))
853+
.or_else(|| tool.get("id"))
854+
.and_then(Value::as_str)
855+
.filter(|name| !name.is_empty());
856+
let Some(name) = name else {
857+
continue;
858+
};
859+
match namespaces.get(name) {
860+
None => {
861+
namespaces.insert(name.to_string(), Some(namespace.to_string()));
862+
}
863+
Some(Some(existing)) if existing != namespace => {
864+
namespaces.insert(name.to_string(), None);
865+
}
866+
Some(_) => {}
867+
}
868+
}
869+
}
870+
871+
// Adds a Codex-compatible namespace field to every outbound Responses function
872+
// call whose name originated from an unambiguously flattened MCP namespace.
873+
// This visits buffered Responses bodies and all Responses streaming events.
874+
fn restore_responses_tool_namespaces(body: &mut Value, namespaces: &HashMap<String, String>) {
875+
if namespaces.is_empty() {
876+
return;
877+
}
878+
match body {
879+
Value::Array(values) => {
880+
for value in values {
881+
restore_responses_tool_namespaces(value, namespaces);
882+
}
883+
}
884+
Value::Object(object) => {
885+
if object.get("type").and_then(Value::as_str) == Some("function_call")
886+
&& let Some(name) = object.get("name").and_then(Value::as_str)
887+
&& let Some(namespace) = namespaces.get(name)
888+
{
889+
object
890+
.entry("namespace".to_string())
891+
.or_insert_with(|| Value::String(namespace.clone()));
892+
}
893+
for value in object.values_mut() {
894+
restore_responses_tool_namespaces(value, namespaces);
895+
}
896+
}
897+
_ => {}
898+
}
899+
}
900+
795901
#[cfg(test)]
796902
mod tests {
797903
use std::collections::BTreeMap;
@@ -1873,6 +1979,70 @@ mod tests {
18731979
Ok(())
18741980
}
18751981

1982+
// A Codex MCP namespace is flattened for the Chat upstream, then restored
1983+
// on the Responses function call that comes back to Codex.
1984+
#[tokio::test]
1985+
async fn call_rewrite_model_raw_restores_codex_mcp_namespace()
1986+
-> std::result::Result<(), Box<dyn Error + Sync + Send + 'static>> {
1987+
let server = MockServer::start().await;
1988+
Mock::given(method("POST"))
1989+
.and(path("/v1/chat/completions"))
1990+
.and(wiremock::matchers::body_partial_json(json!({
1991+
"tools": [{
1992+
"type": "function",
1993+
"function": {"name": "search"}
1994+
}]
1995+
})))
1996+
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
1997+
"id": "chatcmpl-1",
1998+
"model": "gpt",
1999+
"choices": [{
2000+
"index": 0,
2001+
"message": {
2002+
"role": "assistant",
2003+
"content": null,
2004+
"tool_calls": [{
2005+
"id": "call_1",
2006+
"type": "function",
2007+
"function": {"name": "search", "arguments": "{\\\"q\\\":\\\"rust\\\"}"}
2008+
}]
2009+
},
2010+
"finish_reason": "tool_calls"
2011+
}],
2012+
"usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}
2013+
})))
2014+
.mount(&server)
2015+
.await;
2016+
2017+
let client = TranslatingLlmClient::new(&chat_map(&format!("{}/v1", server.uri())))?;
2018+
let raw = json!({
2019+
"model": "client-facing",
2020+
"input": "Search for Rust.",
2021+
"tools": [{
2022+
"type": "namespace",
2023+
"name": "mcp__open_websearch__",
2024+
"tools": [{
2025+
"type": "function",
2026+
"name": "search",
2027+
"description": "Search the web",
2028+
"parameters": {"type": "object", "properties": {"q": {"type": "string"}}}
2029+
}]
2030+
}]
2031+
});
2032+
2033+
let RawResponse::Buffered(body) = client
2034+
.call_rewrite_model_raw(raw, None, Some("gpt"), WireFormat::OpenAiResponses)
2035+
.await?
2036+
else {
2037+
panic!("expected a buffered response");
2038+
};
2039+
2040+
assert_eq!(body["output"][0]["type"], "function_call");
2041+
assert_eq!(body["output"][0]["name"], "search");
2042+
assert_eq!(body["output"][0]["namespace"], "mcp__open_websearch__");
2043+
Ok(())
2044+
}
2045+
18762046
// Raw path, streaming: an inbound `stream: true` request yields an unframed stream
18772047
// of OpenAI Chat chunk objects whose deltas reassemble the completion.
18782048
#[tokio::test]

crates/switchyard-server/src/lib.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ use tracing::{Instrument, Level};
4444

4545
use switchyard_translation::{WireFormat, decode_request};
4646

47-
use crate::response::into_http_response;
47+
use crate::response::{into_http_response, responses_tool_namespaces};
4848
use crate::stats::{StatsAccumulator, StatsSnapshot, prefix_probe, tracking_enabled_from_env};
4949

5050
pub use observability::{flush_observability, initialize_observability};
@@ -686,6 +686,7 @@ async fn handle_llm_request(
686686
wire_format: WireFormat,
687687
routing_log_context: Option<routing_log::RoutingLogContext>,
688688
) -> Response {
689+
let response_namespaces = responses_tool_namespaces(&body, wire_format);
689690
let cache_probe = state.track_cache_eligibility.then(|| prefix_probe(&body));
690691
let (route, request) = match resolve_route(&state, metadata, body, wire_format) {
691692
Ok(resolved) => resolved,
@@ -728,10 +729,11 @@ async fn handle_llm_request(
728729
};
729730

730731
let served_model = decision.map(|decision| decision.selected_model_id().to_string());
731-
let mut response = match into_http_response(response, wire_format, served_model) {
732-
Ok(response) => response,
733-
Err(error) => return server_error(error.to_string()),
734-
};
732+
let mut response =
733+
match into_http_response(response, wire_format, served_model, response_namespaces) {
734+
Ok(response) => response,
735+
Err(error) => return server_error(error.to_string()),
736+
};
735737
if let Some(decision) = decision {
736738
attach_routing_headers(&mut response, decision);
737739
}

0 commit comments

Comments
 (0)