diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index aaf0484a58..987342abf4 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -2285,6 +2285,12 @@ pub struct ThreadStartParams { pub personality: Option, #[ts(optional = nullable)] pub ephemeral: Option, + #[experimental("thread/start.builtinTools")] + #[ts(optional = nullable)] + pub builtin_tools: Option>, + #[experimental("thread/start.manualToolExecution")] + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub manual_tool_execution: bool, #[experimental("thread/start.dynamicTools")] #[ts(optional = nullable)] pub dynamic_tools: Option>, diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index a4909c7745..c9029c0d2d 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -1818,6 +1818,8 @@ impl CodexMessageProcessor { service_name, base_instructions, developer_instructions, + builtin_tools, + manual_tool_execution, dynamic_tools, mock_experimental_field: _mock_experimental_field, experimental_raw_events, @@ -1856,6 +1858,8 @@ impl CodexMessageProcessor { request_id, config, typesafe_overrides, + builtin_tools, + manual_tool_execution, dynamic_tools, persist_extended_history, service_name, @@ -1873,6 +1877,8 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, config_overrides: Option>, typesafe_overrides: ConfigOverrides, + builtin_tools: Option>, + manual_tool_execution: bool, dynamic_tools: Option>, persist_extended_history: bool, service_name: Option, @@ -1902,6 +1908,20 @@ impl CodexMessageProcessor { }; let dynamic_tools = dynamic_tools.unwrap_or_default(); + if let Some(ref builtin_tools) = builtin_tools + && let Err(message) = codex_core::validate_builtin_tools_request(builtin_tools) + { + let error = JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message, + data: None, + }; + listener_task_context + .outgoing + .send_error(request_id, error) + .await; + return; + } let core_dynamic_tools = if dynamic_tools.is_empty() { Vec::new() } else { @@ -1931,6 +1951,8 @@ impl CodexMessageProcessor { .thread_manager .start_thread_with_tools_and_service_name( config, + builtin_tools, + manual_tool_execution, core_dynamic_tools, persist_extended_history, service_name, diff --git a/codex-rs/app-server/tests/suite/v2/skills_list.rs b/codex-rs/app-server/tests/suite/v2/skills_list.rs index 03443a47eb..233f4533b4 100644 --- a/codex-rs/app-server/tests/suite/v2/skills_list.rs +++ b/codex-rs/app-server/tests/suite/v2/skills_list.rs @@ -240,6 +240,8 @@ async fn skills_changed_notification_is_emitted_after_skill_change() -> Result<( developer_instructions: None, personality: None, ephemeral: None, + builtin_tools: None, + manual_tool_execution: false, dynamic_tools: None, mock_experimental_field: None, experimental_raw_events: false, diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index bc383a19d1..b62b5078e9 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -237,6 +237,35 @@ async fn thread_start_accepts_metrics_service_name() -> Result<()> { Ok(()) } +#[tokio::test] +async fn thread_start_accepts_builtin_tool_controls() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let req_id = mcp + .send_thread_start_request(ThreadStartParams { + builtin_tools: Some(vec!["apply_patch".to_string()]), + manual_tool_execution: true, + ..Default::default() + }) + .await?; + + let resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(req_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(resp)?; + assert!(!thread.id.is_empty(), "thread id should not be empty"); + + Ok(()) +} + #[tokio::test] async fn thread_start_ephemeral_remains_pathless() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; diff --git a/codex-rs/core/src/apply_patch.rs b/codex-rs/core/src/apply_patch.rs index 1928a95621..6287d085b0 100644 --- a/codex-rs/core/src/apply_patch.rs +++ b/codex-rs/core/src/apply_patch.rs @@ -50,9 +50,19 @@ pub(crate) async fn apply_patch( } => InternalApplyPatchInvocation::DelegateToExec(ApplyPatchExec { action, auto_approved: !user_explicitly_approved, - exec_approval_requirement: ExecApprovalRequirement::Skip { - bypass_sandbox: false, - proposed_execpolicy_amendment: None, + exec_approval_requirement: if turn_context + .tools_config + .should_force_manual_tool_execution() + { + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + } + } else { + ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + } }, }), SafetyCheck::AskUser => { diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index b2bbae4a78..47d61bc06d 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -364,6 +364,8 @@ impl Codex { conversation_history: InitialHistory, session_source: SessionSource, agent_control: AgentControl, + builtin_tools: Option>, + manual_tool_execution: bool, dynamic_tools: Vec, persist_extended_history: bool, metrics_service_name: Option, @@ -492,7 +494,6 @@ impl Codex { } else { dynamic_tools }; - // TODO (aibrahim): Consolidate config.model and config.model_reasoning_effort into config.collaboration_mode // to avoid extracting these fields separately and constructing CollaborationMode here. let collaboration_mode = CollaborationMode { @@ -525,6 +526,8 @@ impl Codex { metrics_service_name, app_server_client_name: None, session_source, + builtin_tools, + manual_tool_execution, dynamic_tools, persist_extended_history, inherited_shell_snapshot, @@ -786,6 +789,8 @@ impl TurnContext { }) .with_web_search_config(self.tools_config.web_search_config.clone()) .with_allow_login_shell(self.tools_config.allow_login_shell) + .with_builtin_tools(self.tools_config.requested_builtin_tools.clone()) + .with_manual_tool_execution(self.tools_config.manual_tool_execution) .with_agent_roles(config.agent_roles.clone()); Self { @@ -945,6 +950,8 @@ pub(crate) struct SessionConfiguration { app_server_client_name: Option, /// Source of the session (cli, vscode, exec, mcp, ...) session_source: SessionSource, + builtin_tools: Option>, + manual_tool_execution: bool, dynamic_tools: Vec, persist_extended_history: bool, inherited_shell_snapshot: Option>, @@ -1196,6 +1203,8 @@ impl Session { }) .with_web_search_config(per_turn_config.web_search_config.clone()) .with_allow_login_shell(per_turn_config.permissions.allow_login_shell) + .with_builtin_tools(session_configuration.builtin_tools.clone()) + .with_manual_tool_execution(session_configuration.manual_tool_execution) .with_agent_roles(per_turn_config.agent_roles.clone()); let cwd = session_configuration.cwd.clone(); @@ -5153,6 +5162,13 @@ async fn spawn_review_thread( }) .with_web_search_config(None) .with_allow_login_shell(config.permissions.allow_login_shell) + .with_builtin_tools( + parent_turn_context + .tools_config + .requested_builtin_tools + .clone(), + ) + .with_manual_tool_execution(parent_turn_context.tools_config.manual_tool_execution) .with_agent_roles(config.agent_roles.clone()); let review_prompt = resolved.prompt.clone(); diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index f353febd04..531b6aeb39 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -66,6 +66,8 @@ pub(crate) async fn run_codex_thread_interactive( initial_history.unwrap_or(InitialHistory::New), SessionSource::SubAgent(subagent_source), parent_session.services.agent_control.clone(), + None, + false, Vec::new(), false, None, diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 69ce86b61b..4e4169ba08 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -1475,6 +1475,8 @@ async fn set_rate_limits_retains_previous_credits() { metrics_service_name: None, app_server_client_name: None, session_source: SessionSource::Exec, + builtin_tools: None, + manual_tool_execution: false, dynamic_tools: Vec::new(), persist_extended_history: false, inherited_shell_snapshot: None, @@ -1571,6 +1573,8 @@ async fn set_rate_limits_updates_plan_type_when_present() { metrics_service_name: None, app_server_client_name: None, session_source: SessionSource::Exec, + builtin_tools: None, + manual_tool_execution: false, dynamic_tools: Vec::new(), persist_extended_history: false, inherited_shell_snapshot: None, @@ -1925,6 +1929,8 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati metrics_service_name: None, app_server_client_name: None, session_source: SessionSource::Exec, + builtin_tools: None, + manual_tool_execution: false, dynamic_tools: Vec::new(), persist_extended_history: false, inherited_shell_snapshot: None, @@ -2078,6 +2084,8 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() { metrics_service_name: None, app_server_client_name: None, session_source: SessionSource::Exec, + builtin_tools: None, + manual_tool_execution: false, dynamic_tools: Vec::new(), persist_extended_history: false, inherited_shell_snapshot: None, @@ -2171,6 +2179,8 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { metrics_service_name: None, app_server_client_name: None, session_source: SessionSource::Exec, + builtin_tools: None, + manual_tool_execution: false, dynamic_tools: Vec::new(), persist_extended_history: false, inherited_shell_snapshot: None, @@ -2732,6 +2742,8 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( metrics_service_name: None, app_server_client_name: None, session_source: SessionSource::Exec, + builtin_tools: None, + manual_tool_execution: false, dynamic_tools, persist_extended_history: false, inherited_shell_snapshot: None, diff --git a/codex-rs/core/src/codex_tests_guardian.rs b/codex-rs/core/src/codex_tests_guardian.rs index 6bed9fd37b..10ed41444f 100644 --- a/codex-rs/core/src/codex_tests_guardian.rs +++ b/codex-rs/core/src/codex_tests_guardian.rs @@ -298,6 +298,8 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { InitialHistory::New, SessionSource::SubAgent(SubAgentSource::Other(GUARDIAN_SUBAGENT_NAME.to_string())), AgentControl::default(), + None, + false, Vec::new(), false, None, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 7e84577ee6..f39c9d01fb 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -139,6 +139,7 @@ pub use rollout::list::read_session_meta_line; pub use rollout::policy::EventPersistenceMode; pub use rollout::rollout_date_parts; pub use rollout::session_index::find_thread_names_by_ids; +pub use tools::spec::validate_builtin_tools_request; mod function_tool; mod state; mod tasks; diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index cc6de47104..9cd1acb0db 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -315,17 +315,21 @@ impl ThreadManager { pub async fn start_thread(&self, config: Config) -> CodexResult { // Box delegated thread-spawn futures so these convenience wrappers do // not inline the full spawn path into every caller's async state. - Box::pin(self.start_thread_with_tools(config, Vec::new(), false)).await + Box::pin(self.start_thread_with_tools(config, None, false, Vec::new(), false)).await } pub async fn start_thread_with_tools( &self, config: Config, + builtin_tools: Option>, + manual_tool_execution: bool, dynamic_tools: Vec, persist_extended_history: bool, ) -> CodexResult { Box::pin(self.start_thread_with_tools_and_service_name( config, + builtin_tools, + manual_tool_execution, dynamic_tools, persist_extended_history, None, @@ -336,6 +340,8 @@ impl ThreadManager { pub async fn start_thread_with_tools_and_service_name( &self, config: Config, + builtin_tools: Option>, + manual_tool_execution: bool, dynamic_tools: Vec, persist_extended_history: bool, metrics_service_name: Option, @@ -345,6 +351,8 @@ impl ThreadManager { InitialHistory::New, Arc::clone(&self.state.auth_manager), self.agent_control(), + builtin_tools, + manual_tool_execution, dynamic_tools, persist_extended_history, metrics_service_name, @@ -375,6 +383,8 @@ impl ThreadManager { initial_history, auth_manager, self.agent_control(), + None, + false, Vec::new(), persist_extended_history, None, @@ -416,6 +426,8 @@ impl ThreadManager { history, Arc::clone(&self.state.auth_manager), self.agent_control(), + None, + false, Vec::new(), persist_extended_history, None, @@ -499,6 +511,8 @@ impl ThreadManagerState { Arc::clone(&self.auth_manager), agent_control, session_source, + None, + false, Vec::new(), persist_extended_history, metrics_service_name, @@ -522,6 +536,8 @@ impl ThreadManagerState { Arc::clone(&self.auth_manager), agent_control, session_source, + None, + false, Vec::new(), false, None, @@ -545,6 +561,8 @@ impl ThreadManagerState { Arc::clone(&self.auth_manager), agent_control, session_source, + None, + false, Vec::new(), persist_extended_history, None, @@ -561,6 +579,8 @@ impl ThreadManagerState { initial_history: InitialHistory, auth_manager: Arc, agent_control: AgentControl, + builtin_tools: Option>, + manual_tool_execution: bool, dynamic_tools: Vec, persist_extended_history: bool, metrics_service_name: Option, @@ -571,6 +591,8 @@ impl ThreadManagerState { auth_manager, agent_control, self.session_source.clone(), + builtin_tools, + manual_tool_execution, dynamic_tools, persist_extended_history, metrics_service_name, @@ -587,6 +609,8 @@ impl ThreadManagerState { auth_manager: Arc, agent_control: AgentControl, session_source: SessionSource, + builtin_tools: Option>, + manual_tool_execution: bool, dynamic_tools: Vec, persist_extended_history: bool, metrics_service_name: Option, @@ -608,6 +632,8 @@ impl ThreadManagerState { initial_history, session_source, agent_control, + builtin_tools, + manual_tool_execution, dynamic_tools, persist_extended_history, metrics_service_name, diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index b98a721e36..7012aab8dc 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -237,6 +237,9 @@ pub(crate) async fn intercept_apply_patch( call_id: &str, tool_name: &str, ) -> Result, FunctionCallError> { + if !turn.tools_config.builtin_tool_enabled("apply_patch") { + return Ok(None); + } match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd) { codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => { session diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 8c8d668f85..7077d17d16 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -411,6 +411,14 @@ impl ShellHandler { prefix_rule, }) .await; + let exec_approval_requirement = if turn.tools_config.should_force_manual_tool_execution() { + crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + } + } else { + exec_approval_requirement + }; let req = ShellRequest { command: exec_params.command.clone(), diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index f78df2f1a7..648d3a7062 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -123,11 +123,18 @@ where pub struct ToolRegistry { handlers: HashMap>, + builtin_handler_names: std::collections::HashSet, } impl ToolRegistry { - fn new(handlers: HashMap>) -> Self { - Self { handlers } + fn new( + handlers: HashMap>, + builtin_handler_names: std::collections::HashSet, + ) -> Self { + Self { + handlers, + builtin_handler_names, + } } fn handler(&self, name: &str) -> Option> { @@ -147,6 +154,16 @@ impl ToolRegistry { invocation: ToolInvocation, ) -> Result { let tool_name = invocation.tool_name.clone(); + if self.builtin_handler_names.contains(tool_name.as_str()) + && !invocation + .turn + .tools_config + .builtin_tool_enabled(tool_name.as_str()) + { + return Err(FunctionCallError::RespondToModel(format!( + "unsupported call: {tool_name}" + ))); + } let call_id_owned = invocation.call_id.clone(); let otel = invocation.turn.session_telemetry.clone(); let payload_for_response = invocation.payload.clone(); @@ -301,19 +318,22 @@ impl ToolRegistry { pub struct ConfiguredToolSpec { pub spec: ToolSpec, pub supports_parallel_tool_calls: bool, + pub builtin: bool, } impl ConfiguredToolSpec { - pub fn new(spec: ToolSpec, supports_parallel_tool_calls: bool) -> Self { + pub fn new(spec: ToolSpec, supports_parallel_tool_calls: bool, builtin: bool) -> Self { Self { spec, supports_parallel_tool_calls, + builtin, } } } pub struct ToolRegistryBuilder { handlers: HashMap>, + builtin_handler_names: std::collections::HashSet, specs: Vec, } @@ -321,6 +341,7 @@ impl ToolRegistryBuilder { pub fn new() -> Self { Self { handlers: HashMap::new(), + builtin_handler_names: std::collections::HashSet::new(), specs: Vec::new(), } } @@ -334,8 +355,27 @@ impl ToolRegistryBuilder { spec: ToolSpec, supports_parallel_tool_calls: bool, ) { - self.specs - .push(ConfiguredToolSpec::new(spec, supports_parallel_tool_calls)); + self.specs.push(ConfiguredToolSpec::new( + spec, + supports_parallel_tool_calls, + false, + )); + } + + pub fn push_builtin_spec(&mut self, spec: ToolSpec) { + self.push_builtin_spec_with_parallel_support(spec, false); + } + + pub fn push_builtin_spec_with_parallel_support( + &mut self, + spec: ToolSpec, + supports_parallel_tool_calls: bool, + ) { + self.specs.push(ConfiguredToolSpec::new( + spec, + supports_parallel_tool_calls, + true, + )); } pub fn register_handler(&mut self, name: impl Into, handler: Arc) @@ -353,6 +393,15 @@ impl ToolRegistryBuilder { } } + pub fn register_builtin_handler(&mut self, name: impl Into, handler: Arc) + where + H: ToolHandler + 'static, + { + let name = name.into(); + self.builtin_handler_names.insert(name.clone()); + self.register_handler(name, handler); + } + // TODO(jif) for dynamic tools. // pub fn register_many(&mut self, names: I, handler: Arc) // where @@ -372,7 +421,7 @@ impl ToolRegistryBuilder { // } pub fn build(self) -> (Vec, ToolRegistry) { - let registry = ToolRegistry::new(self.handlers); + let registry = ToolRegistry::new(self.handlers, self.builtin_handler_names); (self.specs, registry) } } diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 7095a38cea..0d78c2d952 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -46,6 +46,10 @@ impl ToolRouter { ) -> Self { let builder = build_specs(config, mcp_tools, app_tools, dynamic_tools); let (specs, registry) = builder.build(); + let specs = specs + .into_iter() + .filter(|spec| !spec.builtin || config.builtin_tool_enabled(spec.spec.name())) + .collect(); Self { registry, specs } } @@ -347,4 +351,57 @@ mod tests { Ok(()) } + + #[tokio::test] + async fn builtin_tool_allowlist_blocks_dispatch_for_hidden_builtin() -> anyhow::Result<()> { + let (session, mut turn) = make_session_and_context().await; + turn.tools_config.requested_builtin_tools = Some(vec!["apply_patch".to_string()]); + + let session = Arc::new(session); + let turn = Arc::new(turn); + let mcp_tools = session + .services + .mcp_connection_manager + .read() + .await + .list_all_tools() + .await; + let app_tools = Some(mcp_tools.clone()); + let router = ToolRouter::from_config( + &turn.tools_config, + Some( + mcp_tools + .into_iter() + .map(|(name, tool)| (name, tool.tool)) + .collect(), + ), + app_tools, + turn.dynamic_tools.as_slice(), + ); + + let call = ToolCall { + tool_name: "shell".to_string(), + call_id: "call-3".to_string(), + payload: ToolPayload::Function { + arguments: "{}".to_string(), + }, + }; + let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())); + let response = router + .dispatch_tool_call(session, turn, tracker, call, ToolCallSource::Direct) + .await?; + + match response { + ResponseInputItem::FunctionCallOutput { output, .. } => { + let content = output.text_content().unwrap_or_default(); + assert!( + content.contains("unsupported call: shell"), + "unexpected tool call message: {content}", + ); + } + other => panic!("expected function call output, got {other:?}"), + } + + Ok(()) + } } diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index cf9eaa32f7..8882feef8d 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -38,6 +38,7 @@ use serde::Serialize; use serde_json::Value as JsonValue; use serde_json::json; use std::collections::BTreeMap; +use std::collections::BTreeSet; use std::collections::HashMap; const SEARCH_TOOL_BM25_DESCRIPTION_TEMPLATE: &str = @@ -96,6 +97,8 @@ pub(crate) struct ToolsConfig { shell_command_backend: ShellCommandBackendConfig, pub unified_exec_backend: UnifiedExecBackendConfig, pub allow_login_shell: bool, + pub requested_builtin_tools: Option>, + pub manual_tool_execution: bool, pub apply_patch_tool_type: Option, pub web_search_mode: Option, pub web_search_config: Option, @@ -204,6 +207,8 @@ impl ToolsConfig { shell_command_backend, unified_exec_backend, allow_login_shell: true, + requested_builtin_tools: None, + manual_tool_execution: false, apply_patch_tool_type, web_search_mode: *web_search_mode, web_search_config: None, @@ -236,6 +241,16 @@ impl ToolsConfig { self } + pub fn with_builtin_tools(mut self, builtin_tools: Option>) -> Self { + self.requested_builtin_tools = builtin_tools; + self + } + + pub fn with_manual_tool_execution(mut self, manual_tool_execution: bool) -> Self { + self.manual_tool_execution = manual_tool_execution; + self + } + pub fn with_web_search_config(mut self, web_search_config: Option) -> Self { self.web_search_config = web_search_config; self @@ -246,6 +261,78 @@ impl ToolsConfig { nested.code_mode_enabled = false; nested } + + pub fn builtin_tool_enabled(&self, tool_name: &str) -> bool { + self.requested_builtin_tools + .as_ref() + .is_none_or(|requested| requested.iter().any(|name| name == tool_name)) + } + + pub fn should_force_manual_tool_execution(&self) -> bool { + self.manual_tool_execution + } +} + +fn builtin_tool_names() -> BTreeSet<&'static str> { + BTreeSet::from([ + "apply_patch", + "artifacts", + "close_agent", + "code_mode", + "container.exec", + "exec_command", + "grep_files", + "image_generation", + "js_repl", + "js_repl_reset", + "list_dir", + "list_mcp_resource_templates", + "list_mcp_resources", + "local_shell", + "read_file", + "read_mcp_resource", + "report_agent_job_result", + "request_permissions", + "request_user_input", + "resume_agent", + "search_tool_bm25", + "send_input", + "shell", + "shell_command", + "spawn_agent", + "spawn_agents_on_csv", + "test_sync_tool", + "update_plan", + "view_image", + "wait", + "web_search", + "write_stdin", + ]) +} + +pub fn validate_builtin_tools_request(requested_builtin_tools: &[String]) -> Result<(), String> { + let known_names = builtin_tool_names(); + let invalid_names = requested_builtin_tools + .iter() + .filter(|name| !known_names.contains(name.as_str())) + .cloned() + .collect::>(); + if !invalid_names.is_empty() { + return Err(format!( + "unknown builtin tool name(s): {}", + invalid_names.join(", ") + )); + } + if requested_builtin_tools + .iter() + .any(|name| name == "write_stdin") + && !requested_builtin_tools + .iter() + .any(|name| name == "exec_command") + { + return Err("builtinTools cannot enable write_stdin without exec_command".to_string()); + } + Ok(()) } fn supports_image_generation(model_info: &ModelInfo) -> bool { @@ -2146,10 +2233,10 @@ pub(crate) fn build_specs( if config.shell_type != ConfigShellToolType::Disabled { // Always register shell aliases so older prompts remain compatible. - builder.register_handler("shell", shell_handler.clone()); - builder.register_handler("container.exec", shell_handler.clone()); - builder.register_handler("local_shell", shell_handler); - builder.register_handler("shell_command", shell_command_handler); + builder.register_builtin_handler("shell", shell_handler.clone()); + builder.register_builtin_handler("container.exec", shell_handler.clone()); + builder.register_builtin_handler("local_shell", shell_handler); + builder.register_builtin_handler("shell_command", shell_command_handler); } if mcp_tools.is_some() { @@ -2253,7 +2340,7 @@ pub(crate) fn build_specs( ); } } - builder.register_handler("apply_patch", apply_patch_handler); + builder.register_builtin_handler("apply_patch", apply_patch_handler); } if config @@ -3985,6 +4072,51 @@ mod tests { assert!(!description.contains("{{app_names}}")); } + #[test] + fn builtin_tools_allowlist_reports_enabled_builtin_names() { + let features = Features::with_defaults(); + let model_info = model_info_from_models_json("gpt-5-codex"); + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + features: &features, + web_search_mode: Some(WebSearchMode::Cached), + session_source: SessionSource::Cli, + }) + .with_builtin_tools(Some(vec!["apply_patch".to_string()])); + + assert!(tools_config.builtin_tool_enabled("apply_patch")); + assert!(!tools_config.builtin_tool_enabled("update_plan")); + assert!(!tools_config.builtin_tool_enabled("request_user_input")); + } + + #[test] + fn validate_builtin_tools_request_rejects_invalid_name() { + assert_eq!( + validate_builtin_tools_request(&["not_a_tool".to_string()]), + Err("unknown builtin tool name(s): not_a_tool".to_string()) + ); + } + + #[test] + fn validate_builtin_tools_request_accepts_alias_names() { + assert_eq!( + validate_builtin_tools_request(&[ + "shell".to_string(), + "local_shell".to_string(), + "container.exec".to_string(), + ]), + Ok(()) + ); + } + + #[test] + fn validate_builtin_tools_request_requires_exec_for_write_stdin() { + assert_eq!( + validate_builtin_tools_request(&["write_stdin".to_string()]), + Err("builtinTools cannot enable write_stdin without exec_command".to_string()) + ); + } + #[test] fn test_mcp_tool_property_missing_type_defaults_to_string() { let config = test_config(); diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 29311b1ff4..73ecaa647d 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -26,6 +26,7 @@ use crate::tools::network_approval::finish_deferred_network_approval; use crate::tools::orchestrator::ToolOrchestrator; use crate::tools::runtimes::unified_exec::UnifiedExecRequest as UnifiedExecToolRequest; use crate::tools::runtimes::unified_exec::UnifiedExecRuntime; +use crate::tools::sandboxing::ExecApprovalRequirement; use crate::tools::sandboxing::ToolCtx; use crate::truncate::approx_token_count; use crate::unified_exec::ExecCommandRequest; @@ -593,6 +594,18 @@ impl UnifiedExecProcessManager { prefix_rule: request.prefix_rule.clone(), }) .await; + let exec_approval_requirement = if context + .turn + .tools_config + .should_force_manual_tool_execution() + { + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + } + } else { + exec_approval_requirement + }; let req = UnifiedExecToolRequest { command: request.command.clone(), cwd,