diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index ee0fd5a5d018..e7c4525ea7ab 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -632,7 +632,7 @@ pub struct ExperimentalRequestUserInput { #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct UpdatePlanToolConfig { - #[serde(default = "default_true")] + #[serde(default)] pub enabled: bool, } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index bf704872e236..54415cd19dd7 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -5546,7 +5546,7 @@ "additionalProperties": false, "properties": { "enabled": { - "default": true, + "default": false, "type": "boolean" } }, diff --git a/codex-rs/core/src/agent/control/spawn.rs b/codex-rs/core/src/agent/control/spawn.rs index ab82ac355009..2f31539798c6 100644 --- a/codex-rs/core/src/agent/control/spawn.rs +++ b/codex-rs/core/src/agent/control/spawn.rs @@ -882,8 +882,11 @@ impl AgentControl { let multi_agent_v2_usage_hint_texts_to_filter: Vec = if multi_agent_version == MultiAgentVersion::V2 { let parent_config = parent_thread.session.get_config().await; - let parent_usage_hints = - resolve_usage_hints(&parent_config.multi_agent_v2, /*catalog*/ None); + let parent_usage_hints = resolve_usage_hints( + &parent_config.multi_agent_v2, + /*catalog*/ None, + !parent_config.update_plan_enabled, + ); [parent_usage_hints.root, parent_usage_hints.subagent] .into_iter() .flatten() @@ -1041,7 +1044,12 @@ impl AgentControl { .as_ref() .map(|hints| hints.subagent.clone()) .unwrap_or_else(|| { - resolve_usage_hints(&config.multi_agent_v2, /*catalog*/ None).subagent + resolve_usage_hints( + &config.multi_agent_v2, + /*catalog*/ None, + !config.update_plan_enabled, + ) + .subagent }) { let subagent_usage_hint_message = ContextualUserFragment::into(subagent_usage_hint); diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index 999458501176..1651ab1b9eee 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -281,7 +281,7 @@ async fn run_compact_task_inner_impl( let turn_input_len = turn_input.len(); let prompt = Prompt { input: turn_input, - base_instructions: sess.get_base_instructions().await, + base_instructions: sess.get_prompt_base_instructions().await, ..Default::default() }; let attempt_result = drain_to_completed( diff --git a/codex-rs/core/src/compact_remote_request.rs b/codex-rs/core/src/compact_remote_request.rs index 149be4b3d968..b6b418c7d0fd 100644 --- a/codex-rs/core/src/compact_remote_request.rs +++ b/codex-rs/core/src/compact_remote_request.rs @@ -30,7 +30,7 @@ pub(super) async fn run_remote_compact_attempt( ) -> CodexResult { let turn_context = &step_context.turn; let mut history = sess.clone_history().await; - let base_instructions = sess.get_base_instructions().await; + let base_instructions = sess.get_prompt_base_instructions().await; let (rewritten_outputs, estimated_deleted_tokens) = trim_function_call_history_to_fit_context_window( &mut history, diff --git a/codex-rs/core/src/compact_remote_v2_attempt.rs b/codex-rs/core/src/compact_remote_v2_attempt.rs index 81861af1145f..1c58c7e4c44d 100644 --- a/codex-rs/core/src/compact_remote_v2_attempt.rs +++ b/codex-rs/core/src/compact_remote_v2_attempt.rs @@ -39,7 +39,7 @@ pub(super) async fn run_remote_compact_v2_attempt( ) -> CodexResult { let turn_context = &step_context.turn; let mut history = sess.clone_history().await; - let base_instructions = sess.get_base_instructions().await; + let base_instructions = sess.get_prompt_base_instructions().await; let (rewritten_outputs, estimated_deleted_tokens) = trim_function_call_history_to_fit_context_window( &mut history, diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index da5c622ffe79..8f5a3fc2e2cd 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -566,8 +566,8 @@ async fn load_config_resolves_non_prefixed_mcp_tool_servers() -> std::io::Result async fn load_config_resolves_update_plan_enabled() -> std::io::Result<()> { let codex_home = tempdir()?; for (config_toml, expected_enabled) in [ - ("", true), - ("[tools.update_plan]", true), + ("", false), + ("[tools.update_plan]", false), ("[tools.update_plan]\nenabled = false", false), ("[tools.update_plan]\nenabled = true", true), ] { @@ -11604,7 +11604,9 @@ max_concurrent_threads_per_session = 17 for wait_agent_enabled in [true, false] { let mut config = config.clone(); config.wait_agent_enabled = wait_agent_enabled; - let usage_hints = resolve_usage_hints(&config, /*catalog*/ None); + let usage_hints = resolve_usage_hints( + &config, /*catalog*/ None, /*omit_update_plan_instructions*/ false, + ); for hint in [usage_hints.root, usage_hints.subagent] { let hint = hint.expect("default usage hints should be present").body(); assert!(hint.contains(concurrency_guidance)); @@ -11621,6 +11623,7 @@ max_concurrent_threads_per_session = 17 root: Some(String::new()), subagent: Some(String::new()), }), + /*omit_update_plan_instructions*/ false, ); assert!(usage_hints.root.is_none() && usage_hints.subagent.is_none()); } @@ -11628,12 +11631,12 @@ max_concurrent_threads_per_session = 17 #[test] fn multi_agent_v2_model_override_exposure_preserves_configured_usage_hints() { let config_toml = toml::from_str( - r#"[features.multi_agent_v2] + r###"[features.multi_agent_v2] enabled = true -root_agent_usage_hint_text = "Root guidance." -subagent_usage_hint_text = "Subagent guidance." +root_agent_usage_hint_text = "## Plan tool\nRoot guidance." +subagent_usage_hint_text = "## `update_plan`\nSubagent guidance." expose_spawn_agent_model_overrides = true -"#, +"###, ) .expect("multi-agent v2 config should parse"); @@ -11641,11 +11644,11 @@ expose_spawn_agent_model_overrides = true assert!(config.expose_spawn_agent_model_overrides); assert_eq!( config.root_agent_usage_hint_text.as_deref(), - Some("Root guidance.") + Some("## Plan tool\nRoot guidance.") ); assert_eq!( config.subagent_usage_hint_text.as_deref(), - Some("Subagent guidance.") + Some("## `update_plan`\nSubagent guidance.") ); let usage_hints = resolve_usage_hints( &config, @@ -11653,6 +11656,7 @@ expose_spawn_agent_model_overrides = true root: Some("Catalog root base.".to_string()), subagent: Some("Catalog subagent base.".to_string()), }), + /*omit_update_plan_instructions*/ true, ); assert_eq!( ( @@ -11660,8 +11664,8 @@ expose_spawn_agent_model_overrides = true usage_hints.subagent.map(|hint| hint.body()), ), ( - Some("Root guidance.".to_string()), - Some("Subagent guidance.".to_string()), + Some("## Plan tool\nRoot guidance.".to_string()), + Some("## `update_plan`\nSubagent guidance.".to_string()), ) ); } @@ -11673,9 +11677,13 @@ fn multi_agent_v2_exposes_model_overrides_by_default() { let mut config = resolve_multi_agent_v2_config(&config_toml); assert!(config.expose_spawn_agent_model_overrides); - let usage_hints = resolve_usage_hints(&config, /*catalog*/ None); + let usage_hints = resolve_usage_hints( + &config, /*catalog*/ None, /*omit_update_plan_instructions*/ false, + ); config.expose_spawn_agent_model_overrides = false; - let usage_hints_without_model_overrides = resolve_usage_hints(&config, /*catalog*/ None); + let usage_hints_without_model_overrides = resolve_usage_hints( + &config, /*catalog*/ None, /*omit_update_plan_instructions*/ false, + ); for (hint, hint_without_model_overrides) in [ (usage_hints.root, usage_hints_without_model_overrides.root), @@ -11790,6 +11798,7 @@ subagent_usage_hint_text = "" root: Some("catalog root".to_string()), subagent: Some("catalog subagent".to_string()), }), + /*omit_update_plan_instructions*/ false, ); assert_eq!( ( diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 9fb4f141edd2..44a0411504c4 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2641,7 +2641,7 @@ fn resolve_update_plan_enabled(config_toml: &ConfigToml) -> bool { .tools .as_ref() .and_then(|tools| tools.update_plan.as_ref()) - .is_none_or(|config| config.enabled) + .is_some_and(|config| config.enabled) } fn resolve_orchestrator_feature_enabled( diff --git a/codex-rs/core/src/context/mod.rs b/codex-rs/core/src/context/mod.rs index 281913a82fd8..58ad8140c863 100644 --- a/codex-rs/core/src/context/mod.rs +++ b/codex-rs/core/src/context/mod.rs @@ -42,6 +42,7 @@ mod subagent_notification; mod token_budget_context; mod turn_aborted; mod unsupported_media; +mod update_plan_instructions; mod user_instructions; mod user_shell_command; pub(crate) mod world_state; @@ -108,6 +109,7 @@ pub(crate) use token_budget_context::TokenBudgetRemainingContext; pub(crate) use token_budget_context::TokenBudgetReminder; pub(crate) use turn_aborted::TurnAborted; pub(crate) use unsupported_media::UnsupportedMedia; +pub use update_plan_instructions::without_update_plan_instructions; pub(crate) use user_instructions::UserInstructions; pub(crate) use user_shell_command::UserShellCommand; pub(crate) use world_state::ManagedDeveloperInstructions; diff --git a/codex-rs/core/src/context/update_plan_instructions.rs b/codex-rs/core/src/context/update_plan_instructions.rs new file mode 100644 index 000000000000..37040176206a --- /dev/null +++ b/codex-rs/core/src/context/update_plan_instructions.rs @@ -0,0 +1,63 @@ +//! Omits checklist-tool guidance from Codex-owned prompts, before adding caller text. + +/// Call only for Codex-owned prompt text; custom instructions must remain unchanged. +pub fn without_update_plan_instructions(instructions: &str) -> String { + let lines = instructions.split_inclusive('\n').collect::>(); + let mut rendered = String::with_capacity(instructions.len()); + let mut index = 0; + while index < lines.len() { + let line = lines[index].trim_end(); + if matches!( + line, + "## Planning" + | "## `update_plan`" + | "## Plan tool" + | "## Plan Mode vs update_plan tool" + ) { + let end = (index + 1..lines.len()) + .find(|&next| lines[next].starts_with("# ") || lines[next].starts_with("## ")) + .unwrap_or(lines.len()); + let is_checklist_section = line != "## Planning" + || lines[index..end].iter().any(|line| { + line.starts_with("You have access to an `update_plan` tool") + || line.starts_with("When `update_plan` is available, follow this section") + }); + if is_checklist_section { + index = end; + continue; + } + } + + if line == "Progress visibility:" + && lines + .get(index + 1) + .is_some_and(|line| line.starts_with("If update_plan is available")) + { + index += 2; + if lines.get(index).is_some_and(|line| line.trim().is_empty()) { + index += 1; + } + continue; + } + + if line.starts_with("- Use the plan tool ") + || line.starts_with("- If you create a checklist or task list,") + { + index += 1; + while index < lines.len() + && (lines[index].starts_with(' ') || lines[index].starts_with('\t')) + { + index += 1; + } + continue; + } + + rendered.push_str(lines[index]); + index += 1; + } + rendered +} + +#[cfg(test)] +#[path = "update_plan_instructions_tests.rs"] +mod tests; diff --git a/codex-rs/core/src/context/update_plan_instructions_tests.rs b/codex-rs/core/src/context/update_plan_instructions_tests.rs new file mode 100644 index 000000000000..0e8593b81cd1 --- /dev/null +++ b/codex-rs/core/src/context/update_plan_instructions_tests.rs @@ -0,0 +1,31 @@ +//! Checks that only the known checklist sections are omitted. + +use super::without_update_plan_instructions; +use pretty_assertions::assert_eq; + +#[test] +fn disabled_omits_checklist_sections_and_preserves_other_planning() { + let instructions = "Before.\n\n## Planning\nYou have access to an `update_plan` tool which tracks steps.\n\n### Examples\nKeep steps current.\n\n## Work\nImplement.\n\n## `update_plan`\nUpdate the checklist.\n\n# Next\n## Planning\nDiscuss architecture and inspect update_plan before editing.\n"; + assert_eq!( + without_update_plan_instructions(instructions), + "Before.\n\n## Work\nImplement.\n\n# Next\n## Planning\nDiscuss architecture and inspect update_plan before editing.\n", + ); +} + +#[test] +fn disabled_omits_legacy_tool_and_plan_mode_cross_reference_sections() { + let instructions = "# Plan Mode\nKeep planning.\n\n## Plan Mode vs update_plan tool\nThe tools are separate.\n\n## Execution\nDo not edit.\n\n## Plan tool\nWhen using the planning tool:\n- Update steps.\n"; + assert_eq!( + without_update_plan_instructions(instructions), + "# Plan Mode\nKeep planning.\n\n## Execution\nDo not edit.\n\n", + ); +} + +#[test] +fn disabled_omits_only_checklist_list_items_and_goal_guidance() { + let instructions = "Keep working.\n- Use the plan tool to explain the work\n - Skip simple tasks.\n - Keep steps current.\n- Explain discoveries.\n- If you create a checklist or task list, update its statuses.\n\nProgress visibility:\nIf update_plan is available, use it for complex work.\n\nCompletion:\nVerify the result.\n"; + assert_eq!( + without_update_plan_instructions(instructions), + "Keep working.\n- Explain discoveries.\n\nCompletion:\nVerify the result.\n", + ); +} diff --git a/codex-rs/core/src/context/world_state/collaboration_mode.rs b/codex-rs/core/src/context/world_state/collaboration_mode.rs index 9963ad7aa9f4..b4050a217e0d 100644 --- a/codex-rs/core/src/context/world_state/collaboration_mode.rs +++ b/codex-rs/core/src/context/world_state/collaboration_mode.rs @@ -2,6 +2,8 @@ use super::PreviousSectionState; use super::WorldStateHash; use super::WorldStateSection; use crate::context::ContextualUserFragment; +use crate::context::without_update_plan_instructions; +use codex_models_manager::collaboration_mode_presets::builtin_collaboration_mode_presets; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::models::ContentItemKind; @@ -22,6 +24,8 @@ impl CollaborationModeState { pub(crate) fn from_collaboration_mode( collaboration_mode: &CollaborationMode, catalog_messages: Option<&CollaborationModeMessages>, + update_plan_enabled: bool, + custom_model_catalog: bool, ) -> Self { let catalog_instructions = catalog_messages.and_then(|messages| match collaboration_mode.mode { @@ -36,6 +40,28 @@ impl CollaborationModeState { .clone() .filter(|instructions| !instructions.is_empty()) }); + let instructions = instructions.map(|instructions| { + if update_plan_enabled { + return instructions; + } + // Clients send built-in presets through the override field too. Match the + // entire preset, never a heading that could also occur in custom text. + let builtin = match catalog_instructions { + Some(_) => !custom_model_catalog, + None => builtin_collaboration_mode_presets().iter().any(|preset| { + preset + .developer_instructions + .as_ref() + .and_then(Option::as_ref) + == Some(&instructions) + }), + }; + if builtin { + without_update_plan_instructions(&instructions) + } else { + instructions + } + }); // Keep an empty-state snapshot so removing instructions clears retained history only once. let fragment = CollaborationModeInstructions { instructions: instructions.clone().unwrap_or_default(), diff --git a/codex-rs/core/src/context/world_state/collaboration_mode_tests.rs b/codex-rs/core/src/context/world_state/collaboration_mode_tests.rs index ece7d8234b9a..7d39d4c0aa84 100644 --- a/codex-rs/core/src/context/world_state/collaboration_mode_tests.rs +++ b/codex-rs/core/src/context/world_state/collaboration_mode_tests.rs @@ -43,6 +43,8 @@ fn instruction_updates_are_applied_once_in_retained_history() { world_state.add_section(CollaborationModeState::from_collaboration_mode( &collaboration_mode(ModeKind::Default, instructions), /*catalog_messages*/ None, + /*update_plan_enabled*/ true, + /*custom_model_catalog*/ false, )); let expected: ResponseItem = ContextualUserFragment::into(CollaborationModeInstructions { instructions: instructions.unwrap_or_default().to_string(), @@ -84,6 +86,8 @@ fn catalog_collaboration_messages_select_mode_variant() { let state = CollaborationModeState::from_collaboration_mode( &collaboration_mode(mode, Some("legacy instructions")), Some(&messages), + /*update_plan_enabled*/ true, + /*custom_model_catalog*/ false, ); assert_eq!(state.instructions.as_deref(), Some(expected)); @@ -99,6 +103,8 @@ fn empty_catalog_collaboration_message_suppresses_legacy_instructions() { let state = CollaborationModeState::from_collaboration_mode( &collaboration_mode(ModeKind::Plan, Some("legacy plan instructions")), Some(&messages), + /*update_plan_enabled*/ true, + /*custom_model_catalog*/ false, ); assert_eq!( @@ -119,6 +125,8 @@ fn missing_catalog_collaboration_message_uses_legacy_instructions() { let state = CollaborationModeState::from_collaboration_mode( &collaboration_mode(ModeKind::Plan, Some("legacy plan instructions")), Some(&messages), + /*update_plan_enabled*/ true, + /*custom_model_catalog*/ false, ); assert_eq!( @@ -141,6 +149,8 @@ fn legacy_collaboration_mode_snapshots_refresh_catalog_messages_once() { let state = CollaborationModeState::from_collaboration_mode( &collaboration_mode(ModeKind::Default, Some("stale legacy instructions")), Some(&messages), + /*update_plan_enabled*/ true, + /*custom_model_catalog*/ false, ); assert_eq!( @@ -176,5 +186,7 @@ fn collaboration_mode_state(mode: ModeKind, instructions: &str) -> Collaboration CollaborationModeState::from_collaboration_mode( &collaboration_mode(mode, Some(instructions)), /*catalog_messages*/ None, + /*update_plan_enabled*/ true, + /*custom_model_catalog*/ false, ) } diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 8e9337024ece..8f0eb2ad8db4 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1314,6 +1314,26 @@ impl Session { } } + /// Render the request copy without changing instructions persisted or inherited by forks. + pub(crate) async fn get_prompt_base_instructions(&self) -> BaseInstructions { + let config = self.get_config().await; + let instructions = self.get_base_instructions().await; + if !config.update_plan_enabled + && config.model_catalog.is_none() + && matches!( + instructions.provenance, + Some(BaseInstructionsProvenance::Model { .. }) + ) + { + BaseInstructions { + text: crate::context::without_update_plan_instructions(&instructions.text), + ..instructions + } + } else { + instructions + } + } + // Merges connector IDs into the session-level explicit connector selection. #[tracing::instrument( level = "trace", diff --git a/codex-rs/core/src/session/multi_agents.rs b/codex-rs/core/src/session/multi_agents.rs index 8658c5ae92c3..666f32cce208 100644 --- a/codex-rs/core/src/session/multi_agents.rs +++ b/codex-rs/core/src/session/multi_agents.rs @@ -78,7 +78,11 @@ pub(super) fn usage_hint_text( .as_ref() .and_then(|messages| messages.multi_agent.as_ref()) .and_then(|messages| messages.role.as_ref()); - let snapshot = resolve_usage_hints(&turn_context.config.multi_agent_v2, catalog); + let snapshot = resolve_usage_hints( + &turn_context.config.multi_agent_v2, + catalog, + !turn_context.config.update_plan_enabled && turn_context.config.model_catalog.is_none(), + ); match session_source { SessionSource::SubAgent(SubAgentSource::ThreadSpawn { .. }) => snapshot.subagent, SessionSource::Cli @@ -94,6 +98,7 @@ pub(super) fn usage_hint_text( pub(crate) fn resolve_usage_hints( config: &MultiAgentV2Config, catalog: Option<&MultiAgentRoleMessages>, + omit_update_plan_instructions: bool, ) -> ResolvedMultiAgentV2UsageHints { let resolve_role = |configured: Option<&str>, catalog: Option<&str>, bundled: &str| { // Configured roles take precedence; empty configured or catalog roles suppress fallback. @@ -106,6 +111,11 @@ pub(crate) fn resolve_usage_hints( if base.is_empty() { return None; } + let base = if omit_update_plan_instructions { + crate::context::without_update_plan_instructions(base) + } else { + base.to_string() + }; let max_concurrency = config.max_concurrent_threads_per_session; let wait_agent_guidance = if config.wait_agent_enabled { diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 273b48470cc3..8dbfad88d381 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1579,7 +1579,7 @@ impl Session { ) .await?; sess.start_mcp_prewarm_worker(mcp_prewarm_rx, mcp_auth_changes); - sess.schedule_startup_prewarm(session_configuration.base_instructions.clone()) + sess.schedule_startup_prewarm(sess.get_prompt_base_instructions().await.text) .await; let session_start_source = match &initial_history { InitialHistory::Resumed(_) => codex_hooks::SessionStartSource::Resume, diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 124f27bb4346..9ab25ca1519d 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -1370,7 +1370,7 @@ async fn run_sampling_request( cancellation_token: CancellationToken, ) -> CodexResult<(SamplingRequestResult, Vec)> { let turn_context = Arc::clone(&step_context.turn); - let base_instructions = sess.get_base_instructions().await; + let base_instructions = sess.get_prompt_base_instructions().await; let tool_runtime = ToolCallRuntime::new( Arc::clone(&sess), diff --git a/codex-rs/core/src/session/world_state.rs b/codex-rs/core/src/session/world_state.rs index 740665bfa7bf..322dbbd56dbf 100644 --- a/codex-rs/core/src/session/world_state.rs +++ b/codex-rs/core/src/session/world_state.rs @@ -44,9 +44,20 @@ impl Session { let model_instructions = turn_context .model_info() .get_model_instructions(turn_context.personality()); + let model_instructions = if !turn_context.config.update_plan_enabled + && turn_context.config.model_catalog.is_none() + && (turn_context.config.base_instructions.is_none() + || matches!( + turn_context.config.base_instructions_provenance, + Some(BaseInstructionsProvenance::Model { .. }) + )) { + crate::context::without_update_plan_instructions(&model_instructions) + } else { + model_instructions + }; + let base_instructions = self.get_prompt_base_instructions().await.text; let (previous_model, previous_context, base_instructions) = { let state = self.state.lock().await; - let base_instructions = state.session_configuration.base_instructions.clone(); ( state .previous_turn_settings() @@ -189,6 +200,8 @@ impl Session { .model_messages .as_ref() .and_then(|messages| messages.collaboration_modes.as_ref()), + turn_context.config.update_plan_enabled, + turn_context.config.model_catalog.is_some(), )); } if !crate::guardian::is_basic_session_source(&turn_context.session_source) { diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs index 3f54ff7ae8ac..3684156f985e 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs @@ -196,7 +196,11 @@ async fn handle_spawn_agent( .as_ref() .and_then(|messages| messages.multi_agent.as_ref()) .and_then(|messages| messages.role.as_ref()); - Some(resolve_usage_hints(&config.multi_agent_v2, child_catalog)) + Some(resolve_usage_hints( + &config.multi_agent_v2, + child_catalog, + !config.update_plan_enabled && config.model_catalog.is_none(), + )) } else { None }; diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 44466999f914..c77936fc9055 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -855,18 +855,18 @@ async fn request_user_input_tool_respects_experimental_config_gate() { #[tokio::test] async fn update_plan_tool_respects_config_gate() { - let enabled = probe(|_| {}).await; - enabled.assert_visible_contains(&["update_plan"]); - enabled.assert_registered_contains(&["update_plan"]); + let disabled = probe(|_| {}).await; + disabled.assert_visible_lacks(&["update_plan"]); + disabled.assert_registered_lacks(&["update_plan"]); - let disabled = probe(|turn| { + let enabled = probe(|turn| { update_config(turn, |config| { - config.update_plan_enabled = false; + config.update_plan_enabled = true; }); }) .await; - disabled.assert_visible_lacks(&["update_plan"]); - disabled.assert_registered_lacks(&["update_plan"]); + enabled.assert_visible_contains(&["update_plan"]); + enabled.assert_registered_contains(&["update_plan"]); } #[tokio::test] diff --git a/codex-rs/core/tests/suite/agent_websocket.rs b/codex-rs/core/tests/suite/agent_websocket.rs index cad03435cf3f..e600b21ae449 100644 --- a/codex-rs/core/tests/suite/agent_websocket.rs +++ b/codex-rs/core/tests/suite/agent_websocket.rs @@ -165,8 +165,12 @@ async fn websocket_test_codex_shell_chain() -> Result<()> { Ok(()) } +#[test_case::test_case(false; "update_plan disabled")] +#[test_case::test_case(true; "update_plan enabled")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn websocket_first_turn_uses_startup_prewarm_and_create() -> Result<()> { +async fn websocket_first_turn_uses_startup_prewarm_and_create( + update_plan_enabled: bool, +) -> Result<()> { skip_if_no_network!(Ok(())); let server = start_websocket_server(vec![vec![ @@ -179,7 +183,9 @@ async fn websocket_first_turn_uses_startup_prewarm_and_create() -> Result<()> { ]]) .await; - let mut builder = test_codex(); + let mut builder = test_codex() + .with_model("gpt-5.2") + .with_config(move |config| config.update_plan_enabled = update_plan_enabled); let test = builder.build_with_websocket_server(&server).await?; test.submit_turn_with_policy("hello", test.config.legacy_sandbox_policy()) .await?; @@ -192,6 +198,14 @@ async fn websocket_first_turn_uses_startup_prewarm_and_create() -> Result<()> { .expect("missing warmup request") .body_json(); let turn = connection.get(1).expect("missing turn request").body_json(); + assert_eq!(warmup["instructions"], turn["instructions"]); + assert_eq!( + warmup["instructions"] + .as_str() + .expect("warmup base instructions") + .contains("update_plan"), + update_plan_enabled + ); assert_eq!(warmup["type"].as_str(), Some("response.create")); assert_eq!(warmup["generate"].as_bool(), Some(false)); let warmup_metadata: Value = serde_json::from_str( diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index 6d6bd7d4afee..57f98df1d1ef 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -1596,6 +1596,31 @@ text(output.output); Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn code_mode_does_not_expose_update_plan_by_default() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let (_test, second_mock) = run_code_mode_turn( + &server, + "inspect the available tools", + r#" +text(JSON.stringify({ + callable: typeof tools.update_plan === "function", + listed: ALL_TOOLS.some(({ name }) => name === "update_plan"), +})); +"#, + ) + .await?; + + let (output, _) = custom_tool_output_body_and_success(&second_mock.single_request(), "call-1"); + assert_eq!( + serde_json::from_str::(&output)?, + serde_json::json!({ "callable": false, "listed": false }) + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn code_mode_update_plan_nested_tool_result_is_empty_object() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/core/tests/suite/collaboration_instructions.rs b/codex-rs/core/tests/suite/collaboration_instructions.rs index 8d54fcd68925..7c34c61a90a2 100644 --- a/codex-rs/core/tests/suite/collaboration_instructions.rs +++ b/codex-rs/core/tests/suite/collaboration_instructions.rs @@ -170,8 +170,8 @@ async fn catalog_collaboration_messages_track_mode_changes() -> Result<()> { .await; let model_slug = "catalog-collaboration-model"; - let default_text = "catalog default instructions"; - let plan_text = "catalog plan instructions"; + let default_text = "## Plan tool\nPreserve the custom default policy.\n"; + let plan_text = "## `update_plan`\nPreserve the custom Plan Mode policy.\n"; let model = model_with_collaboration_messages(model_slug, Some(default_text), Some(plan_text)); let mut builder = test_codex() .with_model(model_slug) diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index 7d91404f8e13..0a2c462d51cb 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -506,9 +506,17 @@ fn format_labeled_requests_snapshot( ) } +#[test_case::test_case(false, false; "checklist disabled")] +#[test_case::test_case(true, false; "checklist enabled")] +#[test_case::test_case(false, true; "custom instructions with checklist disabled")] +#[test_case::test_case(true, true; "custom instructions with checklist enabled")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn summarize_context_three_requests_and_instructions() { - skip_if_no_network!(); +async fn summarize_context_three_requests_and_instructions( + enable_plan: bool, + custom_instructions: bool, +) -> Result<()> { + skip_if_no_network!(Ok(())); + const CUSTOM_INSTRUCTIONS: &str = "## Plan tool\nNever deploy without explicit approval.\n"; // Set up a mock server that we can inspect after the run. let server = start_mock_server().await; @@ -535,11 +543,16 @@ async fn summarize_context_three_requests_and_instructions() { // Build config pointing to the mock server and spawn Codex. let model_provider = non_openai_model_provider(&server); let mut builder = test_codex().with_config(move |config| { + config.model = Some("gpt-5.2".to_string()); + config.update_plan_enabled = enable_plan; + if custom_instructions { + config.base_instructions = Some(CUSTOM_INSTRUCTIONS.to_string()); + } config.model_provider = model_provider; set_test_compact_prompt(config); config.model_auto_compact_token_limit = Some(200_000); }); - let test = builder.build(&server).await.unwrap(); + let test = builder.build(&server).await?; let codex = test.codex.clone(); let rollout_path = test.session_configured.rollout_path.expect("rollout path"); @@ -555,12 +568,11 @@ async fn summarize_context_three_requests_and_instructions() { text_elements: Vec::new(), }, ])) - .await - .unwrap(); + .await?; wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; // 2) Summarize – second hit should include the summarization prompt. - codex.submit(Op::Compact).await.unwrap(); + codex.submit(Op::Compact).await?; let warning_event = wait_for_event(&codex, |ev| matches!(ev, EventMsg::Warning(_))).await; let EventMsg::Warning(WarningEvent { message }) = warning_event else { panic!("expected warning event after compact"); @@ -574,45 +586,51 @@ async fn summarize_context_three_requests_and_instructions() { text: THIRD_USER_MSG.into(), text_elements: Vec::new(), }])) - .await - .unwrap(); + .await?; wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; // Inspect the three captured requests. let requests = request_log.requests(); assert_eq!(requests.len(), 3, "expected exactly three requests"); - let body1 = requests[0].body_json(); let body2 = requests[1].body_json(); let body3 = requests[2].body_json(); // Manual compact should keep the baseline developer instructions. - let instr1 = body1.get("instructions").and_then(|v| v.as_str()).unwrap(); - let instr2 = body2.get("instructions").and_then(|v| v.as_str()).unwrap(); + let instr1 = requests[0].instructions_text(); + let instr2 = requests[1].instructions_text(); + if custom_instructions { + assert_eq!(instr1, CUSTOM_INSTRUCTIONS); + } else { + assert_eq!(instr1.contains("update_plan"), enable_plan); + } assert_eq!( instr1, instr2, "manual compact should keep the standard developer instructions" ); + assert_eq!(requests[2].instructions_text(), instr1); // The summarization request should include the injected user input marker. let body2_str = body2.to_string(); - let input2 = body2.get("input").and_then(|v| v.as_array()).unwrap(); + let input2 = body2["input"].as_array().expect("compaction input array"); let has_compact_prompt = body_contains_text(&body2_str, SUMMARIZATION_PROMPT); assert!( has_compact_prompt, "compaction request should include the summarize trigger" ); // The last item is the user message created from the injected input. - let last2 = input2.last().unwrap(); - assert_eq!(last2.get("type").unwrap().as_str().unwrap(), "message"); - assert_eq!(last2.get("role").unwrap().as_str().unwrap(), "user"); - let text2 = last2["content"][0]["text"].as_str().unwrap(); + let last2 = input2.last().expect("summarization prompt"); + assert_eq!(last2["type"], "message"); + assert_eq!(last2["role"], "user"); + let text2 = last2["content"][0]["text"] + .as_str() + .expect("summarization prompt text"); assert_eq!( text2, SUMMARIZATION_PROMPT, "expected summarize trigger, got `{text2}`" ); // Third request must contain the refreshed instructions, compacted user history, and new user message. - let input3 = body3.get("input").and_then(|v| v.as_array()).unwrap(); + let input3 = body3["input"].as_array().expect("follow-up input array"); assert!( input3.len() >= 3, @@ -670,7 +688,7 @@ async fn summarize_context_three_requests_and_instructions() { ); // Shut down Codex to flush rollout entries before inspecting the file. - codex.submit(Op::Shutdown).await.unwrap(); + codex.submit(Op::Shutdown).await?; wait_for_event(&codex, |ev| matches!(ev, EventMsg::ShutdownComplete)).await; let replacement_history = replacement_history_from_rollout(&rollout_path) @@ -750,6 +768,7 @@ async fn summarize_context_three_requests_and_instructions() { saw_compacted_summary, "expected a Compacted entry containing the summarizer output" ); + Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/codex-rs/core/tests/suite/personality.rs b/codex-rs/core/tests/suite/personality.rs index e139bb7373ef..fd3d7e9a05ed 100644 --- a/codex-rs/core/tests/suite/personality.rs +++ b/codex-rs/core/tests/suite/personality.rs @@ -543,6 +543,7 @@ async fn user_turn_personality_skips_if_feature_disabled() -> anyhow::Result<()> let mut builder = test_codex() .with_model("exp-codex-personality") .with_config(|config| { + config.update_plan_enabled = true; config .features .disable(Feature::Personality) diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index 79de7435c737..6c7e8b7686cb 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -6,6 +6,7 @@ use std::path::Path; use codex_core::TurnInputRequest; use codex_core::shell::default_user_shell; use codex_features::Feature; +use codex_models_manager::collaboration_mode_presets::builtin_collaboration_mode_presets; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::ReasoningSummary; @@ -117,10 +118,10 @@ fn normalize_newlines(text: &str) -> String { text.replace("\r\n", "\n") } -#[test_case(None, true, false; "default with model instructions")] +#[test_case(None, false, false; "default with model instructions")] #[test_case(Some(true), true, false; "enabled with model instructions")] #[test_case(Some(false), false, false; "disabled with model instructions")] -#[test_case(None, true, true; "default with custom instructions")] +#[test_case(None, false, true; "default with custom instructions")] #[test_case(Some(true), true, true; "enabled with custom instructions")] #[test_case(Some(false), false, true; "disabled with custom instructions")] #[tokio::test(flavor = "multi_thread", worker_threads = 4)] @@ -189,14 +190,44 @@ async fn prompt_tools_are_consistent_across_requests( let base_instructions = if custom_instructions { CUSTOM_BASE_INSTRUCTIONS.to_string() } else { - model_info.get_model_instructions(config.personality) + let original = model_info.get_model_instructions(config.personality); + if expected_update_plan_enabled { + original + } else { + let (before, planning) = original.split_once("## Planning\n").unwrap(); + let (_, after) = planning.split_once("\n## ").unwrap(); + let (after, _) = after.split_once("## `update_plan`\n").unwrap(); + format!("{before}## {after}") + } }; + let mode_instructions = if custom_instructions { + "## Plan tool\nPreserve this custom collaboration policy.\n".to_string() + } else { + builtin_collaboration_mode_presets() + .into_iter() + .find(|preset| preset.mode == Some(ModeKind::Plan)) + .and_then(|preset| preset.developer_instructions.flatten()) + .expect("built-in Plan mode instructions") + }; codex - .start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text { - text: "hello 1".into(), - text_elements: Vec::new(), - }])) + .start_or_steer_turn( + TurnInputRequest::user_input(vec![UserInput::Text { + text: "hello 1".into(), + text_elements: Vec::new(), + }]) + .with_thread_settings(ThreadSettingsOverrides { + collaboration_mode: Some(CollaborationMode { + mode: ModeKind::Plan, + settings: Settings { + model: "gpt-5.2".to_string(), + reasoning_effort: None, + developer_instructions: Some(mode_instructions.clone()), + }, + }), + ..Default::default() + }), + ) .await?; wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; @@ -229,6 +260,16 @@ async fn prompt_tools_are_consistent_across_requests( assert_tool_names(&body1, &expected_tools_names); for request in [&req1, &req2] { + let developer_text = request + .single_request() + .message_input_texts("developer") + .join("\n"); + if custom_instructions || expected_update_plan_enabled { + assert!(developer_text.contains(&mode_instructions)); + } else { + assert!(!developer_text.contains("update_plan")); + assert!(developer_text.contains("Plan Mode (Conversational)")); + } if let Some(instructions) = &config.developer_instructions { assert!( request diff --git a/codex-rs/core/tests/suite/remote_models.rs b/codex-rs/core/tests/suite/remote_models.rs index 01207a01db15..ba73e2ebf6be 100644 --- a/codex-rs/core/tests/suite/remote_models.rs +++ b/codex-rs/core/tests/suite/remote_models.rs @@ -62,6 +62,43 @@ use wiremock::matchers::path; const REMOTE_MODEL_SLUG: &str = "codex-test"; +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn disabled_update_plan_preserves_custom_catalog_instructions() -> Result<()> { + skip_if_no_network!(Ok(())); + const INSTRUCTIONS: &str = "## Plan tool\nNever deploy without explicit approval.\n"; + let server = start_mock_server().await; + let response = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + let mut catalog = bundled_models_response()?; + let model = catalog + .models + .iter_mut() + .find(|model| model.slug == "gpt-5.2") + .expect("bundled gpt-5.2 model"); + let messages = model + .model_messages + .as_mut() + .expect("model prompt templates"); + messages.instructions_template = Some(INSTRUCTIONS.to_string()); + messages.instructions_variables = None; + let test = test_codex() + .with_model("gpt-5.2") + .with_config(move |config| { + config.update_plan_enabled = false; + config.model_catalog = Some(catalog); + }) + .build_with_auto_env(&server) + .await?; + test.submit_turn("hello").await?; + let request = response.single_request().body_json(); + assert_eq!(request["instructions"], INSTRUCTIONS); + assert!(!request["tools"].to_string().contains("update_plan")); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn unknown_model_sends_builtin_instructions() -> Result<()> { skip_if_no_network!(Ok(())); @@ -72,7 +109,9 @@ async fn unknown_model_sends_builtin_instructions() -> Result<()> { sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), ) .await; - let mut builder = test_codex().with_model("future-custom-model"); + let mut builder = test_codex() + .with_model("future-custom-model") + .with_config(|config| config.update_plan_enabled = true); let test = builder.build_with_auto_env(&server).await?; test.submit_turn("use fallback model metadata").await?; @@ -919,6 +958,7 @@ async fn remote_models_apply_legacy_instructions() -> Result<()> { let mut builder = test_codex() .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) .with_config(|config| { + config.update_plan_enabled = true; config.model = Some("gpt-5.2".to_string()); }); let TestCodex { diff --git a/codex-rs/ext/goal/BUILD.bazel b/codex-rs/ext/goal/BUILD.bazel index 42f2d430e953..c8f3c96c845d 100644 --- a/codex-rs/ext/goal/BUILD.bazel +++ b/codex-rs/ext/goal/BUILD.bazel @@ -6,5 +6,8 @@ codex_rust_crate( "templates/**", ]), crate_name = "codex_goal_extension", - integration_compile_data_extra = ["src/accounting.rs"], + integration_compile_data_extra = [ + "src/accounting.rs", + "src/steering.rs", + ] + glob(["templates/**"]), ) diff --git a/codex-rs/ext/goal/src/runtime.rs b/codex-rs/ext/goal/src/runtime.rs index dca0736c9103..9a3594b760ad 100644 --- a/codex-rs/ext/goal/src/runtime.rs +++ b/codex-rs/ext/goal/src/runtime.rs @@ -448,7 +448,10 @@ impl GoalRuntimeHandle { .get::() .map(|options| options.as_ref().clone()) .unwrap_or_default(); - let item = continuation_steering_item(&protocol_goal_from_state(goal)); + let item = continuation_steering_item( + &protocol_goal_from_state(goal), + thread.config().await.update_plan_enabled, + ); match thread .start_turn_if_idle( diff --git a/codex-rs/ext/goal/src/steering.rs b/codex-rs/ext/goal/src/steering.rs index 6615b6edaae0..88de89d09e17 100644 --- a/codex-rs/ext/goal/src/steering.rs +++ b/codex-rs/ext/goal/src/steering.rs @@ -1,6 +1,7 @@ use codex_core::context::ContextualUserFragment; use codex_core::context::InternalContextSource; use codex_core::context::InternalModelContextFragment; +use codex_core::context::without_update_plan_instructions; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::ThreadGoal; use codex_utils_template::Template; @@ -13,6 +14,13 @@ static CONTINUATION_PROMPT_TEMPLATE: LazyLock