diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 70c9fda438..52a72b8970 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -80,7 +80,7 @@ pub(crate) async fn run_codex_thread_interactive( warnings: Vec::new(), }; let session_source = SessionSource::SubAgent(subagent_source.clone()); - let is_guardian_reviewer = crate::guardian::is_guardian_reviewer_source(&session_source); + let is_guardian_reviewer = crate::guardian::is_basic_session_source(&session_source); let extensions = if is_guardian_reviewer { codex_extension_api::empty_extension_registry() } else { diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index 0486ccb063..eaf2826c84 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -38,7 +38,7 @@ pub(crate) use prompt::BUNDLED_GUARDIAN_POLICY; pub(crate) use prompt::guardian_truncate_text; pub(crate) use review::GuardianReviewOptions; pub(crate) use review::guardian_timeout_message; -pub(crate) use review::is_guardian_reviewer_source; +pub(crate) use review::is_basic_session_source; pub(crate) use review::new_guardian_review_id; #[cfg(test)] pub(crate) use review::record_guardian_denial_for_test; diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index 29c5c08e08..de63bd97db 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -19,7 +19,9 @@ use codex_protocol::protocol::GuardianAssessmentEvent; use codex_protocol::protocol::GuardianAssessmentStatus; use codex_protocol::protocol::GuardianRiskLevel; use codex_protocol::protocol::GuardianUserAuthorization; +use codex_protocol::protocol::InternalSessionSource; use codex_protocol::protocol::ReviewDecision; +use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; use codex_protocol::protocol::TurnAbortReason; use codex_protocol::protocol::WarningEvent; @@ -209,14 +211,12 @@ pub(crate) fn routes_approval_policy_to_guardian( ) && approvals_reviewer == ApprovalsReviewer::AutoReview } -pub(crate) fn is_guardian_reviewer_source( - session_source: &codex_protocol::protocol::SessionSource, -) -> bool { - matches!( - session_source, - codex_protocol::protocol::SessionSource::SubAgent(SubAgentSource::Other(label)) - if label == GUARDIAN_REVIEWER_NAME - ) +pub(crate) fn is_basic_session_source(session_source: &SessionSource) -> bool { + match session_source { + SessionSource::SubAgent(SubAgentSource::Other(label)) => label == GUARDIAN_REVIEWER_NAME, + SessionSource::Internal(InternalSessionSource::Guardian) => true, + _ => false, + } } fn track_guardian_review( diff --git a/codex-rs/core/src/session/mcp_runtime.rs b/codex-rs/core/src/session/mcp_runtime.rs index 9802e7e4c3..e569bf1ac2 100644 --- a/codex-rs/core/src/session/mcp_runtime.rs +++ b/codex-rs/core/src/session/mcp_runtime.rs @@ -150,7 +150,7 @@ impl Session { mut projection: McpRuntimeProjection, ) -> BoxFuture<'a, McpRuntimeProjection> { Box::pin(async move { - if crate::guardian::is_guardian_reviewer_source(session_source) { + if crate::guardian::is_basic_session_source(session_source) { return projection; } diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 10b2b6b696..3fc12c2d71 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -547,11 +547,16 @@ impl Session { config .startup_warnings .extend(user_instruction_provider_warnings); - let exec_policy = if crate::guardian::is_guardian_reviewer_source(&session_source) { - // Guardian review should rely on the built-in shell safety checks, - // not on caller-provided exec-policy rules that could shape the - // reviewer or silently auto-approve commands. - Arc::new(ExecPolicyManager::default()) + let exec_policy = if crate::guardian::is_basic_session_source(&session_source) { + let managed_policy = config + .config_layer_stack + .requirements() + .exec_policy + .as_deref() + .map_or_else(codex_execpolicy::Policy::empty, |policy| { + policy.as_ref().clone() + }); + Arc::new(ExecPolicyManager::new(Arc::new(managed_policy))) } else if let Some(exec_policy) = &inherited_exec_policy { Arc::clone(exec_policy) } else { @@ -602,9 +607,8 @@ impl Session { config.http_client_factory(), ) .await; - let trusted_guardian_reviewer = - crate::guardian::is_guardian_reviewer_source(&session_source) - && !matches!(conversation_history, InitialHistory::Resumed(_)); + let trusted_guardian_reviewer = crate::guardian::is_basic_session_source(&session_source) + && !matches!(conversation_history, InitialHistory::Resumed(_)); if config .config_layer_stack .requirements() @@ -3592,7 +3596,7 @@ impl Session { ) }; let separate_guardian_developer_message = - crate::guardian::is_guardian_reviewer_source(&session_source); + crate::guardian::is_basic_session_source(&session_source); // Keep the guardian policy prompt out of the aggregated developer bundle so it // stays isolated as its own top-level developer message for guardian subagents. if !separate_guardian_developer_message diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 244c116de5..792b2bf4c8 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -659,7 +659,7 @@ async fn required_mcp_servers_for_input( turn_context: &TurnContext, user_input: &[UserInput], ) -> (Vec, Vec) { - if crate::guardian::is_guardian_reviewer_source(&turn_context.session_source) { + if crate::guardian::is_basic_session_source(&turn_context.session_source) { return (Vec::new(), Vec::new()); } @@ -765,7 +765,7 @@ async fn build_skills_and_plugins( let turn_context = step_context.turn.as_ref(); // Guardian input embeds the parent transcript as untrusted evidence. Do not interpret skill or // plugin mentions from that generated prompt as requests to inject additional instructions. - if crate::guardian::is_guardian_reviewer_source(&turn_context.session_source) { + if crate::guardian::is_basic_session_source(&turn_context.session_source) { return Some((Vec::new(), HashSet::new())); } @@ -1322,7 +1322,7 @@ pub(crate) fn build_prompt( parallel_tool_calls: true, base_instructions, output_schema: turn_context.final_output_json_schema.clone(), - output_schema_strict: !crate::guardian::is_guardian_reviewer_source( + output_schema_strict: !crate::guardian::is_basic_session_source( &turn_context.session_source, ), } diff --git a/codex-rs/core/src/session/world_state.rs b/codex-rs/core/src/session/world_state.rs index 90a5328a87..d9640fcb20 100644 --- a/codex-rs/core/src/session/world_state.rs +++ b/codex-rs/core/src/session/world_state.rs @@ -285,14 +285,16 @@ impl Session { world_state.add_section(usage_hint); } world_state.add_section(multi_agent_mode); - world_state.add_section(ManagedDeveloperInstructionsState::new( - turn_context - .config - .config_layer_stack - .requirements() - .additional_developer_instructions - .as_ref(), - )); + if !crate::guardian::is_basic_session_source(&turn_context.session_source) { + world_state.add_section(ManagedDeveloperInstructionsState::new( + turn_context + .config + .config_layer_stack + .requirements() + .additional_developer_instructions + .as_ref(), + )); + } Ok(world_state) } } diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 33d9adee7e..5cbba9778b 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -1882,21 +1882,44 @@ impl ThreadManagerState { threads.remove(&resumed.conversation_id); } } - let user_instructions = self - .user_instructions_for_spawn(&session_source, parent_thread_id, forked_from_thread_id) - .await; + let ( + user_instructions, + inherited_exec_policy, + extensions, + mcp_manager, + multi_agent_version, + ) = if crate::guardian::is_basic_session_source(&session_source) { + ( + LoadedUserInstructions::default(), + None, + empty_extension_registry(), + Arc::new(McpManager::new(Arc::clone(&self.plugins_manager))), + Some(MultiAgentVersion::Disabled), + ) + } else { + ( + self.user_instructions_for_spawn( + &session_source, + parent_thread_id, + forked_from_thread_id, + ) + .await, + inherited_exec_policy, + Arc::clone(&self.extensions), + Arc::clone(&self.mcp_manager), + self.initial_multi_agent_version_for_spawn( + &initial_history, + Some(&session_source), + parent_thread_id, + forked_from_thread_id, + ) + .await, + ) + }; let parent_rollout_thread_trace = self .parent_rollout_thread_trace_for_source(&session_source, &initial_history) .await; let tracked_session_source = session_source.clone(); - let multi_agent_version = self - .initial_multi_agent_version_for_spawn( - &initial_history, - Some(&session_source), - parent_thread_id, - forked_from_thread_id, - ) - .await; let originator = self .effective_originator( &initial_history, @@ -1925,9 +1948,9 @@ impl ThreadManagerState { environment_manager: Arc::clone(&self.environment_manager), skills_service: Arc::clone(&self.skills_service), plugins_manager: Arc::clone(&self.plugins_manager), - mcp_manager: Arc::clone(&self.mcp_manager), + mcp_manager, code_mode_session_provider: Arc::clone(&self.code_mode_session_provider), - extensions: Arc::clone(&self.extensions), + extensions, conversation_history: initial_history, requested_history_mode: history_mode, fork_persistence, diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 017745548a..fea6e44ecc 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -878,18 +878,106 @@ async fn start_thread_keeps_internal_threads_hidden_from_normal_lookups() { #[tokio::test] async fn spawn_internal_session_preserves_parent_lineage_without_forking_history() { + struct ParentLifecycleContributor { + observed_mcp_sources: Arc>>, + } + + impl codex_extension_api::ThreadLifecycleContributor for ParentLifecycleContributor {} + + impl codex_extension_api::McpServerContributor for ParentLifecycleContributor { + fn id(&self) -> &'static str { + "parent_mcp_contributor" + } + + fn contribute<'a>( + &'a self, + context: codex_extension_api::McpServerContributionContext<'a, Config>, + ) -> codex_extension_api::ExtensionFuture<'a, Vec> + { + Box::pin(async move { + if let Some(session_source) = context.session_source() { + self.observed_mcp_sources + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .push(session_source.clone()); + } + Vec::new() + }) + } + } + + struct ParentInstructionsProvider(codex_extension_api::UserInstructions); + + impl codex_extension_api::UserInstructionsProvider for ParentInstructionsProvider { + fn load_user_instructions(&self) -> codex_extension_api::LoadUserInstructionsFuture<'_> { + Box::pin(async move { + codex_extension_api::LoadedUserInstructions { + instructions: Some(self.0.clone()), + warnings: Vec::new(), + } + }) + } + } + let temp_dir = tempdir().expect("tempdir"); let mut config = test_config().await; config.codex_home = temp_dir.path().join("codex-home").abs(); config.cwd = config.codex_home.abs(); std::fs::create_dir_all(&config.codex_home).expect("create codex home"); - let manager = ThreadManager::with_models_provider_and_home_for_tests( + let mut managed_exec_policy = codex_execpolicy::Policy::empty(); + managed_exec_policy + .add_prefix_rule(&["rm".to_string()], codex_execpolicy::Decision::Forbidden) + .expect("add managed execution restriction"); + let mut requirements = config.config_layer_stack.requirements().clone(); + requirements.exec_policy = Some(codex_config::Sourced::new( + codex_execpolicy::RequirementsExecPolicy::new(managed_exec_policy), + codex_config::RequirementSource::Unknown, + )); + requirements.additional_developer_instructions = Some(codex_config::Sourced::new( + "managed instructions must not shape the reviewer".to_string(), + codex_config::RequirementSource::Unknown, + )); + let mut requirements_toml = config.config_layer_stack.requirements_toml().clone(); + requirements_toml.additional_developer_instructions = + Some("managed instructions must not shape the reviewer".to_string()); + config.config_layer_stack = codex_config::ConfigLayerStack::new( + config + .config_layer_stack + .all_layers_low_to_high() + .cloned() + .collect(), + requirements, + requirements_toml, + ) + .expect("managed requirements stack"); + + let parent_instructions = codex_extension_api::UserInstructions { + text: "parent user instructions must not be inherited".to_string(), + source: config.codex_home.join("AGENTS.md"), + }; + let mut manager = ThreadManager::with_models_provider_and_home_for_tests( CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); + let observed_mcp_sources = Arc::new(std::sync::Mutex::new(Vec::new())); + let parent_contributor = Arc::new(ParentLifecycleContributor { + observed_mcp_sources: Arc::clone(&observed_mcp_sources), + }); + let mut extensions = codex_extension_api::ExtensionRegistryBuilder::new(); + extensions.thread_lifecycle_contributor(parent_contributor.clone()); + extensions.mcp_server_contributor(parent_contributor); + let manager_state = Arc::get_mut(&mut manager.state).expect("unshared thread manager state"); + manager_state.extensions = Arc::new(extensions.build()); + manager_state.mcp_manager = Arc::new(McpManager::new_with_extensions( + Arc::clone(&manager_state.plugins_manager), + Arc::clone(&manager_state.extensions), + manager_state.mcp_manager.codex_apps_tools_cache(), + )); + manager_state.user_instructions_provider = + Arc::new(ParentInstructionsProvider(parent_instructions.clone())); let parent = manager .start_thread(StartThreadOptions { metrics_service_name: Some("codex_work_desktop".to_string()), @@ -897,6 +985,45 @@ async fn spawn_internal_session_preserves_parent_lineage_without_forking_history }) .await .expect("start parent thread"); + parent + .thread + .session + .set_multi_agent_version_if_unset(MultiAgentVersion::V2); + assert_eq!( + parent.thread.session.user_instructions().await, + Some(parent_instructions) + ); + assert_eq!( + parent + .thread + .session + .services + .extensions + .thread_lifecycle_contributors() + .len(), + 1 + ); + let mut reviewer_environments = parent + .thread + .session + .services + .turn_environments + .selections(); + let reviewer_environment = reviewer_environments + .first_mut() + .expect("parent should have an environment"); + reviewer_environment.config = + EnvironmentConfigState::Ready(codex_protocol::protocol::EnvironmentConfig { + allow_login_shell: true, + permission_profile: config.permissions.permission_profile_state().snapshot(), + shell_environment_policy: Default::default(), + exec_policy: Some(codex_execpolicy::RequirementsExecPolicy::new( + codex_execpolicy::Policy::empty(), + )), + mcp_policy: None, + network_policy: None, + selected_capability_roots: Vec::new(), + }); let reviewer = manager .spawn_internal_session( parent.thread_id, @@ -905,7 +1032,7 @@ async fn spawn_internal_session_preserves_parent_lineage_without_forking_history initial_history: InitialHistory::Forked(vec![RolloutItem::ResponseItem( user_msg("parent history must not be inherited").into(), )]), - environments: Some(Vec::new()), + environments: Some(reviewer_environments), ..StartThreadOptions::new(config) }, ) @@ -934,11 +1061,70 @@ async fn spawn_internal_session_preserves_parent_lineage_without_forking_history assert_eq!(reviewer_config.parent_thread_id, Some(parent.thread_id)); assert_eq!(reviewer_config.forked_from_thread_id, None); assert_eq!(reviewer_config.originator, "codex_work_desktop"); + assert_eq!( + reviewer.thread.multi_agent_version(), + Some(MultiAgentVersion::Disabled) + ); assert_eq!( reviewer.session_configured.parent_thread_id, Some(parent.thread_id) ); assert_eq!(reviewer.session_configured.forked_from_id, None); + assert!(reviewer.thread.session.user_instructions().await.is_none()); + assert!( + reviewer + .thread + .session + .services + .exec_policy + .current() + .rules() + .contains_key("rm") + ); + let reviewer_turn = reviewer.thread.session.new_default_turn().await; + let reviewer_world_state = + build_world_state_from_turn_context(&reviewer.thread.session, &reviewer_turn).await; + let reviewer_context = reviewer + .thread + .session + .build_initial_context_with_world_state(&reviewer_turn, &reviewer_world_state) + .await; + assert!( + !serde_json::to_string(&reviewer_context) + .expect("reviewer context should serialize") + .contains("managed instructions must not shape the reviewer") + ); + let reviewer_environment = reviewer + .thread + .environment_selections() + .await + .into_iter() + .next() + .expect("reviewer should retain its selected environment"); + assert!(matches!( + reviewer_environment.config, + EnvironmentConfigState::Ready(config) if config.exec_policy.is_some() + )); + assert!( + reviewer + .thread + .session + .services + .extensions + .thread_lifecycle_contributors() + .is_empty() + ); + { + let observed_mcp_sources = observed_mcp_sources + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + assert!(!observed_mcp_sources.is_empty()); + assert!( + observed_mcp_sources + .iter() + .all(|source| !source.is_internal()) + ); + } assert_eq!(manager.list_thread_ids().await, vec![parent.thread_id]); assert!(manager.get_thread(reviewer.thread_id).await.is_err()); assert!( diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index f0ab350319..b317ca2c02 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -141,8 +141,7 @@ pub(crate) fn build_tool_router( let mut registry = ToolRegistry::default(); add_core_tool_sources(&context, &mut registry); - let hosted_specs = if crate::guardian::is_guardian_reviewer_source(&turn_context.session_source) - { + let hosted_specs = if crate::guardian::is_basic_session_source(&turn_context.session_source) { Vec::new() } else { let registered_mcp_tools = session.services.mcp_handler_cache.append_mcp_tools( @@ -278,7 +277,7 @@ pub(crate) fn append_source_tools( extension_tool_executors: impl IntoIterator>>, dynamic_tools: &[DynamicToolSpec], ) -> Vec { - if crate::guardian::is_guardian_reviewer_source(&turn_context.session_source) { + if crate::guardian::is_basic_session_source(&turn_context.session_source) { return Vec::new(); } @@ -549,7 +548,7 @@ fn hosted_model_tool_specs( ) -> Vec { // Responses Lite accepts schemas for client-executed tools, not hosted Responses tools. if turn_context.model_info.use_responses_lite - || crate::guardian::is_guardian_reviewer_source(&turn_context.session_source) + || crate::guardian::is_basic_session_source(&turn_context.session_source) { return Vec::new(); } @@ -890,27 +889,40 @@ fn add_core_tool_sources(context: &CoreToolPlanContext<'_>, registry: &mut ToolR // Guardian reviewers receive only `exec_command`, `write_stdin`, and `view_image` // when a managed sandbox can enforce the parent's filesystem restrictions; // all general tool sources stay excluded. - if crate::guardian::is_guardian_reviewer_source(&context.turn_context.session_source) { + if crate::guardian::is_basic_session_source(&context.turn_context.session_source) { let turn_context = context.turn_context; if !matches!( turn_context.permission_profile(), PermissionProfile::Managed { .. } - ) { + ) || context.environments.turn_environments().any(|environment| { + !matches!( + environment.permission_profile(), + PermissionProfile::Managed { .. } + ) + }) { return; } let environment_mode = tool_environment_mode(context.environments); if environment_mode.has_environment() { let include_environment_id = matches!(environment_mode, ToolEnvironmentMode::Multiple); - registry.add(ExecCommandHandler::new(ExecCommandHandlerOptions { - allow_login_shell: any_environment_allows_login_shell(context.environments), - exec_permission_approvals_enabled: false, - include_environment_id, - include_shell_parameter: unified_exec_should_include_shell_parameter( - turn_context, - context.environments, - ), - })); - registry.add(WriteStdinHandler); + if turn_context.config.features.enabled(Feature::ShellTool) + && turn_context.config.features.enabled(Feature::UnifiedExec) + && !matches!( + turn_context.model_info.shell_type, + ConfigShellToolType::Disabled + ) + { + registry.add(ExecCommandHandler::new(ExecCommandHandlerOptions { + allow_login_shell: any_environment_allows_login_shell(context.environments), + exec_permission_approvals_enabled: false, + include_environment_id, + include_shell_parameter: unified_exec_should_include_shell_parameter( + turn_context, + context.environments, + ), + })); + registry.add(WriteStdinHandler); + } if turn_context.config.features.enabled(Feature::ViewImage) { registry.add(ViewImageHandler::new(ViewImageToolOptions { can_request_original_image_detail: can_request_original_image_detail( diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index a31e576788..a476cf1518 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -22,6 +22,7 @@ use codex_protocol::openai_models::InputModality; use codex_protocol::openai_models::ToolMode; use codex_protocol::openai_models::WebSearchToolType; use codex_protocol::protocol::EnvironmentConfigState; +use codex_protocol::protocol::InternalSessionSource; use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; @@ -476,6 +477,191 @@ fn apply_patch_accepts_environment_id(spec: &ToolSpec) -> bool { } } +#[tokio::test] +async fn internal_guardian_sessions_exclude_optional_core_tools() { + let (session, mut turn) = make_session_and_context().await; + turn.session_source = SessionSource::Internal(InternalSessionSource::Guardian); + set_feature(&mut turn, Feature::ViewImage, /*enabled*/ true); + Arc::make_mut(&mut turn.config).update_plan_enabled = true; + turn.multi_agent_version = MultiAgentVersion::V2; + let turn = Arc::new(turn); + let step_context = StepContext::for_test(Arc::clone(&turn)); + + let router = super::build_tool_router( + &session, + step_context.turn.as_ref(), + &step_context.environments, + &step_context.mcp, + /*apps_enabled*/ false, + &turn.extension_data, + /*tool_suggest_candidates*/ None, + ) + .expect("build internal Guardian tool router"); + + assert_eq!( + router + .model_visible_specs() + .iter() + .map(codex_tools::ToolSpec::name) + .collect::>(), + vec!["exec_command", "write_stdin", "view_image"] + ); +} + +#[tokio::test] +async fn internal_guardian_sessions_respect_managed_shell_restrictions() { + for (disabled_feature, shell_type) in [ + (Some(Feature::ShellTool), ConfigShellToolType::UnifiedExec), + (Some(Feature::UnifiedExec), ConfigShellToolType::UnifiedExec), + (None, ConfigShellToolType::Disabled), + ] { + let (session, mut turn) = make_session_and_context().await; + turn.session_source = SessionSource::Internal(InternalSessionSource::Guardian); + set_feature(&mut turn, Feature::ViewImage, /*enabled*/ true); + set_feature(&mut turn, Feature::CodeMode, /*enabled*/ true); + if let Some(feature) = disabled_feature { + let config = Arc::make_mut(&mut turn.config); + config.features = crate::config::ManagedFeatures::from_configured( + config.features.get().clone(), + Some(codex_config::Sourced::new( + codex_config::FeatureRequirementsToml { + entries: BTreeMap::from([(feature.key().to_string(), false)]), + }, + codex_config::RequirementSource::Unknown, + )), + ) + .expect("managed shell restriction should be valid"); + } + Arc::make_mut(&mut turn.model_info).shell_type = shell_type; + let turn = Arc::new(turn); + let step_context = StepContext::for_test(Arc::clone(&turn)); + + let router = super::build_tool_router( + &session, + step_context.turn.as_ref(), + &step_context.environments, + &step_context.mcp, + /*apps_enabled*/ false, + &turn.extension_data, + /*tool_suggest_candidates*/ None, + ) + .expect("build internal Guardian tool router"); + + assert_eq!( + router + .model_visible_specs() + .iter() + .map(codex_tools::ToolSpec::name) + .collect::>(), + vec![ + codex_code_mode::PUBLIC_TOOL_NAME, + codex_code_mode::WAIT_TOOL_NAME, + "view_image", + ], + "disabled feature: {disabled_feature:?}, shell type: {shell_type:?}" + ); + } +} + +#[tokio::test] +async fn internal_guardian_sessions_preserve_code_mode() { + let (session, mut turn) = make_session_and_context().await; + turn.session_source = SessionSource::Internal(InternalSessionSource::Guardian); + set_feature(&mut turn, Feature::CodeMode, /*enabled*/ true); + let turn = Arc::new(turn); + let step_context = StepContext::for_test(Arc::clone(&turn)); + + let router = super::build_tool_router( + &session, + step_context.turn.as_ref(), + &step_context.environments, + &step_context.mcp, + /*apps_enabled*/ false, + &turn.extension_data, + /*tool_suggest_candidates*/ None, + ) + .expect("build internal Guardian tool router"); + + assert!( + router + .model_visible_specs() + .iter() + .any(|tool| tool.name() == codex_code_mode::PUBLIC_TOOL_NAME) + ); + assert!( + router + .model_visible_specs() + .iter() + .any(|tool| tool.name() == codex_code_mode::WAIT_TOOL_NAME) + ); +} + +#[tokio::test] +async fn internal_guardian_sessions_require_managed_secondary_environments() { + for (secondary_profile, expected_tools) in [ + ( + codex_protocol::models::PermissionProfile::workspace_write(), + vec!["exec_command", "write_stdin", "view_image"], + ), + ( + codex_protocol::models::PermissionProfile::Disabled, + Vec::new(), + ), + ] { + let (session, mut turn) = make_session_and_context().await; + turn.session_source = SessionSource::Internal(InternalSessionSource::Guardian); + set_feature(&mut turn, Feature::ViewImage, /*enabled*/ true); + let TurnEnvironmentState::Ready(primary) = turn + .environments + .environments + .first_mut() + .expect("primary environment") + else { + panic!("primary environment should be ready"); + }; + primary.config_mut().permission_profile = + codex_protocol::models::PermissionProfileSnapshot::legacy( + codex_protocol::models::PermissionProfile::workspace_write(), + ); + duplicate_primary_environment(&mut turn); + let secondary_workspace_root = + codex_utils_path_uri::PathUri::from_abs_path(&turn.config.cwd.join("secondary")); + let TurnEnvironmentState::Ready(secondary) = turn + .environments + .environments + .get_mut(1) + .expect("secondary environment") + else { + panic!("secondary environment should be ready"); + }; + secondary.selection.workspace_roots = vec![secondary_workspace_root]; + secondary.config_mut().permission_profile = + codex_protocol::models::PermissionProfileSnapshot::legacy(secondary_profile); + let turn = Arc::new(turn); + let step_context = StepContext::for_test(Arc::clone(&turn)); + + let router = super::build_tool_router( + &session, + step_context.turn.as_ref(), + &step_context.environments, + &step_context.mcp, + /*apps_enabled*/ false, + &turn.extension_data, + /*tool_suggest_candidates*/ None, + ) + .expect("build internal Guardian tool router"); + + assert_eq!( + router + .model_visible_specs() + .iter() + .map(codex_tools::ToolSpec::name) + .collect::>(), + expected_tools + ); + } +} + #[tokio::test] async fn wait_for_environment_requires_feature_and_uses_host_config_when_present() { const TOOL_DESCRIPTION: &str = "Host-provided wait tool description";