From 3833b952baacba2156583a3ef1286642c985553b Mon Sep 17 00:00:00 2001 From: "erik.hauan" Date: Fri, 16 Oct 2026 08:34:36 -0700 Subject: [PATCH 1/4] H9 local wire codec; retire H8 Flatten (LM Studio) / DottedInbound (Ollama) codec in codex-local-models, OnceLock hooks in model-provider, client.rs and stream_events_utils.rs. router.rs/router_tests.rs reverted to rust-v0.162.1. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPCzgwU247gMtjybR5v4iR --- codex-rs/Cargo.lock | 1 + codex-rs/core/src/client.rs | 7 +- codex-rs/core/src/stream_events_utils.rs | 1 + codex-rs/core/src/tools/router.rs | 16 - codex-rs/core/src/tools/router_tests.rs | 18 - codex-rs/local/cli/src/main.rs | 1 + codex-rs/local/models/Cargo.toml | 3 +- codex-rs/local/models/src/codec.rs | 493 +++++++++++++++++++++++ codex-rs/local/models/src/lib.rs | 2 + codex-rs/model-provider/src/lib.rs | 1 + codex-rs/model-provider/src/provider.rs | 33 ++ local/FORK_SURFACE.md | 4 +- local/fork-surface.toml | 8 +- 13 files changed, 544 insertions(+), 44 deletions(-) create mode 100644 codex-rs/local/models/src/codec.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 2d6d224f4eaa..7a619efd2636 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3954,6 +3954,7 @@ dependencies = [ "codex-protocol", "reqwest 0.12.28", "serde_json", + "sha2 0.10.9", "tokio", "tracing", "wiremock", diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 24f2964ffe56..392ba9ea279b 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -912,7 +912,7 @@ impl ModelClient { self.state.thread_id.to_string().as_bytes(), ); let mut prefix = Vec::new(); - let tools = if model_info.use_responses_lite { + let mut tools = if model_info.use_responses_lite { if !prompt.tools.is_empty() { let tools = create_tools_json_for_responses_lite(&prompt.tools)?; prefix.push(ResponseItem::AdditionalTools { @@ -926,7 +926,7 @@ impl ModelClient { } None } else { - Some(create_tools_raw_json_for_responses_api(&prompt.tools)?.into()) + Some(create_tools_raw_json_for_responses_api(&prompt.tools)?) }; if !prompt.base_instructions.text.is_empty() { let mut instructions = ContextualUserFragment::into(BaseInstructionsFragment( @@ -950,6 +950,7 @@ impl ModelClient { *encrypted_function_args = None; } } + codex_model_provider::codec_outbound(&mut tools, &mut input)?; } let reasoning = self.build_reasoning(model_info, effort, summary); let stream_options = (self.state.concurrent_reasoning_summaries_enabled @@ -996,7 +997,7 @@ impl ModelClient { let request = ResponsesApiRequest { model: model_info.slug.clone(), input, - tools, + tools: tools.map(Into::into), tool_choice: "auto".to_string(), parallel_tool_calls: prompt.parallel_tool_calls && !model_info.use_responses_lite, reasoning: Some(reasoning), diff --git a/codex-rs/core/src/stream_events_utils.rs b/codex-rs/core/src/stream_events_utils.rs index 0264a09389b5..6572df610de6 100644 --- a/codex-rs/core/src/stream_events_utils.rs +++ b/codex-rs/core/src/stream_events_utils.rs @@ -317,6 +317,7 @@ pub(crate) async fn handle_output_item_done( item: ResponseItem, previously_active_item: Option, ) -> Result { + let item = codex_model_provider::codec_inbound(item); let mut output = OutputItemResult::default(); let plan_mode = ctx.step_context.turn.mode() == ModeKind::Plan; diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index c21f732774c8..baf39f082af7 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -344,21 +344,6 @@ impl ToolRouter { .await } - /// codex-local H8: some backends (Ollama) flatten `namespace` into the name - /// as `ns.tool`. Split only when the literal name misses but the split hits. - pub(crate) fn unflatten_tool_name(&self, name: ToolName) -> ToolName { - match name.name.split_once('.') { - Some((ns, n)) - if name.is_default_namespace() - && self.registry.tool(&name).is_none() - && self.registry.tool(&ToolName::namespaced(ns, n)).is_some() => - { - ToolName::namespaced(ns, n) - } - _ => name, - } - } - #[allow(clippy::too_many_arguments)] async fn dispatch_tool_call_with_code_mode_result_inner( &self, @@ -376,7 +361,6 @@ impl ToolRouter { payload, .. } = call; - let tool_name = self.unflatten_tool_name(tool_name); // Keep the legacy ToolInvocation.turn field tied to the same request state until handlers migrate. let turn = Arc::clone(&step_context.turn); diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index d0b691af800e..05a1e2e65a3b 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -649,21 +649,3 @@ fn namespace_function_names(specs: &[ToolSpec], namespace_name: &str) -> Vec anyhow::Result<()> { // SAFETY: still single-threaded; no runtime or worker threads exist yet. unsafe { std::env::set_var("CODEX_HOME", &codex_home) }; codex_local_models::register(); + codex_local_models::codec::register(codex_local_models::codec::Mode::DottedInbound); codex_local_resume::register(); let remote_control_disabled = codex_app_server::take_remote_control_disabled_env(); arg0_dispatch_or_else(move |arg0_paths: Arg0DispatchPaths| async move { diff --git a/codex-rs/local/models/Cargo.toml b/codex-rs/local/models/Cargo.toml index 0b7e3235cc9e..790a5cfd6178 100644 --- a/codex-rs/local/models/Cargo.toml +++ b/codex-rs/local/models/Cargo.toml @@ -18,7 +18,8 @@ codex-model-provider-info = { workspace = true } codex-models-manager = { workspace = true } codex-protocol = { workspace = true } reqwest = { workspace = true, features = ["json"] } -serde_json = { workspace = true } +serde_json = { workspace = true, features = ["raw_value"] } +sha2 = { workspace = true } tracing = { workspace = true } tokio = { workspace = true, features = ["rt", "macros", "sync"] } diff --git a/codex-rs/local/models/src/codec.rs b/codex-rs/local/models/src/codec.rs new file mode 100644 index 000000000000..7838b5a32e12 --- /dev/null +++ b/codex-rs/local/models/src/codec.rs @@ -0,0 +1,493 @@ +//! H9 local wire codec. Some local engines (LM Studio) reject Responses `namespace` +//! and `custom` tools. In `Flatten` mode both become plain function tools with +//! collision-checked aliases, history is translated the same way, and inbound calls are +//! mapped back to canonical items through an exact reverse map (aliases are never parsed). +//! `DottedInbound` (Ollama) sends tools unchanged and maps `ns.tool` call names back. + +use std::collections::HashMap; +use std::collections::HashSet; +use std::sync::Arc; +use std::sync::Mutex; +use std::sync::OnceLock; + +use codex_model_provider::LocalWireCodec; +use codex_protocol::models::ResponseItem; +use serde_json::Value; +use serde_json::json; +use serde_json::value::RawValue; +use sha2::Digest; +use sha2::Sha256; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Mode { + Flatten, + DottedInbound, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Kind { + Function, + Custom, +} + +type Pair = (Option, String); + +#[derive(Default)] +struct Codec { + fwd: HashMap, + rev: HashMap, +} + +static MODE: OnceLock = OnceLock::new(); +static CODEC: Mutex> = Mutex::new(None); + +/// Registers the codec (once, before the first request). +pub fn register(mode: Mode) { + if MODE.set(mode).is_ok() { + codex_model_provider::register_local_codec(LocalWireCodec { outbound, inbound }); + } +} + +fn with_codec(f: impl FnOnce(Mode, &mut Codec) -> R) -> R { + let mut guard = CODEC.lock().unwrap_or_else(|e| e.into_inner()); + f( + MODE.get().copied().unwrap_or(Mode::DottedInbound), + guard.get_or_insert_with(Codec::default), + ) +} + +fn outbound( + tools: &mut Option>, + input: &mut Vec, +) -> serde_json::Result<()> { + with_codec(|mode, c| c.outbound(mode, tools, input)) +} + +fn inbound(item: ResponseItem) -> ResponseItem { + with_codec(|mode, c| c.inbound(mode, item)) +} + +fn sanitize(s: &str) -> String { + s.chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '_' || c == '-' { + c + } else { + '_' + } + }) + .collect() +} + +fn hashed(ns: Option<&str>, name: &str, naive: &str) -> String { + let digest = Sha256::digest(format!("{}\0{name}", ns.unwrap_or(""))); + let hex: String = digest.iter().take(4).map(|b| format!("{b:02x}")).collect(); + let head: String = naive.chars().take(54).collect(); + format!("{head}_{hex}") +} + +/// `Some(alias)` when the plain alias is usable as-is. +fn plain_alias(ns: Option<&str>, name: &str) -> Option { + let alias = match ns { + Some(ns) => format!("{}__{}", sanitize(ns), sanitize(name)), + None => sanitize(name), + }; + let clean = ns.is_none_or(|n| sanitize(n) == n) && sanitize(name) == name; + (clean && alias.len() <= 64).then_some(alias) +} + +fn kind_of(tool: &Value) -> Kind { + if tool["type"] == "custom" { + Kind::Custom + } else { + Kind::Function + } +} + +fn as_function(mut tool: Value, alias: &str) -> Value { + if kind_of(&tool) == Kind::Custom { + let desc = tool["description"].as_str().unwrap_or(""); + tool = json!({ + "type": "function", + "description": format!("{desc}\nPass the full raw tool input as the `input` string."), + "parameters": { + "type": "object", + "properties": {"input": {"type": "string"}}, + "required": ["input"], + "additionalProperties": false, + }, + }); + } + tool["name"] = json!(alias); + tool +} + +impl Codec { + fn outbound( + &mut self, + mode: Mode, + tools: &mut Option>, + input: &mut Vec, + ) -> serde_json::Result<()> { + if let Some(raw) = tools.as_ref() { + let list: Vec = serde_json::from_str(raw.get())?; + match mode { + Mode::Flatten => { + let flat = self.flatten_tools(list); + *tools = Some(Arc::from(serde_json::value::to_raw_value(&flat)?)); + } + Mode::DottedInbound => self.index_dotted(&list), + } + } + if mode == Mode::Flatten { + for item in input.iter_mut() { + let taken = std::mem::replace(item, ResponseItem::Other); + *item = self.out_item(taken); + } + } + Ok(()) + } + + /// Dotted names are looked up as `ns.tool` through the same exact map. + fn index_dotted(&mut self, list: &[Value]) { + for t in list.iter().filter(|t| t["type"] == "namespace") { + let ns = t["name"].as_str().unwrap_or_default(); + for inner in t["tools"].as_array().into_iter().flatten() { + let name = inner["name"].as_str().unwrap_or_default(); + let pair = (Some(ns.to_string()), name.to_string()); + self.rev + .insert(format!("{ns}.{name}"), (pair, kind_of(inner))); + } + } + } + + fn flatten_tools(&mut self, list: Vec) -> Vec { + // Pass 1: collect (ns, name, tool) to translate, and literal top-level names. + let mut literals: HashSet = HashSet::new(); + let mut entries: Vec<(Pair, Value)> = Vec::new(); + let mut slots: Vec> = Vec::new(); + for t in list { + let name = t["name"].as_str().unwrap_or_default().to_string(); + if t["type"] == "namespace" { + for inner in t["tools"].as_array().cloned().unwrap_or_default() { + let n = inner["name"].as_str().unwrap_or_default().to_string(); + slots.push(Err(entries.len())); + entries.push(((Some(name.clone()), n), inner)); + } + } else if t["type"] == "custom" { + slots.push(Err(entries.len())); + entries.push(((None, name), t)); + } else { + literals.insert(name); + slots.push(Ok(t)); + } + } + // Pass 2: an alias is plain only if clean, short, unique and not a literal tool. + let mut owners: HashMap> = HashMap::new(); + for (pair, _) in &entries { + if let Some(a) = plain_alias(pair.0.as_deref(), &pair.1) { + owners.entry(a).or_default().insert(pair); + } + } + let aliases: Vec = entries + .iter() + .map(|(pair, _)| { + let (ns, name) = (pair.0.as_deref(), pair.1.as_str()); + plain_alias(ns, name) + .filter(|a| owners[a].len() == 1 && !literals.contains(a)) + .unwrap_or_else(|| { + let naive = match ns { + Some(ns) => format!("{}__{}", sanitize(ns), sanitize(name)), + None => sanitize(name), + }; + hashed(ns, name, &naive) + }) + }) + .collect(); + let mut out = Vec::new(); + for slot in slots { + match slot { + Ok(t) => out.push(t), + Err(i) => { + let (pair, tool) = &entries[i]; + let kind = kind_of(tool); + self.fwd.insert(pair.clone(), aliases[i].clone()); + self.rev.insert(aliases[i].clone(), (pair.clone(), kind)); + out.push(as_function(tool.clone(), &aliases[i])); + } + } + } + out + } + + fn alias(&mut self, ns: &Option, name: &str) -> String { + let key = (ns.clone(), name.to_string()); + if let Some(a) = self.fwd.get(&key) { + return a.clone(); + } + let a = plain_alias(ns.as_deref(), name) + .filter(|a| self.rev.get(a).is_none_or(|(p, _)| *p == key)) + .unwrap_or_else(|| hashed(ns.as_deref(), name, &sanitize(name))); + self.fwd.insert(key.clone(), a.clone()); + self.rev.insert(a.clone(), (key, Kind::Function)); + a + } + + fn out_item(&mut self, item: ResponseItem) -> ResponseItem { + match item { + ResponseItem::FunctionCall { + namespace: Some(ns), + name, + id, + arguments, + encrypted_function_args, + call_id, + internal_chat_message_metadata_passthrough, + } => { + let name = self.alias(&Some(ns), &name); + ResponseItem::FunctionCall { + id, + name, + namespace: None, + arguments, + encrypted_function_args, + call_id, + internal_chat_message_metadata_passthrough, + } + } + ResponseItem::FunctionCallOutput { + id, + call_id, + name, + namespace: Some(ns), + output, + internal_chat_message_metadata_passthrough, + } => { + let name = name.map(|n| self.alias(&Some(ns), &n)); + ResponseItem::FunctionCallOutput { + id, + call_id, + name, + namespace: None, + output, + internal_chat_message_metadata_passthrough, + } + } + ResponseItem::CustomToolCall { + id, + call_id, + name, + namespace, + input, + internal_chat_message_metadata_passthrough, + .. + } => { + let name = self.alias(&namespace, &name); + ResponseItem::FunctionCall { + id, + name, + namespace: None, + arguments: json!({ "input": input }).to_string(), + encrypted_function_args: None, + call_id, + internal_chat_message_metadata_passthrough, + } + } + ResponseItem::CustomToolCallOutput { + id, + call_id, + name, + output, + internal_chat_message_metadata_passthrough, + } => ResponseItem::FunctionCallOutput { + id, + call_id: Some(call_id), + name, + namespace: None, + output, + internal_chat_message_metadata_passthrough, + }, + other => other, + } + } + + fn inbound(&self, _mode: Mode, item: ResponseItem) -> ResponseItem { + let ResponseItem::FunctionCall { + namespace: None, + name, + .. + } = &item + else { + return item; + }; + // Flatten maps aliases; DottedInbound maps `ns.tool` (both live in `rev`). + let Some(((ns, orig), kind)) = self.rev.get(name).cloned() else { + return item; + }; + let ResponseItem::FunctionCall { + id, + arguments, + call_id, + internal_chat_message_metadata_passthrough, + encrypted_function_args, + .. + } = item + else { + unreachable!() + }; + match kind { + Kind::Function => ResponseItem::FunctionCall { + id, + name: orig, + namespace: ns, + arguments, + encrypted_function_args, + call_id, + internal_chat_message_metadata_passthrough, + }, + Kind::Custom => { + let input = serde_json::from_str::(&arguments) + .ok() + .and_then(|v| v["input"].as_str().map(str::to_string)) + .unwrap_or(arguments); + ResponseItem::CustomToolCall { + id, + status: None, + call_id, + name: orig, + namespace: ns, + input, + internal_chat_message_metadata_passthrough, + } + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn raw(v: Value) -> Option> { + Some(Arc::from(serde_json::value::to_raw_value(&v).unwrap())) + } + + fn ns_tool(ns: &str, names: &[&str]) -> Value { + let tools: Vec = names + .iter() + .map(|n| json!({"type": "function", "name": n, "description": "d", "parameters": {"type": "object"}})) + .collect(); + json!({"type": "namespace", "name": ns, "description": "", "tools": tools}) + } + + fn sent(tools: &Option>) -> Vec { + serde_json::from_str(tools.as_ref().unwrap().get()).unwrap() + } + + fn call(ns: Option<&str>, name: &str) -> ResponseItem { + ResponseItem::FunctionCall { + id: None, + name: name.into(), + namespace: ns.map(Into::into), + arguments: "{}".into(), + encrypted_function_args: None, + call_id: "c1".into(), + internal_chat_message_metadata_passthrough: None, + } + } + + fn flatten(c: &mut Codec, tools: Value) -> Vec { + let mut t = raw(tools); + c.outbound(Mode::Flatten, &mut t, &mut Vec::new()).unwrap(); + sent(&t) + } + + #[test] + fn collision_pair_gets_distinct_aliases_with_exact_reverse() { + let mut c = Codec::default(); + let out = flatten( + &mut c, + json!([ns_tool("mcp__s", &["a__b"]), ns_tool("mcp__s__a", &["b"])]), + ); + let (a, b) = ( + out[0]["name"].as_str().unwrap(), + out[1]["name"].as_str().unwrap(), + ); + assert_ne!(a, b); + assert!(out.iter().all(|t| t["type"] == "function")); + let back = c.inbound(Mode::Flatten, call(None, a)); + assert_eq!(back, call(Some("mcp__s"), "a__b")); + let back = c.inbound(Mode::Flatten, call(None, b)); + assert_eq!(back, call(Some("mcp__s__a"), "b")); + } + + #[test] + fn long_and_dotted_names_get_hashed_aliases_that_round_trip() { + let mut c = Codec::default(); + let long = "x".repeat(80); + let out = flatten(&mut c, json!([ns_tool("mcp__s", &[&long, "a.b"])])); + for (t, name) in out.iter().zip([long.as_str(), "a.b"]) { + let alias = t["name"].as_str().unwrap(); + assert!( + alias.len() <= 64 + && alias + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') + ); + assert_ne!(alias, name); + assert_eq!( + c.inbound(Mode::Flatten, call(None, alias)), + call(Some("mcp__s"), name) + ); + } + } + + #[test] + fn custom_tool_round_trips_and_history_keeps_call_id() { + let mut c = Codec::default(); + let custom = json!({"type": "custom", "name": "apply_patch", "description": "p", "format": {"type": "grammar"}}); + let mut tools = raw(json!([custom, ns_tool("multi", &["spawn"])])); + let hist_call = ResponseItem::CustomToolCall { + id: None, + status: None, + call_id: "c9".into(), + name: "apply_patch".into(), + namespace: None, + input: "*** Begin".into(), + internal_chat_message_metadata_passthrough: None, + }; + let mut input = vec![hist_call.clone(), call(Some("multi"), "spawn")]; + c.outbound(Mode::Flatten, &mut tools, &mut input).unwrap(); + assert_eq!(sent(&tools)[0]["parameters"]["required"], json!(["input"])); + let ResponseItem::FunctionCall { + call_id, + namespace: None, + .. + } = &input[0] + else { + panic!("{:?}", input[0]) + }; + assert_eq!(call_id, "c9"); + assert!( + matches!(&input[1], ResponseItem::FunctionCall { namespace: None, call_id, .. } if call_id == "c1") + ); + assert_eq!(c.inbound(Mode::Flatten, input[0].clone()), hist_call); + } + + #[test] + fn dotted_inbound_maps_only_sent_namespaces() { + let mut c = Codec::default(); + let mut tools = raw(json!([ns_tool("multi_agent_v1", &["spawn_agent"])])); + let before = tools.clone(); + c.outbound(Mode::DottedInbound, &mut tools, &mut Vec::new()) + .unwrap(); + assert_eq!(tools.unwrap().get(), before.unwrap().get()); + let got = c.inbound( + Mode::DottedInbound, + call(None, "multi_agent_v1.spawn_agent"), + ); + assert_eq!(got, call(Some("multi_agent_v1"), "spawn_agent")); + let unknown = call(None, "nope.spawn_agent"); + assert_eq!(c.inbound(Mode::DottedInbound, unknown.clone()), unknown); + } +} diff --git a/codex-rs/local/models/src/lib.rs b/codex-rs/local/models/src/lib.rs index a06148f827d8..b9045c1938a5 100644 --- a/codex-rs/local/models/src/lib.rs +++ b/codex-rs/local/models/src/lib.rs @@ -12,6 +12,8 @@ pub use engine::resolve_engine; pub use v1::V1Model; pub use v1::list_v1_models; +pub mod codec; + use std::fmt; use std::sync::Arc; use std::time::Duration; diff --git a/codex-rs/model-provider/src/lib.rs b/codex-rs/model-provider/src/lib.rs index eee2c75fe29c..18ecaf5afcaf 100644 --- a/codex-rs/model-provider/src/lib.rs +++ b/codex-rs/model-provider/src/lib.rs @@ -41,6 +41,7 @@ pub use provider::ProviderAccountState; pub use provider::ProviderAuthRecoveryMessages; pub use provider::ProviderUnauthorizedRecovery; pub use provider::SharedModelProvider; +pub use provider::{CodecTools, LocalWireCodec, codec_inbound, codec_outbound, register_local_codec}; pub use provider::create_model_provider; #[cfg(test)] diff --git a/codex-rs/model-provider/src/provider.rs b/codex-rs/model-provider/src/provider.rs index d58d16e618c6..ed538b368b0f 100644 --- a/codex-rs/model-provider/src/provider.rs +++ b/codex-rs/model-provider/src/provider.rs @@ -20,6 +20,7 @@ use codex_models_manager::manager::SharedModelsManager; use codex_models_manager::manager::StaticModelsManager; use codex_protocol::account::ProviderAccount; use codex_protocol::error::CodexErr; +use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ModelsResponse; use crate::ProviderCapabilities; @@ -369,6 +370,29 @@ fn local_models_manager(info: &ModelProviderInfo) -> Option (info.name == "codex-local").then(|| LOCAL_MODELS_FACTORY.get().map(|f| f(info)))? } +/// codex-local hook H9: wire codec registered by `codex-local-cli`; no-ops when unregistered. +pub type CodecTools = Option>; +pub struct LocalWireCodec { + pub outbound: fn(&mut CodecTools, &mut Vec) -> serde_json::Result<()>, + pub inbound: fn(ResponseItem) -> ResponseItem, +} +static LOCAL_CODEC: std::sync::OnceLock = std::sync::OnceLock::new(); + +pub fn register_local_codec(codec: LocalWireCodec) { + let _ = LOCAL_CODEC.set(codec); +} + +pub fn codec_outbound(t: &mut CodecTools, i: &mut Vec) -> serde_json::Result<()> { + LOCAL_CODEC.get().map_or(Ok(()), |c| (c.outbound)(t, i)) +} + +pub fn codec_inbound(item: ResponseItem) -> ResponseItem { + match LOCAL_CODEC.get() { + Some(c) => (c.inbound)(item), + None => item, + } +} + /// Runtime model provider that orchestrates primary and gateway credentials. #[derive(Clone, Debug)] struct ConfiguredModelProvider { @@ -1437,4 +1461,13 @@ printf '%s\n' '{"AccessKeyId":"exported","SecretAccessKey":"secret"}' assert!(local_models_manager(&named("codex-local")).is_some()); assert_eq!(CALLS.load(std::sync::atomic::Ordering::SeqCst), 1); } + + #[test] + fn local_codec_is_identity_when_unregistered() { + let mut input = vec![ResponseItem::Other]; + let mut tools = None; + codec_outbound(&mut tools, &mut input).unwrap(); + assert!(tools.is_none() && input == vec![ResponseItem::Other]); + assert_eq!(codec_inbound(ResponseItem::Other), ResponseItem::Other); + } } diff --git a/local/FORK_SURFACE.md b/local/FORK_SURFACE.md index 43390abad84e..6c5d7af546d0 100644 --- a/local/FORK_SURFACE.md +++ b/local/FORK_SURFACE.md @@ -9,7 +9,7 @@ | H5 | TODO | TODO | TODO | | H6 | TODO | TODO | TODO | | H7 | TODO | TODO | TODO | -| H8 | `codex-rs/core/src/tools/router.rs` | `ToolRouter::unflatten_tool_name` (+1 call in `dispatch_tool_call_with_code_mode_result_inner`): when a call has no/default namespace, its dotted name misses the registry, and the split at the first dot hits, dispatch under the split `(namespace, name)`. Works around Ollama flattening Responses `namespace` into the name (`multi_agent_v1.spawn_agent`). Unconditional: OpenAI names match `^[a-zA-Z0-9_-]+$`, so stock behaviour is unchanged. History keeps the model's original item. 16 upstream lines in router.rs (tests in router_tests.rs). Re-apply: add the method and call it on `tool_name` right after destructuring `call`. | done | +| H9 | `codex-rs/model-provider/src/provider.rs` (+ 1 `pub use` line in `lib.rs`), `codex-rs/core/src/client.rs`, `codex-rs/core/src/stream_events_utils.rs` | Local wire codec (replaces retired H8). `OnceLock` `LOCAL_CODEC` (`LocalWireCodec{outbound,inbound}`) + `register_local_codec`, `codec_outbound`, `codec_inbound`; both wrappers return immediately when unregistered. client.rs: `codec_outbound(&mut tools, &mut input)?` after the `!is_openai` input loop (`tools` is now `Option>` until the request struct, `tools.map(Into::into)`). stream_events_utils.rs: `let item = codec_inbound(item);` at the top of `handle_output_item_done`. Logic in `codex-rs/local/models/src/codec.rs` (`Mode::{Flatten, DottedInbound}`), registered by `codex-local-cli`. Re-apply: add the static/wrappers + identity test in provider.rs and the re-export in lib.rs; make the 4 client.rs edits (`let mut tools`, drop `.into()`, hook, `tools.map(Into::into)`); add the 1 line in stream_events_utils.rs. | done | ## Hook tests @@ -17,7 +17,7 @@ One line per hook, read by `local/scripts/sync_upstream.py check` and the rust j - Tests: `cargo test -p codex-model-provider local_models_factory` (H2) - Tests: `cargo test -p codex-local-resume` (H3) -- Tests: `cargo test -p codex-core --lib unflatten_tool_name` (H8) +- Tests: `cargo test -p codex-model-provider local_codec` and `cargo test -p codex-local-models codec` (H9) ## H1 re-apply recipe diff --git a/local/fork-surface.toml b/local/fork-surface.toml index 7e20af78ca0b..461c6d410ad1 100644 --- a/local/fork-surface.toml +++ b/local/fork-surface.toml @@ -26,8 +26,8 @@ allow = [ "codex-rs/Cargo.toml", "codex-rs/cli/src/main.rs", "codex-rs/cli/build.rs", - "codex-rs/core/src/tools/router.rs", - "codex-rs/core/src/tools/router_tests.rs", + "codex-rs/core/src/client.rs", + "codex-rs/core/src/stream_events_utils.rs", "codex-rs/model-provider/src/provider.rs", "codex-rs/model-provider/src/lib.rs", "codex-rs/core/src/lib.rs", @@ -38,7 +38,7 @@ allow = [ generated = ["codex-rs/Cargo.lock"] [budget] -# Measured 122 lines / 11 files across H1-H4 and H8 (Cargo.toml 1, H1 14, H2 36, H3 19, H4 18, -# H8 34 incl. tests) plus ~10% headroom. +# Measured across H1-H4 and H9 (Cargo.toml 1, H1 14, H2 36, H3 19, H4 18, H9 see FORK_SURFACE.md, +# H8 retired) plus headroom. max_lines = 135 max_files = 12 From 954cb76c770bb5188741814e2095c3ba2690d0b4 Mon Sep 17 00:00:00 2001 From: "erik.hauan" Date: Fri, 16 Oct 2026 08:42:12 -0700 Subject: [PATCH 2/4] codec: pure deterministic aliases, literal-conflict salting, canonicalise OutputItemAdded Alias is a pure function of (ns,name) shared by tool defs and history; literal top-level names are never reverse-mapped. Document known limits. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPCzgwU247gMtjybR5v4iR --- codex-rs/core/src/session/turn.rs | 1 + codex-rs/local/models/src/codec.rs | 245 +++++++++++++++++++++-------- local/FORK_SURFACE.md | 2 +- local/fork-surface.toml | 1 + 4 files changed, 186 insertions(+), 63 deletions(-) diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 64e8786a15b4..47ff63a85f12 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -2809,6 +2809,7 @@ async fn try_run_sampling_request( } } ResponseEvent::OutputItemAdded(mut item) => { + item = codex_model_provider::codec_inbound(item); assign_missing_streamed_response_item_id(&mut item, /*active_item*/ None); sess.reserve_assistant_message_order(&turn_context, &item) .await; diff --git a/codex-rs/local/models/src/codec.rs b/codex-rs/local/models/src/codec.rs index 7838b5a32e12..437984077960 100644 --- a/codex-rs/local/models/src/codec.rs +++ b/codex-rs/local/models/src/codec.rs @@ -3,6 +3,16 @@ //! collision-checked aliases, history is translated the same way, and inbound calls are //! mapped back to canonical items through an exact reverse map (aliases are never parsed). //! `DottedInbound` (Ollama) sends tools unchanged and maps `ns.tool` call names back. +//! +//! Known limits (documented, not fixed): aliases appear in telemetry and raw-capture +//! because those observers run before inbound canonicalisation; one engine per +//! codex-local process (no mixed-engine threads); the per-request literal set is +//! process-wide, so concurrent sub-agents with different tool sets can race on it; +//! `AdditionalTools`/`ToolSearchOutput` are only produced for OpenAI-hosted paths +//! (responses-lite, hosted tool_search) and never reach local engines, so they are not +//! translated; streamed argument deltas of flattened custom calls arrive as function +//! argument JSON, which the SSE layer does not turn into `ToolCallInputDelta`, so such +//! calls get no live diff preview (the final item is canonical and executes normally). use std::collections::HashMap; use std::collections::HashSet; @@ -34,8 +44,9 @@ type Pair = (Option, String); #[derive(Default)] struct Codec { - fwd: HashMap, rev: HashMap, + /// Literal top-level function names of the latest request; never reverse-mapped. + literals: HashSet, } static MODE: OnceLock = OnceLock::new(); @@ -79,21 +90,62 @@ fn sanitize(s: &str) -> String { .collect() } -fn hashed(ns: Option<&str>, name: &str, naive: &str) -> String { - let digest = Sha256::digest(format!("{}\0{name}", ns.unwrap_or(""))); - let hex: String = digest.iter().take(4).map(|b| format!("{b:02x}")).collect(); - let head: String = naive.chars().take(54).collect(); +fn naive(ns: Option<&str>, name: &str) -> String { + match ns { + Some(ns) => format!("{}__{}", sanitize(ns), sanitize(name)), + None => sanitize(name), + } +} + +/// Hashed alias: `trunc54(naive) + "_" + hex8(sha256(tag ns \0 name [\0 salt]))`. +/// `tag` separates `None` from `Some("")`; `salt == 0` hashes no salt. +fn hashed(ns: Option<&str>, name: &str, salt: u32) -> String { + let mut h = Sha256::new(); + h.update([u8::from(ns.is_some())]); + h.update(ns.unwrap_or("")); + h.update([0]); + h.update(name); + if salt > 0 { + h.update([0]); + h.update(salt.to_string()); + } + let hex: String = h + .finalize() + .iter() + .take(4) + .map(|b| format!("{b:02x}")) + .collect(); + let head: String = naive(ns, name).chars().take(54).collect(); format!("{head}_{hex}") } -/// `Some(alias)` when the plain alias is usable as-is. -fn plain_alias(ns: Option<&str>, name: &str) -> Option { - let alias = match ns { - Some(ns) => format!("{}__{}", sanitize(ns), sanitize(name)), - None => sanitize(name), - }; - let clean = ns.is_none_or(|n| sanitize(n) == n) && sanitize(name) == name; - (clean && alias.len() <= 64).then_some(alias) +/// Pure, deterministic alias of `(ns, name)` (depends on nothing else). +/// +/// Plain form `P(ns, name)`: `name` when `ns` is `None`, else `ns + "__" + name`. It is +/// used only when `ns`/`name` are unchanged by `sanitize`, `len(P) <= 64`, and: +/// * `ns == None`: `name` contains no `"__"`; +/// * `ns == Some`: `name` contains no `"__"` and does not start with `'_'` +/// (`ns` is unrestricted, so MCP namespaces `mcp__server__` work; they yield +/// `mcp__server____tool`). +/// Injectivity: a plain `None` alias never contains `"__"`, a plain `Some` alias +/// always does, so the two never collide. For `Some`, the last `"__"` in `P` starts +/// exactly at `len(ns)`: an occurrence starting later would lie inside `name` (barred) +/// or straddle the separator and `name[0]` (barred by the no-leading-`'_'` rule), and +/// `P` has no later separator. So `ns = P[..i]`, `name = P[i+2..]` with `i` the last +/// `"__"` index recovers the pair uniquely. Otherwise the hashed form is used +/// (`salt` 0), which is deterministic too; `salt > 0` is only for literal conflicts. +fn alias_of(ns: Option<&str>, name: &str, salt: u32) -> String { + let clean = ns.is_none_or(|n| sanitize(n) == n) + && sanitize(name) == name + && !name.contains("__") + && (ns.is_none() || !name.starts_with('_')); + if salt == 0 && clean { + let plain = naive(ns, name); + if plain.len() <= 64 { + return plain; + } + } + hashed(ns, name, salt) } fn kind_of(tool: &Value) -> Kind { @@ -155,8 +207,7 @@ impl Codec { for inner in t["tools"].as_array().into_iter().flatten() { let name = inner["name"].as_str().unwrap_or_default(); let pair = (Some(ns.to_string()), name.to_string()); - self.rev - .insert(format!("{ns}.{name}"), (pair, kind_of(inner))); + self.remember(&format!("{ns}.{name}"), pair, kind_of(inner), true); } } } @@ -182,28 +233,8 @@ impl Codec { slots.push(Ok(t)); } } - // Pass 2: an alias is plain only if clean, short, unique and not a literal tool. - let mut owners: HashMap> = HashMap::new(); - for (pair, _) in &entries { - if let Some(a) = plain_alias(pair.0.as_deref(), &pair.1) { - owners.entry(a).or_default().insert(pair); - } - } - let aliases: Vec = entries - .iter() - .map(|(pair, _)| { - let (ns, name) = (pair.0.as_deref(), pair.1.as_str()); - plain_alias(ns, name) - .filter(|a| owners[a].len() == 1 && !literals.contains(a)) - .unwrap_or_else(|| { - let naive = match ns { - Some(ns) => format!("{}__{}", sanitize(ns), sanitize(name)), - None => sanitize(name), - }; - hashed(ns, name, &naive) - }) - }) - .collect(); + // Aliases are pure; only a literal-name conflict in this request forces a salt. + self.literals = literals; let mut out = Vec::new(); for slot in slots { match slot { @@ -211,25 +242,44 @@ impl Codec { Err(i) => { let (pair, tool) = &entries[i]; let kind = kind_of(tool); - self.fwd.insert(pair.clone(), aliases[i].clone()); - self.rev.insert(aliases[i].clone(), (pair.clone(), kind)); - out.push(as_function(tool.clone(), &aliases[i])); + let alias = self.alias(pair.0.as_deref(), &pair.1, kind, true); + out.push(as_function(tool.clone(), &alias)); } } } out } - fn alias(&mut self, ns: &Option, name: &str) -> String { - let key = (ns.clone(), name.to_string()); - if let Some(a) = self.fwd.get(&key) { - return a.clone(); + /// Records `alias -> pair`. The mapping is injective, so a different pair under the + /// same alias is a bug (or a 32-bit hash collision): log it and keep the first. + fn remember(&mut self, alias: &str, pair: Pair, kind: Kind, authoritative: bool) { + match self.rev.get_mut(alias) { + Some((p, k)) if *p == pair => { + if authoritative { + *k = kind; + } + } + Some((p, _)) => { + tracing::error!(alias, existing = ?p, new = ?pair, "codec alias conflict"); + debug_assert!(false, "codec alias conflict for {alias}"); + } + None => { + self.rev.insert(alias.to_string(), (pair, kind)); + } + } + } + + /// Alias for tool definitions and history alike: the pure alias, salted only while it + /// equals a literal top-level function name of the current request. + fn alias(&mut self, ns: Option<&str>, name: &str, kind: Kind, tool_def: bool) -> String { + let mut salt = 0; + let mut a = alias_of(ns, name, salt); + while self.literals.contains(&a) { + salt += 1; + a = alias_of(ns, name, salt); } - let a = plain_alias(ns.as_deref(), name) - .filter(|a| self.rev.get(a).is_none_or(|(p, _)| *p == key)) - .unwrap_or_else(|| hashed(ns.as_deref(), name, &sanitize(name))); - self.fwd.insert(key.clone(), a.clone()); - self.rev.insert(a.clone(), (key, Kind::Function)); + let pair = (ns.map(str::to_string), name.to_string()); + self.remember(&a, pair, kind, tool_def); a } @@ -244,7 +294,7 @@ impl Codec { call_id, internal_chat_message_metadata_passthrough, } => { - let name = self.alias(&Some(ns), &name); + let name = self.alias(Some(&ns), &name, Kind::Function, false); ResponseItem::FunctionCall { id, name, @@ -263,7 +313,7 @@ impl Codec { output, internal_chat_message_metadata_passthrough, } => { - let name = name.map(|n| self.alias(&Some(ns), &n)); + let name = name.map(|n| self.alias(Some(&ns), &n, Kind::Function, false)); ResponseItem::FunctionCallOutput { id, call_id, @@ -282,7 +332,7 @@ impl Codec { internal_chat_message_metadata_passthrough, .. } => { - let name = self.alias(&namespace, &name); + let name = self.alias(namespace.as_deref(), &name, Kind::Custom, false); ResponseItem::FunctionCall { id, name, @@ -320,6 +370,10 @@ impl Codec { else { return item; }; + // A literal function name of this request is never redirected. + if self.literals.contains(name) { + return item; + } // Flatten maps aliases; DottedInbound maps `ns.tool` (both live in `rev`). let Some(((ns, orig), kind)) = self.rev.get(name).cloned() else { return item; @@ -403,22 +457,89 @@ mod tests { } #[test] - fn collision_pair_gets_distinct_aliases_with_exact_reverse() { + fn aliases_are_pure_and_stable_across_requests() { let mut c = Codec::default(); - let out = flatten( + let first = flatten(&mut c, json!([ns_tool("mcp__s__", &["t"])])); + let other = flatten( &mut c, - json!([ns_tool("mcp__s", &["a__b"]), ns_tool("mcp__s__a", &["b"])]), + json!([ns_tool("x", &["y"]), ns_tool("mcp__s__", &["t"])]), ); + assert_eq!(first[0]["name"], other[1]["name"]); + assert_eq!(first[0]["name"], "mcp__s____t"); + assert_eq!(first[0]["name"], alias_of(Some("mcp__s__"), "t", 0)); + } + + #[test] + fn colliding_pairs_get_distinct_stable_aliases_across_requests() { + let mut c = Codec::default(); + let r1 = flatten(&mut c, json!([ns_tool("a", &["b__c"])])); + let r2 = flatten(&mut c, json!([ns_tool("a__b", &["c"])])); let (a, b) = ( - out[0]["name"].as_str().unwrap(), - out[1]["name"].as_str().unwrap(), + r1[0]["name"].as_str().unwrap(), + r2[0]["name"].as_str().unwrap(), ); assert_ne!(a, b); - assert!(out.iter().all(|t| t["type"] == "function")); - let back = c.inbound(Mode::Flatten, call(None, a)); - assert_eq!(back, call(Some("mcp__s"), "a__b")); - let back = c.inbound(Mode::Flatten, call(None, b)); - assert_eq!(back, call(Some("mcp__s__a"), "b")); + assert_eq!( + c.inbound(Mode::Flatten, call(None, a)), + call(Some("a"), "b__c") + ); + assert_eq!( + c.inbound(Mode::Flatten, call(None, b)), + call(Some("a__b"), "c") + ); + let again = flatten(&mut c, json!([ns_tool("a", &["b__c"])])); + assert_eq!(again[0]["name"], a); + } + + #[test] + fn literal_equal_to_alias_is_salted_and_never_redirected() { + let mut c = Codec::default(); + let hashed_lit = alias_of(Some("a.b"), "x", 0); + let lit = + |n: &str| json!({"type": "function", "name": n, "parameters": {"type": "object"}}); + let out = flatten( + &mut c, + json!([ + ns_tool("a", &["b"]), + lit("a__b"), + ns_tool("a.b", &["x"]), + lit(&hashed_lit) + ]), + ); + let (plain, salted) = ( + out[0]["name"].as_str().unwrap(), + out[2]["name"].as_str().unwrap(), + ); + assert_ne!(plain, "a__b"); + assert_ne!(salted, hashed_lit); + assert_eq!( + c.inbound(Mode::Flatten, call(None, plain)), + call(Some("a"), "b") + ); + assert_eq!( + c.inbound(Mode::Flatten, call(None, salted)), + call(Some("a.b"), "x") + ); + for l in ["a__b", hashed_lit.as_str()] { + assert_eq!(c.inbound(Mode::Flatten, call(None, l)), call(None, l)); + } + } + + #[test] + fn flattened_custom_call_maps_back_to_custom_tool_call() { + let mut c = Codec::default(); + let custom = json!({"type": "custom", "name": "apply_patch", "description": "p"}); + flatten(&mut c, json!([custom])); + let mut fc = call(None, "apply_patch"); + if let ResponseItem::FunctionCall { arguments, .. } = &mut fc { + *arguments = json!({"input": "*** Begin"}).to_string(); + } + // router.rs maps `CustomToolCall` straight to `ToolPayload::Custom { input }`. + assert!(matches!( + c.inbound(Mode::Flatten, fc), + ResponseItem::CustomToolCall { namespace: None, name, input, .. } + if name == "apply_patch" && input == "*** Begin" + )); } #[test] diff --git a/local/FORK_SURFACE.md b/local/FORK_SURFACE.md index 6c5d7af546d0..30b48209ed24 100644 --- a/local/FORK_SURFACE.md +++ b/local/FORK_SURFACE.md @@ -9,7 +9,7 @@ | H5 | TODO | TODO | TODO | | H6 | TODO | TODO | TODO | | H7 | TODO | TODO | TODO | -| H9 | `codex-rs/model-provider/src/provider.rs` (+ 1 `pub use` line in `lib.rs`), `codex-rs/core/src/client.rs`, `codex-rs/core/src/stream_events_utils.rs` | Local wire codec (replaces retired H8). `OnceLock` `LOCAL_CODEC` (`LocalWireCodec{outbound,inbound}`) + `register_local_codec`, `codec_outbound`, `codec_inbound`; both wrappers return immediately when unregistered. client.rs: `codec_outbound(&mut tools, &mut input)?` after the `!is_openai` input loop (`tools` is now `Option>` until the request struct, `tools.map(Into::into)`). stream_events_utils.rs: `let item = codec_inbound(item);` at the top of `handle_output_item_done`. Logic in `codex-rs/local/models/src/codec.rs` (`Mode::{Flatten, DottedInbound}`), registered by `codex-local-cli`. Re-apply: add the static/wrappers + identity test in provider.rs and the re-export in lib.rs; make the 4 client.rs edits (`let mut tools`, drop `.into()`, hook, `tools.map(Into::into)`); add the 1 line in stream_events_utils.rs. | done | +| H9 | `codex-rs/model-provider/src/provider.rs` (+ 1 `pub use` line in `lib.rs`), `codex-rs/core/src/client.rs`, `codex-rs/core/src/stream_events_utils.rs`, `codex-rs/core/src/session/turn.rs` | Local wire codec (replaces retired H8). `OnceLock` `LOCAL_CODEC` (`LocalWireCodec{outbound,inbound}`) + `register_local_codec`, `codec_outbound`, `codec_inbound`; both wrappers return immediately when unregistered. client.rs: `codec_outbound(&mut tools, &mut input)?` after the `!is_openai` input loop (`tools` is now `Option>` until the request struct, `tools.map(Into::into)`). stream_events_utils.rs: `let item = codec_inbound(item);` at the top of `handle_output_item_done`. session/turn.rs: `item = codex_model_provider::codec_inbound(item);` as the first line of the `ResponseEvent::OutputItemAdded(mut item)` arm (so flattened custom calls get a diff consumer; their streamed args are function JSON and produce no `ToolCallInputDelta`, so no live diff preview: known limitation). Known limits: aliases show in telemetry/raw-capture (observers run pre-canonicalisation); one engine per codex-local process (no mixed-engine threads); `AdditionalTools`/`ToolSearchOutput` are OpenAI-hosted-only and never reach local engines, so untranslated. Aliases are a pure function of `(ns,name)` (see `alias_of` doc). Logic in `codex-rs/local/models/src/codec.rs` (`Mode::{Flatten, DottedInbound}`), registered by `codex-local-cli`. Re-apply: add the static/wrappers + identity test in provider.rs and the re-export in lib.rs; make the 4 client.rs edits (`let mut tools`, drop `.into()`, hook, `tools.map(Into::into)`); add the 1 line in stream_events_utils.rs and the 1 line in turn.rs. | done | ## Hook tests diff --git a/local/fork-surface.toml b/local/fork-surface.toml index 461c6d410ad1..67b8e0e55f2e 100644 --- a/local/fork-surface.toml +++ b/local/fork-surface.toml @@ -28,6 +28,7 @@ allow = [ "codex-rs/cli/build.rs", "codex-rs/core/src/client.rs", "codex-rs/core/src/stream_events_utils.rs", + "codex-rs/core/src/session/turn.rs", "codex-rs/model-provider/src/provider.rs", "codex-rs/model-provider/src/lib.rs", "codex-rs/core/src/lib.rs", From 232fdc75bc1369b2ccb5dbfe7157e33c5d4205ee Mon Sep 17 00:00:00 2001 From: "erik.hauan" Date: Fri, 16 Oct 2026 08:49:26 -0700 Subject: [PATCH 3/4] codec: select wire-codec mode from the resolved engine Ollama -> DottedInbound; LM Studio and llama.cpp -> Flatten (llama.cpp namespace handling is unverified; flatten is always safe). Action::Run carries the engine so the single registration point in main covers exec, tui and resume. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPCzgwU247gMtjybR5v4iR --- codex-rs/local/cli/src/lib.rs | 20 ++++++++++++++++++++ codex-rs/local/cli/src/main.rs | 7 ++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/codex-rs/local/cli/src/lib.rs b/codex-rs/local/cli/src/lib.rs index 6133fa0db792..9fee6bad5a86 100644 --- a/codex-rs/local/cli/src/lib.rs +++ b/codex-rs/local/cli/src/lib.rs @@ -61,6 +61,7 @@ pub enum Action { /// Delegate to upstream with this argv; the caller must first set /// `CODEX_HOME` to `codex_home`. Run { + engine: Engine, argv: Vec, codex_home: PathBuf, /// Engine root to probe before delegating; `None` skips the preflight. @@ -127,6 +128,7 @@ pub fn prepare(args: Vec, env: EnvFn, user_home: Option) -> R .flatten() .filter(|_| want_preflight); Ok(Action::Run { + engine: applied.engine, argv: applied.argv, codex_home: homes.local, preflight_root, @@ -134,6 +136,16 @@ pub fn prepare(args: Vec, env: EnvFn, user_home: Option) -> R }) } +/// Wire-codec mode for an engine. llama.cpp's namespace handling is unverified, and +/// flatten is always safe, so only Ollama gets the dotted-inbound mode. +pub fn codec_mode(engine: Engine) -> codex_local_models::codec::Mode { + use codex_local_models::codec::Mode; + match engine { + Engine::Ollama => Mode::DottedInbound, + Engine::LmStudio | Engine::LlamaCpp => Mode::Flatten, + } +} + /// Root that upstream will end up using: the injected base_url override, else the /// seeded config's base_url, else the env/built-in root. fn effective_root(argv: &[OsString], homes: &Homes, fallback: &str) -> String { @@ -182,6 +194,14 @@ pub(crate) fn env_ollama_root(env: EnvFn) -> Result> { #[cfg(test)] mod tests { + #[test] + fn engine_to_codec_mode() { + use codex_local_models::codec::Mode; + assert_eq!(codec_mode(Engine::Ollama), Mode::DottedInbound); + assert_eq!(codec_mode(Engine::LmStudio), Mode::Flatten); + assert_eq!(codec_mode(Engine::LlamaCpp), Mode::Flatten); + } + use super::*; use std::fs; diff --git a/codex-rs/local/cli/src/main.rs b/codex-rs/local/cli/src/main.rs index 0d93b0ee4fd2..c95e4ce13d56 100644 --- a/codex-rs/local/cli/src/main.rs +++ b/codex-rs/local/cli/src/main.rs @@ -51,18 +51,19 @@ fn main() -> anyhow::Result<()> { &codex_local_cli::process_env, codex_local_cli::user_home_dir(), )?; - let (argv, codex_home, preflight_root, probe_model) = match action { + let (engine, argv, codex_home, preflight_root, probe_model) = match action { codex_local_cli::Action::Version(v) => { println!("{v}"); return Ok(()); } codex_local_cli::Action::SyncConfig => return Ok(()), codex_local_cli::Action::Run { + engine, argv, codex_home, preflight_root, probe_model, - } => (argv, codex_home, preflight_root, probe_model), + } => (engine, argv, codex_home, preflight_root, probe_model), }; if let Some(root) = &preflight_root { let down = codex_local_cli::preflight_check(root); @@ -80,7 +81,7 @@ fn main() -> anyhow::Result<()> { // SAFETY: still single-threaded; no runtime or worker threads exist yet. unsafe { std::env::set_var("CODEX_HOME", &codex_home) }; codex_local_models::register(); - codex_local_models::codec::register(codex_local_models::codec::Mode::DottedInbound); + codex_local_models::codec::register(codex_local_cli::codec_mode(engine)); codex_local_resume::register(); let remote_control_disabled = codex_app_server::take_remote_control_disabled_env(); arg0_dispatch_or_else(move |arg0_paths: Arg0DispatchPaths| async move { From bcf6e45e75c77d9a6dc68444be5cb230d79c86e7 Mon Sep 17 00:00:00 2001 From: "erik.hauan" Date: Fri, 16 Oct 2026 08:54:30 -0700 Subject: [PATCH 4/4] codec: disjoint plain/hashed alias spaces; grow-only literal set Co-Authored-By: Claude Opus 5.5 --- codex-rs/local/models/src/codec.rs | 167 ++++++++++++++++++++--------- 1 file changed, 115 insertions(+), 52 deletions(-) diff --git a/codex-rs/local/models/src/codec.rs b/codex-rs/local/models/src/codec.rs index 437984077960..2a6207a6fb21 100644 --- a/codex-rs/local/models/src/codec.rs +++ b/codex-rs/local/models/src/codec.rs @@ -7,7 +7,8 @@ //! Known limits (documented, not fixed): aliases appear in telemetry and raw-capture //! because those observers run before inbound canonicalisation; one engine per //! codex-local process (no mixed-engine threads); the per-request literal set is -//! process-wide, so concurrent sub-agents with different tool sets can race on it; +//! process-wide and grow-only (if a literal first appears after a plain alias with the +//! same text was sent, the literal wins); //! `AdditionalTools`/`ToolSearchOutput` are only produced for OpenAI-hosted paths //! (responses-lite, hosted tool_search) and never reach local engines, so they are not //! translated; streamed argument deltas of flattened custom calls arrive as function @@ -45,7 +46,7 @@ type Pair = (Option, String); #[derive(Default)] struct Codec { rev: HashMap, - /// Literal top-level function names of the latest request; never reverse-mapped. + /// Every literal top-level function name ever sent (grow-only); never reverse-mapped. literals: HashSet, } @@ -97,7 +98,7 @@ fn naive(ns: Option<&str>, name: &str) -> String { } } -/// Hashed alias: `trunc54(naive) + "_" + hex8(sha256(tag ns \0 name [\0 salt]))`. +/// Hashed alias: `trunc54(naive) + "_h" + hex8(sha256(tag ns \0 name [\0 salt]))`. /// `tag` separates `None` from `Some("")`; `salt == 0` hashes no salt. fn hashed(ns: Option<&str>, name: &str, salt: u32) -> String { let mut h = Sha256::new(); @@ -116,32 +117,44 @@ fn hashed(ns: Option<&str>, name: &str, salt: u32) -> String { .map(|b| format!("{b:02x}")) .collect(); let head: String = naive(ns, name).chars().take(54).collect(); - format!("{head}_{hex}") + format!("{head}_h{hex}") +} + +/// True when `s` ends in `_h` + 8 lowercase hex characters (the hashed alias space). +fn in_hashed_space(s: &str) -> bool { + let b = s.as_bytes(); + b.len() >= 10 + && b[b.len() - 10..b.len() - 8] == *b"_h" + && b[b.len() - 8..] + .iter() + .all(|c| matches!(c, b'0'..=b'9' | b'a'..=b'f')) } /// Pure, deterministic alias of `(ns, name)` (depends on nothing else). /// -/// Plain form `P(ns, name)`: `name` when `ns` is `None`, else `ns + "__" + name`. It is -/// used only when `ns`/`name` are unchanged by `sanitize`, `len(P) <= 64`, and: -/// * `ns == None`: `name` contains no `"__"`; -/// * `ns == Some`: `name` contains no `"__"` and does not start with `'_'` -/// (`ns` is unrestricted, so MCP namespaces `mcp__server__` work; they yield -/// `mcp__server____tool`). -/// Injectivity: a plain `None` alias never contains `"__"`, a plain `Some` alias -/// always does, so the two never collide. For `Some`, the last `"__"` in `P` starts -/// exactly at `len(ns)`: an occurrence starting later would lie inside `name` (barred) -/// or straddle the separator and `name[0]` (barred by the no-leading-`'_'` rule), and -/// `P` has no later separator. So `ns = P[..i]`, `name = P[i+2..]` with `i` the last -/// `"__"` index recovers the pair uniquely. Otherwise the hashed form is used -/// (`salt` 0), which is deterministic too; `salt > 0` is only for literal conflicts. +/// Two disjoint alias spaces. Hashed aliases always end in `_h` + 8 lowercase hex. +/// A plain alias `P(ns, name)` (`name` when `ns` is `None`, else `ns + "__" + name`) is +/// used only when `ns`/`name` are unchanged by `sanitize`, `len(P) <= 64`, `name` +/// contains no `"__"` and does not start with `'_'`, and `P` is NOT in the hashed +/// space. Otherwise (or when `salt > 0`, used only for literal conflicts) the hashed +/// form is used. +/// 1. plain vs plain: a plain `None` alias has no `"__"` (name has none, no leading +/// `'_'`), a plain `Some` alias has one; so the kinds never collide. For `Some`, +/// the last `"__"` in `P` starts exactly at `len(ns)`: a later occurrence would +/// lie inside `name` (barred) or straddle the separator and `name[0]` (barred), so +/// splitting at the last `"__"` recovers `(ns, name)` uniquely. `ns` is otherwise +/// unrestricted, so MCP namespaces `mcp__server__` work. +/// 2. plain vs hashed: disjoint by construction (plain never matches `_h[0-9a-f]{8}$`). +/// 3. hashed vs hashed: equal only on a 32-bit sha256 prefix collision, which +/// `Codec::remember` detects in the reverse map and turns into an outbound error. fn alias_of(ns: Option<&str>, name: &str, salt: u32) -> String { let clean = ns.is_none_or(|n| sanitize(n) == n) && sanitize(name) == name && !name.contains("__") - && (ns.is_none() || !name.starts_with('_')); + && !name.starts_with('_'); if salt == 0 && clean { let plain = naive(ns, name); - if plain.len() <= 64 { + if plain.len() <= 64 && !in_hashed_space(&plain) { return plain; } } @@ -185,36 +198,40 @@ impl Codec { let list: Vec = serde_json::from_str(raw.get())?; match mode { Mode::Flatten => { - let flat = self.flatten_tools(list); + let flat = self.flatten_tools(list)?; *tools = Some(Arc::from(serde_json::value::to_raw_value(&flat)?)); } - Mode::DottedInbound => self.index_dotted(&list), + Mode::DottedInbound => self.index_dotted(&list)?, } } if mode == Mode::Flatten { for item in input.iter_mut() { let taken = std::mem::replace(item, ResponseItem::Other); - *item = self.out_item(taken); + *item = self.out_item(taken)?; } } Ok(()) } /// Dotted names are looked up as `ns.tool` through the same exact map. - fn index_dotted(&mut self, list: &[Value]) { + fn index_dotted(&mut self, list: &[Value]) -> serde_json::Result<()> { + for t in list.iter().filter(|t| t["type"] == "function") { + self.literals + .insert(t["name"].as_str().unwrap_or_default().to_string()); + } for t in list.iter().filter(|t| t["type"] == "namespace") { let ns = t["name"].as_str().unwrap_or_default(); for inner in t["tools"].as_array().into_iter().flatten() { let name = inner["name"].as_str().unwrap_or_default(); let pair = (Some(ns.to_string()), name.to_string()); - self.remember(&format!("{ns}.{name}"), pair, kind_of(inner), true); + self.remember(&format!("{ns}.{name}"), pair, kind_of(inner), true)?; } } + Ok(()) } - fn flatten_tools(&mut self, list: Vec) -> Vec { + fn flatten_tools(&mut self, list: Vec) -> serde_json::Result> { // Pass 1: collect (ns, name, tool) to translate, and literal top-level names. - let mut literals: HashSet = HashSet::new(); let mut entries: Vec<(Pair, Value)> = Vec::new(); let mut slots: Vec> = Vec::new(); for t in list { @@ -229,12 +246,11 @@ impl Codec { slots.push(Err(entries.len())); entries.push(((None, name), t)); } else { - literals.insert(name); + self.literals.insert(name); slots.push(Ok(t)); } } - // Aliases are pure; only a literal-name conflict in this request forces a salt. - self.literals = literals; + // Aliases are pure; only a conflict with a literal ever sent forces a salt. let mut out = Vec::new(); for slot in slots { match slot { @@ -242,17 +258,23 @@ impl Codec { Err(i) => { let (pair, tool) = &entries[i]; let kind = kind_of(tool); - let alias = self.alias(pair.0.as_deref(), &pair.1, kind, true); + let alias = self.alias(pair.0.as_deref(), &pair.1, kind, true)?; out.push(as_function(tool.clone(), &alias)); } } } - out + Ok(out) } - /// Records `alias -> pair`. The mapping is injective, so a different pair under the - /// same alias is a bug (or a 32-bit hash collision): log it and keep the first. - fn remember(&mut self, alias: &str, pair: Pair, kind: Kind, authoritative: bool) { + /// Records `alias -> pair`. A different pair under the same alias can only be a + /// 32-bit hash collision: fail the request rather than send an ambiguous alias. + fn remember( + &mut self, + alias: &str, + pair: Pair, + kind: Kind, + authoritative: bool, + ) -> serde_json::Result<()> { match self.rev.get_mut(alias) { Some((p, k)) if *p == pair => { if authoritative { @@ -261,17 +283,26 @@ impl Codec { } Some((p, _)) => { tracing::error!(alias, existing = ?p, new = ?pair, "codec alias conflict"); - debug_assert!(false, "codec alias conflict for {alias}"); + return Err(serde_json::Error::io(std::io::Error::other(format!( + "codec alias conflict for {alias}: {p:?} vs {pair:?}" + )))); } None => { self.rev.insert(alias.to_string(), (pair, kind)); } } + Ok(()) } - /// Alias for tool definitions and history alike: the pure alias, salted only while it - /// equals a literal top-level function name of the current request. - fn alias(&mut self, ns: Option<&str>, name: &str, kind: Kind, tool_def: bool) -> String { + /// Alias for tool definitions and history alike: the pure alias, salted (still in + /// hashed space) while it equals any literal top-level function name ever sent. + fn alias( + &mut self, + ns: Option<&str>, + name: &str, + kind: Kind, + tool_def: bool, + ) -> serde_json::Result { let mut salt = 0; let mut a = alias_of(ns, name, salt); while self.literals.contains(&a) { @@ -279,12 +310,12 @@ impl Codec { a = alias_of(ns, name, salt); } let pair = (ns.map(str::to_string), name.to_string()); - self.remember(&a, pair, kind, tool_def); - a + self.remember(&a, pair, kind, tool_def)?; + Ok(a) } - fn out_item(&mut self, item: ResponseItem) -> ResponseItem { - match item { + fn out_item(&mut self, item: ResponseItem) -> serde_json::Result { + Ok(match item { ResponseItem::FunctionCall { namespace: Some(ns), name, @@ -294,7 +325,7 @@ impl Codec { call_id, internal_chat_message_metadata_passthrough, } => { - let name = self.alias(Some(&ns), &name, Kind::Function, false); + let name = self.alias(Some(&ns), &name, Kind::Function, false)?; ResponseItem::FunctionCall { id, name, @@ -313,7 +344,9 @@ impl Codec { output, internal_chat_message_metadata_passthrough, } => { - let name = name.map(|n| self.alias(Some(&ns), &n, Kind::Function, false)); + let name = name + .map(|n| self.alias(Some(&ns), &n, Kind::Function, false)) + .transpose()?; ResponseItem::FunctionCallOutput { id, call_id, @@ -332,7 +365,7 @@ impl Codec { internal_chat_message_metadata_passthrough, .. } => { - let name = self.alias(namespace.as_deref(), &name, Kind::Custom, false); + let name = self.alias(namespace.as_deref(), &name, Kind::Custom, false)?; ResponseItem::FunctionCall { id, name, @@ -358,7 +391,7 @@ impl Codec { internal_chat_message_metadata_passthrough, }, other => other, - } + }) } fn inbound(&self, _mode: Mode, item: ResponseItem) -> ResponseItem { @@ -370,7 +403,7 @@ impl Codec { else { return item; }; - // A literal function name of this request is never redirected. + // A literal function name ever sent is never redirected. if self.literals.contains(name) { return item; } @@ -470,10 +503,10 @@ mod tests { } #[test] - fn colliding_pairs_get_distinct_stable_aliases_across_requests() { + fn review_case1_pairs_get_distinct_aliases() { let mut c = Codec::default(); let r1 = flatten(&mut c, json!([ns_tool("a", &["b__c"])])); - let r2 = flatten(&mut c, json!([ns_tool("a__b", &["c"])])); + let r2 = flatten(&mut c, json!([ns_tool("a__b", &["c_4bf34dff"])])); let (a, b) = ( r1[0]["name"].as_str().unwrap(), r2[0]["name"].as_str().unwrap(), @@ -485,10 +518,40 @@ mod tests { ); assert_eq!( c.inbound(Mode::Flatten, call(None, b)), - call(Some("a__b"), "c") + call(Some("a__b"), "c_4bf34dff") + ); + } + + #[test] + fn salted_literal_case_gets_distinct_aliases() { + let mut c = Codec::default(); + let lit = json!({"type": "function", "name": "a__b", "parameters": {"type": "object"}}); + let out = flatten(&mut c, json!([lit, ns_tool("a", &["b", "b_94fa8649"])])); + let (x, y) = ( + out[1]["name"].as_str().unwrap(), + out[2]["name"].as_str().unwrap(), + ); + assert_ne!(x, y); + assert_ne!(x, "a__b"); + assert!(in_hashed_space(x)); + assert_eq!( + c.inbound(Mode::Flatten, call(None, x)), + call(Some("a"), "b") ); - let again = flatten(&mut c, json!([ns_tool("a", &["b__c"])])); - assert_eq!(again[0]["name"], a); + assert_eq!( + c.inbound(Mode::Flatten, call(None, y)), + call(Some("a"), "b_94fa8649") + ); + } + + #[test] + fn literal_from_earlier_request_is_never_reverse_mapped() { + let mut c = Codec::default(); + let lit = json!({"type": "function", "name": "a__b", "parameters": {"type": "object"}}); + flatten(&mut c, json!([lit, ns_tool("a.b", &["x"])])); + flatten(&mut c, json!([ns_tool("a", &["b"])])); + let l = call(None, "a__b"); + assert_eq!(c.inbound(Mode::Flatten, l.clone()), l); } #[test]