mirror of
https://github.com/openai/codex.git
synced 2026-11-02 19:43:24 +00:00
builtintools and manualexecutionmode
This commit is contained in:
@@ -2285,6 +2285,12 @@ pub struct ThreadStartParams {
|
||||
pub personality: Option<Personality>,
|
||||
#[ts(optional = nullable)]
|
||||
pub ephemeral: Option<bool>,
|
||||
#[experimental("thread/start.builtinTools")]
|
||||
#[ts(optional = nullable)]
|
||||
pub builtin_tools: Option<Vec<String>>,
|
||||
#[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<Vec<DynamicToolSpec>>,
|
||||
|
||||
@@ -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<HashMap<String, serde_json::Value>>,
|
||||
typesafe_overrides: ConfigOverrides,
|
||||
builtin_tools: Option<Vec<String>>,
|
||||
manual_tool_execution: bool,
|
||||
dynamic_tools: Option<Vec<ApiDynamicToolSpec>>,
|
||||
persist_extended_history: bool,
|
||||
service_name: Option<String>,
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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::<ThreadStartResponse>(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;
|
||||
|
||||
@@ -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 => {
|
||||
|
||||
@@ -364,6 +364,8 @@ impl Codex {
|
||||
conversation_history: InitialHistory,
|
||||
session_source: SessionSource,
|
||||
agent_control: AgentControl,
|
||||
builtin_tools: Option<Vec<String>>,
|
||||
manual_tool_execution: bool,
|
||||
dynamic_tools: Vec<DynamicToolSpec>,
|
||||
persist_extended_history: bool,
|
||||
metrics_service_name: Option<String>,
|
||||
@@ -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<String>,
|
||||
/// Source of the session (cli, vscode, exec, mcp, ...)
|
||||
session_source: SessionSource,
|
||||
builtin_tools: Option<Vec<String>>,
|
||||
manual_tool_execution: bool,
|
||||
dynamic_tools: Vec<DynamicToolSpec>,
|
||||
persist_extended_history: bool,
|
||||
inherited_shell_snapshot: Option<Arc<ShellSnapshot>>,
|
||||
@@ -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();
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -315,17 +315,21 @@ impl ThreadManager {
|
||||
pub async fn start_thread(&self, config: Config) -> CodexResult<NewThread> {
|
||||
// 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<Vec<String>>,
|
||||
manual_tool_execution: bool,
|
||||
dynamic_tools: Vec<codex_protocol::dynamic_tools::DynamicToolSpec>,
|
||||
persist_extended_history: bool,
|
||||
) -> CodexResult<NewThread> {
|
||||
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<Vec<String>>,
|
||||
manual_tool_execution: bool,
|
||||
dynamic_tools: Vec<codex_protocol::dynamic_tools::DynamicToolSpec>,
|
||||
persist_extended_history: bool,
|
||||
metrics_service_name: Option<String>,
|
||||
@@ -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<AuthManager>,
|
||||
agent_control: AgentControl,
|
||||
builtin_tools: Option<Vec<String>>,
|
||||
manual_tool_execution: bool,
|
||||
dynamic_tools: Vec<codex_protocol::dynamic_tools::DynamicToolSpec>,
|
||||
persist_extended_history: bool,
|
||||
metrics_service_name: Option<String>,
|
||||
@@ -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<AuthManager>,
|
||||
agent_control: AgentControl,
|
||||
session_source: SessionSource,
|
||||
builtin_tools: Option<Vec<String>>,
|
||||
manual_tool_execution: bool,
|
||||
dynamic_tools: Vec<codex_protocol::dynamic_tools::DynamicToolSpec>,
|
||||
persist_extended_history: bool,
|
||||
metrics_service_name: Option<String>,
|
||||
@@ -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,
|
||||
|
||||
@@ -237,6 +237,9 @@ pub(crate) async fn intercept_apply_patch(
|
||||
call_id: &str,
|
||||
tool_name: &str,
|
||||
) -> Result<Option<FunctionToolOutput>, 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
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -123,11 +123,18 @@ where
|
||||
|
||||
pub struct ToolRegistry {
|
||||
handlers: HashMap<String, Arc<dyn AnyToolHandler>>,
|
||||
builtin_handler_names: std::collections::HashSet<String>,
|
||||
}
|
||||
|
||||
impl ToolRegistry {
|
||||
fn new(handlers: HashMap<String, Arc<dyn AnyToolHandler>>) -> Self {
|
||||
Self { handlers }
|
||||
fn new(
|
||||
handlers: HashMap<String, Arc<dyn AnyToolHandler>>,
|
||||
builtin_handler_names: std::collections::HashSet<String>,
|
||||
) -> Self {
|
||||
Self {
|
||||
handlers,
|
||||
builtin_handler_names,
|
||||
}
|
||||
}
|
||||
|
||||
fn handler(&self, name: &str) -> Option<Arc<dyn AnyToolHandler>> {
|
||||
@@ -147,6 +154,16 @@ impl ToolRegistry {
|
||||
invocation: ToolInvocation,
|
||||
) -> Result<AnyToolResult, FunctionCallError> {
|
||||
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<String, Arc<dyn AnyToolHandler>>,
|
||||
builtin_handler_names: std::collections::HashSet<String>,
|
||||
specs: Vec<ConfiguredToolSpec>,
|
||||
}
|
||||
|
||||
@@ -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<H>(&mut self, name: impl Into<String>, handler: Arc<H>)
|
||||
@@ -353,6 +393,15 @@ impl ToolRegistryBuilder {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn register_builtin_handler<H>(&mut self, name: impl Into<String>, handler: Arc<H>)
|
||||
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<I>(&mut self, names: I, handler: Arc<dyn ToolHandler>)
|
||||
// where
|
||||
@@ -372,7 +421,7 @@ impl ToolRegistryBuilder {
|
||||
// }
|
||||
|
||||
pub fn build(self) -> (Vec<ConfiguredToolSpec>, ToolRegistry) {
|
||||
let registry = ToolRegistry::new(self.handlers);
|
||||
let registry = ToolRegistry::new(self.handlers, self.builtin_handler_names);
|
||||
(self.specs, registry)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<Vec<String>>,
|
||||
pub manual_tool_execution: bool,
|
||||
pub apply_patch_tool_type: Option<ApplyPatchToolType>,
|
||||
pub web_search_mode: Option<WebSearchMode>,
|
||||
pub web_search_config: Option<WebSearchConfig>,
|
||||
@@ -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<Vec<String>>) -> 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<WebSearchConfig>) -> 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::<Vec<_>>();
|
||||
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();
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user