mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[mcp] Add dummy tools for previously called but currently missing tools. (#17853)
- [x] Add dummy tools for previously called but currently missing tools. Currently supporting MCP tools only.
This commit is contained in:
committed by
GitHub
Unverified
parent
9d1bf002c6
commit
28b76d13fe
@@ -500,6 +500,9 @@
|
||||
"tui_app_server": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"unavailable_dummy_tools": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"undo": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -2360,6 +2363,9 @@
|
||||
"tui_app_server": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"unavailable_dummy_tools": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"undo": {
|
||||
"type": "boolean"
|
||||
},
|
||||
|
||||
@@ -44,6 +44,7 @@ use crate::stream_events_utils::last_assistant_message_from_item;
|
||||
use crate::stream_events_utils::raw_assistant_output_text_from_item;
|
||||
use crate::stream_events_utils::record_completed_response_item;
|
||||
use crate::turn_metadata::TurnMetadataState;
|
||||
use crate::unavailable_tool::collect_unavailable_called_tools;
|
||||
use crate::util::error_or_panic;
|
||||
use async_channel::Receiver;
|
||||
use async_channel::Sender;
|
||||
@@ -7229,7 +7230,22 @@ pub(crate) async fn built_tools(
|
||||
&turn_context.config,
|
||||
&turn_context.tools_config,
|
||||
);
|
||||
let direct_mcp_tools = has_mcp_servers.then_some(mcp_tool_exposure.direct_tools);
|
||||
let mcp_tools = has_mcp_servers.then_some(mcp_tool_exposure.direct_tools);
|
||||
let deferred_mcp_tools = mcp_tool_exposure.deferred_tools;
|
||||
let unavailable_called_tools = if turn_context
|
||||
.config
|
||||
.features
|
||||
.enabled(Feature::UnavailableDummyTools)
|
||||
{
|
||||
let exposed_tool_names = mcp_tools
|
||||
.iter()
|
||||
.chain(deferred_mcp_tools.iter())
|
||||
.flat_map(|tools| tools.keys().map(String::as_str))
|
||||
.collect::<HashSet<_>>();
|
||||
collect_unavailable_called_tools(input, &exposed_tool_names)
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
|
||||
let parallel_mcp_server_names = turn_context
|
||||
.config
|
||||
@@ -7246,8 +7262,9 @@ pub(crate) async fn built_tools(
|
||||
Ok(Arc::new(ToolRouter::from_config(
|
||||
&turn_context.tools_config,
|
||||
ToolRouterParams {
|
||||
mcp_tools: direct_mcp_tools,
|
||||
deferred_mcp_tools: mcp_tool_exposure.deferred_tools,
|
||||
mcp_tools,
|
||||
deferred_mcp_tools,
|
||||
unavailable_called_tools,
|
||||
parallel_mcp_server_names,
|
||||
discoverable_tools,
|
||||
dynamic_tools: turn_context.dynamic_tools.as_slice(),
|
||||
|
||||
@@ -313,6 +313,7 @@ fn test_tool_runtime(session: Arc<Session>, turn_context: Arc<TurnContext>) -> T
|
||||
crate::tools::router::ToolRouterParams {
|
||||
mcp_tools: None,
|
||||
deferred_mcp_tools: None,
|
||||
unavailable_called_tools: Vec::new(),
|
||||
parallel_mcp_server_names: HashSet::new(),
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: turn_context.dynamic_tools.as_slice(),
|
||||
@@ -5451,6 +5452,7 @@ async fn fatal_tool_error_stops_turn_and_reports_error() {
|
||||
crate::tools::router::ToolRouterParams {
|
||||
deferred_mcp_tools,
|
||||
mcp_tools: Some(tools),
|
||||
unavailable_called_tools: Vec::new(),
|
||||
parallel_mcp_server_names: HashSet::new(),
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: turn_context.dynamic_tools.as_slice(),
|
||||
|
||||
@@ -150,6 +150,7 @@ mod tools;
|
||||
pub(crate) mod turn_diff_tracker;
|
||||
mod turn_metadata;
|
||||
mod turn_timing;
|
||||
mod unavailable_tool;
|
||||
pub use rollout::ARCHIVED_SESSIONS_SUBDIR;
|
||||
pub use rollout::Cursor;
|
||||
pub use rollout::EventPersistenceMode;
|
||||
|
||||
@@ -287,6 +287,7 @@ async fn build_nested_router(exec: &ExecContext) -> ToolRouter {
|
||||
ToolRouterParams {
|
||||
deferred_mcp_tools: None,
|
||||
mcp_tools: Some(listed_mcp_tools),
|
||||
unavailable_called_tools: Vec::new(),
|
||||
parallel_mcp_server_names,
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: exec.turn.dynamic_tools.as_slice(),
|
||||
|
||||
@@ -15,6 +15,7 @@ mod shell;
|
||||
mod test_sync;
|
||||
mod tool_search;
|
||||
mod tool_suggest;
|
||||
mod unavailable_tool;
|
||||
pub(crate) mod unified_exec;
|
||||
mod view_image;
|
||||
|
||||
@@ -49,6 +50,8 @@ pub use shell::ShellHandler;
|
||||
pub use test_sync::TestSyncHandler;
|
||||
pub use tool_search::ToolSearchHandler;
|
||||
pub use tool_suggest::ToolSuggestHandler;
|
||||
pub use unavailable_tool::UnavailableToolHandler;
|
||||
pub(crate) use unavailable_tool::unavailable_tool_message;
|
||||
pub use unified_exec::UnifiedExecHandler;
|
||||
pub use view_image::ViewImageHandler;
|
||||
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::tools::context::FunctionToolOutput;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::registry::ToolHandler;
|
||||
use crate::tools::registry::ToolKind;
|
||||
|
||||
pub struct UnavailableToolHandler;
|
||||
|
||||
pub(crate) fn unavailable_tool_message(
|
||||
tool_name: impl std::fmt::Display,
|
||||
next_step: &str,
|
||||
) -> String {
|
||||
format!(
|
||||
"Tool `{tool_name}` is not currently available. It appeared in earlier tool calls in this conversation, but its implementation is not available in the current request. {next_step}"
|
||||
)
|
||||
}
|
||||
|
||||
impl ToolHandler for UnavailableToolHandler {
|
||||
type Output = FunctionToolOutput;
|
||||
|
||||
fn kind(&self) -> ToolKind {
|
||||
ToolKind::Function
|
||||
}
|
||||
|
||||
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
|
||||
let ToolInvocation {
|
||||
tool_name, payload, ..
|
||||
} = invocation;
|
||||
|
||||
match payload {
|
||||
ToolPayload::Function { .. } => Ok(FunctionToolOutput::from_text(
|
||||
unavailable_tool_message(
|
||||
tool_name.display(),
|
||||
"Retry after the tool becomes available or ask the user to re-enable it.",
|
||||
),
|
||||
Some(false),
|
||||
)),
|
||||
_ => Err(FunctionCallError::RespondToModel(
|
||||
"unavailable tool handler received unsupported payload".to_string(),
|
||||
)),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1568,6 +1568,7 @@ impl JsReplManager {
|
||||
crate::tools::router::ToolRouterParams {
|
||||
deferred_mcp_tools: None,
|
||||
mcp_tools: Some(mcp_tools),
|
||||
unavailable_called_tools: Vec::new(),
|
||||
// JS REPL dispatches nested tool calls directly, not through
|
||||
// `ToolCallRuntime`'s parallel scheduling lock.
|
||||
parallel_mcp_server_names: std::collections::HashSet::new(),
|
||||
|
||||
@@ -44,6 +44,7 @@ pub struct ToolRouter {
|
||||
pub(crate) struct ToolRouterParams<'a> {
|
||||
pub(crate) mcp_tools: Option<HashMap<String, ToolInfo>>,
|
||||
pub(crate) deferred_mcp_tools: Option<HashMap<String, ToolInfo>>,
|
||||
pub(crate) unavailable_called_tools: Vec<ToolName>,
|
||||
pub(crate) parallel_mcp_server_names: HashSet<String>,
|
||||
pub(crate) discoverable_tools: Option<Vec<DiscoverableTool>>,
|
||||
pub(crate) dynamic_tools: &'a [DynamicToolSpec],
|
||||
@@ -54,6 +55,7 @@ impl ToolRouter {
|
||||
let ToolRouterParams {
|
||||
mcp_tools,
|
||||
deferred_mcp_tools,
|
||||
unavailable_called_tools,
|
||||
parallel_mcp_server_names,
|
||||
discoverable_tools,
|
||||
dynamic_tools,
|
||||
@@ -62,6 +64,7 @@ impl ToolRouter {
|
||||
config,
|
||||
mcp_tools,
|
||||
deferred_mcp_tools,
|
||||
unavailable_called_tools,
|
||||
discoverable_tools,
|
||||
dynamic_tools,
|
||||
);
|
||||
|
||||
@@ -33,6 +33,7 @@ async fn js_repl_tools_only_blocks_direct_tool_calls() -> anyhow::Result<()> {
|
||||
ToolRouterParams {
|
||||
deferred_mcp_tools,
|
||||
mcp_tools: Some(mcp_tools),
|
||||
unavailable_called_tools: Vec::new(),
|
||||
parallel_mcp_server_names: HashSet::new(),
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: turn.dynamic_tools.as_slice(),
|
||||
@@ -86,6 +87,7 @@ async fn js_repl_tools_only_allows_js_repl_source_calls() -> anyhow::Result<()>
|
||||
ToolRouterParams {
|
||||
deferred_mcp_tools,
|
||||
mcp_tools: Some(mcp_tools),
|
||||
unavailable_called_tools: Vec::new(),
|
||||
parallel_mcp_server_names: HashSet::new(),
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: turn.dynamic_tools.as_slice(),
|
||||
@@ -132,6 +134,7 @@ async fn js_repl_tools_only_blocks_namespaced_js_repl_tool() -> anyhow::Result<(
|
||||
ToolRouterParams {
|
||||
deferred_mcp_tools: None,
|
||||
mcp_tools: None,
|
||||
unavailable_called_tools: Vec::new(),
|
||||
parallel_mcp_server_names: HashSet::new(),
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: turn.dynamic_tools.as_slice(),
|
||||
@@ -182,6 +185,7 @@ async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow
|
||||
ToolRouterParams {
|
||||
deferred_mcp_tools: None,
|
||||
mcp_tools: Some(mcp_tools),
|
||||
unavailable_called_tools: Vec::new(),
|
||||
parallel_mcp_server_names: HashSet::new(),
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: turn.dynamic_tools.as_slice(),
|
||||
@@ -254,6 +258,7 @@ async fn mcp_parallel_support_uses_exact_payload_server() -> anyhow::Result<()>
|
||||
ToolRouterParams {
|
||||
deferred_mcp_tools: None,
|
||||
mcp_tools: None,
|
||||
unavailable_called_tools: Vec::new(),
|
||||
parallel_mcp_server_names: HashSet::from(["echo".to_string()]),
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: turn.dynamic_tools.as_slice(),
|
||||
|
||||
@@ -7,8 +7,12 @@ use crate::tools::handlers::multi_agents_common::MIN_WAIT_TIMEOUT_MS;
|
||||
use crate::tools::registry::ToolRegistryBuilder;
|
||||
use codex_mcp::ToolInfo;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_tools::AdditionalProperties;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use codex_tools::JsonSchema;
|
||||
use codex_tools::ResponsesApiTool;
|
||||
use codex_tools::ToolHandlerKind;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolNamespace;
|
||||
use codex_tools::ToolRegistryPlanDeferredTool;
|
||||
use codex_tools::ToolRegistryPlanMcpTool;
|
||||
@@ -16,8 +20,10 @@ use codex_tools::ToolRegistryPlanParams;
|
||||
use codex_tools::ToolUserShellType;
|
||||
use codex_tools::ToolsConfig;
|
||||
use codex_tools::WaitAgentTimeoutOptions;
|
||||
use codex_tools::augment_tool_spec_for_code_mode;
|
||||
use codex_tools::build_tool_registry_plan;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
|
||||
pub(crate) fn tool_user_shell_type(user_shell: &Shell) -> ToolUserShellType {
|
||||
@@ -66,6 +72,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
config: &ToolsConfig,
|
||||
mcp_tools: Option<HashMap<String, ToolInfo>>,
|
||||
deferred_mcp_tools: Option<HashMap<String, ToolInfo>>,
|
||||
unavailable_called_tools: Vec<ToolName>,
|
||||
discoverable_tools: Option<Vec<DiscoverableTool>>,
|
||||
dynamic_tools: &[DynamicToolSpec],
|
||||
) -> ToolRegistryBuilder {
|
||||
@@ -86,6 +93,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
use crate::tools::handlers::TestSyncHandler;
|
||||
use crate::tools::handlers::ToolSearchHandler;
|
||||
use crate::tools::handlers::ToolSuggestHandler;
|
||||
use crate::tools::handlers::UnavailableToolHandler;
|
||||
use crate::tools::handlers::UnifiedExecHandler;
|
||||
use crate::tools::handlers::ViewImageHandler;
|
||||
use crate::tools::handlers::multi_agents::CloseAgentHandler;
|
||||
@@ -99,6 +107,7 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
use crate::tools::handlers::multi_agents_v2::SendMessageHandler as SendMessageHandlerV2;
|
||||
use crate::tools::handlers::multi_agents_v2::SpawnAgentHandler as SpawnAgentHandlerV2;
|
||||
use crate::tools::handlers::multi_agents_v2::WaitAgentHandler as WaitAgentHandlerV2;
|
||||
use crate::tools::handlers::unavailable_tool_message;
|
||||
|
||||
let mut builder = ToolRegistryBuilder::new();
|
||||
let mcp_tool_plan_inputs = mcp_tools.as_ref().map(map_mcp_tools_for_plan);
|
||||
@@ -154,6 +163,12 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
let code_mode_wait_handler = Arc::new(CodeModeWaitHandler);
|
||||
let js_repl_handler = Arc::new(JsReplHandler);
|
||||
let js_repl_reset_handler = Arc::new(JsReplResetHandler);
|
||||
let unavailable_tool_handler = Arc::new(UnavailableToolHandler);
|
||||
let mut existing_spec_names = plan
|
||||
.specs
|
||||
.iter()
|
||||
.map(|configured_tool| configured_tool.name().to_string())
|
||||
.collect::<HashSet<_>>();
|
||||
|
||||
for spec in plan.specs {
|
||||
if spec.supports_parallel_tool_calls {
|
||||
@@ -278,6 +293,34 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
builder.register_handler(name.clone(), mcp_handler.clone());
|
||||
}
|
||||
}
|
||||
|
||||
for unavailable_tool in unavailable_called_tools {
|
||||
let tool_name = unavailable_tool.display();
|
||||
if existing_spec_names.insert(tool_name.clone()) {
|
||||
let spec = codex_tools::ToolSpec::Function(ResponsesApiTool {
|
||||
name: tool_name.clone(),
|
||||
description: unavailable_tool_message(
|
||||
&tool_name,
|
||||
"Calling this placeholder returns an error explaining that the tool is unavailable.",
|
||||
),
|
||||
strict: false,
|
||||
parameters: JsonSchema::object(
|
||||
Default::default(),
|
||||
/*required*/ None,
|
||||
Some(AdditionalProperties::Boolean(false)),
|
||||
),
|
||||
output_schema: None,
|
||||
defer_loading: None,
|
||||
});
|
||||
let spec = if config.code_mode_enabled {
|
||||
augment_tool_spec_for_code_mode(spec)
|
||||
} else {
|
||||
spec
|
||||
};
|
||||
builder.push_spec(spec);
|
||||
}
|
||||
builder.register_handler(unavailable_tool, unavailable_tool_handler.clone());
|
||||
}
|
||||
builder
|
||||
}
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ use codex_protocol::openai_models::ConfigShellToolType;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_tools::AdditionalProperties;
|
||||
use codex_tools::ConfiguredToolSpec;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use codex_tools::JsonSchema;
|
||||
@@ -267,11 +268,28 @@ fn build_specs(
|
||||
mcp_tools: Option<HashMap<String, ToolInfo>>,
|
||||
deferred_mcp_tools: Option<HashMap<String, ToolInfo>>,
|
||||
dynamic_tools: &[DynamicToolSpec],
|
||||
) -> ToolRegistryBuilder {
|
||||
build_specs_with_unavailable_tools(
|
||||
config,
|
||||
mcp_tools,
|
||||
deferred_mcp_tools,
|
||||
Vec::new(),
|
||||
dynamic_tools,
|
||||
)
|
||||
}
|
||||
|
||||
fn build_specs_with_unavailable_tools(
|
||||
config: &ToolsConfig,
|
||||
mcp_tools: Option<HashMap<String, ToolInfo>>,
|
||||
deferred_mcp_tools: Option<HashMap<String, ToolInfo>>,
|
||||
unavailable_called_tools: Vec<ToolName>,
|
||||
dynamic_tools: &[DynamicToolSpec],
|
||||
) -> ToolRegistryBuilder {
|
||||
build_specs_with_discoverable_tools(
|
||||
config,
|
||||
mcp_tools,
|
||||
deferred_mcp_tools,
|
||||
unavailable_called_tools,
|
||||
/*discoverable_tools*/ None,
|
||||
dynamic_tools,
|
||||
)
|
||||
@@ -356,6 +374,7 @@ fn assert_model_tools(
|
||||
ToolRouterParams {
|
||||
mcp_tools: None,
|
||||
deferred_mcp_tools: None,
|
||||
unavailable_called_tools: Vec::new(),
|
||||
parallel_mcp_server_names: std::collections::HashSet::new(),
|
||||
discoverable_tools: None,
|
||||
dynamic_tools: &[],
|
||||
@@ -770,6 +789,7 @@ fn tool_suggest_requires_apps_and_plugins_features() {
|
||||
&tools_config,
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
Vec::new(),
|
||||
discoverable_tools.clone(),
|
||||
&[],
|
||||
)
|
||||
@@ -991,6 +1011,55 @@ fn direct_mcp_tools_register_namespaced_handlers() {
|
||||
assert!(!registry.has_handler(&ToolName::plain("mcp__test_server__echo")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unavailable_mcp_tools_are_exposed_as_dummy_function_tools() {
|
||||
let config = test_config();
|
||||
let model_info = construct_model_info_offline("gpt-5-codex", &config);
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::UnifiedExec);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features,
|
||||
image_generation_tool_auth_allowed: true,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
sandbox_policy: &SandboxPolicy::DangerFullAccess,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
});
|
||||
|
||||
let unavailable_tool = ToolName::namespaced("mcp__codex_apps__calendar", "_create_event");
|
||||
let (tools, registry) = build_specs_with_unavailable_tools(
|
||||
&tools_config,
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
vec![unavailable_tool],
|
||||
&[],
|
||||
)
|
||||
.build();
|
||||
|
||||
let tool = find_tool(&tools, "mcp__codex_apps__calendar_create_event");
|
||||
let ToolSpec::Function(ResponsesApiTool {
|
||||
description,
|
||||
parameters,
|
||||
..
|
||||
}) = &tool.spec
|
||||
else {
|
||||
panic!("unavailable MCP tool should be exposed as a function tool");
|
||||
};
|
||||
assert!(description.contains("not currently available"));
|
||||
assert_eq!(
|
||||
parameters.additional_properties,
|
||||
Some(AdditionalProperties::Boolean(false))
|
||||
);
|
||||
assert!(registry.has_handler(&ToolName::namespaced(
|
||||
"mcp__codex_apps__calendar",
|
||||
"_create_event"
|
||||
)));
|
||||
assert!(!registry.has_handler(&ToolName::plain("mcp__codex_apps__calendar_create_event")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mcp_tool_property_missing_type_defaults_to_string() {
|
||||
let config = test_config();
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashSet;
|
||||
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_tools::ToolName;
|
||||
|
||||
pub(crate) fn collect_unavailable_called_tools(
|
||||
input: &[ResponseItem],
|
||||
exposed_tool_names: &HashSet<&str>,
|
||||
) -> Vec<ToolName> {
|
||||
let mut unavailable_tools = BTreeMap::new();
|
||||
|
||||
for item in input {
|
||||
let ResponseItem::FunctionCall {
|
||||
name, namespace, ..
|
||||
} = item
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
if !should_collect_unavailable_tool(name, namespace.as_deref()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
let tool_name = match namespace {
|
||||
Some(namespace) => ToolName::namespaced(namespace.clone(), name.clone()),
|
||||
None => ToolName::plain(name.clone()),
|
||||
};
|
||||
let display_name = tool_name.display();
|
||||
if exposed_tool_names.contains(display_name.as_str()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
unavailable_tools
|
||||
.entry(display_name)
|
||||
.or_insert_with(|| tool_name);
|
||||
}
|
||||
|
||||
unavailable_tools.into_values().collect()
|
||||
}
|
||||
|
||||
fn should_collect_unavailable_tool(name: &str, namespace: Option<&str>) -> bool {
|
||||
namespace.is_some_and(|namespace| namespace.starts_with("mcp__")) || name.starts_with("mcp__")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn function_call(name: &str, namespace: Option<&str>) -> ResponseItem {
|
||||
ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: name.to_string(),
|
||||
namespace: namespace.map(str::to_string),
|
||||
arguments: "{}".to_string(),
|
||||
call_id: format!("call-{name}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_unavailable_called_tools_detects_mcp_function_calls() {
|
||||
let input = vec![
|
||||
function_call("shell", /*namespace*/ None),
|
||||
function_call("mcp__server__lookup", /*namespace*/ None),
|
||||
function_call("_create_event", Some("mcp__codex_apps__calendar")),
|
||||
];
|
||||
|
||||
let tools = collect_unavailable_called_tools(&input, &HashSet::new());
|
||||
|
||||
assert_eq!(
|
||||
tools,
|
||||
vec![
|
||||
ToolName::namespaced("mcp__codex_apps__calendar", "_create_event"),
|
||||
ToolName::plain("mcp__server__lookup"),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_unavailable_called_tools_skips_currently_available_tools() {
|
||||
let exposed_tool_names = HashSet::from(["mcp__server__lookup", "mcp__server__search"]);
|
||||
let input = vec![
|
||||
function_call("mcp__server__lookup", /*namespace*/ None),
|
||||
function_call("mcp__server__search", /*namespace*/ None),
|
||||
function_call("mcp__server__missing", /*namespace*/ None),
|
||||
];
|
||||
|
||||
let tools = collect_unavailable_called_tools(&input, &exposed_tool_names);
|
||||
|
||||
assert_eq!(tools, vec![ToolName::plain("mcp__server__missing")]);
|
||||
}
|
||||
}
|
||||
@@ -1,12 +1,16 @@
|
||||
#![cfg(not(target_os = "windows"))]
|
||||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::fs;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_core::sandboxing::SandboxPermissions;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -22,10 +26,13 @@ use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::stdio_server_bin;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use regex_lite::Regex;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use serial_test::serial;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn tool_names(body: &Value) -> Vec<String> {
|
||||
body.get("tools")
|
||||
@@ -44,6 +51,24 @@ fn tool_names(body: &Value) -> Vec<String> {
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
fn ev_namespaced_function_call(
|
||||
call_id: &str,
|
||||
namespace: &str,
|
||||
name: &str,
|
||||
arguments: &str,
|
||||
) -> Value {
|
||||
json!({
|
||||
"type": "response.output_item.done",
|
||||
"item": {
|
||||
"type": "function_call",
|
||||
"call_id": call_id,
|
||||
"namespace": namespace,
|
||||
"name": name,
|
||||
"arguments": arguments,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn custom_tool_unknown_returns_custom_output_error() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -91,6 +116,147 @@ async fn custom_tool_unknown_returns_custom_output_error() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[serial(mcp_test_value)]
|
||||
async fn historical_unavailable_mcp_call_is_exposed_as_placeholder_tool() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let historical_call_id = "historical-mcp-call";
|
||||
let retry_call_id = "retry-mcp-call";
|
||||
let server_name = "rmcp";
|
||||
let unavailable_tool_namespace = "mcp__rmcp__";
|
||||
let unavailable_tool_name = "echo";
|
||||
let unavailable_tool_display_name = "mcp__rmcp__echo";
|
||||
let server = start_mock_server().await;
|
||||
let rmcp_test_server_bin = match stdio_server_bin() {
|
||||
Ok(bin) => bin,
|
||||
Err(err) => {
|
||||
eprintln!("test_stdio_server binary not available, skipping test: {err}");
|
||||
return Ok(());
|
||||
}
|
||||
};
|
||||
let codex_home = Arc::new(TempDir::new()?);
|
||||
let mut builder = test_codex()
|
||||
.with_model("gpt-5.1")
|
||||
.with_home(Arc::clone(&codex_home))
|
||||
.with_config(move |config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::UnavailableDummyTools)
|
||||
.expect("unavailable dummy tools should be enabled for this test");
|
||||
let mut servers = config.mcp_servers.get().clone();
|
||||
servers.insert(
|
||||
server_name.to_string(),
|
||||
McpServerConfig {
|
||||
transport: McpServerTransportConfig::Stdio {
|
||||
command: rmcp_test_server_bin,
|
||||
args: Vec::new(),
|
||||
env: Some(HashMap::new()),
|
||||
env_vars: Vec::new(),
|
||||
cwd: None,
|
||||
},
|
||||
enabled: true,
|
||||
required: false,
|
||||
supports_parallel_tool_calls: false,
|
||||
disabled_reason: None,
|
||||
startup_timeout_sec: Some(Duration::from_secs(10)),
|
||||
tool_timeout_sec: None,
|
||||
enabled_tools: None,
|
||||
disabled_tools: None,
|
||||
scopes: None,
|
||||
oauth_resource: None,
|
||||
tools: HashMap::new(),
|
||||
},
|
||||
);
|
||||
config
|
||||
.mcp_servers
|
||||
.set(servers)
|
||||
.expect("test mcp servers should accept any configuration");
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let first_turn_mock = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_namespaced_function_call(
|
||||
historical_call_id,
|
||||
unavailable_tool_namespace,
|
||||
unavailable_tool_name,
|
||||
r#"{"message":"ping"}"#,
|
||||
),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-1", "rmcp echo tool completed"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
test.submit_turn("call the rmcp echo tool").await?;
|
||||
let rollout_path = test.codex.rollout_path().context("rollout path")?;
|
||||
assert_eq!(first_turn_mock.requests().len(), 2);
|
||||
drop(test);
|
||||
|
||||
let retry_mock = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-3"),
|
||||
ev_namespaced_function_call(
|
||||
retry_call_id,
|
||||
unavailable_tool_namespace,
|
||||
unavailable_tool_name,
|
||||
r#"{"message":"ping again"}"#,
|
||||
),
|
||||
ev_completed("resp-3"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-4"),
|
||||
ev_assistant_message("msg-2", "done"),
|
||||
ev_completed("resp-4"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut resume_builder = test_codex().with_model("gpt-5.1").with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::UnavailableDummyTools)
|
||||
.expect("unavailable dummy tools should be enabled for this test");
|
||||
});
|
||||
let test = resume_builder
|
||||
.resume(&server, codex_home, rollout_path)
|
||||
.await?;
|
||||
|
||||
test.submit_turn("retry the rmcp echo tool").await?;
|
||||
|
||||
let requests = retry_mock.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
let first_request_tools = tool_names(&requests[0].body_json());
|
||||
assert!(
|
||||
first_request_tools
|
||||
.iter()
|
||||
.any(|name| name == unavailable_tool_display_name),
|
||||
"historical unavailable MCP call should add a placeholder tool; got {first_request_tools:?}"
|
||||
);
|
||||
let output_text = requests[1]
|
||||
.function_call_output_text(retry_call_id)
|
||||
.context("placeholder tool output present")?;
|
||||
assert!(output_text.contains("not currently available"));
|
||||
assert!(
|
||||
!output_text.contains("unsupported call"),
|
||||
"placeholder handler should answer instead of falling back to unsupported call: {output_text}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn shell_escalated_permissions_rejected_then_ok() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -146,6 +146,8 @@ pub enum Feature {
|
||||
Apps,
|
||||
/// Enable the tool_search tool for apps.
|
||||
ToolSearch,
|
||||
/// Expose placeholder tools for unavailable historical tool calls.
|
||||
UnavailableDummyTools,
|
||||
/// Enable discoverable tool suggestions for apps.
|
||||
ToolSuggest,
|
||||
/// Enable plugins.
|
||||
@@ -789,6 +791,12 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
stage: Stage::UnderDevelopment,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::UnavailableDummyTools,
|
||||
key: "unavailable_dummy_tools",
|
||||
stage: Stage::UnderDevelopment,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ToolSuggest,
|
||||
key: "tool_suggest",
|
||||
|
||||
@@ -133,6 +133,15 @@ fn tool_search_is_under_development_and_disabled_by_default() {
|
||||
assert_eq!(Feature::ToolSearch.default_enabled(), false);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unavailable_dummy_tools_is_under_development_and_disabled_by_default() {
|
||||
assert_eq!(
|
||||
Feature::UnavailableDummyTools.stage(),
|
||||
Stage::UnderDevelopment
|
||||
);
|
||||
assert_eq!(Feature::UnavailableDummyTools.default_enabled(), false);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn general_analytics_is_stable_and_enabled_by_default() {
|
||||
assert_eq!(Feature::GeneralAnalytics.stage(), Stage::Stable);
|
||||
|
||||
Reference in New Issue
Block a user