diff --git a/codex-rs/code-mode-protocol/src/description.rs b/codex-rs/code-mode-protocol/src/description.rs index 3e0d1affde..4e485889c0 100644 --- a/codex-rs/code-mode-protocol/src/description.rs +++ b/codex-rs/code-mode-protocol/src/description.rs @@ -7,6 +7,7 @@ use serde::Deserialize; use serde::Serialize; use serde_json::Value as JsonValue; use std::collections::BTreeMap; +use std::collections::BTreeSet; use crate::PUBLIC_TOOL_NAME; use crate::json_schema_types::render_json_schema_to_typescript; @@ -159,6 +160,15 @@ pub struct ToolNamespaceDescription { pub description: String, } +/// One complete tool definition or namespace instruction and its identity in an update. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ExecDefinition { + /// Stable identity (`Tool: tools.` or `Namespace instructions: `) used for diffing. + pub key: String, + /// Complete rendered entry, including its heading, used to replace an earlier definition. + pub description: String, +} + #[derive(Debug, Default, Deserialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] struct CodeModeExecPragma { @@ -275,8 +285,9 @@ pub enum DeferredToolDiscovery { RankedSearch, } +/// Builds the full description and optionally collects stable entries in one rendering pass. #[allow(clippy::too_many_arguments)] -pub fn build_exec_tool_description( +pub fn build_exec_tool_description_and_definitions( enabled_tools: &[ToolDefinition], _deferred_tools: &[ToolDefinition], namespace_descriptions: &BTreeMap, @@ -284,8 +295,9 @@ pub fn build_exec_tool_description( code_mode_only: bool, image_detail_visibility: ImageDetailVisibility, deferred_tool_discovery: DeferredToolDiscovery, + incremental_exec_enabled: bool, messages: Option<&CodeModeToolMessages>, -) -> String { +) -> (String, Vec) { let mut sections = Vec::new(); let image_helper = match image_detail_visibility { ImageDetailVisibility::Visible => LEGACY_IMAGE_HELPER_DESCRIPTION, @@ -319,7 +331,7 @@ pub fn build_exec_tool_description( sections.push(guidance.to_string()); } if !code_mode_only { - return sections.join("\n\n"); + return (sections.join("\n\n"), Vec::new()); } let preamble = messages @@ -329,8 +341,10 @@ pub fn build_exec_tool_description( sections.push(format!("Shared MCP Types:\n```ts\n{preamble}\n```")); } + let mut entries = Vec::new(); if !enabled_tools.is_empty() { let mut current_namespace: Option<&str> = None; + let mut seen_namespaces = BTreeSet::new(); let mut nested_tool_sections = Vec::with_capacity(enabled_tools.len()); for tool in enabled_tools { @@ -343,35 +357,57 @@ pub fn build_exec_tool_description( .and_then(|namespace| namespace_descriptions.get(namespace)); let next_namespace = namespace_description .map(|namespace_description| namespace_description.name.as_str()); - if next_namespace != current_namespace { - if let Some(namespace_description) = namespace_description { - let namespace_description_text = namespace_description.description.trim(); - if !namespace_description_text.is_empty() { - nested_tool_sections.push(format!( + if let Some(namespace_description) = namespace_description { + let namespace_description_text = namespace_description.description.trim(); + if !namespace_description_text.is_empty() { + let new_entry = incremental_exec_enabled + && seen_namespaces.insert(&namespace_description.name); + let new_group = next_namespace != current_namespace; + if new_entry || new_group { + let description = format!( "## {}\n{namespace_description_text}", namespace_description.name - )); + ); + if new_entry { + entries.push(ExecDefinition { + key: format!( + "Namespace instructions: {}", + namespace_description.name + ), + description: description.clone(), + }); + } + if new_group { + nested_tool_sections.push(description); + } } } - current_namespace = next_namespace; } + current_namespace = next_namespace; let global_name = normalize_code_mode_identifier(name); let nested_description = nested_description.trim(); - if nested_description.is_empty() { - nested_tool_sections.push(render_tool_heading(&global_name, name)); + let description = if nested_description.is_empty() { + render_tool_heading(&global_name, name) } else { - nested_tool_sections.push(format!( + format!( "{}\n{nested_description}", render_tool_heading(&global_name, name) - )); + ) + }; + if incremental_exec_enabled { + entries.push(ExecDefinition { + key: format!("Tool: tools.{global_name}"), + description: description.clone(), + }); } + nested_tool_sections.push(description); } sections.push(nested_tool_sections.join("\n\n")); } - sections.join("\n\n") + (sections.join("\n\n"), entries) } pub fn build_wait_tool_description() -> &'static str { @@ -553,7 +589,7 @@ mod tests { use super::ToolDefinition; use super::ToolNamespaceDescription; use super::augment_tool_definition; - use super::build_exec_tool_description; + use super::build_exec_tool_description_and_definitions; use super::normalize_code_mode_identifier; use super::parse_exec_source; use codex_protocol::ToolName; @@ -741,7 +777,7 @@ mod tests { #[test] fn code_mode_only_description_includes_nested_tools() { - let description = build_exec_tool_description( + let (description, entries) = build_exec_tool_description_and_definitions( &[ToolDefinition { name: "foo".to_string(), tool_name: ToolName::plain("foo"), @@ -757,8 +793,10 @@ mod tests { /*code_mode_only*/ true, ImageDetailVisibility::Visible, DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ false, /*messages*/ None, ); + assert!(entries.is_empty()); assert!(description.contains( "### `foo` bar" @@ -768,7 +806,7 @@ bar" #[test] fn exec_description_mentions_timeout_helpers() { - let description = build_exec_tool_description( + let (description, _) = build_exec_tool_description_and_definitions( &[], &[], &BTreeMap::new(), @@ -776,6 +814,7 @@ bar" /*code_mode_only*/ false, ImageDetailVisibility::Visible, DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ false, /*messages*/ None, ); assert!(description.contains("`audio(audioUrlOrItem:")); @@ -784,7 +823,7 @@ bar" } #[test] - fn code_mode_only_description_groups_namespace_instructions_once() { + fn code_mode_only_definitions_match_full_description() { let namespace_descriptions = BTreeMap::from([( "mcp__sample__".to_string(), ToolNamespaceDescription { @@ -792,42 +831,94 @@ bar" description: "Shared namespace guidance.".to_string(), }, )]); - let description = build_exec_tool_description( + let enabled_tools = [ + ToolDefinition { + name: "mcp__sample__alpha-tool".to_string(), + tool_name: ToolName::namespaced("mcp__sample__", "alpha-tool"), + description: "First tool".to_string(), + kind: CodeModeToolKind::Function, + input_schema: Some(json!({ + "type": "object", + "properties": { "city": { "type": "string" } }, + "required": ["city"], + "additionalProperties": false + })), + input_schema_max_bytes: None, + output_schema: Some(mcp_call_tool_result_schema(json!({ + "type": "object", + "properties": { "ok": { "type": "boolean" } }, + "required": ["ok"], + "additionalProperties": false + }))), + }, + ToolDefinition { + name: "mcp__sample__beta".to_string(), + tool_name: ToolName::namespaced("mcp__sample__", "beta"), + description: "Second tool".to_string(), + kind: CodeModeToolKind::Function, + input_schema: Some(json!({ + "type": "object", + "properties": {}, + "additionalProperties": false + })), + input_schema_max_bytes: None, + output_schema: Some(mcp_call_tool_result_schema(json!({ + "type": "object", + "properties": {}, + "additionalProperties": false + }))), + }, + ]; + let (description, entries) = build_exec_tool_description_and_definitions( + &enabled_tools, + &[], + &namespace_descriptions, + crate::DEFAULT_EXEC_YIELD_TIME_MS, + /*code_mode_only*/ true, + ImageDetailVisibility::Visible, + DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ true, + /*messages*/ None, + ); + assert_eq!(description.matches("## mcp__sample").count(), 1); + assert!(description.contains("## mcp__sample\nShared namespace guidance.")); + assert!(description.contains( + "declare const tools: { mcp__sample__alpha_tool(args: { city: string; }): Promise>; };" + )); + assert!(description.contains( + "declare const tools: { mcp__sample__beta(args: {}): Promise>; };" + )); + + assert_eq!( + entries + .iter() + .map(|entry| entry.key.as_str()) + .collect::>(), + vec![ + "Namespace instructions: mcp__sample", + "Tool: tools.mcp__sample__alpha_tool", + "Tool: tools.mcp__sample__beta", + ] + ); + let catalog_start = description.find("## mcp__sample\n").unwrap(); + assert_eq!( + entries + .iter() + .map(|entry| entry.description.as_str()) + .collect::>() + .join("\n\n"), + &description[catalog_start..] + ); + + // Full descriptions repeat a returning namespace heading; update entries keep one identity. + let mut plain_tool = enabled_tools[1].clone(); + plain_tool.name = "plain".to_string(); + plain_tool.tool_name = ToolName::plain("plain"); + let (description, interleaved_entries) = build_exec_tool_description_and_definitions( &[ - ToolDefinition { - name: "mcp__sample__alpha".to_string(), - tool_name: ToolName::namespaced("mcp__sample__", "alpha"), - description: "First tool".to_string(), - kind: CodeModeToolKind::Function, - input_schema: Some(json!({ - "type": "object", - "properties": {}, - "additionalProperties": false - })), - input_schema_max_bytes: None, - output_schema: Some(mcp_call_tool_result_schema(json!({ - "type": "object", - "properties": {}, - "additionalProperties": false - }))), - }, - ToolDefinition { - name: "mcp__sample__beta".to_string(), - tool_name: ToolName::namespaced("mcp__sample__", "beta"), - description: "Second tool".to_string(), - kind: CodeModeToolKind::Function, - input_schema: Some(json!({ - "type": "object", - "properties": {}, - "additionalProperties": false - })), - input_schema_max_bytes: None, - output_schema: Some(mcp_call_tool_result_schema(json!({ - "type": "object", - "properties": {}, - "additionalProperties": false - }))), - }, + enabled_tools[0].clone(), + plain_tool, + enabled_tools[1].clone(), ], &[], &namespace_descriptions, @@ -835,16 +926,17 @@ bar" /*code_mode_only*/ true, ImageDetailVisibility::Visible, DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ true, /*messages*/ None, ); - assert_eq!(description.matches("## mcp__sample").count(), 1); - assert!(description.contains("## mcp__sample\nShared namespace guidance.")); - assert!(description.contains( - "declare const tools: { mcp__sample__alpha(args: {}): Promise>; };" - )); - assert!(description.contains( - "declare const tools: { mcp__sample__beta(args: {}): Promise>; };" - )); + assert_eq!(description.matches("## mcp__sample").count(), 2); + assert_eq!( + interleaved_entries + .iter() + .filter(|entry| entry.key.starts_with("Namespace instructions:")) + .collect::>(), + vec![&entries[0]], + ); } #[test] @@ -856,7 +948,7 @@ bar" description: String::new(), }, )]); - let description = build_exec_tool_description( + let (description, _) = build_exec_tool_description_and_definitions( &[ToolDefinition { name: "mcp__sample__alpha".to_string(), tool_name: ToolName::namespaced("mcp__sample__", "alpha"), @@ -880,6 +972,7 @@ bar" /*code_mode_only*/ true, ImageDetailVisibility::Visible, DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ false, /*messages*/ None, ); @@ -960,7 +1053,7 @@ bar" })), }); - let description = build_exec_tool_description( + let (description, _) = build_exec_tool_description_and_definitions( &[ ToolDefinition { name: first_tool.name, @@ -987,6 +1080,7 @@ bar" /*code_mode_only*/ true, ImageDetailVisibility::Visible, DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ false, /*messages*/ None, ); @@ -1019,7 +1113,7 @@ bar" }))), }; - let description = build_exec_tool_description( + let (description, _) = build_exec_tool_description_and_definitions( &[], &[deferred_tool], &BTreeMap::new(), @@ -1027,6 +1121,7 @@ bar" /*code_mode_only*/ true, ImageDetailVisibility::Visible, DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ false, /*messages*/ None, ); @@ -1064,7 +1159,7 @@ bar" DeferredToolDiscovery::Catalog => Vec::new(), DeferredToolDiscovery::RankedSearch => vec![search_tool.clone()], }; - let description = build_exec_tool_description( + let (description, _) = build_exec_tool_description_and_definitions( &enabled_tools, &deferred_tools, &BTreeMap::new(), @@ -1072,6 +1167,7 @@ bar" code_mode_only, ImageDetailVisibility::Visible, discovery, + /*incremental_exec_enabled*/ false, /*messages*/ None, ); assert!(description.contains("Some deferred nested tools may be omitted")); @@ -1081,7 +1177,7 @@ bar" usize::from(discovery == DeferredToolDiscovery::RankedSearch), ); - let empty_catalog_description = build_exec_tool_description( + let (empty_catalog_description, _) = build_exec_tool_description_and_definitions( &enabled_tools, &[], &BTreeMap::new(), @@ -1089,6 +1185,7 @@ bar" code_mode_only, ImageDetailVisibility::Visible, discovery, + /*incremental_exec_enabled*/ false, /*messages*/ None, ); assert_eq!(description, empty_catalog_description); diff --git a/codex-rs/code-mode-protocol/src/description_override_tests.rs b/codex-rs/code-mode-protocol/src/description_override_tests.rs index 294aa1011e..e151964b1b 100644 --- a/codex-rs/code-mode-protocol/src/description_override_tests.rs +++ b/codex-rs/code-mode-protocol/src/description_override_tests.rs @@ -9,7 +9,7 @@ use super::MCP_TYPESCRIPT_PREAMBLE; use super::TOOL_SEARCH_GUIDANCE; use super::ToolDefinition; use super::UNIFIED_IMAGE_HELPER_DESCRIPTION; -use super::build_exec_tool_description; +use super::build_exec_tool_description_and_definitions; use codex_protocol::ToolName; use codex_protocol::openai_models::CodeModeToolMessages; use codex_protocol::openai_models::ToolMessage; @@ -29,7 +29,7 @@ fn exec_override_renders_only_known_literal_placeholders() { UNIFIED_IMAGE_HELPER_DESCRIPTION, ), ] { - let description = build_exec_tool_description( + let (description, _) = build_exec_tool_description_and_definitions( &[], &[], &BTreeMap::new(), @@ -37,6 +37,7 @@ fn exec_override_renders_only_known_literal_placeholders() { /*code_mode_only*/ false, image_detail_visibility, DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ false, Some(&CodeModeToolMessages { exec: Some(ToolMessage { description: Some(" \nDefaults to 10000 ms. {{ default_exec_yield_time_ms }} ms.\n{{ image_helper }}\n{{ unknown }} {{default_exec_yield_time_ms}}\t ".to_string()), @@ -59,7 +60,7 @@ fn exec_override_renders_only_known_literal_placeholders() { fn exec_override_preserves_empty_and_whitespace_only_text() { for description_override in ["", " \n\t "] { assert_eq!( - build_exec_tool_description( + build_exec_tool_description_and_definitions( &[], &[], &BTreeMap::new(), @@ -67,6 +68,7 @@ fn exec_override_preserves_empty_and_whitespace_only_text() { /*code_mode_only*/ true, ImageDetailVisibility::Visible, DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ false, Some(&CodeModeToolMessages { exec: Some(ToolMessage { description: Some(description_override.to_string()), @@ -78,7 +80,8 @@ fn exec_override_preserves_empty_and_whitespace_only_text() { mcp_typescript_preamble: Some("No MCP tools; omit this.".to_string()), ..Default::default() }), - ), + ) + .0, [ description_override, "No deferred tools; omit this.", @@ -150,7 +153,7 @@ fn exec_override_preserves_runtime_sections() { ), ] { assert_eq!( - build_exec_tool_description( + build_exec_tool_description_and_definitions( &enabled_tools, &deferred_tools, &BTreeMap::new(), @@ -158,6 +161,7 @@ fn exec_override_preserves_runtime_sections() { code_mode_only, ImageDetailVisibility::Visible, DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ false, Some(&CodeModeToolMessages { exec: Some(ToolMessage { description: Some(String::new()), @@ -167,7 +171,8 @@ fn exec_override_preserves_runtime_sections() { mcp_typescript_preamble: preamble.map(str::to_string), ..Default::default() }), - ), + ) + .0, expected, ); } @@ -197,7 +202,7 @@ fn ranked_search_guidance_honors_overrides() { (Some(""), String::new()), (Some("Catalog discovery."), "Catalog discovery.".to_string()), ] { - let description = build_exec_tool_description( + let (description, _) = build_exec_tool_description_and_definitions( &[], &deferred_tools, &BTreeMap::new(), @@ -205,6 +210,7 @@ fn ranked_search_guidance_honors_overrides() { code_mode_only, ImageDetailVisibility::Visible, DeferredToolDiscovery::RankedSearch, + /*incremental_exec_enabled*/ false, Some(&CodeModeToolMessages { exec: Some(ToolMessage { description: Some(String::new()), @@ -232,7 +238,7 @@ fn mcp_types_stay_stable_when_a_deferred_tool_changes_its_output_schema() { output_schema: None, }; let render = |tool: &ToolDefinition, preamble: Option<&str>| { - build_exec_tool_description( + build_exec_tool_description_and_definitions( &[], std::slice::from_ref(tool), &BTreeMap::new(), @@ -240,11 +246,13 @@ fn mcp_types_stay_stable_when_a_deferred_tool_changes_its_output_schema() { /*code_mode_only*/ true, ImageDetailVisibility::Visible, DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ false, Some(&CodeModeToolMessages { mcp_typescript_preamble: preamble.map(str::to_string), ..Default::default() }), ) + .0 }; // Keep one deferred tool throughout to isolate this from the independent // discovery guidance transition when the entire deferred catalog empties. @@ -275,7 +283,7 @@ fn nested_guidance_survives_catalog_changes_and_respects_overrides() { output_schema: None, }]; let render = |tools: &[ToolDefinition], code_mode_only, guidance: Option<&str>| { - build_exec_tool_description( + build_exec_tool_description_and_definitions( &[], tools, &BTreeMap::new(), @@ -283,6 +291,7 @@ fn nested_guidance_survives_catalog_changes_and_respects_overrides() { code_mode_only, ImageDetailVisibility::Visible, DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ false, Some(&CodeModeToolMessages { exec: Some(ToolMessage { description: Some(String::new()), @@ -293,6 +302,7 @@ fn nested_guidance_survives_catalog_changes_and_respects_overrides() { ..Default::default() }), ) + .0 }; for code_mode_only in [false, true] { for (guidance, expected) in [ diff --git a/codex-rs/code-mode-protocol/src/lib.rs b/codex-rs/code-mode-protocol/src/lib.rs index 0d60218234..81d9c0ae84 100644 --- a/codex-rs/code-mode-protocol/src/lib.rs +++ b/codex-rs/code-mode-protocol/src/lib.rs @@ -11,12 +11,13 @@ pub use description::CODE_MODE_PRAGMA_PREFIX; pub use description::CodeModeToolKind; pub use description::DeferredToolDiscovery; pub use description::EnabledToolMetadata; +pub use description::ExecDefinition; pub use description::ImageDetailVisibility; pub use description::TOOL_SEARCH_GUIDANCE; pub use description::ToolDefinition; pub use description::ToolNamespaceDescription; pub use description::augment_tool_definition; -pub use description::build_exec_tool_description; +pub use description::build_exec_tool_description_and_definitions; pub use description::build_wait_tool_description; pub use description::enabled_tool_metadata; pub use description::is_code_mode_nested_tool; diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index c8662e89a0..9c65c797b8 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -27,7 +27,7 @@ use crate::context::ManagedDeveloperInstructions; use crate::context::ModelInstructions; use crate::context::MultiAgentRoleInstructions; use crate::context::SubagentNotification; -use crate::context::world_state::IncrementalToolsHint; +use crate::context::world_state::IncrementalToolsUpdate; use crate::init_state_db; use crate::responses_metadata::CodexResponsesRequestKind; use crate::session::SessionSettingsUpdate; @@ -3351,7 +3351,7 @@ async fn spawn_agent_fork_strips_parent_usage_hints_from_compacted_history(marke role: "developer".to_string(), tools: Arc::from([serde_json::json!({"type": "function", "name": "compacted_tool"})]), }; - let tools_update = ContextualUserFragment::into(IncrementalToolsHint( + let tools_update = ContextualUserFragment::into(IncrementalToolsUpdate( codex_prompts::ResolvedIncrementalToolMessages::default().tool_update_hint, )); let replacement_history = vec![ @@ -3517,7 +3517,7 @@ async fn spawn_agent_fork_strips_parent_usage_hints_from_compacted_history(marke || matches!(item, ResponseItem::Message { internal_chat_message_metadata_passthrough: Some(metadata), .. } if metadata.content_item_kinds.as_ref().is_some_and(|kinds| { - kinds.iter().any(|kind| kind.as_str() == IncrementalToolsHint::KIND) + kinds.iter().any(|kind| kind.as_str() == IncrementalToolsUpdate::KIND) })) }) .cloned() @@ -3929,7 +3929,7 @@ async fn spawn_agent_full_fork_legacy_compaction_rebuilds_child_instructions_onc })), ]); rollout_items.push(rollout_response_item(ContextualUserFragment::into( - IncrementalToolsHint( + IncrementalToolsUpdate( codex_prompts::ResolvedIncrementalToolMessages::default().tool_update_hint, ), ))); @@ -4001,7 +4001,7 @@ async fn spawn_agent_full_fork_legacy_compaction_rebuilds_child_instructions_onc if matches!(&envelope.item, ResponseItem::Message { internal_chat_message_metadata_passthrough: Some(metadata), .. } if metadata.content_item_kinds.as_ref().is_some_and(|kinds| { - kinds.iter().any(|kind| kind.as_str() == IncrementalToolsHint::KIND) + kinds.iter().any(|kind| kind.as_str() == IncrementalToolsUpdate::KIND) })) => Some(envelope.item.clone()), _ => None, diff --git a/codex-rs/core/src/agent/history.rs b/codex-rs/core/src/agent/history.rs index 9cba9c4260..f82446b371 100644 --- a/codex-rs/core/src/agent/history.rs +++ b/codex-rs/core/src/agent/history.rs @@ -13,7 +13,7 @@ use crate::context::ModelInstructions; use crate::context::MultiAgentModeInstructions; use crate::context::MultiAgentRoleInstructions; use crate::context::MultiAgentUsageHint; -use crate::context::world_state::IncrementalToolsHint; +use crate::context::world_state::IncrementalToolsUpdate; use crate::context::world_state::PersistentModeState; use crate::session::multi_agents::resolve_usage_hints; use crate::tasks::InterruptedTurnHistoryMarker; @@ -320,7 +320,7 @@ pub(super) async fn prepare_forked_history( content.retain_mut(|content_item| { if matches!( content_item.kind().as_str(), - BaseInstructionsFragment::KIND | IncrementalToolsHint::KIND + BaseInstructionsFragment::KIND | IncrementalToolsUpdate::KIND ) { return preserve_context_baselines; } diff --git a/codex-rs/core/src/context/world_state/mod.rs b/codex-rs/core/src/context/world_state/mod.rs index 8fb392b961..7b3dbe3908 100644 --- a/codex-rs/core/src/context/world_state/mod.rs +++ b/codex-rs/core/src/context/world_state/mod.rs @@ -66,7 +66,7 @@ pub(crate) use persistent_mode::PersistentModeState; pub(crate) use plugins_instructions::PluginsInstructionsState; pub(crate) use realtime::RealtimeState; pub(crate) use tools::ToolsState; -pub(crate) use top_level_tools::IncrementalToolsHint; +pub(crate) use top_level_tools::IncrementalToolsUpdate; pub(crate) use top_level_tools::TopLevelToolsState; /// One contribution to model context and its placement policy. diff --git a/codex-rs/core/src/context/world_state/top_level_tools.rs b/codex-rs/core/src/context/world_state/top_level_tools.rs index 4dbab8b181..04978ef8cd 100644 --- a/codex-rs/core/src/context/world_state/top_level_tools.rs +++ b/codex-rs/core/src/context/world_state/top_level_tools.rs @@ -11,11 +11,14 @@ use super::WorldStateSection; use super::WorldStateUpdate; use crate::context::ContextualUserFragment; use crate::context::DeveloperInstructions; +use codex_code_mode::ExecDefinition; +use codex_code_mode::PUBLIC_TOOL_NAME; use codex_extension_api::ExtensionMetrics; use codex_otel::TOOL_INCREMENTAL_UPDATES_METRIC; use codex_otel::TOOL_SPEC_DESCRIPTION_BYTES_METRIC; use codex_otel::TOOL_SPEC_DESCRIPTION_BYTES_METRIC_BUCKETS; use codex_prompts::ResolvedIncrementalToolMessages; +use codex_protocol::DEFAULT_FUNCTION_NAMESPACE; use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result; use codex_protocol::models::ContentItem; @@ -23,6 +26,8 @@ use codex_protocol::models::ContentItemKind; use codex_protocol::models::ContentItemMetadata; use codex_protocol::models::ResponseItem; use codex_protocol::models::input_text; +use serde::Deserialize; +use serde::Serialize; use serde_json::Value; use std::collections::BTreeMap; use std::collections::BTreeSet; @@ -32,12 +37,31 @@ const ACTION_ADDED: &str = "added"; const ACTION_REMOVED: &str = "removed"; const ACTION_SCHEMA_CHANGED: &str = "schema_changed"; +const EXEC_TOOLS_UPDATE_HINT: &str = "Use these definitions and namespace instructions with `functions.exec`. Each replaces its earlier version in full. Unlisted tools keep their latest definitions and `tools` methods."; +const EXEC_TOOLS_REMOVED_DEFINITIONS_HEADER: &str = "Listed `Tool:` entries are unavailable until redefined or rediscovered. `Namespace instructions:` removes guidance only. Unlisted entries are unchanged."; + /// The exact serialized Responses Lite declarations visible in one captured step. pub(crate) struct TopLevelToolsState { messages: ResolvedIncrementalToolMessages, definitions: Vec, hashes: BTreeMap, metrics: Option>, + /// Whether this captured step uses granular exec catalog updates. + incremental_exec_tools: bool, + /// Diffable entries for the current CodeModeOnly catalog; absent in other tool modes. + exec_definitions: Option>>, +} + +/// Ordinary catalogs keep their landed flat format; exec mode adds its own comparison state. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(untagged)] +pub(crate) enum TopLevelToolsSnapshot { + WithExec { + tools: BTreeMap, + /// Stable entry keys and definition hashes, retained while exec is unavailable. + exec: BTreeMap, + }, + ToolsOnly(BTreeMap), } impl TopLevelToolsState { @@ -77,8 +101,24 @@ impl TopLevelToolsState { definitions, hashes, metrics, + incremental_exec_tools: false, + exec_definitions: None, }) } + + /// Track granular exec updates as part of this tool catalog. + #[cfg_attr( + not(test), + expect(dead_code, reason = "Activated by the follow-up runtime integration") + )] + pub(crate) fn with_incremental_exec_tools( + mut self, + exec_definitions: Option>>, + ) -> Self { + self.incremental_exec_tools = true; + self.exec_definitions = exec_definitions; + self + } } // Input comes from create_tools_json_for_responses_lite: namespaces and their members have @@ -129,20 +169,66 @@ fn record_changed_tool_metrics( impl WorldStateSection for TopLevelToolsState { const ID: &'static str = "top_level_tools"; - type Snapshot = BTreeMap; + type Snapshot = TopLevelToolsSnapshot; fn render_diff( &self, previous: PreviousSectionState<'_, Self::Snapshot>, ) -> SectionTransition { - let previous = match previous { - PreviousSectionState::Known(previous) => Some(previous), - PreviousSectionState::Absent | PreviousSectionState::Unknown => None, + let (previous, previous_exec) = match previous { + PreviousSectionState::Known(TopLevelToolsSnapshot::WithExec { tools, exec }) => { + (Some(tools), Some(exec)) + } + PreviousSectionState::Known(TopLevelToolsSnapshot::ToolsOnly(tools)) => { + (Some(tools), None) + } + PreviousSectionState::Absent | PreviousSectionState::Unknown => (None, None), }; + // Only an advertised CodeModeOnly exec declaration gets granular updates. + let exec_catalog = self.exec_definitions.as_ref().and_then(|entries| { + self.definitions.iter().find_map(|namespace| { + if namespace["type"] != "namespace" + || namespace["name"] != DEFAULT_FUNCTION_NAMESPACE + { + return None; + } + let exec = namespace["tools"] + .as_array()? + .iter() + .find(|tool| tool["type"] == "custom" && tool["name"] == PUBLIC_TOOL_NAME)?; + Some((namespace, exec, entries.as_slice())) + }) + }); + let manages_exec = exec_catalog.is_some(); + let mut updates = self.render_declaration_updates(previous, manages_exec); + self.append_removed_tools(previous, &mut updates); + let snapshot = if self.incremental_exec_tools { + let exec = + self.append_exec_updates(previous, previous_exec, exec_catalog, &mut updates); + TopLevelToolsSnapshot::WithExec { + tools: self.hashes.clone(), + exec, + } + } else { + TopLevelToolsSnapshot::ToolsOnly(self.hashes.clone()) + }; + // Persist empty catalogs as known comparison baselines. + (Some(snapshot), updates) + } +} + +impl TopLevelToolsState { + /// Render added or changed declarations and namespace instruction updates. + fn render_declaration_updates( + &self, + previous: Option<&BTreeMap>, + manages_exec: bool, + ) -> Vec { let changed = |name: &str| previous.and_then(|previous| previous.get(name)) != self.hashes.get(name); let mut namespace_updates = Vec::new(); let mut tools = Vec::new(); + // Tool declarations: collect changed standalone tools and namespace members. for definition in &self.definitions { let name = definition_name(definition); let Some(members) = definition["tools"].as_array() else { @@ -160,6 +246,9 @@ impl WorldStateSection for TopLevelToolsState { let mut changed_members = Vec::new(); for tool in members { let tool_name = format!("{name}.{}", definition_name(tool)); + if manages_exec && previous.is_some() && tool_name == "functions.exec" { + continue; + } if changed(&tool_name) { record_changed_tool_metrics( &tool_name, @@ -174,9 +263,8 @@ impl WorldStateSection for TopLevelToolsState { let mut namespace = definition.clone(); namespace["tools"] = Value::Array(changed_members); tools.push(namespace); - } else if changed(name) { - // Namespace declarations require a member. Update metadata as text without - // repeating unchanged tool definitions. + } else if changed(name) && (previous.is_some() || !manages_exec) { + // Namespace instruction updates use text because declarations require a member. let instructions = definition["description"].as_str().unwrap_or_default(); let text = if instructions.is_empty() { self.messages @@ -201,12 +289,13 @@ impl WorldStateSection for TopLevelToolsState { } let mut updates = Vec::new(); if !tools.is_empty() { + // Incremental tool updates: introduce an appended batch of namespace declarations. if previous.is_some() && tools.iter().any(|tool| tool["type"] == "namespace") && !self.messages.tool_update_hint.is_empty() { updates.push(WorldStateUpdate::prefix_item(ContextualUserFragment::into( - IncrementalToolsHint(self.messages.tool_update_hint.clone()), + IncrementalToolsUpdate(self.messages.tool_update_hint.clone()), ))); } updates.push(WorldStateUpdate::prefix_item( @@ -218,7 +307,17 @@ impl WorldStateSection for TopLevelToolsState { )); } updates.extend(namespace_updates); + updates + } + + /// Append a removal notice, listing removed namespaces instead of their individual members. + fn append_removed_tools( + &self, + previous: Option<&BTreeMap>, + updates: &mut Vec, + ) { if let Some(previous) = previous { + // Removal notices: withdraw namespaces and tools absent from the current catalog. // Namespace and member names cannot contain dots. A qualified key identifies // its old namespace without changing the persisted header/member hash format. let namespaces = previous @@ -259,19 +358,94 @@ impl WorldStateSection for TopLevelToolsState { ); } } - // Persist the empty map too: it is a known empty catalog, not missing state. - (Some(self.hashes.clone()), updates) + } + + /// Append exec catalog updates and return the matching comparison snapshot. + fn append_exec_updates( + &self, + previous: Option<&BTreeMap>, + previous_exec: Option<&BTreeMap>, + exec_catalog: Option<(&Value, &Value, &[ExecDefinition])>, + updates: &mut Vec, + ) -> BTreeMap { + // Retain entry hashes while functions.exec is not advertised. + // When it returns, send the full current catalog and withdraw removed entries. + let mut exec = BTreeMap::new(); + if let Some((namespace, declaration, entries)) = exec_catalog { + exec = entries + .iter() + .map(|entry| { + ( + entry.key.clone(), + WorldStateHash::from_json(&Value::String(entry.description.clone())), + ) + }) + .collect(); + if previous_exec.is_some() + && previous.is_some_and(|tools| tools.contains_key("functions.exec")) + { + // Incremental tool updates: append entries; the exec declaration stays frozen. + let changed = entries + .iter() + .filter(|entry| { + previous_exec.and_then(|previous| previous.get(&entry.key)) + != exec.get(&entry.key) + }) + .collect::>(); + if !changed.is_empty() { + updates.push( + WorldStateUpdate::fragment(render_exec_update(&changed)).standalone(), + ); + } + } else if previous.is_some() { + // Install the full catalog when enabling granular updates or restoring exec. + if previous_exec.is_some() + && let Some(metrics) = self.metrics.as_deref() + { + metrics.counter( + TOOL_INCREMENTAL_UPDATES_METRIC, + /*inc*/ 1, + &[("action", ACTION_ADDED), ("tool", "functions.exec")], + ); + } + let mut namespace = namespace.clone(); + namespace["tools"] = Value::Array(vec![declaration.clone()]); + updates.push(WorldStateUpdate::prefix_item( + ResponseItem::AdditionalTools { + id: None, + role: "developer".to_string(), + tools: Arc::from(vec![namespace]), + }, + )); + } + // Removal notices: include exec entries removed while it was unavailable. + if let Some(previous) = previous_exec { + let removed = previous + .keys() + .filter(|key| !exec.contains_key(*key)) + .cloned() + .collect::>(); + if !removed.is_empty() { + updates.push( + WorldStateUpdate::fragment(RemovedExecDefinitions(removed)).standalone(), + ); + } + } + } else if let Some(previous) = previous_exec { + exec = previous.clone(); + } + exec } } -/// Guidance for an appended catalog batch, retained or discarded with its declarations. -pub(crate) struct IncrementalToolsHint(pub(crate) String); +/// An appended catalog hint or exec definition update, retained or discarded with its baseline. +pub(crate) struct IncrementalToolsUpdate(pub(crate) String); -impl IncrementalToolsHint { +impl IncrementalToolsUpdate { pub(crate) const KIND: &str = "tools.incremental_update"; } -impl ContextualUserFragment for IncrementalToolsHint { +impl ContextualUserFragment for IncrementalToolsUpdate { fn role(&self) -> &'static str { "developer" } @@ -344,6 +518,46 @@ impl ContextualUserFragment for RemovedTools { } } +/// Render a nonempty batch of complete replacement definitions with the exec update hint. +fn render_exec_update(definitions: &[&ExecDefinition]) -> IncrementalToolsUpdate { + let description = definitions + .iter() + .map(|entry| entry.description.as_str()) + .collect::>() + .join("\n\n"); + IncrementalToolsUpdate(format!( + "{EXEC_TOOLS_UPDATE_HINT}\n\n## Namespace: functions\n{description}" + )) +} + +/// Removal notice containing the stable keys of withdrawn tools or namespace instructions. +struct RemovedExecDefinitions(Vec); + +impl ContextualUserFragment for RemovedExecDefinitions { + fn role(&self) -> &'static str { + "developer" + } + + fn content_kind(&self) -> ContentItemKind { + ContentItemKind("tools.removed_definition".to_string()) + } + + fn markers(&self) -> (&'static str, &'static str) { + Self::type_markers() + } + + fn type_markers() -> (&'static str, &'static str) { + ("", "") + } + + fn body(&self) -> ContentItem { + input_text(format!( + "{EXEC_TOOLS_REMOVED_DEFINITIONS_HEADER}\n- {}", + self.0.join("\n- ") + )) + } +} + #[cfg(test)] #[path = "top_level_tools_tests.rs"] mod tests; diff --git a/codex-rs/core/src/context/world_state/top_level_tools_tests.rs b/codex-rs/core/src/context/world_state/top_level_tools_tests.rs index 4ac92c2603..0e85b553de 100644 --- a/codex-rs/core/src/context/world_state/top_level_tools_tests.rs +++ b/codex-rs/core/src/context/world_state/top_level_tools_tests.rs @@ -4,6 +4,7 @@ use super::*; use crate::context::world_state::test_support::render_section_cases; use crate::context_manager::updates::merge_world_state_updates; use crate::session::extension_metrics::from_session_telemetry; +use codex_code_mode::ExecDefinition; use codex_otel::MetricsClient; use codex_otel::MetricsConfig; use codex_otel::SessionTelemetry; @@ -33,6 +34,12 @@ fn namespace(name: &str, tools: Vec) -> Value { json!({"type": "namespace", "name": name, "description": "Tools.", "tools": tools}) } +fn exec_declaration(description: &str, grammar: &str) -> Value { + json!({"type": "namespace", "name": "functions", "description": "", + "tools": [{"type": "custom", "name": "exec", "description": description, + "format": {"type": "grammar", "syntax": "lark", "definition": grammar}}]}) +} + #[test] fn snapshots_catalog_transitions() { use PreviousSectionState::Absent; @@ -278,10 +285,17 @@ fn initial_catalog_preserves_order_and_unchanged_catalog_emits_nothing() { vec![item(definitions.clone())] ); let snapshot = snapshot.unwrap(); + let saved = serde_json::to_value(&snapshot).unwrap(); assert_eq!( - snapshot.keys().map(String::as_str).collect::>(), + saved + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect::>(), vec!["alpha", "alpha.lookup", "zebra", "zebra.lookup"] ); + let snapshot: TopLevelToolsSnapshot = serde_json::from_value(saved).unwrap(); let (unchanged_snapshot, updates) = state.render_diff(PreviousSectionState::Known(&snapshot)); assert_eq!(unchanged_snapshot, Some(snapshot.clone())); assert!(updates.is_empty()); @@ -321,7 +335,10 @@ fn empty_catalog_is_persisted_and_can_add_tools_again() { ) .unwrap(); let (snapshot, updates) = empty.render_diff(PreviousSectionState::Known(&previous)); - assert_eq!(snapshot, Some(BTreeMap::new())); + assert_eq!( + snapshot, + Some(TopLevelToolsSnapshot::ToolsOnly(BTreeMap::new())) + ); assert_eq!( merge_world_state_updates(updates), vec![ContextualUserFragment::into(RemovedTools { @@ -334,7 +351,7 @@ fn empty_catalog_is_persisted_and_can_add_tools_again() { assert_eq!( merge_world_state_updates(updates), vec![ - ContextualUserFragment::into(IncrementalToolsHint( + ContextualUserFragment::into(IncrementalToolsUpdate( ResolvedIncrementalToolMessages::default().tool_update_hint )), item(definitions), @@ -384,7 +401,7 @@ fn namespace_diff_contains_only_changed_and_added_tools() { assert_eq!( merge_world_state_updates(updates), vec![ - ContextualUserFragment::into(IncrementalToolsHint( + ContextualUserFragment::into(IncrementalToolsUpdate( ResolvedIncrementalToolMessages::default().tool_update_hint )), item(vec![expected]), @@ -428,7 +445,7 @@ fn new_namespaces_share_one_notice_before_their_declarations() { assert_eq!( prefix, vec![ - ContextualUserFragment::into(IncrementalToolsHint( + ContextualUserFragment::into(IncrementalToolsUpdate( ResolvedIncrementalToolMessages::default().tool_update_hint )), item(additions.clone()), @@ -543,7 +560,7 @@ fn tool_type_change_keeps_the_callable_name_available() { assert_eq!( merge_world_state_updates(updates), vec![ - ContextualUserFragment::into(IncrementalToolsHint( + ContextualUserFragment::into(IncrementalToolsUpdate( ResolvedIncrementalToolMessages::default().tool_update_hint )), item(vec![updated]), @@ -633,3 +650,158 @@ fn namespace_replaced_by_builtin_keeps_the_replacement_available() { ], ); } + +#[test] +fn hidden_exec_is_not_reintroduced_and_returning_exec_restores_full_catalog() { + let original = exec_declaration("Shared MCP Types:", "grammar"); + let exec_definitions = Arc::new(vec![ExecDefinition { + key: "Tool: tools.a".to_string(), + description: "### a\ndeclare const tools: { a(): Promise };".to_string(), + }]); + let current = TopLevelToolsState::new( + /*messages*/ Default::default(), + vec![original.clone()], + /*metrics*/ None, + ) + .unwrap() + .with_incremental_exec_tools(Some(Arc::clone(&exec_definitions))); + let (baseline, full) = current.render_diff(PreviousSectionState::Absent); + assert_eq!( + merge_world_state_updates(full), + vec![ResponseItem::AdditionalTools { + id: None, + role: "developer".to_string(), + tools: Arc::from(vec![original.clone()]) + },] + ); + let hidden = TopLevelToolsState::new( + /*messages*/ Default::default(), + Vec::new(), + /*metrics*/ None, + ) + .unwrap() + .with_incremental_exec_tools(Some(exec_definitions)); + let (empty, updates) = hidden.render_diff(PreviousSectionState::Known(&baseline.unwrap())); + assert_eq!( + merge_world_state_updates(updates), + vec![ContextualUserFragment::into(RemovedTools { + namespaces: vec!["functions".to_string()], + ..Default::default() + })], + ); + // Retain identities for removals on return, but require a fresh exec declaration. + let restored: TopLevelToolsSnapshot = + serde_json::from_value(serde_json::to_value(empty.unwrap()).unwrap()).unwrap(); + let namespace_instructions = ContextualUserFragment::into( + DeveloperInstructions::new( + "The functions namespace no longer has additional instructions.".to_string(), + ) + .with_metadata(ContentItemMetadata::tool(Some( + "functions".to_string().into(), + ))), + ); + assert_eq!( + merge_world_state_updates( + current + .render_diff(PreviousSectionState::Known(&restored)) + .1 + ), + vec![ + namespace_instructions.clone(), + ResponseItem::AdditionalTools { + id: None, + role: "developer".to_string(), + tools: Arc::from(vec![original.clone()]) + } + ], + ); + + // A full restore also withdraws definitions removed while exec was hidden. + let without_entries = TopLevelToolsState::new( + /*messages*/ Default::default(), + vec![original.clone()], + /*metrics*/ None, + ) + .unwrap() + .with_incremental_exec_tools(Some(Arc::new(Vec::new()))); + assert_eq!( + merge_world_state_updates( + without_entries + .render_diff(PreviousSectionState::Known(&restored)) + .1 + ), + vec![ + namespace_instructions, + ResponseItem::AdditionalTools { + id: None, + role: "developer".to_string(), + tools: Arc::from(vec![original]), + }, + ContextualUserFragment::into(RemovedExecDefinitions( + vec!["Tool: tools.a".to_string(),] + )), + ], + ); +} + +#[test] +fn changed_and_added_entries_share_one_update_and_removed_entries_are_withdrawn() { + let definition = exec_declaration("Shared MCP Types:", "grammar"); + let original = TopLevelToolsState::new( + /*messages*/ Default::default(), + vec![definition.clone()], + /*metrics*/ None, + ) + .unwrap() + .with_incremental_exec_tools(Some(Arc::new(vec![ + ExecDefinition { + key: "Tool: tools.a".to_string(), + description: "### a\ndeclare const tools: { a(): Promise };".to_string(), + }, + ExecDefinition { + key: "Tool: tools.b".to_string(), + description: "### b\ndeclare const tools: { b(): Promise };".to_string(), + }, + ]))); + let baseline = original + .render_diff(PreviousSectionState::Absent) + .0 + .unwrap(); + + let current = TopLevelToolsState::new( + /*messages*/ Default::default(), + vec![definition], + /*metrics*/ None, + ) + .unwrap() + .with_incremental_exec_tools(Some(Arc::new(vec![ + ExecDefinition { + key: "Tool: tools.a".to_string(), + description: + "### a\nChanged a.\ndeclare const tools: { a(value: number): Promise };" + .to_string(), + }, + ExecDefinition { + key: "Tool: tools.c".to_string(), + description: "### c\ndeclare const tools: { c(): Promise };".to_string(), + }, + ]))); + let (snapshot, updates) = current.render_diff(PreviousSectionState::Known(&baseline)); + assert_eq!( + merge_world_state_updates(updates), + vec![ + ContextualUserFragment::into(IncrementalToolsUpdate( + "Use these definitions and namespace instructions with `functions.exec`. Each replaces its earlier version in full. Unlisted tools keep their latest definitions and `tools` methods.\n\n## Namespace: functions\n### a\nChanged a.\ndeclare const tools: { a(value: number): Promise };\n\n### c\ndeclare const tools: { c(): Promise };".to_string(), + )), + ContextualUserFragment::into(RemovedExecDefinitions(vec![ + "Tool: tools.b".to_string(), + ])), + ] + ); + assert!( + current + .render_diff(PreviousSectionState::Known(&snapshot.unwrap())) + .1 + .is_empty() + ); +} diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 6274bc6fb7..8c736b191c 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -1109,7 +1109,7 @@ fn register_code_mode_executors( } enabled_tools .sort_by(|left, right| compare_code_mode_tools(left, right, &namespace_descriptions)); - let exec_description = codex_code_mode::build_exec_tool_description( + let (exec_description, _) = codex_code_mode::build_exec_tool_description_and_definitions( &enabled_tools, &deferred_tools, &namespace_descriptions, @@ -1121,6 +1121,7 @@ fn register_code_mode_executors( codex_code_mode::ImageDetailVisibility::Visible }, deferred_tool_discovery, + /*incremental_exec_enabled*/ false, model_messages.code_mode(), ); let execute_handler = CodeModeExecuteHandler::new( diff --git a/codex-rs/tools/src/indirect_namespace_prefixes_tests.rs b/codex-rs/tools/src/indirect_namespace_prefixes_tests.rs index 6f2df660a1..cb2fc2189c 100644 --- a/codex-rs/tools/src/indirect_namespace_prefixes_tests.rs +++ b/codex-rs/tools/src/indirect_namespace_prefixes_tests.rs @@ -31,7 +31,7 @@ fn plain_tools_join_an_existing_functions_group_only_with_a_prefix() { )]); let render = |tools: &[ToolDefinition], namespaces: &BTreeMap| { - codex_code_mode::build_exec_tool_description( + codex_code_mode::build_exec_tool_description_and_definitions( tools, &[], namespaces, @@ -39,8 +39,10 @@ fn plain_tools_join_an_existing_functions_group_only_with_a_prefix() { /*code_mode_only*/ true, codex_code_mode::ImageDetailVisibility::Visible, codex_code_mode::DeferredToolDiscovery::Catalog, + /*incremental_exec_enabled*/ false, /*messages*/ None, ) + .0 }; let original = render(&original_tools, &original_namespaces); for messages in [