diff --git a/codex-rs/app-server/tests/suite/v2/legacy_multi_agent_v2_namespace_repair.rs b/codex-rs/app-server/tests/suite/v2/legacy_multi_agent_v2_namespace_repair.rs new file mode 100644 index 0000000000..bf70f716cf --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/legacy_multi_agent_v2_namespace_repair.rs @@ -0,0 +1,182 @@ +use anyhow::Result; +use app_test_support::TestAppServer; +use app_test_support::create_fake_rollout; +use app_test_support::rollout_path; +use app_test_support::to_response; +use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ThreadResumeParams; +use codex_app_server_protocol::ThreadResumeResponse; +use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::UserInput; +use codex_protocol::models::FunctionCallOutputPayload; +use codex_protocol::models::ResponseItem; +use codex_protocol::protocol::MultiAgentVersion; +use codex_protocol::protocol::RolloutItem; +use codex_rollout::append_rollout_item_to_path; +use codex_rollout::read_session_meta_line; +use core_test_support::responses; +use core_test_support::skip_if_no_network; +use pretty_assertions::assert_eq; +use serde_json::Value; +use serde_json::json; +use std::time::Duration; +use tempfile::TempDir; +use tokio::time::timeout; + +const CUSTOM_NAMESPACE: &str = "agents"; +const LEGACY_CALL_ID: &str = "legacy-spawn-call"; +const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10); + +#[tokio::test] +async fn resumed_legacy_multi_agent_v2_call_uses_configured_namespace_in_request() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let response_mock = responses::mount_sse_once( + &server, + responses::sse(vec![ + responses::ev_response_created("resp-after-resume"), + responses::ev_completed("resp-after-resume"), + ]), + ) + .await; + let codex_home = TempDir::new()?; + write_config(codex_home.path(), &server.uri())?; + + let filename_ts = "2025-01-05T12-00-00"; + let thread_id = create_fake_rollout( + codex_home.path(), + filename_ts, + "2025-01-05T12:00:00Z", + "Saved user message", + Some("mock_provider"), + /*git_info*/ None, + )?; + let rollout_path = rollout_path(codex_home.path(), filename_ts, &thread_id); + append_legacy_multi_agent_v2_history(&rollout_path).await?; + + let mut app_server = TestAppServer::new_with_auto_env(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, app_server.initialize()).await??; + let resume_id = app_server + .send_thread_resume_request(ThreadResumeParams { + thread_id: thread_id.clone(), + ..Default::default() + }) + .await?; + let resume_response: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + app_server.read_stream_until_response_message(RequestId::Integer(resume_id)), + ) + .await??; + let ThreadResumeResponse { thread, .. } = to_response(resume_response)?; + assert_eq!(thread.id, thread_id); + + let turn_id = app_server + .send_turn_start_request(TurnStartParams { + thread_id, + input: vec![UserInput::Text { + text: "Continue the legacy thread".to_string(), + text_elements: Vec::new(), + }], + environments: Some(vec![app_server.auto_env_params()?]), + ..Default::default() + }) + .await?; + let _: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + app_server.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + timeout( + DEFAULT_READ_TIMEOUT, + app_server.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let input = response_mock.single_request().input(); + let call_index = input + .iter() + .position(|item| item.get("call_id").and_then(Value::as_str) == Some(LEGACY_CALL_ID)) + .expect("legacy function call should be present"); + assert_eq!( + input.get(call_index), + Some(&json!({ + "type": "function_call", + "name": "spawn_agent", + "namespace": CUSTOM_NAMESPACE, + "arguments": "{}", + "call_id": LEGACY_CALL_ID, + })) + ); + assert_eq!( + input.get(call_index + 1), + Some(&json!({ + "type": "function_call_output", + "call_id": LEGACY_CALL_ID, + "output": "legacy spawn result", + })) + ); + + Ok(()) +} + +async fn append_legacy_multi_agent_v2_history(rollout_path: &std::path::Path) -> Result<()> { + // Mark the rollout as MAv2 while preserving the legacy unnamespaced call shape that predates + // namespace support. The paired output makes this a complete historical tool exchange. + let mut session_meta = read_session_meta_line(rollout_path).await?; + session_meta.meta.multi_agent_version = Some(MultiAgentVersion::V2); + append_rollout_item_to_path(rollout_path, &RolloutItem::SessionMeta(session_meta)).await?; + append_rollout_item_to_path( + rollout_path, + &RolloutItem::ResponseItem(ResponseItem::FunctionCall { + id: None, + name: "spawn_agent".to_string(), + namespace: None, + arguments: "{}".to_string(), + call_id: LEGACY_CALL_ID.to_string(), + internal_chat_message_metadata_passthrough: None, + }), + ) + .await?; + append_rollout_item_to_path( + rollout_path, + &RolloutItem::ResponseItem(ResponseItem::FunctionCallOutput { + id: None, + call_id: LEGACY_CALL_ID.to_string(), + output: FunctionCallOutputPayload::from_text("legacy spawn result".to_string()), + internal_chat_message_metadata_passthrough: None, + }), + ) + .await?; + Ok(()) +} + +fn write_config(codex_home: &std::path::Path, server_uri: &str) -> std::io::Result<()> { + std::fs::write( + codex_home.join("config.toml"), + format!( + r#" +model = "gpt-5.3-codex" +approval_policy = "never" +sandbox_mode = "read-only" +model_provider = "mock_provider" + +[features] +personality = true + +[features.multi_agent_v2] +enabled = true +tool_namespace = "{CUSTOM_NAMESPACE}" +non_code_mode_only = false + +[model_providers.mock_provider] +name = "Mock provider for test" +base_url = "{server_uri}/v1" +wire_api = "responses" +request_max_retries = 0 +stream_max_retries = 0 +"#, + ), + ) +} diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index bbd9952754..626fa5cb71 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -25,6 +25,7 @@ mod fs; mod hooks_list; mod imagegen_extension; mod initialize; +mod legacy_multi_agent_v2_namespace_repair; mod marketplace_add; mod marketplace_remove; mod marketplace_upgrade; diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 649f52e549..4abf4c01a4 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -110,6 +110,7 @@ use crate::client_common::Prompt; use crate::client_common::ResponseEvent; use crate::client_common::ResponseStream; use crate::feedback_tags; +use crate::legacy_multi_agent_v2_namespace_repair::repair_legacy_multi_agent_v2_function_call_namespaces; use crate::responses_metadata::CodexResponsesMetadata; use crate::responses_metadata::subagent_header_value; use crate::util::emit_feedback_auth_recovery_tags; @@ -797,6 +798,7 @@ impl ModelClient { responses_metadata: &CodexResponsesMetadata, ) -> Result { let mut input = prompt.get_formatted_input_for_request(model_info.use_responses_lite); + repair_legacy_multi_agent_v2_function_call_namespaces(&mut input, &prompt.tools); if !self.state.provider.info().is_openai() { input .iter_mut() @@ -2338,3 +2340,7 @@ impl WebsocketTelemetry for ApiTelemetry { #[cfg(test)] #[path = "client_tests.rs"] mod tests; + +#[cfg(test)] +#[path = "legacy_multi_agent_v2_namespace_repair_request_tests.rs"] +mod legacy_multi_agent_v2_namespace_repair_request_tests; diff --git a/codex-rs/core/src/legacy_multi_agent_v2_namespace_repair.rs b/codex-rs/core/src/legacy_multi_agent_v2_namespace_repair.rs new file mode 100644 index 0000000000..ca0a1d712d --- /dev/null +++ b/codex-rs/core/src/legacy_multi_agent_v2_namespace_repair.rs @@ -0,0 +1,93 @@ +use codex_protocol::models::ResponseItem; +use codex_tools::ResponsesApiNamespaceTool; +use codex_tools::ToolSpec; + +/// Function names that uniquely identify the complete MultiAgentV2 tool family. +const MULTI_AGENT_V2_FUNCTION_NAMES: [&str; 6] = [ + "spawn_agent", + "send_message", + "followup_task", + "wait_agent", + "interrupt_agent", + "list_agents", +]; + +/// Repairs function calls persisted before MultiAgentV2 tools moved into a namespace. +/// +/// The prompt's typed tool specs are the source of truth. The repair deliberately fails closed +/// unless one namespace exposes the complete MultiAgentV2 family and no flat function or second +/// namespace makes the legacy call ambiguous. +pub(crate) fn repair_legacy_multi_agent_v2_function_call_namespaces( + input: &mut [ResponseItem], + tools: &[ToolSpec], +) { + for item in input { + let ResponseItem::FunctionCall { + name, namespace, .. + } = item + else { + continue; + }; + if namespace.is_some() + || !MULTI_AGENT_V2_FUNCTION_NAMES.contains(&name.as_str()) + || has_flat_function(tools, name) + { + continue; + } + + let mut matching_namespaces = tools + .iter() + .filter_map(|tool| namespace_containing_function(tool, name)); + let Some(candidate_namespace) = matching_namespaces.next() else { + continue; + }; + if matching_namespaces.any(|namespace| namespace != candidate_namespace) + || !has_multi_agent_v2_fingerprint(tools, candidate_namespace) + { + continue; + } + + *namespace = Some(candidate_namespace.to_string()); + } +} + +fn has_flat_function(tools: &[ToolSpec], function_name: &str) -> bool { + tools + .iter() + .any(|tool| matches!(tool, ToolSpec::Function(function) if function.name == function_name)) +} + +fn namespace_containing_function<'a>(tool: &'a ToolSpec, function_name: &str) -> Option<&'a str> { + let ToolSpec::Namespace(namespace) = tool else { + return None; + }; + namespace + .tools + .iter() + .any(|tool| { + matches!(tool, ResponsesApiNamespaceTool::Function(function) if function.name == function_name) + }) + .then_some(namespace.name.as_str()) +} + +fn has_multi_agent_v2_fingerprint(tools: &[ToolSpec], namespace_name: &str) -> bool { + !namespace_name.is_empty() + && MULTI_AGENT_V2_FUNCTION_NAMES + .iter() + .all(|function_name| { + tools + .iter() + .filter_map(|tool| { + let ToolSpec::Namespace(namespace) = tool else { + return None; + }; + (namespace.name == namespace_name).then_some(&namespace.tools) + }) + .flatten() + .filter(|tool| { + matches!(tool, ResponsesApiNamespaceTool::Function(function) if function.name == *function_name) + }) + .count() + == 1 + }) +} diff --git a/codex-rs/core/src/legacy_multi_agent_v2_namespace_repair_request_tests.rs b/codex-rs/core/src/legacy_multi_agent_v2_namespace_repair_request_tests.rs new file mode 100644 index 0000000000..31e6535fd4 --- /dev/null +++ b/codex-rs/core/src/legacy_multi_agent_v2_namespace_repair_request_tests.rs @@ -0,0 +1,279 @@ +use super::ModelClient; +use crate::Prompt; +use crate::test_support::TestCodexResponsesRequestKind; +use crate::test_support::responses_metadata; +use codex_model_provider_info::WireApi; +use codex_model_provider_info::create_oss_provider_with_base_url; +use codex_protocol::ThreadId; +use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::models::ResponseItem; +use codex_protocol::openai_models::ModelInfo; +use codex_protocol::protocol::SessionSource; +use codex_tools::JsonSchema; +use codex_tools::ResponsesApiNamespace; +use codex_tools::ResponsesApiNamespaceTool; +use codex_tools::ResponsesApiTool; +use codex_tools::ToolSpec; +use pretty_assertions::assert_eq; +use serde_json::json; + +const CUSTOM_NAMESPACE: &str = "agents"; +const MULTI_AGENT_V2_FUNCTION_NAMES: [&str; 6] = [ + "spawn_agent", + "send_message", + "followup_task", + "wait_agent", + "interrupt_agent", + "list_agents", +]; + +fn function(function_name: &str) -> ResponsesApiTool { + ResponsesApiTool { + name: function_name.to_string(), + description: format!("{function_name} description"), + strict: false, + defer_loading: None, + parameters: JsonSchema::default(), + output_schema: None, + } +} + +fn flat_function(function_name: &str) -> ToolSpec { + ToolSpec::Function(function(function_name)) +} + +fn namespace(namespace_name: &str, function_names: &[&str]) -> ToolSpec { + ToolSpec::Namespace(ResponsesApiNamespace { + name: namespace_name.to_string(), + description: format!("{namespace_name} description"), + tools: function_names + .iter() + .map(|function_name| ResponsesApiNamespaceTool::Function(function(function_name))) + .collect(), + }) +} + +fn multi_agent_v2_namespace(namespace_name: &str) -> ToolSpec { + namespace(namespace_name, &MULTI_AGENT_V2_FUNCTION_NAMES) +} + +fn function_call(function_name: &str, namespace: Option<&str>) -> ResponseItem { + ResponseItem::FunctionCall { + id: Some(format!("{function_name}-item")), + name: function_name.to_string(), + namespace: namespace.map(str::to_string), + arguments: format!(r#"{{"function":"{function_name}"}}"#), + call_id: format!("{function_name}-call"), + internal_chat_message_metadata_passthrough: None, + } +} + +fn test_model_client() -> ModelClient { + let provider_info = + create_oss_provider_with_base_url("https://example.com/v1", WireApi::Responses); + ModelClient::new( + /*auth_manager*/ None, + ThreadId::new(), + provider_info, + SessionSource::Cli, + "test_originator".to_string(), + /*model_verbosity*/ None, + /*enable_request_compression*/ false, + /*include_timing_metrics*/ false, + /*beta_features_header*/ None, + /*item_ids_enabled*/ false, + /*attestation_provider*/ None, + ) +} + +fn test_model_info() -> ModelInfo { + serde_json::from_value(json!({ + "slug": "gpt-test", + "display_name": "gpt-test", + "description": "desc", + "default_reasoning_level": "medium", + "supported_reasoning_levels": [ + {"effort": "medium", "description": "medium"} + ], + "shell_type": "shell_command", + "visibility": "list", + "supported_in_api": true, + "priority": 1, + "upgrade": null, + "base_instructions": "base instructions", + "model_messages": null, + "supports_reasoning_summaries": false, + "support_verbosity": false, + "default_verbosity": null, + "apply_patch_tool_type": null, + "truncation_policy": {"mode": "bytes", "limit": 10000}, + "supports_parallel_tool_calls": false, + "supports_image_detail_original": false, + "context_window": 272000, + "auto_compact_token_limit": null, + "experimental_supported_tools": [] + })) + .expect("deserialize test model info") +} + +async fn request_input(prompt: &Prompt) -> Vec { + let client = test_model_client(); + let provider = client + .state + .provider + .api_provider() + .await + .expect("API provider"); + let thread_id = client.state.thread_id.to_string(); + let metadata = responses_metadata( + "11111111-1111-4111-8111-111111111111", + &thread_id, + &thread_id, + /*turn_id*/ None, + format!("{thread_id}:0"), + &client.state.session_source, + /*parent_thread_id*/ None, + TestCodexResponsesRequestKind::Turn, + ); + + client + .build_responses_request( + &provider, + prompt, + &test_model_info(), + /*effort*/ None, + ReasoningSummary::None, + /*service_tier*/ None, + &metadata, + ) + .expect("responses request") + .input +} + +#[tokio::test] +async fn request_body_repairs_every_legacy_multi_agent_v2_function_name_without_mutating_prompt() { + let prompt = Prompt { + input: MULTI_AGENT_V2_FUNCTION_NAMES + .iter() + .map(|function_name| function_call(function_name, None)) + .collect(), + tools: vec![multi_agent_v2_namespace(CUSTOM_NAMESPACE)], + ..Default::default() + }; + let original_input = prompt.input.clone(); + + let request_input = request_input(&prompt).await; + + let expected = MULTI_AGENT_V2_FUNCTION_NAMES + .iter() + .map(|function_name| function_call(function_name, Some(CUSTOM_NAMESPACE))) + .collect::>(); + assert_eq!(request_input, expected); + assert_eq!(prompt.input, original_input); +} + +#[tokio::test] +async fn request_body_fails_closed_for_ambiguous_or_non_multi_agent_v2_tools() { + let mut duplicate_family = MULTI_AGENT_V2_FUNCTION_NAMES.to_vec(); + duplicate_family.push("spawn_agent"); + let cases = [ + ("missing", Vec::new()), + ( + "disabled_flat_family", + MULTI_AGENT_V2_FUNCTION_NAMES + .iter() + .map(|name| flat_function(name)) + .collect(), + ), + ( + "partial_namespace", + vec![namespace(CUSTOM_NAMESPACE, &["spawn_agent"])], + ), + ( + "v1_namespace", + vec![namespace( + "multi_agent_v1", + &[ + "close_agent", + "resume_agent", + "send_input", + "spawn_agent", + "wait_agent", + ], + )], + ), + ( + "flat_conflict", + vec![ + multi_agent_v2_namespace(CUSTOM_NAMESPACE), + flat_function("spawn_agent"), + ], + ), + ( + "multiple_namespaces", + vec![ + multi_agent_v2_namespace(CUSTOM_NAMESPACE), + namespace("dynamic", &["spawn_agent"]), + ], + ), + ("empty_namespace", vec![multi_agent_v2_namespace("")]), + ( + "duplicate_family_child", + vec![namespace(CUSTOM_NAMESPACE, &duplicate_family)], + ), + ]; + + for (case, tools) in cases { + let prompt = Prompt { + input: vec![function_call("spawn_agent", None)], + tools, + ..Default::default() + }; + + let request_input = request_input(&prompt).await; + + assert_eq!(request_input, prompt.input, "case: {case}"); + } +} + +#[tokio::test] +async fn request_body_preserves_explicit_namespaces_and_non_multi_agent_v2_calls() { + let prompt = Prompt { + input: vec![ + function_call("spawn_agent", Some("original_namespace")), + function_call("unrelated_function", None), + ResponseItem::Other, + ], + tools: vec![multi_agent_v2_namespace(CUSTOM_NAMESPACE)], + ..Default::default() + }; + + let request_input = request_input(&prompt).await; + + assert_eq!(request_input, prompt.input); +} + +#[tokio::test] +async fn request_body_combines_distinct_fragments_for_the_same_namespace() { + let prompt = Prompt { + input: vec![function_call("spawn_agent", None)], + tools: vec![ + namespace( + CUSTOM_NAMESPACE, + &["spawn_agent", "send_message", "followup_task"], + ), + namespace( + CUSTOM_NAMESPACE, + &["wait_agent", "interrupt_agent", "list_agents"], + ), + ], + ..Default::default() + }; + + let request_input = request_input(&prompt).await; + + assert_eq!( + request_input, + vec![function_call("spawn_agent", Some(CUSTOM_NAMESPACE))] + ); +} diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 519d11d1ca..0594aab258 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -50,6 +50,7 @@ mod hook_runtime; mod image_preparation; mod installation_id; pub(crate) mod landlock; +mod legacy_multi_agent_v2_namespace_repair; pub use landlock::spawn_command_under_linux_sandbox; pub(crate) mod mcp; mod mcp_skill_dependencies;