mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
code-mode: retain shared MCP types for deferred tools (#31745)
## Why When MCP tools are deferred behind `tool_search`, Code mode keeps them callable but omits their individual declarations from the initial `exec` description. The shared MCP `CallToolResult` types were derived only from directly rendered tools, so deferring every MCP tool also removed the common response contract that models need to interpret MCP results. This restores that contract without undoing the context savings from deferred tool definitions. This is a follow-up to #29486. ## What changed - Track deferred Code-mode tool definitions separately from directly rendered definitions. - Render the shared MCP type preamble when either direct or deferred MCP tools are available. - Keep deferred tool declarations out of the initial prompt. - Add unit and integration coverage for deferred MCP tools. ## Testing - `just test -p codex-code-mode-protocol` - `just test -p codex-core code_mode_only_guides_all_tools_search_and_calls_deferred_app_tools`
This commit is contained in:
@@ -250,25 +250,32 @@ pub fn is_code_mode_nested_tool(tool_name: &str) -> bool {
|
||||
|
||||
pub fn build_exec_tool_description(
|
||||
enabled_tools: &[ToolDefinition],
|
||||
deferred_tools: &[ToolDefinition],
|
||||
namespace_descriptions: &BTreeMap<String, ToolNamespaceDescription>,
|
||||
code_mode_only: bool,
|
||||
deferred_tools_available: bool,
|
||||
) -> String {
|
||||
let mut sections = Vec::new();
|
||||
sections.push(EXEC_DESCRIPTION_TEMPLATE.to_string());
|
||||
if deferred_tools_available {
|
||||
if !deferred_tools.is_empty() {
|
||||
sections.push(DEFERRED_NESTED_TOOLS_GUIDANCE.to_string());
|
||||
}
|
||||
if !code_mode_only {
|
||||
return sections.join("\n\n");
|
||||
}
|
||||
|
||||
let has_mcp_tools = enabled_tools
|
||||
.iter()
|
||||
.chain(deferred_tools)
|
||||
.any(|tool| mcp_structured_content_schema(tool.output_schema.as_ref()).is_some());
|
||||
if has_mcp_tools {
|
||||
sections.push(format!(
|
||||
"Shared MCP Types:\n```ts\n{MCP_TYPESCRIPT_PREAMBLE}\n```"
|
||||
));
|
||||
}
|
||||
|
||||
if !enabled_tools.is_empty() {
|
||||
let mut current_namespace: Option<&str> = None;
|
||||
let mut nested_tool_sections = Vec::with_capacity(enabled_tools.len());
|
||||
let has_mcp_tools = enabled_tools
|
||||
.iter()
|
||||
.any(|tool| mcp_structured_content_schema(tool.output_schema.as_ref()).is_some());
|
||||
|
||||
for tool in enabled_tools {
|
||||
let name = tool.name.as_str();
|
||||
@@ -305,11 +312,6 @@ pub fn build_exec_tool_description(
|
||||
}
|
||||
}
|
||||
|
||||
if has_mcp_tools {
|
||||
sections.push(format!(
|
||||
"Shared MCP Types:\n```ts\n{MCP_TYPESCRIPT_PREAMBLE}\n```"
|
||||
));
|
||||
}
|
||||
let nested_tool_reference = nested_tool_sections.join("\n\n");
|
||||
sections.push(nested_tool_reference);
|
||||
}
|
||||
@@ -863,9 +865,9 @@ mod tests {
|
||||
input_schema: None,
|
||||
output_schema: None,
|
||||
}],
|
||||
&[],
|
||||
&BTreeMap::new(),
|
||||
/*code_mode_only*/ true,
|
||||
/*deferred_tools_available*/ false,
|
||||
);
|
||||
assert!(description.contains(
|
||||
"### `foo`
|
||||
@@ -876,12 +878,8 @@ bar"
|
||||
|
||||
#[test]
|
||||
fn exec_description_mentions_timeout_helpers() {
|
||||
let description = build_exec_tool_description(
|
||||
&[],
|
||||
&BTreeMap::new(),
|
||||
/*code_mode_only*/ false,
|
||||
/*deferred_tools_available*/ false,
|
||||
);
|
||||
let description =
|
||||
build_exec_tool_description(&[], &[], &BTreeMap::new(), /*code_mode_only*/ false);
|
||||
assert!(description.contains("`setTimeout(callback: () => void, delayMs?: number)`"));
|
||||
assert!(description.contains("`clearTimeout(timeoutId?: number)`"));
|
||||
}
|
||||
@@ -930,9 +928,9 @@ bar"
|
||||
}))),
|
||||
},
|
||||
],
|
||||
&[],
|
||||
&namespace_descriptions,
|
||||
/*code_mode_only*/ true,
|
||||
/*deferred_tools_available*/ false,
|
||||
);
|
||||
assert_eq!(description.matches("## mcp__sample").count(), 1);
|
||||
assert!(description.contains("## mcp__sample\nShared namespace guidance."));
|
||||
@@ -970,9 +968,9 @@ bar"
|
||||
"additionalProperties": false
|
||||
}))),
|
||||
}],
|
||||
&[],
|
||||
&namespace_descriptions,
|
||||
/*code_mode_only*/ true,
|
||||
/*deferred_tools_available*/ false,
|
||||
);
|
||||
|
||||
assert!(!description.contains("## mcp__sample"));
|
||||
@@ -1069,9 +1067,9 @@ bar"
|
||||
output_schema: second_tool.output_schema,
|
||||
},
|
||||
],
|
||||
&[],
|
||||
&BTreeMap::new(),
|
||||
/*code_mode_only*/ true,
|
||||
/*deferred_tools_available*/ false,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
@@ -1083,13 +1081,51 @@ bar"
|
||||
assert_eq!(description.matches("Shared MCP Types:").count(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn code_mode_only_description_renders_shared_mcp_types_for_deferred_tools() {
|
||||
let deferred_tool = ToolDefinition {
|
||||
name: "mcp__sample__alpha".to_string(),
|
||||
tool_name: ToolName::namespaced("mcp__sample__", "alpha"),
|
||||
description: "Deferred tool".to_string(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: Some(json!({
|
||||
"type": "object",
|
||||
"properties": {},
|
||||
"additionalProperties": false
|
||||
})),
|
||||
output_schema: Some(mcp_call_tool_result_schema(json!({
|
||||
"type": "object",
|
||||
"properties": {},
|
||||
"additionalProperties": false
|
||||
}))),
|
||||
};
|
||||
|
||||
let description = build_exec_tool_description(
|
||||
&[],
|
||||
&[deferred_tool],
|
||||
&BTreeMap::new(),
|
||||
/*code_mode_only*/ true,
|
||||
);
|
||||
|
||||
assert!(description.contains("Some deferred nested tools may be omitted"));
|
||||
assert!(description.contains("Shared MCP Types:"));
|
||||
assert!(!description.contains("### `mcp__sample__alpha`"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_description_mentions_deferred_nested_tools_when_available() {
|
||||
let description = build_exec_tool_description(
|
||||
&[],
|
||||
&[ToolDefinition {
|
||||
name: "deferred_tool".to_string(),
|
||||
tool_name: ToolName::plain("deferred_tool"),
|
||||
description: "Deferred tool".to_string(),
|
||||
kind: CodeModeToolKind::Function,
|
||||
input_schema: None,
|
||||
output_schema: None,
|
||||
}],
|
||||
&BTreeMap::new(),
|
||||
/*code_mode_only*/ false,
|
||||
/*deferred_tools_available*/ true,
|
||||
);
|
||||
|
||||
assert!(description.contains("Some deferred nested tools may be omitted"));
|
||||
|
||||
Reference in New Issue
Block a user