diff --git a/codex-rs/core/src/context/world_state/snapshots/codex_core__context__world_state__top_level_tools__tests__snapshots_namespace_transitions.snap b/codex-rs/core/src/context/world_state/snapshots/codex_core__context__world_state__top_level_tools__tests__snapshots_namespace_transitions.snap index 0684c5eb53..8c31b171e2 100644 --- a/codex-rs/core/src/context/world_state/snapshots/codex_core__context__world_state__top_level_tools__tests__snapshots_namespace_transitions.snap +++ b/codex-rs/core/src/context/world_state/snapshots/codex_core__context__world_state__top_level_tools__tests__snapshots_namespace_transitions.snap @@ -1,6 +1,6 @@ --- source: core/src/context/world_state/top_level_tools_tests.rs -expression: "render_section_cases(&[(Absent, Known(&original)),\n(Known(&original), Known(&original)), (Known(&original), Known(&updated)),\n(Known(&updated), Known(&only_other)),\n(Known(&only_other), Known(&original)),])" +expression: "render_section_cases(&[(Absent, Known(&original)),\n(Known(&original), Known(&original)), (Known(&original), Known(&updated)),\n(Known(&updated), Known(&only_other)), (Known(&only_other), Known(&original)),\n(Known(&updated), Known(&only_functions)), (Known(&updated), Known(&empty)),\n(Known(&empty), Known(&empty)), (Known(&empty), Known(&restored)),])" --- Absent -> {"functions":"d0971b2901a1d91cd0773dd3014aacad18939724","functions.changed":"201818ec534c933aaf3e31e180ad44845f90de52","functions.removed":"ec12b15fd139075230b516c8cd7cbd0d656b9a38","functions.unchanged":"4d234d07035e05b331e0ca66127362f678bed9ce","other":"d3ba1afd5b23158d1baec4aba38ba2b8d376141b","other.lookup":"957791c1122ca973a75db93abc14a5ae32514005"} (item - prefix) { @@ -104,11 +104,8 @@ The following tools are no longer available. Do not call them: - functions.removed {"functions":"d0971b2901a1d91cd0773dd3014aacad18939724","functions.added":"5fc75ad4d08cd0b73ef983d58fed9272072e5ce7","functions.changed":"ebe9005f973a60ba4ee93b024d7021d21dc27826","functions.unchanged":"4d234d07035e05b331e0ca66127362f678bed9ce","other":"d3ba1afd5b23158d1baec4aba38ba2b8d376141b","other.lookup":"957791c1122ca973a75db93abc14a5ae32514005"} -> {"other":"d3ba1afd5b23158d1baec4aba38ba2b8d376141b","other.lookup":"957791c1122ca973a75db93abc14a5ae32514005"} (role - developer) -The following tools are no longer available. Do not call them: +The following namespaces are no longer available. Do not call tools in them unless those tools are declared in a later update: - functions -- functions.added -- functions.changed -- functions.unchanged {"other":"d3ba1afd5b23158d1baec4aba38ba2b8d376141b","other.lookup":"957791c1122ca973a75db93abc14a5ae32514005"} -> {"functions":"d0971b2901a1d91cd0773dd3014aacad18939724","functions.changed":"201818ec534c933aaf3e31e180ad44845f90de52","functions.removed":"ec12b15fd139075230b516c8cd7cbd0d656b9a38","functions.unchanged":"4d234d07035e05b331e0ca66127362f678bed9ce","other":"d3ba1afd5b23158d1baec4aba38ba2b8d376141b","other.lookup":"957791c1122ca973a75db93abc14a5ae32514005"} (item - prefix) { @@ -151,3 +148,52 @@ The following tools are no longer available. Do not call them: ], "type": "additional_tools" } + +{"functions":"d0971b2901a1d91cd0773dd3014aacad18939724","functions.added":"5fc75ad4d08cd0b73ef983d58fed9272072e5ce7","functions.changed":"ebe9005f973a60ba4ee93b024d7021d21dc27826","functions.unchanged":"4d234d07035e05b331e0ca66127362f678bed9ce","other":"d3ba1afd5b23158d1baec4aba38ba2b8d376141b","other.lookup":"957791c1122ca973a75db93abc14a5ae32514005"} -> {"functions":"d0971b2901a1d91cd0773dd3014aacad18939724","functions.unchanged":"4d234d07035e05b331e0ca66127362f678bed9ce"} (role - developer) +The following namespaces are no longer available. Do not call tools in them unless those tools are declared in a later update: +- other + +The following tools are no longer available. Do not call them: +- functions.added +- functions.changed + +{"functions":"d0971b2901a1d91cd0773dd3014aacad18939724","functions.added":"5fc75ad4d08cd0b73ef983d58fed9272072e5ce7","functions.changed":"ebe9005f973a60ba4ee93b024d7021d21dc27826","functions.unchanged":"4d234d07035e05b331e0ca66127362f678bed9ce","other":"d3ba1afd5b23158d1baec4aba38ba2b8d376141b","other.lookup":"957791c1122ca973a75db93abc14a5ae32514005"} -> {} (role - developer) +The following namespaces are no longer available. Do not call tools in them unless those tools are declared in a later update: +- functions +- other + +{} -> {} +None + +{} -> {"functions":"d0971b2901a1d91cd0773dd3014aacad18939724","functions.restored":"56fbc7d43a57aab411e5352bfbb88a6df939fd4e","functions.unchanged":"4d234d07035e05b331e0ca66127362f678bed9ce"} (item - prefix) +{ + "role": "developer", + "tools": [ + { + "description": "Tools.", + "name": "functions", + "tools": [ + { + "description": "Look up a value.", + "name": "unchanged", + "parameters": { + "properties": {}, + "type": "object" + }, + "type": "function" + }, + { + "description": "Look up a value.", + "name": "restored", + "parameters": { + "properties": {}, + "type": "object" + }, + "type": "function" + } + ], + "type": "namespace" + } + ], + "type": "additional_tools" +} 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 873b550040..5fc9b5b6f3 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 @@ -1,6 +1,7 @@ //! Diffs the one-level Responses Lite catalog: namespaces of callable tools and built-ins. //! Missing state starts a fresh catalog; this section does not migrate legacy history. //! Incremental hints decorate emitted declarations only, leaving catalog hashes unchanged. +//! Whole namespace removals subsume their members in the removal notice. use super::PreviousSectionState; use super::SectionTransition; @@ -15,8 +16,11 @@ use codex_protocol::models::ContentItemKind; use codex_protocol::models::ResponseItem; use serde_json::Value; use std::collections::BTreeMap; +use std::collections::BTreeSet; const NAMESPACE_UPDATE_HINT: &str = "This is an incremental namespace 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."; +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 { @@ -137,13 +141,34 @@ impl WorldStateSection for TopLevelToolsState { } updates.extend(namespace_updates); if let Some(previous) = previous { - let removed = previous + // 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 + .keys() + .filter_map(|key| key.split_once('.').map(|(namespace, _)| namespace)) + .filter(|namespace| { + previous.contains_key(*namespace) && !self.hashes.contains_key(*namespace) + }) + .collect::>(); + let tools = previous .keys() .filter(|key| !self.hashes.contains_key(*key)) + .filter(|key| { + !namespaces.contains(key.as_str()) + && !key + .split_once('.') + .is_some_and(|(namespace, _)| namespaces.contains(namespace)) + }) .cloned() .collect::>(); - if !removed.is_empty() { - updates.push(WorldStateUpdate::fragment(RemovedTools(removed)).standalone()); + if !namespaces.is_empty() || !tools.is_empty() { + updates.push( + WorldStateUpdate::fragment(RemovedTools { + namespaces: namespaces.into_iter().map(str::to_string).collect(), + tools, + }) + .standalone(), + ); } } // Persist the empty map too: it is a known empty catalog, not missing state. @@ -151,7 +176,11 @@ impl WorldStateSection for TopLevelToolsState { } } -struct RemovedTools(Vec); +#[derive(Default)] +struct RemovedTools { + namespaces: Vec, + tools: Vec, +} impl ContextualUserFragment for RemovedTools { fn role(&self) -> &'static str { @@ -170,9 +199,27 @@ impl ContextualUserFragment for RemovedTools { ("", "") } + /// Lists removed namespaces and individual tools in separate sections of one notice. fn body(&self) -> String { - let names = self.0.join("\n- "); - format!("The following tools are no longer available. Do not call them:\n- {names}") + let sections = [ + (REMOVED_NAMESPACES_HEADER, &self.namespaces), + (REMOVED_TOOLS_HEADER, &self.tools), + ]; + let mut body = String::new(); + for (header, names) in sections { + if names.is_empty() { + continue; + } + if !body.is_empty() { + body.push_str("\n\n"); + } + body.push_str(header); + for name in names { + body.push_str("\n- "); + body.push_str(name); + } + } + body } } 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 1145d69340..dd15408619 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 @@ -78,6 +78,15 @@ fn snapshots_namespace_transitions() { .unwrap(); let only_other = TopLevelToolsState::new(vec![namespace("other", vec![declaration("lookup")])]).unwrap(); + let only_functions = + TopLevelToolsState::new(vec![namespace("functions", vec![declaration("unchanged")])]) + .unwrap(); + let empty = TopLevelToolsState::new(Vec::new()).unwrap(); + let restored = TopLevelToolsState::new(vec![namespace( + "functions", + vec![declaration("unchanged"), declaration("restored")], + )]) + .unwrap(); insta::assert_snapshot!(render_section_cases(&[ (Absent, Known(&original)), @@ -85,6 +94,13 @@ fn snapshots_namespace_transitions() { (Known(&original), Known(&updated)), (Known(&updated), Known(&only_other)), (Known(&only_other), Known(&original)), + // A whole namespace and individual members can disappear in the same update. + (Known(&updated), Known(&only_functions)), + (Known(&updated), Known(&empty)), + // An empty catalog is known state; do not repeat its removal notice. + (Known(&empty), Known(&empty)), + // Restoration declares both an old member and one that has never been available. + (Known(&empty), Known(&restored)), ])); } @@ -125,10 +141,10 @@ fn empty_catalog_is_persisted_and_can_add_tools_again() { assert_eq!(snapshot, Some(BTreeMap::new())); assert_eq!( merge_world_state_updates(updates), - vec![ContextualUserFragment::into(RemovedTools(vec![ - "functions".to_string(), - "functions.lookup".to_string(), - ]))] + vec![ContextualUserFragment::into(RemovedTools { + namespaces: vec!["functions".to_string()], + ..Default::default() + })] ); let (_, updates) = original.render_diff(PreviousSectionState::Known(&snapshot.unwrap())); assert_eq!(merge_world_state_updates(updates), vec![item(definitions)]); @@ -170,7 +186,10 @@ fn namespace_diff_contains_only_changed_and_added_tools() { merge_world_state_updates(updates), vec![ item(vec![expected]), - ContextualUserFragment::into(RemovedTools(vec!["functions.removed".to_string()])), + ContextualUserFragment::into(RemovedTools { + tools: vec!["functions.removed".to_string()], + ..Default::default() + }), ] ); assert!( @@ -228,21 +247,29 @@ fn tool_type_change_keeps_the_callable_name_available() { merge_world_state_updates(updates), vec![ item(vec![updated]), - ContextualUserFragment::into(RemovedTools(vec!["functions.removed".to_string()])), + ContextualUserFragment::into(RemovedTools { + tools: vec!["functions.removed".to_string()], + ..Default::default() + }), ] ); } #[test] fn namespace_removals_emit_one_notice() { - let original = TopLevelToolsState::new(vec![namespace( - "functions", - vec![ - declaration("lookup"), - declaration("removed"), - declaration("also_removed"), - ], - )]) + let original = TopLevelToolsState::new(vec![ + namespace( + "functions", + vec![ + declaration("lookup"), + declaration("removed"), + declaration("also_removed"), + ], + ), + namespace("function", vec![declaration("lookup")]), + namespace("other", vec![declaration("lookup")]), + json!({"type": "web_search"}), + ]) .unwrap(); let snapshot = original .render_diff(PreviousSectionState::Absent) @@ -253,9 +280,37 @@ fn namespace_removals_emit_one_notice() { let (_, updates) = current.render_diff(PreviousSectionState::Known(&snapshot)); assert_eq!( merge_world_state_updates(updates), - vec![ContextualUserFragment::into(RemovedTools(vec![ - "functions.also_removed".to_string(), - "functions.removed".to_string(), - ]))] + vec![ContextualUserFragment::into(RemovedTools { + namespaces: vec!["function".to_string(), "other".to_string()], + tools: vec![ + "functions.also_removed".to_string(), + "functions.removed".to_string(), + "web_search".to_string(), + ], + })] + ); +} + +#[test] +fn namespace_replaced_by_builtin_keeps_the_replacement_available() { + let original = + TopLevelToolsState::new(vec![namespace("web_search", vec![declaration("lookup")])]) + .unwrap(); + let snapshot = original + .render_diff(PreviousSectionState::Absent) + .0 + .unwrap(); + let replacement = json!({"type": "web_search"}); + let current = TopLevelToolsState::new(vec![replacement.clone()]).unwrap(); + let (_, updates) = current.render_diff(PreviousSectionState::Known(&snapshot)); + assert_eq!( + merge_world_state_updates(updates), + vec![ + item(vec![replacement]), + ContextualUserFragment::into(RemovedTools { + tools: vec!["web_search.lookup".to_string()], + ..Default::default() + }), + ], ); } diff --git a/codex-rs/core/tests/suite/scenarios_incremental_tools.rs b/codex-rs/core/tests/suite/scenarios_incremental_tools.rs index 236edfc5f9..2236473258 100644 --- a/codex-rs/core/tests/suite/scenarios_incremental_tools.rs +++ b/codex-rs/core/tests/suite/scenarios_incremental_tools.rs @@ -243,6 +243,7 @@ async fn tool_removal_appends_a_standalone_notice_without_rewriting_history() -> }; let initial_server = server_config(&["echo", "cwd"])?; let reduced_server = server_config(&["echo"])?; + let restored_server = server_config(&["echo", "cwd"])?; let test = test_codex() .with_model_info_override("gpt-5.5", |model| { model.use_responses_lite = true; @@ -267,7 +268,7 @@ async fn tool_removal_appends_a_standalone_notice_without_rewriting_history() -> wait_for_mcp_server(&test.codex, "echo_service").await?; let mock = responses::mount_sse_sequence( &server, - (1..=3) + (1..=7) .map(|index| responses::sse(vec![responses::ev_completed(&format!("resp-{index}"))])) .collect(), ) @@ -306,13 +307,70 @@ async fn tool_removal_appends_a_standalone_notice_without_rewriting_history() -> test.submit_text_turn("Continue with the remaining echo tool.") .await?; + // Remove the whole server, then reconcile even though its tools can no longer be called. + let current_config = test.codex.config().await; + let mut removed_config = current_config.as_ref().clone(); + removed_config.mcp_servers.set(Default::default())?; + assert_eq!( + test.codex + .refresh_mcp_config(current_config, removed_config) + .await, + ConfigRefreshOutcome::Published, + ); + test.codex + .call_mcp_tool( + "echo_service", + "echo", + Some(json!({"message": "unavailable"})), + /*meta*/ None, + ) + .await + .expect_err("removed MCP server must not accept tool calls"); + test.submit_text_turn("Continue after removing the echo service.") + .await?; + test.submit_text_turn("Continue without the echo service.") + .await?; + + // Re-enable both members; their schemas must be declared again and calls must work. + let current_config = test.codex.config().await; + let mut restored_config = current_config.as_ref().clone(); + restored_config + .mcp_servers + .set(std::collections::HashMap::from([( + "echo_service".to_string(), + restored_server, + )]))?; + assert_eq!( + test.codex + .refresh_mcp_config(current_config, restored_config) + .await, + ConfigRefreshOutcome::Published, + ); + for (tool, arguments) in [ + ("echo", json!({"message": "ready after restoration"})), + ("cwd", json!({})), + ] { + let result = test + .codex + .call_mcp_tool("echo_service", tool, Some(arguments), /*meta*/ None) + .await?; + assert_ne!(result.is_error, Some(true)); + } + test.submit_text_turn("Continue with the restored echo service.") + .await?; + test.submit_text_turn("Continue with the same restored tools.") + .await?; + let requests = mock.requests(); - assert_eq!(requests.len(), 3); + assert_eq!(requests.len(), 7); assert!(requests[1].has_content_kinds(&["tools.removed_definition"])); + assert!(requests[3].has_content_kinds(&["tools.removed_definition"])); let initial_tools = requests[0].inputs_of_type("additional_tools"); for pair in requests.windows(2) { assert!(pair[1].input().starts_with(&pair[0].input())); - assert_eq!(pair[1].inputs_of_type("additional_tools"), initial_tools); + } + for request in &requests[1..5] { + assert_eq!(request.inputs_of_type("additional_tools"), initial_tools); } let removed_input = requests[1].input(); let updates = &removed_input[requests[0].input().len()..]; @@ -329,18 +387,81 @@ async fn tool_removal_appends_a_standalone_notice_without_rewriting_history() -> "text": "The following tools are no longer available. Do not call them:\n- mcp__echo_service.cwd", }]) ); - let unchanged_input = requests[2].input(); - let follow_up = &unchanged_input[removed_input.len()..]; - assert_eq!(follow_up.len(), 1, "unchanged turn appends only user input"); - assert_eq!(follow_up[0]["role"], "user"); + let removed_namespace_input = requests[3].input(); + let namespace_updates = &removed_namespace_input[requests[2].input().len()..]; assert_eq!( - follow_up[0]["content"], - json!([{"type": "input_text", "text": "Continue with the remaining echo tool."}]) + namespace_updates.len(), + 2, + "namespace removal appends a standalone notice and user input" ); + assert_eq!(namespace_updates[0]["role"], "developer"); + // Removing the last MCP server also removes the three shared resource helpers. + assert_eq!( + namespace_updates[0]["content"], + json!([{ + "type": "input_text", + "text": "The following namespaces are no longer available. Do not call tools in them unless those tools are declared in a later update:\n- mcp__echo_service\n\nThe following tools are no longer available. Do not call them:\n- functions.list_mcp_resource_templates\n- functions.list_mcp_resources\n- functions.read_mcp_resource", + }]) + ); + let restored_tools = requests[5].inputs_of_type("additional_tools"); + assert_eq!(restored_tools.len(), initial_tools.len() + 1); + assert_eq!(&restored_tools[..initial_tools.len()], &initial_tools); + let original_namespace = initial_tools + .iter() + .flat_map(|item| item["tools"].as_array().expect("initial tools")) + .find(|tool| tool["name"] == "mcp__echo_service") + .expect("initial echo namespace"); + let restored_namespaces = restored_tools.last().expect("restored tools")["tools"] + .as_array() + .expect("restored namespaces"); + assert_eq!(restored_namespaces.len(), 2); + assert_eq!( + restored_namespaces + .iter() + .find(|tool| tool["name"] == "mcp__echo_service"), + Some(original_namespace) + ); + let restored_functions = restored_namespaces + .iter() + .find(|tool| tool["name"] == "functions") + .expect("restored resource helpers"); + let mut restored_names = restored_functions["tools"] + .as_array() + .expect("restored functions") + .iter() + .map(|tool| tool["name"].as_str().expect("restored tool name")) + .collect::>(); + restored_names.sort_unstable(); + assert_eq!( + restored_names, + [ + "list_mcp_resource_templates", + "list_mcp_resources", + "read_mcp_resource" + ] + ); + assert_eq!( + requests[6].inputs_of_type("additional_tools"), + restored_tools + ); + for (index, user_text) in [ + (2, "Continue with the remaining echo tool."), + (4, "Continue without the echo service."), + (6, "Continue with the same restored tools."), + ] { + let unchanged_input = requests[index].input(); + let follow_up = &unchanged_input[requests[index - 1].input().len()..]; + assert_eq!(follow_up.len(), 1, "unchanged turn appends only user input"); + assert_eq!(follow_up[0]["role"], "user"); + assert_eq!( + follow_up[0]["content"], + json!([{"type": "input_text", "text": user_text}]) + ); + } insta::assert_snapshot!( "incremental_tool_removals", context_snapshot::format_request_history_snapshot( - "An MCP configuration refresh removes one tool while preserving its namespace and other member. A standalone removal notice preserves earlier declarations and is not repeated on the next turn.", + "MCP refreshes remove one tool, then its whole namespace, before restoring both tools. Standalone removal notices and the restored namespace append to history; unchanged turns do not repeat updates.", &requests, &ContextSnapshotOptions::default() .rewrite_known_segments() diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__incremental_tools__incremental_tool_removals.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__incremental_tools__incremental_tool_removals.snap index b885e40dd7..a57a69ff96 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__incremental_tools__incremental_tool_removals.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__incremental_tools__incremental_tool_removals.snap @@ -1,8 +1,8 @@ --- source: core/tests/suite/scenarios_incremental_tools.rs -expression: "context_snapshot::format_request_history_snapshot(\"An MCP configuration refresh removes one tool while preserving its namespace and other member. A standalone removal notice preserves earlier declarations and is not repeated on the next turn.\",\n&requests,\n&ContextSnapshotOptions::default().rewrite_known_segments().include_request_settings(),)" +expression: "context_snapshot::format_request_history_snapshot(\"MCP refreshes remove one tool, then its whole namespace, before restoring both tools. Standalone removal notices and the restored namespace append to history; unchanged turns do not repeat updates.\",\n&requests,\n&ContextSnapshotOptions::default().rewrite_known_segments().include_request_settings(),)" --- -Scenario: An MCP configuration refresh removes one tool while preserving its namespace and other member. A standalone removal notice preserves earlier declarations and is not repeated on the next turn. +Scenario: MCP refreshes remove one tool, then its whole namespace, before restoring both tools. Standalone removal notices and the restored namespace append to history; unchanged turns do not repeat updates. ## Window 1 Settings: @@ -52,3 +52,31 @@ Settings: -- request 3 (turn) -- 07:message/user: Continue with the remaining echo tool. +-- request 4 (turn) -- +08:message/developer: + The following namespaces are no longer available. Do not call tools in them unless those tools are declared in a later update: + - mcp__echo_service + + The following tools are no longer available. Do not call them: + - 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:additional_tools/developer (2; hash=817ef381d2e3ba83): + - namespace/functions: This is an incremental namespace update. Previously declared tools remain available for direct ca...; hash=b54767a7496e2119 + - 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 +12:message/user: + Continue with the restored echo service. +-- request 7 (turn) -- +13:message/user: + Continue with the same restored tools.