valknar/llmx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: build is broken on main; introduce ToolsConfigParams to help fix (#2663)

Browse Source 
`ToolsConfig::new()` taking a large number of boolean params was hard to
manage and it finally bit us (see
https://github.com/openai/codex/pull/2660). This changes
`ToolsConfig::new()` so that it takes a struct (and also reduces the
visibility of some members, where possible).
This commit is contained in:
Michael Bolin
2025-08-24 22:43:42 -07:00
committed by GitHub
parent ee2ccb5cb6
commit 7b20db942a
2 changed files with 124 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
codex-rs/core/src/codex.rs
View File

@@ -64,6 +64,7 @@ use crate::mcp_tool_call::handle_mcp_tool_call;
use crate::model_family::find_family_for_model; use crate::model_family::find_family_for_model;
use crate::openai_tools::ApplyPatchToolArgs; use crate::openai_tools::ApplyPatchToolArgs;
use crate::openai_tools::ToolsConfig; use crate::openai_tools::ToolsConfig;
use crate::openai_tools::ToolsConfigParams;
use crate::openai_tools::get_openai_tools; use crate::openai_tools::get_openai_tools;
use crate::parse_command::parse_command; use crate::parse_command::parse_command;
use crate::plan_tool::handle_update_plan; use crate::plan_tool::handle_update_plan;
@@ -506,15 +507,15 @@ impl Session {
); );
let turn_context = TurnContext { let turn_context = TurnContext {
client, client,
tools_config: ToolsConfig::new( tools_config: ToolsConfig::new(&ToolsConfigParams {
&config.model_family, model_family: &config.model_family,
approval_policy, approval_policy,
sandbox_policy.clone(), sandbox_policy: sandbox_policy.clone(),
config.include_plan_tool, include_plan_tool: config.include_plan_tool,
config.include_apply_patch_tool, include_apply_patch_tool: config.include_apply_patch_tool,
config.tools_web_search_request, include_web_search_request: config.tools_web_search_request,
config.use_experimental_streamable_shell_tool, use_streamable_shell_tool: config.use_experimental_streamable_shell_tool,
), }),
user_instructions, user_instructions,
base_instructions, base_instructions,
approval_policy, approval_policy,
@@ -1092,15 +1093,15 @@ async fn submission_loop(
.unwrap_or(prev.sandbox_policy.clone()); .unwrap_or(prev.sandbox_policy.clone());
let new_cwd = cwd.clone().unwrap_or_else(|| prev.cwd.clone()); let new_cwd = cwd.clone().unwrap_or_else(|| prev.cwd.clone());
let tools_config = ToolsConfig::new( let tools_config = ToolsConfig::new(&ToolsConfigParams {
&effective_family, model_family: &effective_family,
new_approval_policy, approval_policy: new_approval_policy,
new_sandbox_policy.clone(), sandbox_policy: new_sandbox_policy.clone(),
config.include_plan_tool, include_plan_tool: config.include_plan_tool,
config.include_apply_patch_tool, include_apply_patch_tool: config.include_apply_patch_tool,
config.tools_web_search_request, include_web_search_request: config.tools_web_search_request,
config.use_experimental_streamable_shell_tool, use_streamable_shell_tool: config.use_experimental_streamable_shell_tool,
); });
let new_turn_context = TurnContext { let new_turn_context = TurnContext {
client, client,
@@ -1172,15 +1173,16 @@ async fn submission_loop(
let fresh_turn_context = TurnContext { let fresh_turn_context = TurnContext {
client, client,
tools_config: ToolsConfig::new( tools_config: ToolsConfig::new(&ToolsConfigParams {
&model_family, model_family: &model_family,
approval_policy, approval_policy,
sandbox_policy.clone(), sandbox_policy: sandbox_policy.clone(),
config.include_plan_tool, include_plan_tool: config.include_plan_tool,
config.include_apply_patch_tool, include_apply_patch_tool: config.include_apply_patch_tool,
config.tools_web_search_request, include_web_search_request: config.tools_web_search_request,
config.use_experimental_streamable_shell_tool, use_streamable_shell_tool: config
), .use_experimental_streamable_shell_tool,
}),
user_instructions: turn_context.user_instructions.clone(), user_instructions: turn_context.user_instructions.clone(),
base_instructions: turn_context.base_instructions.clone(), base_instructions: turn_context.base_instructions.clone(),
approval_policy, approval_policy,

182
codex-rs/core/src/openai_tools.rs
View File

@@ -62,24 +62,35 @@ pub enum ConfigShellToolType {
} }
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub struct ToolsConfig { pub(crate) struct ToolsConfig {
pub shell_type: ConfigShellToolType, pub shell_type: ConfigShellToolType,
pub plan_tool: bool, pub plan_tool: bool,
pub apply_patch_tool_type: Option<ApplyPatchToolType>, pub apply_patch_tool_type: Option<ApplyPatchToolType>,
pub web_search_request: bool, pub web_search_request: bool,
} }
pub(crate) struct ToolsConfigParams<'a> {
pub(crate) model_family: &'a ModelFamily,
pub(crate) approval_policy: AskForApproval,
pub(crate) sandbox_policy: SandboxPolicy,
pub(crate) include_plan_tool: bool,
pub(crate) include_apply_patch_tool: bool,
pub(crate) include_web_search_request: bool,
pub(crate) use_streamable_shell_tool: bool,
}
impl ToolsConfig { impl ToolsConfig {
pub fn new( pub fn new(params: &ToolsConfigParams) -> Self {
model_family: &ModelFamily, let ToolsConfigParams {
approval_policy: AskForApproval, model_family,
sandbox_policy: SandboxPolicy, approval_policy,
include_plan_tool: bool, sandbox_policy,
include_apply_patch_tool: bool, include_plan_tool,
include_web_search_request: bool, include_apply_patch_tool,
use_streamable_shell_tool: bool, include_web_search_request,
) -> Self { use_streamable_shell_tool,
let mut shell_type = if use_streamable_shell_tool { } = params;
let mut shell_type = if *use_streamable_shell_tool {
ConfigShellToolType::StreamableShell ConfigShellToolType::StreamableShell
} else if model_family.uses_local_shell_tool { } else if model_family.uses_local_shell_tool {
ConfigShellToolType::LocalShell ConfigShellToolType::LocalShell
@@ -96,7 +107,7 @@ impl ToolsConfig {
Some(ApplyPatchToolType::Freeform) => Some(ApplyPatchToolType::Freeform), Some(ApplyPatchToolType::Freeform) => Some(ApplyPatchToolType::Freeform),
Some(ApplyPatchToolType::Function) => Some(ApplyPatchToolType::Function), Some(ApplyPatchToolType::Function) => Some(ApplyPatchToolType::Function),
None => { None => {
if include_apply_patch_tool { if *include_apply_patch_tool {
Some(ApplyPatchToolType::Freeform) Some(ApplyPatchToolType::Freeform)
} else { } else {
None None
@@ -106,9 +117,9 @@ impl ToolsConfig {
Self { Self {
shell_type, shell_type,
plan_tool: include_plan_tool, plan_tool: *include_plan_tool,
apply_patch_tool_type, apply_patch_tool_type,
web_search_request: include_web_search_request, web_search_request: *include_web_search_request,
} }
} }
} }
@@ -585,15 +596,15 @@ mod tests {
fn test_get_openai_tools() { fn test_get_openai_tools() {
let model_family = find_family_for_model("codex-mini-latest") let model_family = find_family_for_model("codex-mini-latest")
.expect("codex-mini-latest should be a valid model family"); .expect("codex-mini-latest should be a valid model family");
let config = ToolsConfig::new( let config = ToolsConfig::new(&ToolsConfigParams {
&model_family, model_family: &model_family,
AskForApproval::Never, approval_policy: AskForApproval::Never,
SandboxPolicy::ReadOnly, sandbox_policy: SandboxPolicy::ReadOnly,
true, include_plan_tool: true,
false, include_apply_patch_tool: false,
true, include_web_search_request: true,
/*use_experimental_streamable_shell_tool*/ false, use_streamable_shell_tool: false,
); });
let tools = get_openai_tools(&config, Some(HashMap::new())); let tools = get_openai_tools(&config, Some(HashMap::new()));
assert_eq_tool_names(&tools, &["local_shell", "update_plan", "web_search"]); assert_eq_tool_names(&tools, &["local_shell", "update_plan", "web_search"]);
@@ -602,15 +613,15 @@ mod tests {
#[test] #[test]
fn test_get_openai_tools_default_shell() { fn test_get_openai_tools_default_shell() {
let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let model_family = find_family_for_model("o3").expect("o3 should be a valid model family");
let config = ToolsConfig::new( let config = ToolsConfig::new(&ToolsConfigParams {
&model_family, model_family: &model_family,
AskForApproval::Never, approval_policy: AskForApproval::Never,
SandboxPolicy::ReadOnly, sandbox_policy: SandboxPolicy::ReadOnly,
true, include_plan_tool: true,
false, include_apply_patch_tool: false,
true, include_web_search_request: true,
/*use_experimental_streamable_shell_tool*/ false, use_streamable_shell_tool: false,
); });
let tools = get_openai_tools(&config, Some(HashMap::new())); let tools = get_openai_tools(&config, Some(HashMap::new()));
assert_eq_tool_names(&tools, &["shell", "update_plan", "web_search"]); assert_eq_tool_names(&tools, &["shell", "update_plan", "web_search"]);
@@ -619,15 +630,15 @@ mod tests {
#[test] #[test]
fn test_get_openai_tools_mcp_tools() { fn test_get_openai_tools_mcp_tools() {
let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let model_family = find_family_for_model("o3").expect("o3 should be a valid model family");
let config = ToolsConfig::new( let config = ToolsConfig::new(&ToolsConfigParams {
&model_family, model_family: &model_family,
AskForApproval::Never, approval_policy: AskForApproval::Never,
SandboxPolicy::ReadOnly, sandbox_policy: SandboxPolicy::ReadOnly,
false, include_plan_tool: false,
false, include_apply_patch_tool: false,
true, include_web_search_request: true,
/*use_experimental_streamable_shell_tool*/ false, use_streamable_shell_tool: false,
); });
let tools = get_openai_tools( let tools = get_openai_tools(
&config, &config,
Some(HashMap::from([( Some(HashMap::from([(
@@ -718,14 +729,15 @@ mod tests {
#[test] #[test]
fn test_get_openai_tools_mcp_tools_sorted_by_name() { fn test_get_openai_tools_mcp_tools_sorted_by_name() {
let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let model_family = find_family_for_model("o3").expect("o3 should be a valid model family");
let config = ToolsConfig::new( let config = ToolsConfig::new(&ToolsConfigParams {
&model_family, model_family: &model_family,
AskForApproval::Never, approval_policy: AskForApproval::Never,
SandboxPolicy::ReadOnly, sandbox_policy: SandboxPolicy::ReadOnly,
false, include_plan_tool: false,
false, include_apply_patch_tool: false,
/*use_experimental_streamable_shell_tool*/ false, include_web_search_request: false,
); use_streamable_shell_tool: false,
});
// Intentionally construct a map with keys that would sort alphabetically. // Intentionally construct a map with keys that would sort alphabetically.
let tools_map: HashMap<String, mcp_types::Tool> = HashMap::from([ let tools_map: HashMap<String, mcp_types::Tool> = HashMap::from([
@@ -792,15 +804,15 @@ mod tests {
#[test] #[test]
fn test_mcp_tool_property_missing_type_defaults_to_string() { fn test_mcp_tool_property_missing_type_defaults_to_string() {
let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let model_family = find_family_for_model("o3").expect("o3 should be a valid model family");
let config = ToolsConfig::new( let config = ToolsConfig::new(&ToolsConfigParams {
&model_family, model_family: &model_family,
AskForApproval::Never, approval_policy: AskForApproval::Never,
SandboxPolicy::ReadOnly, sandbox_policy: SandboxPolicy::ReadOnly,
false, include_plan_tool: false,
false, include_apply_patch_tool: false,
true, include_web_search_request: true,
/*use_experimental_streamable_shell_tool*/ false, use_streamable_shell_tool: false,
); });
let tools = get_openai_tools( let tools = get_openai_tools(
&config, &config,
@@ -850,15 +862,15 @@ mod tests {
#[test] #[test]
fn test_mcp_tool_integer_normalized_to_number() { fn test_mcp_tool_integer_normalized_to_number() {
let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let model_family = find_family_for_model("o3").expect("o3 should be a valid model family");
let config = ToolsConfig::new( let config = ToolsConfig::new(&ToolsConfigParams {
&model_family, model_family: &model_family,
AskForApproval::Never, approval_policy: AskForApproval::Never,
SandboxPolicy::ReadOnly, sandbox_policy: SandboxPolicy::ReadOnly,
false, include_plan_tool: false,
false, include_apply_patch_tool: false,
true, include_web_search_request: true,
/*use_experimental_streamable_shell_tool*/ false, use_streamable_shell_tool: false,
); });
let tools = get_openai_tools( let tools = get_openai_tools(
&config, &config,
@@ -903,15 +915,15 @@ mod tests {
#[test] #[test]
fn test_mcp_tool_array_without_items_gets_default_string_items() { fn test_mcp_tool_array_without_items_gets_default_string_items() {
let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let model_family = find_family_for_model("o3").expect("o3 should be a valid model family");
let config = ToolsConfig::new( let config = ToolsConfig::new(&ToolsConfigParams {
&model_family, model_family: &model_family,
AskForApproval::Never, approval_policy: AskForApproval::Never,
SandboxPolicy::ReadOnly, sandbox_policy: SandboxPolicy::ReadOnly,
false, include_plan_tool: false,
false, include_apply_patch_tool: false,
true, include_web_search_request: true,
/*use_experimental_streamable_shell_tool*/ false, use_streamable_shell_tool: false,
); });
let tools = get_openai_tools( let tools = get_openai_tools(
&config, &config,
@@ -959,15 +971,15 @@ mod tests {
#[test] #[test]
fn test_mcp_tool_anyof_defaults_to_string() { fn test_mcp_tool_anyof_defaults_to_string() {
let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let model_family = find_family_for_model("o3").expect("o3 should be a valid model family");
let config = ToolsConfig::new( let config = ToolsConfig::new(&ToolsConfigParams {
&model_family, model_family: &model_family,
AskForApproval::Never, approval_policy: AskForApproval::Never,
SandboxPolicy::ReadOnly, sandbox_policy: SandboxPolicy::ReadOnly,
false, include_plan_tool: false,
false, include_apply_patch_tool: false,
true, include_web_search_request: true,
/*use_experimental_streamable_shell_tool*/ false, use_streamable_shell_tool: false,
); });
let tools = get_openai_tools( let tools = get_openai_tools(
&config, &config,