mirror of
https://github.com/openai/codex.git
synced 2026-10-23 18:08:45 +00:00
Gate multi-environment process tool model surface
Keep the legacy single-environment surface unchanged, expose environment_id only when multiple turn environments are selected, and add coverage plus a probe for the multi-environment model surface. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -1982,6 +1982,106 @@ async fn turn_start_resolves_sticky_thread_environments_and_turn_overrides() ->
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_start_model_surface_gates_multi_environment_process_tools() -> Result<()> {
|
||||
let tmp = TempDir::new()?;
|
||||
let codex_home = tmp.path().join("codex_home");
|
||||
std::fs::create_dir(&codex_home)?;
|
||||
let workspace = tmp.path().join("workspace");
|
||||
std::fs::create_dir(&workspace)?;
|
||||
|
||||
let server = create_mock_responses_server_repeating_assistant("done").await;
|
||||
create_config_toml(
|
||||
&codex_home,
|
||||
&server.uri(),
|
||||
"never",
|
||||
&BTreeMap::from([(Feature::UnifiedExec, true)]),
|
||||
)?;
|
||||
|
||||
let mut mcp = McpProcess::new_with_env(
|
||||
&codex_home,
|
||||
&[("CODEX_EXEC_SERVER_URL", Some("http://127.0.0.1:1"))],
|
||||
)
|
||||
.await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
run_model_surface_turn(&mut mcp, &workspace, &["local"], "single").await?;
|
||||
let requests = server
|
||||
.received_requests()
|
||||
.await
|
||||
.expect("failed to fetch received requests");
|
||||
let single_request = requests.last().expect("single-env request");
|
||||
assert!(body_contains(single_request, "<cwd>"));
|
||||
assert!(!body_contains(single_request, "<environments>"));
|
||||
assert!(!body_contains(single_request, "environment_id"));
|
||||
|
||||
run_model_surface_turn(&mut mcp, &workspace, &["local", "remote"], "multi").await?;
|
||||
let requests = server
|
||||
.received_requests()
|
||||
.await
|
||||
.expect("failed to fetch received requests");
|
||||
let multi_request = requests.last().expect("multi-env request");
|
||||
assert!(body_contains(multi_request, "<environments>"));
|
||||
assert!(body_contains(
|
||||
multi_request,
|
||||
r#"<environment id=\"local\" primary=\"true\">"#
|
||||
));
|
||||
assert!(body_contains(
|
||||
multi_request,
|
||||
r#"<environment id=\"remote\" primary=\"false\">"#
|
||||
));
|
||||
assert!(body_contains(multi_request, "environment_id"));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn run_model_surface_turn(
|
||||
mcp: &mut McpProcess,
|
||||
workspace: &Path,
|
||||
environments: &[&str],
|
||||
text: &str,
|
||||
) -> Result<()> {
|
||||
let thread_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
cwd: Some(workspace.to_string_lossy().into_owned()),
|
||||
environments: environment_params(Some(environments), workspace)?,
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let thread_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(thread_req)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
|
||||
|
||||
let turn_req = mcp
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
thread_id: thread.id,
|
||||
input: vec![V2UserInput::Text {
|
||||
text: text.to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
cwd: Some(workspace.to_path_buf()),
|
||||
model: Some("mock-model".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(turn_req)),
|
||||
)
|
||||
.await??;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
mcp.clear_message_buffer();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
struct EnvironmentSelectionCase {
|
||||
name: &'static str,
|
||||
sticky: Option<&'static [&'static str]>,
|
||||
|
||||
@@ -76,6 +76,7 @@ async fn build_arc_monitor_request_includes_relevant_history_and_null_policies()
|
||||
crate::context::EnvironmentContext::new(
|
||||
Some(PathBuf::from("/tmp")),
|
||||
"zsh".to_string(),
|
||||
Vec::new(),
|
||||
/*current_date*/ None,
|
||||
/*timezone*/ None,
|
||||
/*network*/ None,
|
||||
|
||||
@@ -10,12 +10,50 @@ use super::ContextualUserFragment;
|
||||
pub(crate) struct EnvironmentContext {
|
||||
pub(crate) cwd: Option<PathBuf>,
|
||||
pub(crate) shell: String,
|
||||
pub(crate) environments: Vec<EnvironmentContextEnvironment>,
|
||||
pub(crate) current_date: Option<String>,
|
||||
pub(crate) timezone: Option<String>,
|
||||
pub(crate) network: Option<NetworkContext>,
|
||||
pub(crate) subagents: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) struct EnvironmentContextEnvironment {
|
||||
pub(crate) id: String,
|
||||
pub(crate) cwd: PathBuf,
|
||||
pub(crate) primary: bool,
|
||||
}
|
||||
|
||||
impl EnvironmentContextEnvironment {
|
||||
fn from_turn_environments(
|
||||
environments: &[crate::session::turn_context::TurnEnvironment],
|
||||
) -> Vec<Self> {
|
||||
environments
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(index, environment)| Self {
|
||||
id: environment.environment_id.clone(),
|
||||
cwd: environment.cwd.to_path_buf(),
|
||||
primary: index == 0,
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn from_selected_environments(
|
||||
environments: &[codex_protocol::protocol::TurnEnvironmentSelection],
|
||||
) -> Vec<Self> {
|
||||
environments
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(index, environment)| Self {
|
||||
id: environment.environment_id.clone(),
|
||||
cwd: environment.cwd.to_path_buf(),
|
||||
primary: index == 0,
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Default)]
|
||||
pub(crate) struct NetworkContext {
|
||||
allowed_domains: Vec<String>,
|
||||
@@ -35,6 +73,7 @@ impl EnvironmentContext {
|
||||
pub(crate) fn new(
|
||||
cwd: Option<PathBuf>,
|
||||
shell: String,
|
||||
environments: Vec<EnvironmentContextEnvironment>,
|
||||
current_date: Option<String>,
|
||||
timezone: Option<String>,
|
||||
network: Option<NetworkContext>,
|
||||
@@ -43,6 +82,7 @@ impl EnvironmentContext {
|
||||
Self {
|
||||
cwd,
|
||||
shell,
|
||||
environments,
|
||||
current_date,
|
||||
timezone,
|
||||
network,
|
||||
@@ -56,6 +96,7 @@ impl EnvironmentContext {
|
||||
pub(crate) fn equals_except_shell(&self, other: &EnvironmentContext) -> bool {
|
||||
let EnvironmentContext {
|
||||
cwd,
|
||||
environments,
|
||||
current_date,
|
||||
timezone,
|
||||
network,
|
||||
@@ -63,6 +104,7 @@ impl EnvironmentContext {
|
||||
shell: _,
|
||||
} = other;
|
||||
self.cwd == *cwd
|
||||
&& self.environments == *environments
|
||||
&& self.current_date == *current_date
|
||||
&& self.timezone == *timezone
|
||||
&& self.network == *network
|
||||
@@ -83,9 +125,19 @@ impl EnvironmentContext {
|
||||
} else {
|
||||
before_network
|
||||
};
|
||||
let before_had_environments = before
|
||||
.environments
|
||||
.as_ref()
|
||||
.is_some_and(|environments| environments.len() > 1);
|
||||
let environments = if before_had_environments || after.environments.len() > 1 {
|
||||
after.environments.clone()
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
EnvironmentContext::new(
|
||||
cwd,
|
||||
after.shell.clone(),
|
||||
environments,
|
||||
after.current_date.clone(),
|
||||
after.timezone.clone(),
|
||||
network,
|
||||
@@ -97,6 +149,7 @@ impl EnvironmentContext {
|
||||
Self::new(
|
||||
Some(turn_context.cwd.to_path_buf()),
|
||||
shell.name().to_string(),
|
||||
EnvironmentContextEnvironment::from_turn_environments(&turn_context.environments),
|
||||
turn_context.current_date.clone(),
|
||||
turn_context.timezone.clone(),
|
||||
Self::network_from_turn_context(turn_context),
|
||||
@@ -108,9 +161,16 @@ impl EnvironmentContext {
|
||||
turn_context_item: &TurnContextItem,
|
||||
shell: String,
|
||||
) -> Self {
|
||||
let environments = turn_context_item
|
||||
.environments
|
||||
.as_deref()
|
||||
.filter(|environments| environments.len() > 1)
|
||||
.map(EnvironmentContextEnvironment::from_selected_environments)
|
||||
.unwrap_or_default();
|
||||
Self::new(
|
||||
Some(turn_context_item.cwd.clone()),
|
||||
shell,
|
||||
environments,
|
||||
turn_context_item.current_date.clone(),
|
||||
turn_context_item.timezone.clone(),
|
||||
Self::network_from_turn_context_item(turn_context_item),
|
||||
@@ -168,8 +228,27 @@ impl ContextualUserFragment for EnvironmentContext {
|
||||
|
||||
fn body(&self) -> String {
|
||||
let mut lines = Vec::new();
|
||||
if let Some(cwd) = &self.cwd {
|
||||
lines.push(format!(" <cwd>{}</cwd>", cwd.to_string_lossy()));
|
||||
if self.environments.len() <= 1 {
|
||||
// Only change the model-facing surface when more than one selected
|
||||
// environment is in use. Zero- and single-environment turns keep
|
||||
// the existing cwd/shell shape.
|
||||
if let Some(cwd) = &self.cwd {
|
||||
lines.push(format!(" <cwd>{}</cwd>", cwd.to_string_lossy()));
|
||||
}
|
||||
} else {
|
||||
lines.push(" <environments>".to_string());
|
||||
for environment in &self.environments {
|
||||
lines.push(format!(
|
||||
" <environment id=\"{}\" primary=\"{}\">",
|
||||
environment.id, environment.primary
|
||||
));
|
||||
lines.push(format!(
|
||||
" <cwd>{}</cwd>",
|
||||
environment.cwd.to_string_lossy()
|
||||
));
|
||||
lines.push(" </environment>".to_string());
|
||||
}
|
||||
lines.push(" </environments>".to_string());
|
||||
}
|
||||
|
||||
lines.push(format!(" <shell>{}</shell>", self.shell));
|
||||
|
||||
@@ -20,6 +20,7 @@ fn serialize_workspace_write_environment_context() {
|
||||
let context = EnvironmentContext::new(
|
||||
Some(cwd.clone()),
|
||||
fake_shell_name(),
|
||||
Vec::new(),
|
||||
Some("2026-02-26".to_string()),
|
||||
Some("America/Los_Angeles".to_string()),
|
||||
/*network*/ None,
|
||||
@@ -48,6 +49,7 @@ fn serialize_environment_context_with_network() {
|
||||
let context = EnvironmentContext::new(
|
||||
Some(test_path_buf("/repo")),
|
||||
fake_shell_name(),
|
||||
Vec::new(),
|
||||
Some("2026-02-26".to_string()),
|
||||
Some("America/Los_Angeles".to_string()),
|
||||
Some(network),
|
||||
@@ -77,6 +79,7 @@ fn serialize_read_only_environment_context() {
|
||||
let context = EnvironmentContext::new(
|
||||
/*cwd*/ None,
|
||||
fake_shell_name(),
|
||||
Vec::new(),
|
||||
Some("2026-02-26".to_string()),
|
||||
Some("America/Los_Angeles".to_string()),
|
||||
/*network*/ None,
|
||||
@@ -97,6 +100,7 @@ fn equals_except_shell_compares_cwd() {
|
||||
let context1 = EnvironmentContext::new(
|
||||
Some(PathBuf::from("/repo")),
|
||||
fake_shell_name(),
|
||||
Vec::new(),
|
||||
/*current_date*/ None,
|
||||
/*timezone*/ None,
|
||||
/*network*/ None,
|
||||
@@ -105,6 +109,7 @@ fn equals_except_shell_compares_cwd() {
|
||||
let context2 = EnvironmentContext::new(
|
||||
Some(PathBuf::from("/repo")),
|
||||
fake_shell_name(),
|
||||
Vec::new(),
|
||||
/*current_date*/ None,
|
||||
/*timezone*/ None,
|
||||
/*network*/ None,
|
||||
@@ -118,6 +123,7 @@ fn equals_except_shell_compares_cwd_differences() {
|
||||
let context1 = EnvironmentContext::new(
|
||||
Some(PathBuf::from("/repo1")),
|
||||
fake_shell_name(),
|
||||
Vec::new(),
|
||||
/*current_date*/ None,
|
||||
/*timezone*/ None,
|
||||
/*network*/ None,
|
||||
@@ -126,6 +132,7 @@ fn equals_except_shell_compares_cwd_differences() {
|
||||
let context2 = EnvironmentContext::new(
|
||||
Some(PathBuf::from("/repo2")),
|
||||
fake_shell_name(),
|
||||
Vec::new(),
|
||||
/*current_date*/ None,
|
||||
/*timezone*/ None,
|
||||
/*network*/ None,
|
||||
@@ -140,6 +147,7 @@ fn equals_except_shell_ignores_shell() {
|
||||
let context1 = EnvironmentContext::new(
|
||||
Some(PathBuf::from("/repo")),
|
||||
"bash".to_string(),
|
||||
Vec::new(),
|
||||
/*current_date*/ None,
|
||||
/*timezone*/ None,
|
||||
/*network*/ None,
|
||||
@@ -148,6 +156,7 @@ fn equals_except_shell_ignores_shell() {
|
||||
let context2 = EnvironmentContext::new(
|
||||
Some(PathBuf::from("/repo")),
|
||||
"zsh".to_string(),
|
||||
Vec::new(),
|
||||
/*current_date*/ None,
|
||||
/*timezone*/ None,
|
||||
/*network*/ None,
|
||||
@@ -162,6 +171,7 @@ fn serialize_environment_context_with_subagents() {
|
||||
let context = EnvironmentContext::new(
|
||||
Some(test_path_buf("/repo")),
|
||||
fake_shell_name(),
|
||||
Vec::new(),
|
||||
Some("2026-02-26".to_string()),
|
||||
Some("America/Los_Angeles".to_string()),
|
||||
/*network*/ None,
|
||||
@@ -184,3 +194,48 @@ fn serialize_environment_context_with_subagents() {
|
||||
|
||||
assert_eq!(context.render(), expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn serialize_environment_context_with_multiple_selected_environments() {
|
||||
let mut context = EnvironmentContext::new(
|
||||
Some(test_path_buf("/primary")),
|
||||
fake_shell_name(),
|
||||
Vec::new(),
|
||||
Some("2026-02-26".to_string()),
|
||||
Some("America/Los_Angeles".to_string()),
|
||||
/*network*/ None,
|
||||
/*subagents*/ None,
|
||||
);
|
||||
context.environments = vec![
|
||||
EnvironmentContextEnvironment {
|
||||
id: "local".to_string(),
|
||||
cwd: test_path_buf("/primary"),
|
||||
primary: true,
|
||||
},
|
||||
EnvironmentContextEnvironment {
|
||||
id: "remote".to_string(),
|
||||
cwd: test_path_buf("/remote/repo"),
|
||||
primary: false,
|
||||
},
|
||||
];
|
||||
|
||||
let expected = format!(
|
||||
r#"<environment_context>
|
||||
<environments>
|
||||
<environment id="local" primary="true">
|
||||
<cwd>{}</cwd>
|
||||
</environment>
|
||||
<environment id="remote" primary="false">
|
||||
<cwd>{}</cwd>
|
||||
</environment>
|
||||
</environments>
|
||||
<shell>bash</shell>
|
||||
<current_date>2026-02-26</current_date>
|
||||
<timezone>America/Los_Angeles</timezone>
|
||||
</environment_context>"#,
|
||||
test_path_buf("/primary").display(),
|
||||
test_path_buf("/remote/repo").display()
|
||||
);
|
||||
|
||||
assert_eq!(context.render(), expected);
|
||||
}
|
||||
|
||||
@@ -4501,6 +4501,78 @@ async fn primary_environment_uses_first_turn_environment() {
|
||||
assert_eq!(turn_context.environments[1].cwd, second_cwd);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn selected_environment_defaults_to_primary_and_resolves_explicit_ids() {
|
||||
let (session, _turn_context, _rx) = make_session_and_context_with_rx().await;
|
||||
let session_cwd = session.get_config().await.cwd.clone();
|
||||
let selected_cwd =
|
||||
AbsolutePathBuf::try_from(session_cwd.as_path().join("selected")).expect("absolute path");
|
||||
|
||||
let turn_context = session
|
||||
.new_turn_with_sub_id(
|
||||
"sub-1".to_string(),
|
||||
SessionSettingsUpdate {
|
||||
environments: Some(vec![TurnEnvironmentSelection {
|
||||
environment_id: "local".to_string(),
|
||||
cwd: selected_cwd.clone(),
|
||||
}]),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("turn should start");
|
||||
|
||||
assert!(!turn_context.has_multiple_selected_environments());
|
||||
|
||||
let primary = turn_context
|
||||
.selected_environment(/*environment_id*/ None)
|
||||
.expect("primary lookup should succeed")
|
||||
.expect("primary environment should exist");
|
||||
assert_eq!(primary.environment_id, "local");
|
||||
assert_eq!(primary.cwd, selected_cwd);
|
||||
|
||||
let explicit = turn_context
|
||||
.selected_environment(Some("local"))
|
||||
.expect("explicit lookup should succeed")
|
||||
.expect("explicit environment should exist");
|
||||
assert_eq!(explicit.environment_id, "local");
|
||||
assert_eq!(explicit.cwd, selected_cwd);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn selected_environment_rejects_empty_and_unknown_ids() {
|
||||
let (session, _turn_context, _rx) = make_session_and_context_with_rx().await;
|
||||
let selected_cwd =
|
||||
AbsolutePathBuf::try_from(session.get_config().await.cwd.as_path().join("selected"))
|
||||
.expect("absolute path");
|
||||
|
||||
let turn_context = session
|
||||
.new_turn_with_sub_id(
|
||||
"sub-1".to_string(),
|
||||
SessionSettingsUpdate {
|
||||
environments: Some(vec![TurnEnvironmentSelection {
|
||||
environment_id: "local".to_string(),
|
||||
cwd: selected_cwd,
|
||||
}]),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("turn should start");
|
||||
|
||||
assert!(!turn_context.has_multiple_selected_environments());
|
||||
|
||||
let empty_err = turn_context
|
||||
.selected_environment(Some(""))
|
||||
.expect_err("empty id should be rejected");
|
||||
assert_eq!(empty_err, "environment_id cannot be empty");
|
||||
|
||||
let unknown_err = turn_context
|
||||
.selected_environment(Some("remote"))
|
||||
.expect_err("unknown id should be rejected");
|
||||
assert_eq!(unknown_err, "unknown selected environment_id `remote`");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn empty_turn_environments_clear_primary_environment() {
|
||||
let (session, _turn_context, _rx) = make_session_and_context_with_rx().await;
|
||||
|
||||
@@ -198,6 +198,9 @@ impl TurnContext {
|
||||
.with_web_search_config(self.tools_config.web_search_config.clone())
|
||||
.with_allow_login_shell(self.tools_config.allow_login_shell)
|
||||
.with_has_environment(self.tools_config.has_environment)
|
||||
.with_has_multiple_selected_environments(
|
||||
self.tools_config.has_multiple_selected_environments,
|
||||
)
|
||||
.with_spawn_agent_usage_hint(config.multi_agent_v2.usage_hint_enabled)
|
||||
.with_spawn_agent_usage_hint_text(config.multi_agent_v2.usage_hint_text.clone())
|
||||
.with_hide_spawn_agent_metadata(config.multi_agent_v2.hide_spawn_agent_metadata)
|
||||
@@ -274,6 +277,31 @@ impl TurnContext {
|
||||
.map_or_else(|| self.cwd.clone(), |path| self.cwd.join(path))
|
||||
}
|
||||
|
||||
pub(crate) fn has_multiple_selected_environments(&self) -> bool {
|
||||
self.environments.len() > 1
|
||||
}
|
||||
|
||||
pub(crate) fn primary_environment(&self) -> Option<&TurnEnvironment> {
|
||||
self.environments.first()
|
||||
}
|
||||
|
||||
pub(crate) fn selected_environment(
|
||||
&self,
|
||||
environment_id: Option<&str>,
|
||||
) -> Result<Option<&TurnEnvironment>, String> {
|
||||
let Some(environment_id) = environment_id else {
|
||||
return Ok(self.primary_environment());
|
||||
};
|
||||
if environment_id.is_empty() {
|
||||
return Err("environment_id cannot be empty".to_string());
|
||||
}
|
||||
self.environments
|
||||
.iter()
|
||||
.find(|environment| environment.environment_id == environment_id)
|
||||
.map(Some)
|
||||
.ok_or_else(|| format!("unknown selected environment_id `{environment_id}`"))
|
||||
}
|
||||
|
||||
pub(crate) fn file_system_sandbox_context(
|
||||
&self,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
@@ -482,6 +510,7 @@ 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_has_environment(!environments.is_empty())
|
||||
.with_has_multiple_selected_environments(environments.len() > 1)
|
||||
.with_spawn_agent_usage_hint(per_turn_config.multi_agent_v2.usage_hint_enabled)
|
||||
.with_spawn_agent_usage_hint_text(per_turn_config.multi_agent_v2.usage_hint_text.clone())
|
||||
.with_hide_spawn_agent_metadata(per_turn_config.multi_agent_v2.hide_spawn_agent_metadata)
|
||||
|
||||
@@ -27,10 +27,12 @@ use codex_utils_absolute_path::AbsolutePathBufGuard;
|
||||
use serde::Deserialize;
|
||||
use serde_json::Value;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
pub(crate) use crate::tools::code_mode::CodeModeExecuteHandler;
|
||||
pub(crate) use crate::tools::code_mode::CodeModeWaitHandler;
|
||||
pub use apply_patch::ApplyPatchHandler;
|
||||
@@ -86,6 +88,55 @@ fn resolve_workdir_base_path(
|
||||
.map_or_else(|| default_cwd.clone(), |workdir| default_cwd.join(workdir)))
|
||||
}
|
||||
|
||||
pub(crate) fn resolve_environment_id_argument(
|
||||
arguments: &str,
|
||||
) -> Result<Option<String>, FunctionCallError> {
|
||||
let arguments: Value = parse_arguments(arguments)?;
|
||||
Ok(arguments
|
||||
.get("environment_id")
|
||||
.and_then(Value::as_str)
|
||||
.map(str::to_owned))
|
||||
}
|
||||
|
||||
pub(crate) struct ProcessToolEnvironment {
|
||||
pub(crate) environment: Arc<codex_exec_server::Environment>,
|
||||
pub(crate) cwd: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
pub(crate) fn resolve_process_tool_environment(
|
||||
turn: &TurnContext,
|
||||
environment_id: Option<&str>,
|
||||
) -> Result<Option<ProcessToolEnvironment>, FunctionCallError> {
|
||||
if environment_id.is_none() && !turn.has_multiple_selected_environments() {
|
||||
return Ok(turn
|
||||
.environment
|
||||
.as_ref()
|
||||
.map(|environment| ProcessToolEnvironment {
|
||||
environment: Arc::clone(environment),
|
||||
cwd: turn.cwd.clone(),
|
||||
}));
|
||||
}
|
||||
|
||||
let selected_environment = turn
|
||||
.selected_environment(environment_id)
|
||||
.map_err(FunctionCallError::RespondToModel)?;
|
||||
if let Some(environment) = selected_environment {
|
||||
return Ok(Some(ProcessToolEnvironment {
|
||||
environment: Arc::clone(&environment.environment),
|
||||
cwd: environment.cwd.clone(),
|
||||
}));
|
||||
}
|
||||
Ok(None)
|
||||
}
|
||||
|
||||
pub(crate) fn resolve_path_against_cwd(
|
||||
cwd: &AbsolutePathBuf,
|
||||
path: Option<String>,
|
||||
) -> AbsolutePathBuf {
|
||||
path.as_ref()
|
||||
.map_or_else(|| cwd.clone(), |path| cwd.join(path))
|
||||
}
|
||||
|
||||
/// Validates feature/policy constraints for `with_additional_permissions` and
|
||||
/// normalizes any path-based permissions. Errors if the request is invalid.
|
||||
pub(crate) fn normalize_and_validate_additional_permissions(
|
||||
@@ -230,11 +281,14 @@ mod tests {
|
||||
use super::EffectiveAdditionalPermissions;
|
||||
use super::implicit_granted_permissions;
|
||||
use super::normalize_and_validate_additional_permissions;
|
||||
use super::parse_arguments_with_base_path;
|
||||
use super::permissions_are_preapproved;
|
||||
use super::resolve_environment_id_argument;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::models::NetworkPermissions;
|
||||
use codex_protocol::models::ShellCommandToolCallParams;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
@@ -268,6 +322,61 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn environment_id_lookup_does_not_require_typed_argument_parsing() {
|
||||
let arguments = serde_json::json!({
|
||||
"command": "touch relative-write.txt",
|
||||
"workdir": "nested",
|
||||
"environment_id": "remote",
|
||||
"sandbox_permissions": SandboxPermissions::WithAdditionalPermissions,
|
||||
"additional_permissions": {
|
||||
"file_system": {
|
||||
"write": ["."]
|
||||
}
|
||||
}
|
||||
})
|
||||
.to_string();
|
||||
|
||||
assert_eq!(
|
||||
resolve_environment_id_argument(&arguments).expect("environment lookup should succeed"),
|
||||
Some("remote".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shell_command_args_resolve_relative_additional_permissions_against_workdir_base() {
|
||||
let workspace = tempdir().expect("tempdir");
|
||||
let nested = workspace.path().join("nested");
|
||||
std::fs::create_dir_all(&nested).expect("create nested directory");
|
||||
let base = AbsolutePathBuf::from_absolute_path(&nested).expect("absolute nested path");
|
||||
|
||||
let arguments = serde_json::json!({
|
||||
"command": "touch relative-write.txt",
|
||||
"workdir": "nested",
|
||||
"sandbox_permissions": SandboxPermissions::WithAdditionalPermissions,
|
||||
"additional_permissions": {
|
||||
"file_system": {
|
||||
"write": ["."]
|
||||
}
|
||||
}
|
||||
})
|
||||
.to_string();
|
||||
|
||||
let params: ShellCommandToolCallParams =
|
||||
parse_arguments_with_base_path(&arguments, &base).expect("shell args should parse");
|
||||
|
||||
assert_eq!(
|
||||
params.additional_permissions,
|
||||
Some(AdditionalPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
/*read*/ None,
|
||||
Some(vec![base]),
|
||||
)),
|
||||
..Default::default()
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preapproved_permissions_work_when_request_permissions_tool_is_enabled_without_exec_permission_approvals_feature()
|
||||
{
|
||||
|
||||
@@ -24,6 +24,9 @@ use crate::tools::handlers::implicit_granted_permissions;
|
||||
use crate::tools::handlers::normalize_and_validate_additional_permissions;
|
||||
use crate::tools::handlers::parse_arguments;
|
||||
use crate::tools::handlers::parse_arguments_with_base_path;
|
||||
use crate::tools::handlers::resolve_environment_id_argument;
|
||||
use crate::tools::handlers::resolve_path_against_cwd;
|
||||
use crate::tools::handlers::resolve_process_tool_environment;
|
||||
use crate::tools::handlers::resolve_workdir_base_path;
|
||||
use crate::tools::hook_names::HookToolName;
|
||||
use crate::tools::orchestrator::ToolOrchestrator;
|
||||
@@ -81,6 +84,7 @@ struct RunExecLikeArgs {
|
||||
hook_command: String,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
prefix_rule: Option<Vec<String>>,
|
||||
target_environment: crate::tools::handlers::ProcessToolEnvironment,
|
||||
session: Arc<crate::session::session::Session>,
|
||||
turn: Arc<TurnContext>,
|
||||
tracker: crate::tools::context::SharedTurnDiffTracker,
|
||||
@@ -93,11 +97,12 @@ impl ShellHandler {
|
||||
fn to_exec_params(
|
||||
params: &ShellToolCallParams,
|
||||
turn_context: &TurnContext,
|
||||
cwd_base: &codex_utils_absolute_path::AbsolutePathBuf,
|
||||
thread_id: ThreadId,
|
||||
) -> ExecParams {
|
||||
ExecParams {
|
||||
command: params.command.clone(),
|
||||
cwd: turn_context.resolve_path(params.workdir.clone()),
|
||||
cwd: resolve_path_against_cwd(cwd_base, params.workdir.clone()),
|
||||
expiration: params.timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env: create_env(&turn_context.shell_environment_policy, Some(thread_id)),
|
||||
@@ -143,6 +148,7 @@ impl ShellCommandHandler {
|
||||
params: &ShellCommandToolCallParams,
|
||||
session: &crate::session::session::Session,
|
||||
turn_context: &TurnContext,
|
||||
cwd_base: &codex_utils_absolute_path::AbsolutePathBuf,
|
||||
thread_id: ThreadId,
|
||||
allow_login_shell: bool,
|
||||
) -> Result<ExecParams, FunctionCallError> {
|
||||
@@ -152,7 +158,7 @@ impl ShellCommandHandler {
|
||||
|
||||
Ok(ExecParams {
|
||||
command,
|
||||
cwd: turn_context.resolve_path(params.workdir.clone()),
|
||||
cwd: resolve_path_against_cwd(cwd_base, params.workdir.clone()),
|
||||
expiration: params.timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env: create_env(&turn_context.shell_environment_policy, Some(thread_id)),
|
||||
@@ -241,17 +247,30 @@ impl ToolHandler for ShellHandler {
|
||||
|
||||
match payload {
|
||||
ToolPayload::Function { arguments } => {
|
||||
let cwd = resolve_workdir_base_path(&arguments, &turn.cwd)?;
|
||||
let environment_id = resolve_environment_id_argument(&arguments)?;
|
||||
let Some(target_environment) =
|
||||
resolve_process_tool_environment(&turn, environment_id.as_deref())?
|
||||
else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"shell is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
let cwd = resolve_workdir_base_path(&arguments, &target_environment.cwd)?;
|
||||
let params: ShellToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?;
|
||||
let prefix_rule = params.prefix_rule.clone();
|
||||
let exec_params =
|
||||
Self::to_exec_params(¶ms, turn.as_ref(), session.conversation_id);
|
||||
let exec_params = Self::to_exec_params(
|
||||
¶ms,
|
||||
turn.as_ref(),
|
||||
&target_environment.cwd,
|
||||
session.conversation_id,
|
||||
);
|
||||
Self::run_exec_like(RunExecLikeArgs {
|
||||
tool_name: tool_name.display(),
|
||||
exec_params,
|
||||
hook_command: codex_shell_command::parse_command::shlex_join(¶ms.command),
|
||||
additional_permissions: params.additional_permissions.clone(),
|
||||
prefix_rule,
|
||||
target_environment,
|
||||
session,
|
||||
turn,
|
||||
tracker,
|
||||
@@ -262,14 +281,26 @@ impl ToolHandler for ShellHandler {
|
||||
.await
|
||||
}
|
||||
ToolPayload::LocalShell { params } => {
|
||||
let exec_params =
|
||||
Self::to_exec_params(¶ms, turn.as_ref(), session.conversation_id);
|
||||
let Some(target_environment) =
|
||||
resolve_process_tool_environment(&turn, /*environment_id*/ None)?
|
||||
else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"shell is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
let exec_params = Self::to_exec_params(
|
||||
¶ms,
|
||||
turn.as_ref(),
|
||||
&target_environment.cwd,
|
||||
session.conversation_id,
|
||||
);
|
||||
Self::run_exec_like(RunExecLikeArgs {
|
||||
tool_name: tool_name.display(),
|
||||
exec_params,
|
||||
hook_command: codex_shell_command::parse_command::shlex_join(¶ms.command),
|
||||
additional_permissions: None,
|
||||
prefix_rule: None,
|
||||
target_environment,
|
||||
session,
|
||||
turn,
|
||||
tracker,
|
||||
@@ -360,9 +391,17 @@ impl ToolHandler for ShellCommandHandler {
|
||||
)));
|
||||
};
|
||||
|
||||
let cwd = resolve_workdir_base_path(&arguments, &turn.cwd)?;
|
||||
let environment_id = resolve_environment_id_argument(&arguments)?;
|
||||
let Some(target_environment) =
|
||||
resolve_process_tool_environment(&turn, environment_id.as_deref())?
|
||||
else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"shell_command is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
let cwd = resolve_workdir_base_path(&arguments, &target_environment.cwd)?;
|
||||
let params: ShellCommandToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?;
|
||||
let workdir = turn.resolve_path(params.workdir.clone());
|
||||
let workdir = resolve_path_against_cwd(&target_environment.cwd, params.workdir.clone());
|
||||
maybe_emit_implicit_skill_invocation(
|
||||
session.as_ref(),
|
||||
turn.as_ref(),
|
||||
@@ -375,6 +414,7 @@ impl ToolHandler for ShellCommandHandler {
|
||||
¶ms,
|
||||
session.as_ref(),
|
||||
turn.as_ref(),
|
||||
&target_environment.cwd,
|
||||
session.conversation_id,
|
||||
turn.tools_config.allow_login_shell,
|
||||
)?;
|
||||
@@ -384,6 +424,7 @@ impl ToolHandler for ShellCommandHandler {
|
||||
hook_command: params.command,
|
||||
additional_permissions: params.additional_permissions.clone(),
|
||||
prefix_rule,
|
||||
target_environment,
|
||||
session,
|
||||
turn,
|
||||
tracker,
|
||||
@@ -403,6 +444,7 @@ impl ShellHandler {
|
||||
hook_command,
|
||||
additional_permissions,
|
||||
prefix_rule,
|
||||
target_environment,
|
||||
session,
|
||||
turn,
|
||||
tracker,
|
||||
@@ -412,12 +454,7 @@ impl ShellHandler {
|
||||
} = args;
|
||||
|
||||
let mut exec_params = exec_params;
|
||||
let Some(turn_environment) = turn.primary_environment() else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"shell is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
let fs = turn_environment.environment.get_filesystem();
|
||||
let fs = target_environment.environment.get_filesystem();
|
||||
|
||||
let dependency_env = session.dependency_env().await;
|
||||
if !dependency_env.is_empty() {
|
||||
@@ -436,7 +473,7 @@ impl ShellHandler {
|
||||
let requested_additional_permissions = additional_permissions.clone();
|
||||
let effective_additional_permissions = apply_granted_turn_permissions(
|
||||
session.as_ref(),
|
||||
turn.cwd.as_path(),
|
||||
target_environment.cwd.as_path(),
|
||||
exec_params.sandbox_permissions,
|
||||
additional_permissions,
|
||||
)
|
||||
@@ -522,7 +559,7 @@ impl ShellHandler {
|
||||
approval_policy: turn.approval_policy.value(),
|
||||
permission_profile: turn.permission_profile(),
|
||||
file_system_sandbox_policy: &file_system_sandbox_policy,
|
||||
sandbox_cwd: turn.cwd.as_path(),
|
||||
sandbox_cwd: target_environment.cwd.as_path(),
|
||||
sandbox_permissions: if effective_additional_permissions.permissions_preapproved {
|
||||
codex_protocol::models::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
@@ -531,7 +568,6 @@ impl ShellHandler {
|
||||
prefix_rule,
|
||||
})
|
||||
.await;
|
||||
|
||||
let req = ShellRequest {
|
||||
command: exec_params.command.clone(),
|
||||
hook_command,
|
||||
|
||||
@@ -98,6 +98,7 @@ async fn shell_command_handler_to_exec_params_uses_session_shell_and_turn_contex
|
||||
let params = ShellCommandToolCallParams {
|
||||
command,
|
||||
workdir,
|
||||
environment_id: None,
|
||||
login,
|
||||
timeout_ms,
|
||||
sandbox_permissions: Some(sandbox_permissions),
|
||||
@@ -110,6 +111,7 @@ async fn shell_command_handler_to_exec_params_uses_session_shell_and_turn_contex
|
||||
¶ms,
|
||||
&session,
|
||||
&turn_context,
|
||||
&turn_context.cwd,
|
||||
session.conversation_id,
|
||||
/*allow_login_shell*/ true,
|
||||
)
|
||||
@@ -165,6 +167,7 @@ async fn shell_command_handler_defaults_to_non_login_when_disallowed() {
|
||||
let params = ShellCommandToolCallParams {
|
||||
command: "echo hello".to_string(),
|
||||
workdir: None,
|
||||
environment_id: None,
|
||||
login: None,
|
||||
timeout_ms: None,
|
||||
sandbox_permissions: None,
|
||||
@@ -177,6 +180,7 @@ async fn shell_command_handler_defaults_to_non_login_when_disallowed() {
|
||||
¶ms,
|
||||
&session,
|
||||
&turn_context,
|
||||
&turn_context.cwd,
|
||||
session.conversation_id,
|
||||
/*allow_login_shell*/ false,
|
||||
)
|
||||
@@ -213,6 +217,7 @@ async fn shell_pre_tool_use_payload_uses_joined_command() {
|
||||
"printf hi".to_string(),
|
||||
],
|
||||
workdir: None,
|
||||
environment_id: None,
|
||||
timeout_ms: None,
|
||||
sandbox_permissions: None,
|
||||
prefix_rule: None,
|
||||
|
||||
@@ -13,6 +13,9 @@ use crate::tools::handlers::implicit_granted_permissions;
|
||||
use crate::tools::handlers::normalize_and_validate_additional_permissions;
|
||||
use crate::tools::handlers::parse_arguments;
|
||||
use crate::tools::handlers::parse_arguments_with_base_path;
|
||||
use crate::tools::handlers::resolve_environment_id_argument;
|
||||
use crate::tools::handlers::resolve_path_against_cwd;
|
||||
use crate::tools::handlers::resolve_process_tool_environment;
|
||||
use crate::tools::handlers::resolve_workdir_base_path;
|
||||
use crate::tools::hook_names::HookToolName;
|
||||
use crate::tools::registry::PostToolUsePayload;
|
||||
@@ -48,6 +51,9 @@ pub(crate) struct ExecCommandArgs {
|
||||
#[serde(default)]
|
||||
pub(crate) workdir: Option<String>,
|
||||
#[serde(default)]
|
||||
#[allow(dead_code)]
|
||||
environment_id: Option<String>,
|
||||
#[serde(default)]
|
||||
shell: Option<String>,
|
||||
#[serde(default)]
|
||||
login: Option<bool>,
|
||||
@@ -196,22 +202,25 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
}
|
||||
};
|
||||
|
||||
let Some(turn_environment) = turn.primary_environment() else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"unified exec is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
let fs = turn_environment.environment.get_filesystem();
|
||||
|
||||
let manager: &UnifiedExecProcessManager = &session.services.unified_exec_manager;
|
||||
let context = UnifiedExecContext::new(session.clone(), turn.clone(), call_id.clone());
|
||||
|
||||
let response = match tool_name.name.as_str() {
|
||||
"exec_command" => {
|
||||
let cwd = resolve_workdir_base_path(&arguments, &context.turn.cwd)?;
|
||||
let environment_id = resolve_environment_id_argument(&arguments)?;
|
||||
let Some(target_environment) =
|
||||
resolve_process_tool_environment(&turn, environment_id.as_deref())?
|
||||
else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"unified exec is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
let fs = target_environment.environment.get_filesystem();
|
||||
let cwd = resolve_workdir_base_path(&arguments, &target_environment.cwd)?;
|
||||
let args: ExecCommandArgs = parse_arguments_with_base_path(&arguments, &cwd)?;
|
||||
let hook_command = args.cmd.clone();
|
||||
let workdir = context.turn.resolve_path(args.workdir.clone());
|
||||
let workdir =
|
||||
resolve_path_against_cwd(&target_environment.cwd, args.workdir.clone());
|
||||
maybe_emit_implicit_skill_invocation(
|
||||
session.as_ref(),
|
||||
context.turn.as_ref(),
|
||||
@@ -231,6 +240,7 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
|
||||
let ExecCommandArgs {
|
||||
workdir,
|
||||
environment_id: _,
|
||||
tty,
|
||||
yield_time_ms,
|
||||
max_output_tokens,
|
||||
@@ -248,7 +258,7 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
let requested_additional_permissions = additional_permissions.clone();
|
||||
let effective_additional_permissions = apply_granted_turn_permissions(
|
||||
context.session.as_ref(),
|
||||
context.turn.cwd.as_path(),
|
||||
target_environment.cwd.as_path(),
|
||||
sandbox_permissions,
|
||||
additional_permissions,
|
||||
)
|
||||
@@ -277,7 +287,8 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
|
||||
let workdir = workdir.filter(|value| !value.is_empty());
|
||||
|
||||
let workdir = workdir.map(|dir| context.turn.resolve_path(Some(dir)));
|
||||
let workdir =
|
||||
workdir.map(|dir| resolve_path_against_cwd(&target_environment.cwd, Some(dir)));
|
||||
let cwd = workdir.clone().unwrap_or(cwd);
|
||||
let normalized_additional_permissions = match implicit_granted_permissions(
|
||||
sandbox_permissions,
|
||||
@@ -339,7 +350,8 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
process_id,
|
||||
yield_time_ms,
|
||||
max_output_tokens: Some(max_output_tokens),
|
||||
workdir,
|
||||
workdir: Some(cwd.clone()),
|
||||
environment: target_environment.environment,
|
||||
network: context.turn.network.clone(),
|
||||
tty,
|
||||
sandbox_permissions: effective_additional_permissions
|
||||
|
||||
@@ -248,6 +248,7 @@ impl ToolRouter {
|
||||
let params = ShellToolCallParams {
|
||||
command: exec.command,
|
||||
workdir: exec.working_directory,
|
||||
environment_id: None,
|
||||
timeout_ms: exec.timeout_ms,
|
||||
sandbox_permissions: Some(SandboxPermissions::UseDefault),
|
||||
additional_permissions: None,
|
||||
|
||||
@@ -37,6 +37,7 @@ use crate::unified_exec::NoopSpawnLifecycle;
|
||||
use crate::unified_exec::UnifiedExecError;
|
||||
use crate::unified_exec::UnifiedExecProcess;
|
||||
use crate::unified_exec::UnifiedExecProcessManager;
|
||||
use codex_exec_server::Environment;
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::error::SandboxErr;
|
||||
@@ -58,6 +59,7 @@ pub struct UnifiedExecRequest {
|
||||
pub hook_command: String,
|
||||
pub process_id: i32,
|
||||
pub cwd: AbsolutePathBuf,
|
||||
pub environment: std::sync::Arc<Environment>,
|
||||
pub env: HashMap<String, String>,
|
||||
pub exec_server_env_config: Option<ExecServerEnvConfig>,
|
||||
pub explicit_env_overrides: HashMap<String, String>,
|
||||
@@ -252,10 +254,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
if let Some(network) = managed_network {
|
||||
network.apply_to_env(&mut env);
|
||||
}
|
||||
let environment_is_remote = ctx
|
||||
.turn
|
||||
.primary_environment()
|
||||
.is_some_and(|turn_environment| turn_environment.environment.is_remote());
|
||||
let environment_is_remote = req.environment.is_remote();
|
||||
let command = if environment_is_remote {
|
||||
base_command.to_vec()
|
||||
} else {
|
||||
@@ -292,12 +291,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
.await?
|
||||
{
|
||||
Some(prepared) => {
|
||||
let Some(turn_environment) = ctx.turn.primary_environment() else {
|
||||
return Err(ToolError::Rejected(
|
||||
"exec_command is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
if turn_environment.environment.is_remote() {
|
||||
if req.environment.is_remote() {
|
||||
return Err(ToolError::Rejected(
|
||||
"unified_exec zsh-fork is not supported when exec_server_url is configured".to_string(),
|
||||
));
|
||||
@@ -309,7 +303,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
&prepared.exec_request,
|
||||
req.tty,
|
||||
prepared.spawn_lifecycle,
|
||||
turn_environment.environment.as_ref(),
|
||||
req.environment.as_ref(),
|
||||
)
|
||||
.await
|
||||
.map_err(|err| match err {
|
||||
@@ -337,18 +331,13 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
.env_for(command, options, managed_network)
|
||||
.map_err(|err| ToolError::Codex(err.into()))?;
|
||||
exec_env.exec_server_env_config = req.exec_server_env_config.clone();
|
||||
let Some(turn_environment) = ctx.turn.primary_environment() else {
|
||||
return Err(ToolError::Rejected(
|
||||
"exec_command is unavailable in this session".to_string(),
|
||||
));
|
||||
};
|
||||
self.manager
|
||||
.open_session_with_exec_env(
|
||||
req.process_id,
|
||||
&exec_env,
|
||||
req.tty,
|
||||
Box::new(NoopSpawnLifecycle),
|
||||
turn_environment.environment.as_ref(),
|
||||
req.environment.as_ref(),
|
||||
)
|
||||
.await
|
||||
.map_err(|err| match err {
|
||||
|
||||
@@ -94,6 +94,7 @@ pub(crate) struct ExecCommandRequest {
|
||||
pub yield_time_ms: u64,
|
||||
pub max_output_tokens: Option<usize>,
|
||||
pub workdir: Option<AbsolutePathBuf>,
|
||||
pub environment: Arc<codex_exec_server::Environment>,
|
||||
pub network: Option<NetworkProxy>,
|
||||
pub tty: bool,
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
|
||||
@@ -1012,7 +1012,7 @@ impl UnifiedExecProcessManager {
|
||||
approval_policy: context.turn.approval_policy.value(),
|
||||
permission_profile: context.turn.permission_profile(),
|
||||
file_system_sandbox_policy: &file_system_sandbox_policy,
|
||||
sandbox_cwd: context.turn.cwd.as_path(),
|
||||
sandbox_cwd: cwd.as_path(),
|
||||
sandbox_permissions: if request.additional_permissions_preapproved {
|
||||
crate::sandboxing::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
@@ -1026,6 +1026,7 @@ impl UnifiedExecProcessManager {
|
||||
hook_command: request.hook_command.clone(),
|
||||
process_id: request.process_id,
|
||||
cwd,
|
||||
environment: request.environment.clone(),
|
||||
env,
|
||||
exec_server_env_config: Some(exec_server_env_config),
|
||||
explicit_env_overrides: context.turn.shell_environment_policy.r#set.clone(),
|
||||
|
||||
@@ -176,6 +176,7 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() {
|
||||
yield_time_ms: 1000,
|
||||
max_output_tokens: None,
|
||||
workdir: None,
|
||||
environment: turn.environment.as_ref().expect("turn environment").clone(),
|
||||
network: None,
|
||||
tty: true,
|
||||
sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault,
|
||||
|
||||
@@ -1254,6 +1254,9 @@ pub struct SearchToolCallParams {
|
||||
pub struct ShellToolCallParams {
|
||||
pub command: Vec<String>,
|
||||
pub workdir: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub environment_id: Option<String>,
|
||||
|
||||
/// This is the maximum time in milliseconds that the command is allowed to run.
|
||||
#[serde(alias = "timeout")]
|
||||
@@ -1278,6 +1281,9 @@ pub struct ShellToolCallParams {
|
||||
pub struct ShellCommandToolCallParams {
|
||||
pub command: String,
|
||||
pub workdir: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub environment_id: Option<String>,
|
||||
|
||||
/// Whether to run the shell with login shell semantics
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
@@ -2523,6 +2529,7 @@ mod tests {
|
||||
ShellToolCallParams {
|
||||
command: vec!["ls".to_string(), "-l".to_string()],
|
||||
workdir: Some("/tmp".to_string()),
|
||||
environment_id: None,
|
||||
timeout_ms: Some(1000),
|
||||
sandbox_permissions: None,
|
||||
prefix_rule: None,
|
||||
|
||||
@@ -9,11 +9,13 @@ use std::collections::BTreeMap;
|
||||
pub struct CommandToolOptions {
|
||||
pub allow_login_shell: bool,
|
||||
pub exec_permission_approvals_enabled: bool,
|
||||
pub include_environment_id: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct ShellToolOptions {
|
||||
pub exec_permission_approvals_enabled: bool,
|
||||
pub include_environment_id: bool,
|
||||
}
|
||||
|
||||
pub fn create_exec_command_tool(options: CommandToolOptions) -> ToolSpec {
|
||||
@@ -63,6 +65,7 @@ pub fn create_exec_command_tool(options: CommandToolOptions) -> ToolSpec {
|
||||
)),
|
||||
);
|
||||
}
|
||||
maybe_insert_environment_id_parameter(&mut properties, options.include_environment_id);
|
||||
properties.extend(create_approval_parameters(
|
||||
options.exec_permission_approvals_enabled,
|
||||
));
|
||||
@@ -158,6 +161,7 @@ pub fn create_shell_tool(options: ShellToolOptions) -> ToolSpec {
|
||||
properties.extend(create_approval_parameters(
|
||||
options.exec_permission_approvals_enabled,
|
||||
));
|
||||
maybe_insert_environment_id_parameter(&mut properties, options.include_environment_id);
|
||||
|
||||
let description = if cfg!(windows) {
|
||||
format!(
|
||||
@@ -226,6 +230,7 @@ pub fn create_shell_command_tool(options: CommandToolOptions) -> ToolSpec {
|
||||
)),
|
||||
);
|
||||
}
|
||||
maybe_insert_environment_id_parameter(&mut properties, options.include_environment_id);
|
||||
properties.extend(create_approval_parameters(
|
||||
options.exec_permission_approvals_enabled,
|
||||
));
|
||||
@@ -266,6 +271,20 @@ Examples of valid command strings:
|
||||
})
|
||||
}
|
||||
|
||||
fn maybe_insert_environment_id_parameter(
|
||||
properties: &mut BTreeMap<String, JsonSchema>,
|
||||
include_environment_id: bool,
|
||||
) {
|
||||
if include_environment_id {
|
||||
properties.insert(
|
||||
"environment_id".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Optional environment id from the <environment_context> block. If omitted, uses the primary environment.".to_string(),
|
||||
)),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
pub fn create_request_permissions_tool(description: String) -> ToolSpec {
|
||||
let properties = BTreeMap::from([
|
||||
(
|
||||
|
||||
@@ -10,6 +10,7 @@ fn windows_shell_guidance_description() -> String {
|
||||
fn shell_tool_matches_expected_spec() {
|
||||
let tool = create_shell_tool(ShellToolOptions {
|
||||
exec_permission_approvals_enabled: false,
|
||||
include_environment_id: false,
|
||||
});
|
||||
|
||||
let description = if cfg!(windows) {
|
||||
@@ -96,6 +97,7 @@ fn exec_command_tool_matches_expected_spec() {
|
||||
let tool = create_exec_command_tool(CommandToolOptions {
|
||||
allow_login_shell: true,
|
||||
exec_permission_approvals_enabled: false,
|
||||
include_environment_id: false,
|
||||
});
|
||||
|
||||
let description = if cfg!(windows) {
|
||||
@@ -174,6 +176,38 @@ fn exec_command_tool_matches_expected_spec() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn process_tools_include_environment_id_when_requested() {
|
||||
for tool in [
|
||||
create_shell_tool(ShellToolOptions {
|
||||
exec_permission_approvals_enabled: false,
|
||||
include_environment_id: true,
|
||||
}),
|
||||
create_shell_command_tool(CommandToolOptions {
|
||||
allow_login_shell: true,
|
||||
exec_permission_approvals_enabled: false,
|
||||
include_environment_id: true,
|
||||
}),
|
||||
create_exec_command_tool(CommandToolOptions {
|
||||
allow_login_shell: true,
|
||||
exec_permission_approvals_enabled: false,
|
||||
include_environment_id: true,
|
||||
}),
|
||||
] {
|
||||
let ToolSpec::Function(tool) = tool else {
|
||||
panic!("expected function tool");
|
||||
};
|
||||
assert!(
|
||||
tool.parameters
|
||||
.properties
|
||||
.as_ref()
|
||||
.is_some_and(|properties| properties.contains_key("environment_id")),
|
||||
"{} should include environment_id",
|
||||
tool.name
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_stdin_tool_matches_expected_spec() {
|
||||
let tool = create_write_stdin_tool();
|
||||
@@ -228,6 +262,7 @@ fn write_stdin_tool_matches_expected_spec() {
|
||||
fn shell_tool_with_request_permission_includes_additional_permissions() {
|
||||
let tool = create_shell_tool(ShellToolOptions {
|
||||
exec_permission_approvals_enabled: true,
|
||||
include_environment_id: false,
|
||||
});
|
||||
|
||||
let mut properties = BTreeMap::from([
|
||||
@@ -332,6 +367,7 @@ fn shell_command_tool_matches_expected_spec() {
|
||||
let tool = create_shell_command_tool(CommandToolOptions {
|
||||
allow_login_shell: true,
|
||||
exec_permission_approvals_enabled: false,
|
||||
include_environment_id: false,
|
||||
});
|
||||
|
||||
let description = if cfg!(windows) {
|
||||
|
||||
@@ -89,6 +89,7 @@ pub struct ToolsConfig {
|
||||
pub shell_command_backend: ShellCommandBackendConfig,
|
||||
pub unified_exec_shell_mode: UnifiedExecShellMode,
|
||||
pub has_environment: bool,
|
||||
pub has_multiple_selected_environments: bool,
|
||||
pub allow_login_shell: bool,
|
||||
pub apply_patch_tool_type: Option<ApplyPatchToolType>,
|
||||
pub web_search_mode: Option<WebSearchMode>,
|
||||
@@ -206,6 +207,7 @@ impl ToolsConfig {
|
||||
shell_command_backend,
|
||||
unified_exec_shell_mode: UnifiedExecShellMode::Direct,
|
||||
has_environment: true,
|
||||
has_multiple_selected_environments: false,
|
||||
allow_login_shell: true,
|
||||
apply_patch_tool_type,
|
||||
web_search_mode: *web_search_mode,
|
||||
@@ -311,6 +313,14 @@ impl ToolsConfig {
|
||||
self
|
||||
}
|
||||
|
||||
pub fn with_has_multiple_selected_environments(
|
||||
mut self,
|
||||
has_multiple_selected_environments: bool,
|
||||
) -> Self {
|
||||
self.has_multiple_selected_environments = has_multiple_selected_environments;
|
||||
self
|
||||
}
|
||||
|
||||
pub fn with_unified_exec_shell_mode(
|
||||
mut self,
|
||||
unified_exec_shell_mode: UnifiedExecShellMode,
|
||||
|
||||
@@ -141,6 +141,7 @@ pub fn build_tool_registry_plan(
|
||||
plan.push_spec(
|
||||
create_shell_tool(ShellToolOptions {
|
||||
exec_permission_approvals_enabled,
|
||||
include_environment_id: config.has_multiple_selected_environments,
|
||||
}),
|
||||
/*supports_parallel_tool_calls*/ true,
|
||||
config.code_mode_enabled,
|
||||
@@ -158,6 +159,7 @@ pub fn build_tool_registry_plan(
|
||||
create_exec_command_tool(CommandToolOptions {
|
||||
allow_login_shell: config.allow_login_shell,
|
||||
exec_permission_approvals_enabled,
|
||||
include_environment_id: config.has_multiple_selected_environments,
|
||||
}),
|
||||
/*supports_parallel_tool_calls*/ true,
|
||||
config.code_mode_enabled,
|
||||
@@ -176,6 +178,7 @@ pub fn build_tool_registry_plan(
|
||||
create_shell_command_tool(CommandToolOptions {
|
||||
allow_login_shell: config.allow_login_shell,
|
||||
exec_permission_approvals_enabled,
|
||||
include_environment_id: config.has_multiple_selected_environments,
|
||||
}),
|
||||
/*supports_parallel_tool_calls*/ true,
|
||||
config.code_mode_enabled,
|
||||
|
||||
@@ -87,6 +87,7 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() {
|
||||
create_exec_command_tool(CommandToolOptions {
|
||||
allow_login_shell: true,
|
||||
exec_permission_approvals_enabled: false,
|
||||
include_environment_id: false,
|
||||
}),
|
||||
create_write_stdin_tool(),
|
||||
create_update_plan_tool(),
|
||||
@@ -158,6 +159,88 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn process_tool_specs_include_environment_id_only_for_multiple_selected_environments() {
|
||||
let model_info = model_info();
|
||||
let available_models = Vec::new();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::UnifiedExec);
|
||||
let config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features,
|
||||
image_generation_tool_auth_allowed: true,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
permission_profile: &PermissionProfile::Disabled,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
});
|
||||
|
||||
let (single_environment_tools, _) = build_specs(
|
||||
&config,
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
&[],
|
||||
);
|
||||
assert_process_tool_environment_id(
|
||||
&single_environment_tools,
|
||||
"exec_command",
|
||||
/*expected_present*/ false,
|
||||
);
|
||||
|
||||
let multi_environment_config = config
|
||||
.with_has_multiple_selected_environments(/*has_multiple_selected_environments*/ true);
|
||||
let (multi_environment_tools, _) = build_specs(
|
||||
&multi_environment_config,
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
&[],
|
||||
);
|
||||
assert_process_tool_environment_id(
|
||||
&multi_environment_tools,
|
||||
"exec_command",
|
||||
/*expected_present*/ true,
|
||||
);
|
||||
|
||||
let mut shell_command_features = Features::with_defaults();
|
||||
shell_command_features.disable(Feature::UnifiedExec);
|
||||
let shell_command_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &shell_command_features,
|
||||
image_generation_tool_auth_allowed: true,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
permission_profile: &PermissionProfile::Disabled,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
});
|
||||
let (single_environment_shell_tools, _) = build_specs(
|
||||
&shell_command_config,
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
&[],
|
||||
);
|
||||
assert_process_tool_environment_id(
|
||||
&single_environment_shell_tools,
|
||||
"shell_command",
|
||||
/*expected_present*/ false,
|
||||
);
|
||||
|
||||
let multi_environment_shell_config = shell_command_config
|
||||
.with_has_multiple_selected_environments(/*has_multiple_selected_environments*/ true);
|
||||
let (multi_environment_shell_tools, _) = build_specs(
|
||||
&multi_environment_shell_config,
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
&[],
|
||||
);
|
||||
assert_process_tool_environment_id(
|
||||
&multi_environment_shell_tools,
|
||||
"shell_command",
|
||||
/*expected_present*/ true,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_specs_collab_tools_enabled() {
|
||||
let model_info = model_info();
|
||||
@@ -2429,6 +2512,23 @@ fn find_tool<'a>(tools: &'a [ConfiguredToolSpec], expected_name: &str) -> &'a Co
|
||||
.unwrap_or_else(|| panic!("expected tool {expected_name}"))
|
||||
}
|
||||
|
||||
fn assert_process_tool_environment_id(
|
||||
tools: &[ConfiguredToolSpec],
|
||||
expected_name: &str,
|
||||
expected_present: bool,
|
||||
) {
|
||||
let tool = find_tool(tools, expected_name);
|
||||
let ToolSpec::Function(ResponsesApiTool { parameters, .. }) = &tool.spec else {
|
||||
panic!("expected function tool {expected_name}");
|
||||
};
|
||||
let (properties, _) = expect_object_schema(parameters);
|
||||
assert_eq!(
|
||||
properties.contains_key("environment_id"),
|
||||
expected_present,
|
||||
"{expected_name} environment_id parameter presence"
|
||||
);
|
||||
}
|
||||
|
||||
fn find_namespace_function_tool<'a>(
|
||||
tools: &'a [ConfiguredToolSpec],
|
||||
expected_namespace: &str,
|
||||
|
||||
Reference in New Issue
Block a user