Repair legacy MAv2 namespaces when resuming threads

This commit is contained in:
Ari He
2026-06-24 07:17:39 +00:00
parent 31e428a1ef
commit b3fc2cbed1
6 changed files with 562 additions and 0 deletions

View File

@@ -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
"#,
),
)
}

View File

@@ -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;

View File

@@ -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<ResponsesApiRequest> {
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;

View File

@@ -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
})
}

View File

@@ -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<ResponseItem> {
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::<Vec<_>>();
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))]
);
}

View File

@@ -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;