From e4490eff65d9f4dcd986f140e81927058bfa296d Mon Sep 17 00:00:00 2001 From: Alex Kira Date: Sat, 10 Oct 2026 01:04:44 +0000 Subject: [PATCH] Allow model catalogs to override incremental tool notices (#52736) ## What changed Add `model_messages.tools.incremental_tools` overrides for the update hint, tool and namespace removal headers, and namespace instruction update and clearing messages. Missing, null, or over-512-byte values fall back to bundled wording; empty strings omit static text while retaining runtime names and instructions. Substitute `{name}` in namespace message templates and append runtime instructions verbatim. Wording changes alone do not trigger tool catalog updates. ## Testing Extend unit tests and MCP request-history snapshots to cover custom wording, empty notices, tool removal and restoration, and namespace instruction changes and clearing without repeating unchanged tools or notices. GitOrigin-RevId: 22dea0a6fb4cfcd646f5e65346424834c8b7d5a2 --- codex-rs/core/src/agent/control_tests.rs | 8 +- .../context/world_state/top_level_tools.rs | 44 ++-- .../world_state/top_level_tools_tests.rs | 188 +++++++++++++++--- codex-rs/core/src/session/world_state.rs | 1 + .../suite/scenarios_incremental_tools.rs | 103 +++++++++- ...s__incremental_tools_catalog_messages.snap | 109 ++++++++++ codex-rs/prompts/src/lib.rs | 1 + codex-rs/prompts/src/model_messages.rs | 11 + .../src/model_messages/incremental_tools.rs | 51 +++++ codex-rs/protocol/src/openai_models.rs | 5 + .../src/openai_models/incremental_tools.rs | 24 +++ 11 files changed, 493 insertions(+), 52 deletions(-) create mode 100644 codex-rs/core/tests/suite/snapshots/all__suite__scenarios__incremental_tools__incremental_tools_catalog_messages.snap create mode 100644 codex-rs/prompts/src/model_messages/incremental_tools.rs create mode 100644 codex-rs/protocol/src/openai_models/incremental_tools.rs diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 361551c992..6df46c1d78 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -3188,7 +3188,9 @@ async fn spawn_agent_fork_strips_parent_usage_hints_from_compacted_history(marke role: "developer".to_string(), tools: vec![serde_json::json!({"type": "function", "name": "compacted_tool"})], }; - let tools_update = ContextualUserFragment::into(IncrementalToolsHint); + let tools_update = ContextualUserFragment::into(IncrementalToolsHint( + codex_prompts::ResolvedIncrementalToolMessages::default().tool_update_hint, + )); let replacement_history = vec![ tools_update.clone(), tool_declarations.clone(), @@ -3700,7 +3702,9 @@ async fn spawn_agent_full_fork_legacy_compaction_rebuilds_child_instructions_onc internal_chat_message_metadata_passthrough: None, })); rollout_items.push(rollout_response_item(ContextualUserFragment::into( - IncrementalToolsHint, + IncrementalToolsHint( + codex_prompts::ResolvedIncrementalToolMessages::default().tool_update_hint, + ), ))); rollout_items.push(rollout_response_item(ResponseItem::AdditionalTools { id: None, 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 1a4f7005a7..e046f193e9 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 @@ -13,6 +13,7 @@ use crate::context::ContextualUserFragment; use crate::context::DeveloperInstructions; use codex_extension_api::ExtensionMetrics; use codex_otel::TOOL_INCREMENTAL_UPDATES_METRIC; +use codex_prompts::ResolvedIncrementalToolMessages; use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result; use codex_protocol::models::ContentItemKind; @@ -26,11 +27,9 @@ const ACTION_ADDED: &str = "added"; const ACTION_REMOVED: &str = "removed"; const ACTION_SCHEMA_CHANGED: &str = "schema_changed"; -const REMOVED_TOOLS_HEADER: &str = "The following tools are no longer available. Do not call them:"; -const REMOVED_NAMESPACES_HEADER: &str = "The following namespaces are no longer available. Do not call tools in them unless those tools are declared in a later update:"; - /// The exact serialized Responses Lite declarations visible in one captured step. pub(crate) struct TopLevelToolsState { + messages: ResolvedIncrementalToolMessages, definitions: Vec, hashes: BTreeMap, metrics: Option>, @@ -42,6 +41,7 @@ impl TopLevelToolsState { reason = "the Responses Lite serializer emits objects with array-valued namespace members" )] pub(crate) fn new( + messages: ResolvedIncrementalToolMessages, definitions: Vec, metrics: Option>, ) -> Result { @@ -68,6 +68,7 @@ impl TopLevelToolsState { insert_hash(name.to_string(), &header)?; } Ok(Self { + messages, definitions, hashes, metrics, @@ -174,19 +175,31 @@ impl WorldStateSection for TopLevelToolsState { // repeating unchanged tool definitions. let instructions = definition["description"].as_str().unwrap_or_default(); let text = if instructions.is_empty() { - format!("The {name} namespace no longer has additional instructions.") + self.messages + .namespace_instructions_cleared + .replace("{name}", name) } else { - format!("Updated instructions for the {name} namespace:\n{instructions}") + let mut text = self + .messages + .namespace_instructions_prefix + .replace("{name}", name); + text.push_str(instructions); + text }; - namespace_updates - .push(WorldStateUpdate::fragment(DeveloperInstructions::new(text))); + if !text.is_empty() { + namespace_updates + .push(WorldStateUpdate::fragment(DeveloperInstructions::new(text))); + } } } let mut updates = Vec::new(); if !tools.is_empty() { - if previous.is_some() && tools.iter().any(|tool| tool["type"] == "namespace") { + 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, + IncrementalToolsHint(self.messages.tool_update_hint.clone()), ))); } updates.push(WorldStateUpdate::prefix_item( @@ -225,6 +238,7 @@ impl WorldStateSection for TopLevelToolsState { if !namespaces.is_empty() || !tools.is_empty() { updates.push( WorldStateUpdate::fragment(RemovedTools { + messages: self.messages.clone(), namespaces: namespaces.into_iter().map(str::to_string).collect(), tools, }) @@ -239,7 +253,7 @@ impl WorldStateSection for TopLevelToolsState { } /// Guidance for an appended catalog batch, retained or discarded with its declarations. -pub(crate) struct IncrementalToolsHint; +pub(crate) struct IncrementalToolsHint(pub(crate) String); impl IncrementalToolsHint { pub(crate) const KIND: &str = "tools.incremental_update"; @@ -263,12 +277,13 @@ impl ContextualUserFragment for IncrementalToolsHint { } fn body(&self) -> String { - "This is an incremental tools update. Previously declared tools remain available for direct calls unless explicitly marked unavailable. If a tool is redefined here, its latest definition replaces the earlier one.".to_string() + self.0.clone() } } #[derive(Default)] struct RemovedTools { + messages: ResolvedIncrementalToolMessages, namespaces: Vec, tools: Vec, } @@ -293,8 +308,11 @@ impl ContextualUserFragment for RemovedTools { /// Lists removed namespaces and individual tools in separate sections of one notice. fn body(&self) -> String { let sections = [ - (REMOVED_NAMESPACES_HEADER, &self.namespaces), - (REMOVED_TOOLS_HEADER, &self.tools), + ( + self.messages.removed_namespaces_header.as_str(), + &self.namespaces, + ), + (self.messages.removed_tools_header.as_str(), &self.tools), ]; let mut body = String::new(); for (header, names) in sections { 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 a75fd6e44b..d205e37a1f 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 @@ -42,12 +42,32 @@ fn snapshots_catalog_transitions() { "description": "Search tools.", "parameters": {"type": "object", "properties": {}}}); let web = json!({"type": "web_search", "external_web_access": true}); - let original = TopLevelToolsState::new(vec![search.clone()], /*metrics*/ None).unwrap(); + let original = TopLevelToolsState::new( + /*messages*/ Default::default(), + vec![search.clone()], + /*metrics*/ None, + ) + .unwrap(); let mut changed = search; changed["parameters"]["properties"] = json!({"query": {"type": "string"}}); - let updated = TopLevelToolsState::new(vec![changed, web.clone()], /*metrics*/ None).unwrap(); - let only_web = TopLevelToolsState::new(vec![web], /*metrics*/ None).unwrap(); - let empty = TopLevelToolsState::new(Vec::new(), /*metrics*/ None).unwrap(); + let updated = TopLevelToolsState::new( + /*messages*/ Default::default(), + vec![changed, web.clone()], + /*metrics*/ None, + ) + .unwrap(); + let only_web = TopLevelToolsState::new( + /*messages*/ Default::default(), + vec![web], + /*metrics*/ None, + ) + .unwrap(); + let empty = TopLevelToolsState::new( + /*messages*/ Default::default(), + Vec::new(), + /*metrics*/ None, + ) + .unwrap(); insta::assert_snapshot!(render_section_cases(&[ (Absent, Known(&original)), @@ -90,6 +110,7 @@ async fn snapshots_namespace_transitions() { .with_metrics_without_metadata_tags(metrics.clone()), ); let original = TopLevelToolsState::new( + /*messages*/ Default::default(), vec![ namespace( "functions", @@ -107,6 +128,7 @@ async fn snapshots_namespace_transitions() { let mut changed = declaration("changed"); changed["parameters"]["properties"] = json!({"query": {"type": "string"}}); let updated = TopLevelToolsState::new( + /*messages*/ Default::default(), vec![ namespace( "functions", @@ -118,17 +140,25 @@ async fn snapshots_namespace_transitions() { ) .unwrap(); let only_other = TopLevelToolsState::new( + /*messages*/ Default::default(), vec![namespace("other", vec![declaration("lookup")])], Some(Arc::clone(&telemetry)), ) .unwrap(); let only_functions = TopLevelToolsState::new( + /*messages*/ Default::default(), vec![namespace("functions", vec![declaration("unchanged")])], Some(Arc::clone(&telemetry)), ) .unwrap(); - let empty = TopLevelToolsState::new(Vec::new(), Some(Arc::clone(&telemetry))).unwrap(); + let empty = TopLevelToolsState::new( + /*messages*/ Default::default(), + Vec::new(), + Some(Arc::clone(&telemetry)), + ) + .unwrap(); let restored = TopLevelToolsState::new( + /*messages*/ Default::default(), vec![namespace( "functions", vec![declaration("unchanged"), declaration("restored")], @@ -191,38 +221,71 @@ fn initial_catalog_preserves_order_and_unchanged_catalog_emits_nothing() { namespace("zebra", vec![declaration("lookup")]), namespace("alpha", vec![declaration("lookup")]), ]; - let state = TopLevelToolsState::new(definitions.clone(), /*metrics*/ None).unwrap(); + let state = TopLevelToolsState::new( + /*messages*/ Default::default(), + definitions.clone(), + /*metrics*/ None, + ) + .unwrap(); let (snapshot, updates) = state.render_diff(PreviousSectionState::Absent); assert_eq!( merge_world_state_updates(updates), vec![item(definitions.clone())] ); let (_, updates) = state.render_diff(PreviousSectionState::Unknown); - assert_eq!(merge_world_state_updates(updates), vec![item(definitions)]); + assert_eq!( + merge_world_state_updates(updates), + vec![item(definitions.clone())] + ); let snapshot = snapshot.unwrap(); assert_eq!( snapshot.keys().map(String::as_str).collect::>(), vec!["alpha", "alpha.lookup", "zebra", "zebra.lookup"] ); let (unchanged_snapshot, updates) = state.render_diff(PreviousSectionState::Known(&snapshot)); - assert_eq!(unchanged_snapshot, Some(snapshot)); + assert_eq!(unchanged_snapshot, Some(snapshot.clone())); + assert!(updates.is_empty()); + + let reworded = TopLevelToolsState::new( + ResolvedIncrementalToolMessages { + tool_update_hint: "New wording".to_string(), + removed_tools_header: "Removed".to_string(), + ..Default::default() + }, + definitions, + /*metrics*/ None, + ) + .unwrap(); + let (reworded_snapshot, updates) = reworded.render_diff(PreviousSectionState::Known(&snapshot)); + assert_eq!(reworded_snapshot, Some(snapshot)); assert!(updates.is_empty()); } #[test] fn empty_catalog_is_persisted_and_can_add_tools_again() { let definitions = vec![namespace("functions", vec![declaration("lookup")])]; - let original = TopLevelToolsState::new(definitions.clone(), /*metrics*/ None).unwrap(); + let original = TopLevelToolsState::new( + /*messages*/ Default::default(), + definitions.clone(), + /*metrics*/ None, + ) + .unwrap(); let previous = original .render_diff(PreviousSectionState::Absent) .0 .unwrap(); - let empty = TopLevelToolsState::new(Vec::new(), /*metrics*/ None).unwrap(); + let empty = TopLevelToolsState::new( + /*messages*/ Default::default(), + Vec::new(), + /*metrics*/ None, + ) + .unwrap(); let (snapshot, updates) = empty.render_diff(PreviousSectionState::Known(&previous)); assert_eq!(snapshot, Some(BTreeMap::new())); assert_eq!( merge_world_state_updates(updates), vec![ContextualUserFragment::into(RemovedTools { + messages: Default::default(), namespaces: vec!["functions".to_string()], ..Default::default() })] @@ -231,7 +294,9 @@ fn empty_catalog_is_persisted_and_can_add_tools_again() { assert_eq!( merge_world_state_updates(updates), vec![ - ContextualUserFragment::into(IncrementalToolsHint), + ContextualUserFragment::into(IncrementalToolsHint( + ResolvedIncrementalToolMessages::default().tool_update_hint + )), item(definitions), ] ); @@ -240,6 +305,7 @@ fn empty_catalog_is_persisted_and_can_add_tools_again() { #[test] fn namespace_diff_contains_only_changed_and_added_tools() { let original = TopLevelToolsState::new( + /*messages*/ Default::default(), vec![ namespace( "functions", @@ -262,6 +328,7 @@ fn namespace_diff_contains_only_changed_and_added_tools() { changed["parameters"]["properties"] = json!({"query": {"type": "string"}}); let added = declaration("added"); let current = TopLevelToolsState::new( + /*messages*/ Default::default(), vec![ namespace( "functions", @@ -277,9 +344,12 @@ fn namespace_diff_contains_only_changed_and_added_tools() { assert_eq!( merge_world_state_updates(updates), vec![ - ContextualUserFragment::into(IncrementalToolsHint), + ContextualUserFragment::into(IncrementalToolsHint( + ResolvedIncrementalToolMessages::default().tool_update_hint + )), item(vec![expected]), ContextualUserFragment::into(RemovedTools { + messages: Default::default(), tools: vec!["functions.removed".to_string()], ..Default::default() }), @@ -296,13 +366,19 @@ fn namespace_diff_contains_only_changed_and_added_tools() { #[test] fn new_namespaces_share_one_notice_before_their_declarations() { let original = namespace("original", vec![declaration("lookup")]); - let state = TopLevelToolsState::new(vec![original.clone()], /*metrics*/ None).unwrap(); + let state = TopLevelToolsState::new( + /*messages*/ Default::default(), + vec![original.clone()], + /*metrics*/ None, + ) + .unwrap(); let previous = state.render_diff(PreviousSectionState::Absent).0.unwrap(); let additions = vec![ namespace("second", vec![declaration("lookup")]), namespace("third", vec![declaration("lookup")]), ]; - let state = TopLevelToolsState::new( + let mut state = TopLevelToolsState::new( + /*messages*/ Default::default(), std::iter::once(original).chain(additions.clone()).collect(), /*metrics*/ None, ) @@ -312,11 +388,18 @@ fn new_namespaces_share_one_notice_before_their_declarations() { assert_eq!( prefix, vec![ - ContextualUserFragment::into(IncrementalToolsHint), - item(additions), + ContextualUserFragment::into(IncrementalToolsHint( + ResolvedIncrementalToolMessages::default().tool_update_hint + )), + item(additions.clone()), ] ); assert!(remaining.is_empty()); + state.messages.tool_update_hint.clear(); + let (_, updates) = state.render_diff(PreviousSectionState::Known(&previous)); + let (prefix, remaining) = super::super::split_prefix_updates(updates); + assert_eq!(prefix, vec![item(additions)]); + assert!(remaining.is_empty()); assert!( state .render_diff(PreviousSectionState::Known(&snapshot.unwrap())) @@ -325,9 +408,34 @@ fn new_namespaces_share_one_notice_before_their_declarations() { ); } -#[test] -fn namespace_description_change_does_not_repeat_unchanged_tools() { +#[test_case::test_case( + None, + "Updated namespace guidance.", + "Updated instructions for the functions namespace:\nUpdated namespace guidance." +)] +#[test_case::test_case( + Some(("Catalog {name}: ", "Cleared {name}.")), + "Literal {name} and {instructions}.", + "Catalog functions: Literal {name} and {instructions}." +)] +#[test_case::test_case( + Some(("", "Cleared {name}.")), + "Runtime instructions.", + "Runtime instructions." +)] +#[test_case::test_case( + Some(("Catalog {name}: ", "Cleared {name}.")), + "", + "Cleared functions." +)] +#[test_case::test_case(Some(("Catalog {name}: ", "")), "", "")] +fn namespace_description_change_does_not_repeat_unchanged_tools( + wording: Option<(&str, &str)>, + instructions: &str, + expected: &str, +) { let original = TopLevelToolsState::new( + /*messages*/ Default::default(), vec![namespace("functions", vec![declaration("lookup")])], /*metrics*/ None, ) @@ -337,14 +445,23 @@ fn namespace_description_change_does_not_repeat_unchanged_tools() { .0 .unwrap(); let mut updated = namespace("functions", vec![declaration("lookup")]); - updated["description"] = json!("Updated namespace guidance."); - let current = TopLevelToolsState::new(vec![updated], /*metrics*/ None).unwrap(); + updated["description"] = json!(instructions); + let mut messages = ResolvedIncrementalToolMessages::default(); + if let Some((prefix, cleared_template)) = wording { + messages.namespace_instructions_prefix = prefix.to_string(); + messages.namespace_instructions_cleared = cleared_template.to_string(); + } + let current = TopLevelToolsState::new(messages, vec![updated], /*metrics*/ None).unwrap(); let (snapshot, updates) = current.render_diff(PreviousSectionState::Known(&snapshot)); assert_eq!( merge_world_state_updates(updates), - vec![ContextualUserFragment::into(DeveloperInstructions::new( - "Updated instructions for the functions namespace:\nUpdated namespace guidance." - ))] + if expected.is_empty() { + Vec::new() + } else { + vec![ContextualUserFragment::into(DeveloperInstructions::new( + expected, + ))] + } ); assert!( current @@ -357,6 +474,7 @@ fn namespace_description_change_does_not_repeat_unchanged_tools() { #[test] fn tool_type_change_keeps_the_callable_name_available() { let original = TopLevelToolsState::new( + /*messages*/ Default::default(), vec![namespace( "functions", vec![declaration("lookup"), declaration("removed")], @@ -371,14 +489,22 @@ fn tool_type_change_keeps_the_callable_name_available() { let custom = json!({"type": "custom", "name": "lookup", "description": "Look up a value.", "format": {"type": "text"}}); let updated = namespace("functions", vec![custom]); - let current = TopLevelToolsState::new(vec![updated.clone()], /*metrics*/ None).unwrap(); + let current = TopLevelToolsState::new( + /*messages*/ Default::default(), + vec![updated.clone()], + /*metrics*/ None, + ) + .unwrap(); let (_, updates) = current.render_diff(PreviousSectionState::Known(&snapshot)); assert_eq!( merge_world_state_updates(updates), vec![ - ContextualUserFragment::into(IncrementalToolsHint), + ContextualUserFragment::into(IncrementalToolsHint( + ResolvedIncrementalToolMessages::default().tool_update_hint + )), item(vec![updated]), ContextualUserFragment::into(RemovedTools { + messages: Default::default(), tools: vec!["functions.removed".to_string()], ..Default::default() }), @@ -389,6 +515,7 @@ fn tool_type_change_keeps_the_callable_name_available() { #[test] fn namespace_removals_emit_one_notice() { let original = TopLevelToolsState::new( + /*messages*/ Default::default(), vec![ namespace( "functions", @@ -410,6 +537,7 @@ fn namespace_removals_emit_one_notice() { .0 .unwrap(); let current = TopLevelToolsState::new( + /*messages*/ Default::default(), vec![namespace("functions", vec![declaration("lookup")])], /*metrics*/ None, ) @@ -418,6 +546,7 @@ fn namespace_removals_emit_one_notice() { assert_eq!( merge_world_state_updates(updates), vec![ContextualUserFragment::into(RemovedTools { + messages: Default::default(), namespaces: vec!["function".to_string(), "other".to_string()], tools: vec![ "functions.also_removed".to_string(), @@ -431,6 +560,7 @@ fn namespace_removals_emit_one_notice() { #[test] fn namespace_replaced_by_builtin_keeps_the_replacement_available() { let original = TopLevelToolsState::new( + /*messages*/ Default::default(), vec![namespace("web_search", vec![declaration("lookup")])], /*metrics*/ None, ) @@ -440,13 +570,19 @@ fn namespace_replaced_by_builtin_keeps_the_replacement_available() { .0 .unwrap(); let replacement = json!({"type": "web_search"}); - let current = TopLevelToolsState::new(vec![replacement.clone()], /*metrics*/ None).unwrap(); + let current = TopLevelToolsState::new( + /*messages*/ Default::default(), + vec![replacement.clone()], + /*metrics*/ None, + ) + .unwrap(); let (_, updates) = current.render_diff(PreviousSectionState::Known(&snapshot)); assert_eq!( merge_world_state_updates(updates), vec![ item(vec![replacement]), ContextualUserFragment::into(RemovedTools { + messages: Default::default(), tools: vec!["web_search.lookup".to_string()], ..Default::default() }), diff --git a/codex-rs/core/src/session/world_state.rs b/codex-rs/core/src/session/world_state.rs index a556d20a4c..37dda837fc 100644 --- a/codex-rs/core/src/session/world_state.rs +++ b/codex-rs/core/src/session/world_state.rs @@ -147,6 +147,7 @@ impl Session { let specs = step_context.tool_router.model_visible_specs(); let definitions = codex_tools::create_tools_json_for_responses_lite(&specs)?; world_state.add_section(TopLevelToolsState::new( + model_messages.incremental_tools(), definitions, Some(Arc::clone(&extension_metrics)), )?); diff --git a/codex-rs/core/tests/suite/scenarios_incremental_tools.rs b/codex-rs/core/tests/suite/scenarios_incremental_tools.rs index 496864a290..16e7f58c02 100644 --- a/codex-rs/core/tests/suite/scenarios_incremental_tools.rs +++ b/codex-rs/core/tests/suite/scenarios_incremental_tools.rs @@ -3,6 +3,7 @@ use anyhow::Result; use codex_features::Feature; use codex_protocol::openai_models::CodeModeToolMessages; +use codex_protocol::openai_models::IncrementalToolMessages; use codex_protocol::openai_models::ToolMessage; use codex_protocol::openai_models::ToolMode; use codex_protocol::protocol::ThreadSettingsOverrides; @@ -108,8 +109,12 @@ async fn incremental_tools_append_changed_catalog_without_rewriting_history( Ok(()) } +#[test_case(false; "bundled")] +#[test_case(true; "catalog_messages")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn tool_removal_appends_a_standalone_notice_without_rewriting_history() -> Result<()> { +async fn tool_removal_appends_a_standalone_notice_without_rewriting_history( + catalog_messages: bool, +) -> Result<()> { skip_if_wine_exec!( Ok(()), "requires a Windows test_stdio_server in the Wine-exec environment" @@ -118,29 +123,53 @@ async fn tool_removal_appends_a_standalone_notice_without_rewriting_history() -> let server = responses::start_mock_server().await; let command = remote_aware_stdio_server_bin()?; let environment_id = remote_aware_environment_id(); - let server_config = |enabled_tools: &[&str]| { + let server_config = |enabled_tools: &[&str], instructions: &str| { serde_json::from_value(json!({ "command": command, "environment_id": environment_id, - "env": {"MCP_TEST_SERVER_INSTRUCTIONS": "Echo service instructions."}, + "env": {"MCP_TEST_SERVER_INSTRUCTIONS": instructions}, "enabled_tools": enabled_tools, "default_tools_approval_mode": "approve", })) }; - let initial_server = server_config(&["echo", "cwd"])?; - let reduced_server = server_config(&["echo"])?; - let restored_server = server_config(&["echo", "cwd"])?; + let initial_server = server_config(&["echo", "cwd"], "Echo service instructions.")?; + let reduced_server = server_config(&["echo"], "Echo service instructions.")?; + let restored_server = server_config(&["echo", "cwd"], "Echo service instructions.")?; let test = test_codex() - .with_model_info_override("gpt-5.5", |model| { + .with_model_info_override("gpt-5.5", move |model| { model.use_responses_lite = true; model.supports_search_tool = false; model.tool_mode = Some(ToolMode::Direct); + if catalog_messages { + model + .model_messages + .get_or_insert_default() + .tools + .get_or_insert_default() + .incremental_tools = Some(IncrementalToolMessages { + tool_update_hint: Some( + "Catalog update: retain earlier tools unless explicitly removed." + .to_string(), + ), + removed_tools_header: Some("Unavailable tools:".to_string()), + removed_namespaces_header: Some("Unavailable namespaces:".to_string()), + namespace_instructions_prefix: Some("Catalog {name}: ".to_string()), + namespace_instructions_cleared: Some("Cleared {name}.".to_string()), + }); + } }) .with_config(move |config| { config .features .enable(Feature::IncrementalTools) .expect("enable incremental tools"); + if catalog_messages { + config + .features + .enable(Feature::NonPrefixedMcpToolNames) + .expect("enable unprefixed MCP namespaces"); + config.non_prefixed_mcp_tool_servers = Some(vec!["functions".to_string()]); + } config .mcp_servers .set(std::collections::HashMap::from([( @@ -152,9 +181,10 @@ async fn tool_removal_appends_a_standalone_notice_without_rewriting_history() -> .build_with_auto_env(&server) .await?; wait_for_mcp_server(&test.codex, "echo_service").await?; + let response_count = if catalog_messages { 11 } else { 7 }; let mock = responses::mount_sse_sequence( &server, - (1..=7) + (1..=response_count) .map(|index| responses::sse(vec![responses::ev_completed(&format!("resp-{index}"))])) .collect(), ) @@ -237,11 +267,62 @@ async fn tool_removal_appends_a_standalone_notice_without_rewriting_history() -> test.submit_text_turn("Continue with the same restored tools.") .await?; + if catalog_messages { + // The functions namespace preserves empty instructions instead of adding fallback text. + // Move the service there, then change and clear instructions with unchanged tool schemas. + for (instructions, prompt) in [ + ( + "Use these echo tools.", + "Continue with the echo tools in the functions namespace.", + ), + ( + "Keep {name} and {instructions} literal.", + "Continue with updated echo instructions.", + ), + ("", "Continue after clearing echo instructions."), + ] { + let current_config = test.codex.config().await; + let mut updated_config = current_config.as_ref().clone(); + updated_config + .mcp_servers + .set(std::collections::HashMap::from([( + "functions".to_string(), + server_config(&["echo", "cwd"], instructions)?, + )]))?; + let _ = test + .codex + .refresh_mcp_config(current_config, updated_config) + .await; + test.codex + .call_mcp_tool( + "functions", + "echo", + Some(json!({"message": "ready after instructions refresh"})), + /*meta*/ None, + ) + .await?; + test.submit_text_turn(prompt).await?; + } + test.submit_text_turn("Continue with the same cleared instructions.") + .await?; + } + let requests = mock.requests(); - insta::assert_snapshot!( - "incremental_tool_removals", - context_snapshot::format_request_history_snapshot( + let (snapshot_name, scenario) = if catalog_messages { + ( + "incremental_tools_catalog_messages", + "Catalog overrides supply the update hint, both removal headers, and namespace instructions notices. MCP refreshes remove and restore tools, then change and clear only the namespace instructions. Runtime instructions remain verbatim, and unchanged turns do not repeat notices.", + ) + } else { + ( + "incremental_tool_removals", "MCP refreshes remove one tool, then its whole namespace, before restoring both tools. Removal notices append alone; restoration appends one developer notice before both namespace declarations. Unchanged turns do not repeat updates.", + ) + }; + insta::assert_snapshot!( + snapshot_name, + context_snapshot::format_request_history_snapshot( + scenario, &requests, &ContextSnapshotOptions::default() .rewrite_known_segments() diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__incremental_tools__incremental_tools_catalog_messages.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__incremental_tools__incremental_tools_catalog_messages.snap new file mode 100644 index 0000000000..655768d1ab --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__incremental_tools__incremental_tools_catalog_messages.snap @@ -0,0 +1,109 @@ +--- +source: core/tests/suite/scenarios_incremental_tools.rs +expression: "context_snapshot::format_request_history_snapshot(scenario, &requests,\n&ContextSnapshotOptions::default().rewrite_known_segments().include_request_settings(),)" +--- +Scenario: Catalog overrides supply the update hint, both removal headers, and namespace instructions notices. MCP refreshes remove and restore tools, then change and clear only the namespace instructions. Runtime instructions remain verbatim, and unchanged turns do not repeat notices. + +## Window 1 +Settings: + include: ["reasoning.encrypted_content"] + model: "gpt-5.5" + parallel_tool_calls: false + prompt_cache_key: "" + reasoning: {"context":"all_turns","effort":"medium"} + store: false + stream: true + text: {"verbosity":"low"} + tool_choice: "auto" +-- request 1 (turn) -- +00:additional_tools/developer (3; hash=7748a53142f449b7): + - namespace/functions; hash=124fee69a69ed6b2 + - function/exec_command + - function/write_stdin + - function/list_mcp_resources + - function/list_mcp_resource_templates + - function/read_mcp_resource + - function/request_user_input + - custom/apply_patch + - function/view_image + - namespace/multi_agent_v1: Tools for spawning and managing sub-agents.; hash=2327142bda177c50 + - function/close_agent + - function/resume_agent + - function/send_input + - function/spawn_agent + - function/wait_agent + - namespace/mcp__echo_service: Echo service instructions.; hash=a5144e5d70d01799 + - function/cwd + - function/echo +01:message/developer: + +02:message/developer: + +03:message/user: + +04:message/user: + Begin with the echo service. +-- request 2 (turn) -- +05:message/developer: + Unavailable tools: + - mcp__echo_service.cwd +06:message/user: + Continue after disabling the cwd tool. +-- request 3 (turn) -- +07:message/user: + Continue with the remaining echo tool. +-- request 4 (turn) -- +08:message/developer: + Unavailable namespaces: + - mcp__echo_service + + Unavailable tools: + - functions.list_mcp_resource_templates + - functions.list_mcp_resources + - functions.read_mcp_resource +09:message/user: + Continue after removing the echo service. +-- request 5 (turn) -- +10:message/user: + Continue without the echo service. +-- request 6 (turn) -- +11:message/developer: + Catalog update: retain earlier tools unless explicitly removed. +12:additional_tools/developer (2; hash=ba0bceb82aa883e0): + - namespace/functions; hash=88d1efdc7cb932a8 + - function/list_mcp_resources + - function/list_mcp_resource_templates + - function/read_mcp_resource + - namespace/mcp__echo_service: Echo service instructions.; hash=a5144e5d70d01799 + - function/cwd + - function/echo +13:message/user: + Continue with the restored echo service. +-- request 7 (turn) -- +14:message/user: + Continue with the same restored tools. +-- request 8 (turn) -- +15:message/developer: + Catalog update: retain earlier tools unless explicitly removed. +16:additional_tools/developer (1; hash=d436d651bf8b57b0): + - namespace/functions: Use these echo tools.; hash=8b9887e764d8cfd4 + - function/cwd + - function/echo +17:message/developer: + Unavailable namespaces: + - mcp__echo_service +18:message/user: + Continue with the echo tools in the functions namespace. +-- request 9 (turn) -- +19:message/developer: + Catalog functions: Keep {name} and {instructions} literal. +20:message/user: + Continue with updated echo instructions. +-- request 10 (turn) -- +21:message/developer: + Cleared functions. +22:message/user: + Continue after clearing echo instructions. +-- request 11 (turn) -- +23:message/user: + Continue with the same cleared instructions. diff --git a/codex-rs/prompts/src/lib.rs b/codex-rs/prompts/src/lib.rs index 22db073498..c6ba31a387 100644 --- a/codex-rs/prompts/src/lib.rs +++ b/codex-rs/prompts/src/lib.rs @@ -17,6 +17,7 @@ pub use guardian_instructions::render_guardian_rejection; pub use model_instructions::render_model_instructions; pub use model_messages::ResolvedAutoReviewMessages; pub use model_messages::ResolvedCollaborationModeMessages; +pub use model_messages::ResolvedIncrementalToolMessages; pub use model_messages::ResolvedMessage; pub use model_messages::ResolvedModelMessages; pub use model_messages::ResolvedMultiAgentMessages; diff --git a/codex-rs/prompts/src/model_messages.rs b/codex-rs/prompts/src/model_messages.rs index b7d819e776..26d3266e16 100644 --- a/codex-rs/prompts/src/model_messages.rs +++ b/codex-rs/prompts/src/model_messages.rs @@ -15,11 +15,13 @@ use permissions::ResolvedPermissionMessages; mod collaboration; mod guardian; +mod incremental_tools; mod multi_agent; pub(crate) mod permissions; pub use collaboration::ResolvedCollaborationModeMessages; pub use guardian::ResolvedAutoReviewMessages; +pub use incremental_tools::ResolvedIncrementalToolMessages; pub use multi_agent::ResolvedMultiAgentMessages; /// Text together with whether it was supplied by the catalog, even when it equals the default. @@ -215,6 +217,15 @@ impl<'a> ResolvedModelMessages<'a> { .as_ref() } + /// Resolves bounded incremental catalog wording for an owned world-state section. + pub fn incremental_tools(&self) -> ResolvedIncrementalToolMessages { + ResolvedIncrementalToolMessages::new( + self.catalog_messages + .and_then(|messages| messages.tools.as_ref()) + .and_then(|tools| tools.incremental_tools.as_ref()), + ) + } + /// Selects Code Mode messages; bundled text and runtime composition belong to the tool owner. pub fn code_mode(&self) -> Option<&'a CodeModeToolMessages> { self.catalog_messages?.tools.as_ref()?.code_mode.as_ref() diff --git a/codex-rs/prompts/src/model_messages/incremental_tools.rs b/codex-rs/prompts/src/model_messages/incremental_tools.rs new file mode 100644 index 0000000000..5a05d8878e --- /dev/null +++ b/codex-rs/prompts/src/model_messages/incremental_tools.rs @@ -0,0 +1,51 @@ +//! Resolves incremental catalog wording without changing catalog identity or update timing. +//! Text is owned because world-state sections outlive the borrowed model metadata. + +use codex_protocol::openai_models::IncrementalToolMessages; + +/// Resolved static wording; consumers retain ownership of runtime names and instructions. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ResolvedIncrementalToolMessages { + pub tool_update_hint: String, + pub removed_tools_header: String, + pub removed_namespaces_header: String, + pub namespace_instructions_prefix: String, + pub namespace_instructions_cleared: String, +} + +impl Default for ResolvedIncrementalToolMessages { + fn default() -> Self { + Self::new(/*messages*/ None) + } +} + +impl ResolvedIncrementalToolMessages { + pub(crate) fn new(messages: Option<&IncrementalToolMessages>) -> Self { + let resolve = |text: Option<&String>, bundled: &str| { + text.filter(|text| text.len() <= 512) + .map_or_else(|| bundled.to_owned(), Clone::clone) + }; + Self { + tool_update_hint: resolve( + messages.and_then(|m| m.tool_update_hint.as_ref()), + "This is an incremental tools update. Previously declared tools remain available for direct calls unless explicitly marked unavailable. If a tool is redefined here, its latest definition replaces the earlier one.", + ), + removed_tools_header: resolve( + messages.and_then(|m| m.removed_tools_header.as_ref()), + "The following tools are no longer available. Do not call them:", + ), + removed_namespaces_header: resolve( + messages.and_then(|m| m.removed_namespaces_header.as_ref()), + "The following namespaces are no longer available. Do not call tools in them unless those tools are declared in a later update:", + ), + namespace_instructions_prefix: resolve( + messages.and_then(|m| m.namespace_instructions_prefix.as_ref()), + "Updated instructions for the {name} namespace:\n", + ), + namespace_instructions_cleared: resolve( + messages.and_then(|m| m.namespace_instructions_cleared.as_ref()), + "The {name} namespace no longer has additional instructions.", + ), + } + } +} diff --git a/codex-rs/protocol/src/openai_models.rs b/codex-rs/protocol/src/openai_models.rs index 1a203e1972..45e3d5c096 100644 --- a/codex-rs/protocol/src/openai_models.rs +++ b/codex-rs/protocol/src/openai_models.rs @@ -33,6 +33,8 @@ use crate::config_types::Verbosity; use crate::protocol::MultiAgentVersion; mod access_programs; +mod incremental_tools; +pub use incremental_tools::IncrementalToolMessages; #[path = "openai_models/guardian.rs"] mod guardian; pub use guardian::GuardianModelPolicy; @@ -583,6 +585,9 @@ pub struct ConfirmationPolicies { /// Model-owned tool messages and description prefixes. #[derive(Debug, Default, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)] pub struct ToolMessages { + /// Optional wording for incremental catalog notices. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub incremental_tools: Option, /// Prefixes for individual functions in the `functions` namespace, keyed by unqualified name. /// Applies to direct declarations, Code Mode, and tool search, including custom tools. /// Values are trimmed; empty values and unavailable functions add nothing. The trimmed diff --git a/codex-rs/protocol/src/openai_models/incremental_tools.rs b/codex-rs/protocol/src/openai_models/incremental_tools.rs new file mode 100644 index 0000000000..f051318136 --- /dev/null +++ b/codex-rs/protocol/src/openai_models/incremental_tools.rs @@ -0,0 +1,24 @@ +//! Optional model-catalog overrides for harness-owned incremental tool notices. + +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; +use ts_rs::TS; + +/// Incremental tool notice overrides; missing/null or >512-byte values use bundled defaults. +/// Empty strings omit static text but retain runtime names and instructions. +#[derive(Debug, Default, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)] +pub struct IncrementalToolMessages { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub tool_update_hint: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub removed_tools_header: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub removed_namespaces_header: Option, + /// Prefix supporting `{name}`; the namespace instructions are appended verbatim. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub namespace_instructions_prefix: Option, + /// Message supporting `{name}` when a namespace's instructions are cleared. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub namespace_instructions_cleared: Option, +}