Merge 2ab9ff698e into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2025-08-24 22:34:50 -07:00
committed by GitHub
5 changed files with 205 additions and 111 deletions

7
codex-rs/Cargo.lock generated
View File

@@ -2722,6 +2722,7 @@ checksum = "4488594b9328dee448adb906d8b126d9b7deb7cf5c22161ee591610bb1be83c0"
dependencies = [
"bitflags 2.9.1",
"libc",
"redox_syscall",
]
[[package]]
@@ -5777,11 +5778,11 @@ dependencies = [
[[package]]
name = "whoami"
version = "1.6.0"
version = "1.6.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6994d13118ab492c3c80c1f81928718159254c53c472bf9ce36f8dae4add02a7"
checksum = "5d4a4db5077702ca3015d3d02d74974948aba2ad9e12ab7df718ee64ccd7e97d"
dependencies = [
"redox_syscall",
"libredox",
"wasite",
"web-sys",
]

View File

@@ -57,7 +57,7 @@ tracing = { version = "0.1.41", features = ["log"] }
tree-sitter = "0.25.8"
tree-sitter-bash = "0.25.0"
uuid = { version = "1", features = ["serde", "v4"] }
whoami = "1.6.0"
whoami = "1.6.1"
wildmatch = "2.4.0"

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

View File

@@ -79,13 +79,13 @@ pub(crate) fn get_model_info(model_family: &ModelFamily) -> Option<ModelInfo> {
}),
"gpt-5" => Some(ModelInfo {
context_window: 200_000,
max_output_tokens: 100_000,
context_window: 400_000,
max_output_tokens: 128_000,
}),
_ if slug.starts_with("codex-") => Some(ModelInfo {
context_window: 200_000,
max_output_tokens: 100_000,
context_window: 400_000,
max_output_tokens: 128_000,
}),
_ => None,

View File

@@ -62,24 +62,35 @@ pub enum ConfigShellToolType {
}
#[derive(Debug, Clone)]
pub struct ToolsConfig {
pub(crate) struct ToolsConfig {
pub shell_type: ConfigShellToolType,
pub plan_tool: bool,
pub apply_patch_tool_type: Option<ApplyPatchToolType>,
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 {
pub fn new(
model_family: &ModelFamily,
approval_policy: AskForApproval,
sandbox_policy: SandboxPolicy,
include_plan_tool: bool,
include_apply_patch_tool: bool,
include_web_search_request: bool,
use_streamable_shell_tool: bool,
) -> Self {
let mut shell_type = if use_streamable_shell_tool {
pub fn new(params: &ToolsConfigParams) -> Self {
let ToolsConfigParams {
model_family,
approval_policy,
sandbox_policy,
include_plan_tool,
include_apply_patch_tool,
include_web_search_request,
use_streamable_shell_tool,
} = params;
let mut shell_type = if *use_streamable_shell_tool {
ConfigShellToolType::StreamableShell
} else if model_family.uses_local_shell_tool {
ConfigShellToolType::LocalShell
@@ -96,7 +107,7 @@ impl ToolsConfig {
Some(ApplyPatchToolType::Freeform) => Some(ApplyPatchToolType::Freeform),
Some(ApplyPatchToolType::Function) => Some(ApplyPatchToolType::Function),
None => {
if include_apply_patch_tool {
if *include_apply_patch_tool {
Some(ApplyPatchToolType::Freeform)
} else {
None
@@ -106,9 +117,9 @@ impl ToolsConfig {
Self {
shell_type,
plan_tool: include_plan_tool,
plan_tool: *include_plan_tool,
apply_patch_tool_type,
web_search_request: include_web_search_request,
web_search_request: *include_web_search_request,
}
}
}
@@ -531,7 +542,12 @@ pub(crate) fn get_openai_tools(
}
if let Some(mcp_tools) = mcp_tools {
for (name, tool) in mcp_tools {
// Ensure deterministic ordering to maximize prompt cache hits.
// HashMap iteration order is non-deterministic, so sort by fully-qualified tool name.
let mut entries: Vec<(String, mcp_types::Tool)> = mcp_tools.into_iter().collect();
entries.sort_by(|a, b| a.0.cmp(&b.0));
for (name, tool) in entries.into_iter() {
match mcp_tool_to_openai_tool(name.clone(), tool.clone()) {
Ok(converted_tool) => tools.push(OpenAiTool::Function(converted_tool)),
Err(e) => {
@@ -580,15 +596,15 @@ mod tests {
fn test_get_openai_tools() {
let model_family = find_family_for_model("codex-mini-latest")
.expect("codex-mini-latest should be a valid model family");
let config = ToolsConfig::new(
&model_family,
AskForApproval::Never,
SandboxPolicy::ReadOnly,
true,
false,
true,
/*use_experimental_streamable_shell_tool*/ false,
);
let config = ToolsConfig::new(&ToolsConfigParams {
model_family: &model_family,
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
include_plan_tool: true,
include_apply_patch_tool: false,
include_web_search_request: true,
use_streamable_shell_tool: false,
});
let tools = get_openai_tools(&config, Some(HashMap::new()));
assert_eq_tool_names(&tools, &["local_shell", "update_plan", "web_search"]);
@@ -597,15 +613,15 @@ mod tests {
#[test]
fn test_get_openai_tools_default_shell() {
let model_family = find_family_for_model("o3").expect("o3 should be a valid model family");
let config = ToolsConfig::new(
&model_family,
AskForApproval::Never,
SandboxPolicy::ReadOnly,
true,
false,
true,
/*use_experimental_streamable_shell_tool*/ false,
);
let config = ToolsConfig::new(&ToolsConfigParams {
model_family: &model_family,
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
include_plan_tool: true,
include_apply_patch_tool: false,
include_web_search_request: true,
use_streamable_shell_tool: false,
});
let tools = get_openai_tools(&config, Some(HashMap::new()));
assert_eq_tool_names(&tools, &["shell", "update_plan", "web_search"]);
@@ -614,15 +630,15 @@ mod tests {
#[test]
fn test_get_openai_tools_mcp_tools() {
let model_family = find_family_for_model("o3").expect("o3 should be a valid model family");
let config = ToolsConfig::new(
&model_family,
AskForApproval::Never,
SandboxPolicy::ReadOnly,
false,
false,
true,
/*use_experimental_streamable_shell_tool*/ false,
);
let config = ToolsConfig::new(&ToolsConfigParams {
model_family: &model_family,
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
include_plan_tool: false,
include_apply_patch_tool: false,
include_web_search_request: true,
use_streamable_shell_tool: false,
});
let tools = get_openai_tools(
&config,
Some(HashMap::from([(
@@ -710,18 +726,93 @@ mod tests {
);
}
#[test]
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 config = ToolsConfig::new(&ToolsConfigParams {
model_family: &model_family,
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
include_plan_tool: false,
include_apply_patch_tool: false,
include_web_search_request: false,
use_streamable_shell_tool: false,
});
// Intentionally construct a map with keys that would sort alphabetically.
let tools_map: HashMap<String, mcp_types::Tool> = HashMap::from([
(
"test_server/do".to_string(),
mcp_types::Tool {
name: "a".to_string(),
input_schema: ToolInputSchema {
properties: Some(serde_json::json!({})),
required: None,
r#type: "object".to_string(),
},
output_schema: None,
title: None,
annotations: None,
description: Some("a".to_string()),
},
),
(
"test_server/something".to_string(),
mcp_types::Tool {
name: "b".to_string(),
input_schema: ToolInputSchema {
properties: Some(serde_json::json!({})),
required: None,
r#type: "object".to_string(),
},
output_schema: None,
title: None,
annotations: None,
description: Some("b".to_string()),
},
),
(
"test_server/cool".to_string(),
mcp_types::Tool {
name: "c".to_string(),
input_schema: ToolInputSchema {
properties: Some(serde_json::json!({})),
required: None,
r#type: "object".to_string(),
},
output_schema: None,
title: None,
annotations: None,
description: Some("c".to_string()),
},
),
]);
let tools = get_openai_tools(&config, Some(tools_map));
// Expect shell first, followed by MCP tools sorted by fully-qualified name.
assert_eq_tool_names(
&tools,
&[
"shell",
"test_server/cool",
"test_server/do",
"test_server/something",
],
);
}
#[test]
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 config = ToolsConfig::new(
&model_family,
AskForApproval::Never,
SandboxPolicy::ReadOnly,
false,
false,
true,
/*use_experimental_streamable_shell_tool*/ false,
);
let config = ToolsConfig::new(&ToolsConfigParams {
model_family: &model_family,
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
include_plan_tool: false,
include_apply_patch_tool: false,
include_web_search_request: true,
use_streamable_shell_tool: false,
});
let tools = get_openai_tools(
&config,
@@ -771,15 +862,15 @@ mod tests {
#[test]
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 config = ToolsConfig::new(
&model_family,
AskForApproval::Never,
SandboxPolicy::ReadOnly,
false,
false,
true,
/*use_experimental_streamable_shell_tool*/ false,
);
let config = ToolsConfig::new(&ToolsConfigParams {
model_family: &model_family,
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
include_plan_tool: false,
include_apply_patch_tool: false,
include_web_search_request: true,
use_streamable_shell_tool: false,
});
let tools = get_openai_tools(
&config,
@@ -824,15 +915,15 @@ mod tests {
#[test]
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 config = ToolsConfig::new(
&model_family,
AskForApproval::Never,
SandboxPolicy::ReadOnly,
false,
false,
true,
/*use_experimental_streamable_shell_tool*/ false,
);
let config = ToolsConfig::new(&ToolsConfigParams {
model_family: &model_family,
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
include_plan_tool: false,
include_apply_patch_tool: false,
include_web_search_request: true,
use_streamable_shell_tool: false,
});
let tools = get_openai_tools(
&config,
@@ -880,15 +971,15 @@ mod tests {
#[test]
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 config = ToolsConfig::new(
&model_family,
AskForApproval::Never,
SandboxPolicy::ReadOnly,
false,
false,
true,
/*use_experimental_streamable_shell_tool*/ false,
);
let config = ToolsConfig::new(&ToolsConfigParams {
model_family: &model_family,
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
include_plan_tool: false,
include_apply_patch_tool: false,
include_web_search_request: true,
use_streamable_shell_tool: false,
});
let tools = get_openai_tools(
&config,