fix: make the default set of env vars that are passed through platform-specific

This commit is contained in:
Michael Bolin
2026-04-03 11:47:29 -07:00
parent 1cc87019b4
commit 610b0a5dd0
3 changed files with 43 additions and 9 deletions
+35 -7
View File
@@ -21,9 +21,36 @@ pub fn create_env(
policy: &ShellEnvironmentPolicy,
thread_id: Option<ThreadId>,
) -> HashMap<String, String> {
populate_env(std::env::vars(), policy, thread_id)
let mut env_map = populate_env(std::env::vars(), policy, thread_id);
if cfg!(target_os = "windows") {
// This is a workaround to address the failures we are seeing in the
// following tests when run via Bazel on Windows:
//
// ```
// suite::shell_command::unicode_output::with_login
// suite::shell_command::unicode_output::without_login
// ```
//
// Currently, we can only reproduce these failures in CI, which makes
// iteration times long, so we include this quick fix for now to unblock
// getting the Windows Bazel build running.
//
if !env_map.keys().any(|k| k.eq_ignore_ascii_case("PATHEXT")) {
env_map.insert("PATHEXT".to_string(), ".COM;.EXE;.BAT;.CMD".to_string());
}
}
env_map
}
const COMMON_CORE_VARS: &[&str] = &["PATH", "SHELL", "TMPDIR", "TEMP", "TMP"];
#[cfg(target_os = "windows")]
const PLATFORM_CORE_VARS: &[&str] = { &["PATHEXT", "USERNAME", "USERPROFILE"] };
#[cfg(unix)]
const PLATFORM_CORE_VARS: &[&str] = &["HOME", "LANG", "LC_ALL", "LC_CTYPE", "LOGNAME", "USER"];
fn populate_env<I>(
vars: I,
policy: &ShellEnvironmentPolicy,
@@ -38,17 +65,18 @@ where
ShellEnvironmentPolicyInherit::All => vars.into_iter().collect(),
ShellEnvironmentPolicyInherit::None => HashMap::new(),
ShellEnvironmentPolicyInherit::Core => {
const CORE_VARS: &[&str] = &[
"HOME", "LOGNAME", "PATH", "SHELL", "USER", "USERNAME", "TMPDIR", "TEMP", "TMP",
];
let allow: HashSet<&str> = CORE_VARS.iter().copied().collect();
let core_vars: HashSet<&str> = COMMON_CORE_VARS
.iter()
.copied()
.chain(PLATFORM_CORE_VARS.iter().copied())
.collect();
let is_core_var = |name: &str| {
if cfg!(target_os = "windows") {
CORE_VARS
core_vars
.iter()
.any(|allowed| allowed.eq_ignore_ascii_case(name))
} else {
allow.contains(name)
core_vars.contains(name)
}
};
vars.into_iter().filter(|(k, _)| is_core_var(k)).collect()
+2
View File
@@ -171,6 +171,7 @@ fn test_inherit_all_with_default_excludes() {
fn test_core_inherit_respects_case_insensitive_names_on_windows() {
let vars = make_vars(&[
("Path", "C:\\Windows\\System32"),
("PathExt", ".COM;.EXE;.BAT;.CMD"),
("TEMP", "C:\\Temp"),
("FOO", "bar"),
]);
@@ -185,6 +186,7 @@ fn test_core_inherit_respects_case_insensitive_names_on_windows() {
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"Path".to_string() => "C:\\Windows\\System32".to_string(),
"PathExt".to_string() => ".COM;.EXE;.BAT;.CMD".to_string(),
"TEMP".to_string() => "C:\\Temp".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
+6 -2
View File
@@ -256,6 +256,9 @@ async fn shell_command_times_out_with_timeout_ms() -> anyhow::Result<()> {
Ok(())
}
/// This test verifies that a shell, particularly PowerShell, can correctly
/// handle unicode output when the UTF-8 BOM is used. See
/// https://github.com/openai/codex/pull/7902 for more context.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(true ; "with_login")]
#[test_case(false ; "without_login")]
@@ -264,10 +267,11 @@ async fn unicode_output(login: bool) -> anyhow::Result<()> {
let harness = shell_command_harness_with(|builder| builder.with_model("gpt-5.2")).await?;
// We use a child process on windows instead of a direct builtin like 'echo' to ensure that Powershell
// config is actually being set correctly.
let call_id = "unicode_output";
let command = if cfg!(windows) {
// We use a child process on Windows instead of a PowerShell command
// like `Write-Output` to ensure that the Powershell config is set
// correctly.
"cmd.exe /c echo naïve_café"
} else {
"echo \"naïve_café\""