mirror of
https://github.com/openai/codex.git
synced 2026-11-11 21:10:39 +00:00
refactor: hide shell override for zsh fork unified exec
This commit is contained in:
@@ -13,12 +13,15 @@ pub struct CommandToolOptions {
|
||||
|
||||
#[cfg(test)]
|
||||
pub fn create_exec_command_tool(options: CommandToolOptions) -> ToolSpec {
|
||||
create_exec_command_tool_with_environment_id(options, /*include_environment_id*/ false)
|
||||
create_exec_command_tool_with_environment_id(
|
||||
options, /*include_environment_id*/ false, /*include_shell_parameter*/ true,
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn create_exec_command_tool_with_environment_id(
|
||||
options: CommandToolOptions,
|
||||
include_environment_id: bool,
|
||||
include_shell_parameter: bool,
|
||||
) -> ToolSpec {
|
||||
let mut properties = BTreeMap::from([
|
||||
(
|
||||
@@ -32,12 +35,6 @@ pub(crate) fn create_exec_command_tool_with_environment_id(
|
||||
.to_string(),
|
||||
)),
|
||||
),
|
||||
(
|
||||
"shell".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Shell binary to launch. Defaults to the user's default shell.".to_string(),
|
||||
)),
|
||||
),
|
||||
(
|
||||
"tty".to_string(),
|
||||
JsonSchema::boolean(Some(
|
||||
@@ -58,6 +55,14 @@ pub(crate) fn create_exec_command_tool_with_environment_id(
|
||||
)),
|
||||
),
|
||||
]);
|
||||
if include_shell_parameter {
|
||||
properties.insert(
|
||||
"shell".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Shell binary to launch. Defaults to the user's default shell.".to_string(),
|
||||
)),
|
||||
);
|
||||
}
|
||||
if options.allow_login_shell {
|
||||
properties.insert(
|
||||
"login".to_string(),
|
||||
|
||||
@@ -6,6 +6,13 @@ fn windows_shell_guidance_description() -> String {
|
||||
format!("\n\n{}", windows_shell_guidance())
|
||||
}
|
||||
|
||||
fn has_parameter(tool: &ToolSpec, parameter_name: &str) -> bool {
|
||||
serde_json::to_value(tool)
|
||||
.expect("tool spec should serialize")
|
||||
.pointer(&format!("/parameters/properties/{parameter_name}"))
|
||||
.is_some()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_command_tool_matches_expected_spec() {
|
||||
let tool = create_exec_command_tool(CommandToolOptions {
|
||||
@@ -89,6 +96,21 @@ fn exec_command_tool_matches_expected_spec() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_command_tool_can_hide_shell_parameter() {
|
||||
let tool = create_exec_command_tool_with_environment_id(
|
||||
CommandToolOptions {
|
||||
allow_login_shell: true,
|
||||
exec_permission_approvals_enabled: false,
|
||||
},
|
||||
/*include_environment_id*/ false,
|
||||
/*include_shell_parameter*/ false,
|
||||
);
|
||||
|
||||
assert!(!has_parameter(&tool, "shell"));
|
||||
assert!(has_parameter(&tool, "cmd"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_stdin_tool_matches_expected_spec() {
|
||||
let tool = create_write_stdin_tool();
|
||||
|
||||
@@ -44,6 +44,7 @@ pub(crate) struct ExecCommandHandlerOptions {
|
||||
pub(crate) allow_login_shell: bool,
|
||||
pub(crate) exec_permission_approvals_enabled: bool,
|
||||
pub(crate) include_environment_id: bool,
|
||||
pub(crate) include_shell_parameter: bool,
|
||||
}
|
||||
|
||||
pub struct ExecCommandHandler {
|
||||
@@ -57,6 +58,7 @@ impl Default for ExecCommandHandler {
|
||||
allow_login_shell: false,
|
||||
exec_permission_approvals_enabled: false,
|
||||
include_environment_id: false,
|
||||
include_shell_parameter: true,
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -81,6 +83,7 @@ impl ToolExecutor<ToolInvocation> for ExecCommandHandler {
|
||||
exec_permission_approvals_enabled: self.options.exec_permission_approvals_enabled,
|
||||
},
|
||||
self.options.include_environment_id,
|
||||
self.options.include_shell_parameter,
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -72,6 +72,7 @@ use codex_tools::ToolExecutor;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolOutput;
|
||||
use codex_tools::ToolSpec;
|
||||
use codex_tools::UnifiedExecShellMode;
|
||||
use codex_tools::can_request_original_image_detail;
|
||||
use codex_tools::collect_code_mode_exec_prompt_tool_definitions;
|
||||
use codex_tools::collect_request_plugin_install_entries;
|
||||
@@ -566,6 +567,10 @@ fn add_shell_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut Planne
|
||||
allow_login_shell,
|
||||
exec_permission_approvals_enabled,
|
||||
include_environment_id,
|
||||
include_shell_parameter: !matches!(
|
||||
&turn_context.unified_exec_shell_mode,
|
||||
UnifiedExecShellMode::ZshFork(_)
|
||||
),
|
||||
}));
|
||||
planned_tools.add(WriteStdinHandler);
|
||||
|
||||
|
||||
@@ -380,6 +380,7 @@ async fn shell_family_registers_visible_unified_exec_and_hidden_legacy_shell() {
|
||||
plan.assert_visible_lacks(&["shell_command"]);
|
||||
plan.assert_registered_contains(&["exec_command", "write_stdin", "shell_command"]);
|
||||
assert_eq!(plan.exposure("shell_command"), ToolExposure::Hidden);
|
||||
assert!(has_parameter(plan.visible_spec("exec_command"), "shell"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -422,6 +423,39 @@ async fn shell_zsh_fork_stays_standalone_until_unified_exec_composition_is_enabl
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn zsh_fork_unified_exec_hides_shell_parameter() {
|
||||
if !codex_utils_pty::conpty_supported() {
|
||||
return;
|
||||
}
|
||||
|
||||
let exe = codex_utils_absolute_path::AbsolutePathBuf::try_from(
|
||||
std::env::current_exe().expect("current exe path"),
|
||||
)
|
||||
.expect("current exe should be absolute");
|
||||
|
||||
let plan = probe(|turn| {
|
||||
set_features(
|
||||
turn,
|
||||
&[
|
||||
Feature::ShellTool,
|
||||
Feature::UnifiedExec,
|
||||
Feature::ShellZshFork,
|
||||
Feature::UnifiedExecZshFork,
|
||||
],
|
||||
);
|
||||
turn.unified_exec_shell_mode =
|
||||
codex_tools::UnifiedExecShellMode::ZshFork(codex_tools::ZshForkConfig {
|
||||
shell_zsh_path: exe.clone(),
|
||||
main_execve_wrapper_exe: exe,
|
||||
});
|
||||
})
|
||||
.await;
|
||||
|
||||
plan.assert_visible_contains(&["exec_command", "write_stdin"]);
|
||||
assert!(!has_parameter(plan.visible_spec("exec_command"), "shell"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn environment_count_controls_environment_backed_tools() {
|
||||
let no_environment = probe(|turn| {
|
||||
|
||||
Reference in New Issue
Block a user