mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
core: render remote environment cwd natively (#28152)
## Why Model-visible `<environment_context>` should match the environment of the executor, not of the app server. Stacked on #28146. ## What - Keep selected environment cwd values as `PathUri` while building environment context. - Render cwd text using the path convention represented by the URI, with the canonical URI as a fallback. - Preserve compatibility with legacy `TurnContextItem.cwd` values when reconstructing and diffing context. - Extend the Wine-backed remote Windows test to assert that the model sees `powershell` and `C:\windows`.
This commit is contained in:
@@ -10,6 +10,7 @@ use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::protocol::TurnContextItem;
|
||||
use codex_protocol::protocol::TurnContextNetworkItem;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use std::collections::HashSet;
|
||||
use std::path::PathBuf;
|
||||
|
||||
@@ -28,12 +29,12 @@ pub(crate) struct EnvironmentContext {
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) struct EnvironmentContextEnvironment {
|
||||
pub(crate) id: String,
|
||||
pub(crate) cwd: AbsolutePathBuf,
|
||||
pub(crate) cwd: PathUri,
|
||||
pub(crate) shell: String,
|
||||
}
|
||||
|
||||
impl EnvironmentContextEnvironment {
|
||||
fn legacy(cwd: AbsolutePathBuf, shell: String) -> Self {
|
||||
fn legacy(cwd: PathUri, shell: String) -> Self {
|
||||
Self {
|
||||
id: String::new(),
|
||||
cwd,
|
||||
@@ -44,18 +45,14 @@ impl EnvironmentContextEnvironment {
|
||||
fn from_turn_environments(environments: &[TurnEnvironment], shell: &Shell) -> Vec<Self> {
|
||||
environments
|
||||
.iter()
|
||||
.filter_map(|environment| {
|
||||
// TODO(anp): Migrate EnvironmentContextEnvironment to PathUri so foreign
|
||||
// environments remain visible in model context.
|
||||
Some(Self {
|
||||
id: environment.environment_id.clone(),
|
||||
cwd: environment.cwd().to_abs_path().ok()?,
|
||||
shell: environment
|
||||
.shell
|
||||
.as_ref()
|
||||
.map(|shell| shell.name().to_string())
|
||||
.unwrap_or_else(|| shell.name().to_string()),
|
||||
})
|
||||
.map(|environment| Self {
|
||||
id: environment.environment_id.clone(),
|
||||
cwd: environment.cwd().clone(),
|
||||
shell: environment
|
||||
.shell
|
||||
.as_ref()
|
||||
.map(|shell| shell.name().to_string())
|
||||
.unwrap_or_else(|| shell.name().to_string()),
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
@@ -383,12 +380,12 @@ impl EnvironmentContext {
|
||||
pub(crate) fn diff_from_turn_context_item(
|
||||
before: &TurnContextItem,
|
||||
after: &EnvironmentContext,
|
||||
) -> Self {
|
||||
) -> std::io::Result<Self> {
|
||||
let before_network = Self::network_from_turn_context_item(before);
|
||||
let before_filesystem = Self::filesystem_from_turn_context_item(before);
|
||||
let before_filesystem = Self::filesystem_from_turn_context_item(before)?;
|
||||
let environments = match &after.environments {
|
||||
EnvironmentContextEnvironments::Single(environment) => {
|
||||
if before.cwd.as_path() != environment.cwd.as_path() {
|
||||
if before.cwd != environment.cwd {
|
||||
EnvironmentContextEnvironments::Single(EnvironmentContextEnvironment::legacy(
|
||||
environment.cwd.clone(),
|
||||
environment.shell.clone(),
|
||||
@@ -412,14 +409,14 @@ impl EnvironmentContext {
|
||||
} else {
|
||||
before_filesystem
|
||||
};
|
||||
EnvironmentContext::new_with_environments(
|
||||
Ok(EnvironmentContext::new_with_environments(
|
||||
environments,
|
||||
after.current_date.clone(),
|
||||
after.timezone.clone(),
|
||||
network,
|
||||
filesystem,
|
||||
/*subagents*/ None,
|
||||
)
|
||||
))
|
||||
}
|
||||
|
||||
pub(crate) fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self {
|
||||
@@ -443,21 +440,18 @@ impl EnvironmentContext {
|
||||
pub(crate) fn from_turn_context_item(
|
||||
turn_context_item: &TurnContextItem,
|
||||
shell: String,
|
||||
) -> Self {
|
||||
let cwd = match AbsolutePathBuf::try_from(turn_context_item.cwd.clone()) {
|
||||
Ok(cwd) => cwd,
|
||||
Err(_) => AbsolutePathBuf::resolve_path_against_base(&turn_context_item.cwd, "/"),
|
||||
};
|
||||
Self::new_with_environments(
|
||||
) -> std::io::Result<Self> {
|
||||
Ok(Self::new_with_environments(
|
||||
EnvironmentContextEnvironments::from_vec(vec![EnvironmentContextEnvironment::legacy(
|
||||
cwd, shell,
|
||||
turn_context_item.cwd.clone(),
|
||||
shell,
|
||||
)]),
|
||||
turn_context_item.current_date.clone(),
|
||||
turn_context_item.timezone.clone(),
|
||||
Self::network_from_turn_context_item(turn_context_item),
|
||||
Self::filesystem_from_turn_context_item(turn_context_item),
|
||||
Self::filesystem_from_turn_context_item(turn_context_item)?,
|
||||
/*subagents*/ None,
|
||||
)
|
||||
))
|
||||
}
|
||||
|
||||
pub(crate) fn with_subagents(mut self, subagents: String) -> Self {
|
||||
@@ -504,11 +498,11 @@ impl EnvironmentContext {
|
||||
|
||||
fn filesystem_from_turn_context_item(
|
||||
turn_context_item: &TurnContextItem,
|
||||
) -> Option<FileSystemContext> {
|
||||
Some(FileSystemContext::from_permission_profile(
|
||||
&turn_context_item.permission_profile(),
|
||||
) -> std::io::Result<Option<FileSystemContext>> {
|
||||
Ok(Some(FileSystemContext::from_permission_profile(
|
||||
&turn_context_item.permission_profile()?,
|
||||
&workspace_roots_from_turn_context_item(turn_context_item),
|
||||
))
|
||||
)))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -521,7 +515,7 @@ fn workspace_roots_from_turn_context_item(
|
||||
|
||||
// Older rollout items did not persist workspace roots. Fall back to the
|
||||
// legacy cwd binding only when reconstructing that historical context.
|
||||
match AbsolutePathBuf::try_from(turn_context_item.cwd.clone()) {
|
||||
match turn_context_item.cwd.to_abs_path() {
|
||||
Ok(cwd) => vec![cwd],
|
||||
Err(_) => Vec::new(),
|
||||
}
|
||||
@@ -547,20 +541,16 @@ impl ContextualUserFragment for EnvironmentContext {
|
||||
let mut lines = Vec::new();
|
||||
match &self.environments {
|
||||
EnvironmentContextEnvironments::Single(environment) => {
|
||||
lines.push(format!(
|
||||
" <cwd>{}</cwd>",
|
||||
environment.cwd.to_string_lossy()
|
||||
));
|
||||
let cwd = environment.cwd.inferred_native_path_string();
|
||||
lines.push(format!(" <cwd>{cwd}</cwd>"));
|
||||
lines.push(format!(" <shell>{}</shell>", environment.shell));
|
||||
}
|
||||
EnvironmentContextEnvironments::Multiple(environments) => {
|
||||
lines.push(" <environments>".to_string());
|
||||
for environment in environments {
|
||||
lines.push(format!(" <environment id=\"{}\">", environment.id));
|
||||
lines.push(format!(
|
||||
" <cwd>{}</cwd>",
|
||||
environment.cwd.to_string_lossy()
|
||||
));
|
||||
let cwd = environment.cwd.inferred_native_path_string();
|
||||
lines.push(format!(" <cwd>{cwd}</cwd>"));
|
||||
lines.push(format!(" <shell>{}</shell>", environment.shell));
|
||||
lines.push(" </environment>".to_string());
|
||||
}
|
||||
|
||||
@@ -36,7 +36,7 @@ fn serialize_workspace_write_environment_context() {
|
||||
let context = EnvironmentContext::new(
|
||||
vec![EnvironmentContextEnvironment {
|
||||
id: "local".to_string(),
|
||||
cwd: cwd.abs(),
|
||||
cwd: PathUri::from_abs_path(&cwd.abs()),
|
||||
shell: fake_shell_name(),
|
||||
}],
|
||||
Some("2026-02-26".to_string()),
|
||||
@@ -58,6 +58,29 @@ fn serialize_workspace_write_environment_context() {
|
||||
assert_eq!(context.render(), expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn serialize_environment_context_with_foreign_windows_cwd() {
|
||||
let context = EnvironmentContext::new(
|
||||
vec![EnvironmentContextEnvironment {
|
||||
id: "remote".to_string(),
|
||||
cwd: PathUri::parse("file:///C:/windows").expect("Windows cwd URI"),
|
||||
shell: "powershell".to_string(),
|
||||
}],
|
||||
/*current_date*/ None,
|
||||
/*timezone*/ None,
|
||||
/*network*/ None,
|
||||
/*subagents*/ None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
context.render(),
|
||||
r#"<environment_context>
|
||||
<cwd>C:\windows</cwd>
|
||||
<shell>powershell</shell>
|
||||
</environment_context>"#
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn serialize_environment_context_with_network() {
|
||||
let network = NetworkContext::new(
|
||||
@@ -67,7 +90,7 @@ fn serialize_environment_context_with_network() {
|
||||
let context = EnvironmentContext::new(
|
||||
vec![EnvironmentContextEnvironment {
|
||||
id: "local".to_string(),
|
||||
cwd: test_path_buf("/repo").abs(),
|
||||
cwd: PathUri::from_abs_path(&test_abs_path("/repo")),
|
||||
shell: fake_shell_name(),
|
||||
}],
|
||||
Some("2026-02-26".to_string()),
|
||||
@@ -129,7 +152,7 @@ fn serialize_environment_context_with_full_filesystem_profile() {
|
||||
let mut context = EnvironmentContext::new(
|
||||
vec![EnvironmentContextEnvironment {
|
||||
id: "local".to_string(),
|
||||
cwd: test_path_buf("/repo").abs(),
|
||||
cwd: PathUri::from_abs_path(&test_abs_path("/repo")),
|
||||
shell: fake_shell_name(),
|
||||
}],
|
||||
/*current_date*/ None,
|
||||
@@ -167,7 +190,7 @@ fn turn_context_item_filesystem_uses_workspace_roots_instead_of_cwd() {
|
||||
let repo_private = repo.join("private");
|
||||
let item = TurnContextItem {
|
||||
turn_id: None,
|
||||
cwd: test_path_buf("/not-the-workspace"),
|
||||
cwd: PathUri::from_abs_path(&test_abs_path("/not-the-workspace")),
|
||||
workspace_roots: Some(vec![repo.clone(), other_repo.clone()]),
|
||||
current_date: None,
|
||||
timezone: None,
|
||||
@@ -186,7 +209,9 @@ fn turn_context_item_filesystem_uses_workspace_roots_instead_of_cwd() {
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
};
|
||||
|
||||
let context = EnvironmentContext::from_turn_context_item(&item, fake_shell_name()).render();
|
||||
let context = EnvironmentContext::from_turn_context_item(&item, fake_shell_name())
|
||||
.expect("turn context should hydrate")
|
||||
.render();
|
||||
|
||||
assert!(
|
||||
context.contains(&format!(
|
||||
@@ -234,7 +259,7 @@ fn equals_except_shell_compares_cwd() {
|
||||
let context1 = EnvironmentContext::new(
|
||||
vec![EnvironmentContextEnvironment {
|
||||
id: "local".to_string(),
|
||||
cwd: test_abs_path("/repo"),
|
||||
cwd: PathUri::from_abs_path(&test_abs_path("/repo")),
|
||||
shell: fake_shell_name(),
|
||||
}],
|
||||
/*current_date*/ None,
|
||||
@@ -245,7 +270,7 @@ fn equals_except_shell_compares_cwd() {
|
||||
let context2 = EnvironmentContext::new(
|
||||
vec![EnvironmentContextEnvironment {
|
||||
id: "local".to_string(),
|
||||
cwd: test_abs_path("/repo"),
|
||||
cwd: PathUri::from_abs_path(&test_abs_path("/repo")),
|
||||
shell: fake_shell_name(),
|
||||
}],
|
||||
/*current_date*/ None,
|
||||
@@ -261,7 +286,7 @@ fn equals_except_shell_compares_cwd_differences() {
|
||||
let context1 = EnvironmentContext::new(
|
||||
vec![EnvironmentContextEnvironment {
|
||||
id: "local".to_string(),
|
||||
cwd: test_abs_path("/repo1"),
|
||||
cwd: PathUri::from_abs_path(&test_abs_path("/repo1")),
|
||||
shell: fake_shell_name(),
|
||||
}],
|
||||
/*current_date*/ None,
|
||||
@@ -272,7 +297,7 @@ fn equals_except_shell_compares_cwd_differences() {
|
||||
let context2 = EnvironmentContext::new(
|
||||
vec![EnvironmentContextEnvironment {
|
||||
id: "local".to_string(),
|
||||
cwd: test_abs_path("/repo2"),
|
||||
cwd: PathUri::from_abs_path(&test_abs_path("/repo2")),
|
||||
shell: fake_shell_name(),
|
||||
}],
|
||||
/*current_date*/ None,
|
||||
@@ -289,7 +314,7 @@ fn equals_except_shell_ignores_shell() {
|
||||
let context1 = EnvironmentContext::new(
|
||||
vec![EnvironmentContextEnvironment {
|
||||
id: "local".to_string(),
|
||||
cwd: test_abs_path("/repo"),
|
||||
cwd: PathUri::from_abs_path(&test_abs_path("/repo")),
|
||||
shell: "bash".to_string(),
|
||||
}],
|
||||
/*current_date*/ None,
|
||||
@@ -300,7 +325,7 @@ fn equals_except_shell_ignores_shell() {
|
||||
let context2 = EnvironmentContext::new(
|
||||
vec![EnvironmentContextEnvironment {
|
||||
id: "other".to_string(),
|
||||
cwd: test_abs_path("/repo"),
|
||||
cwd: PathUri::from_abs_path(&test_abs_path("/repo")),
|
||||
shell: "zsh".to_string(),
|
||||
}],
|
||||
/*current_date*/ None,
|
||||
@@ -317,7 +342,7 @@ fn serialize_environment_context_with_subagents() {
|
||||
let context = EnvironmentContext::new(
|
||||
vec![EnvironmentContextEnvironment {
|
||||
id: "local".to_string(),
|
||||
cwd: test_path_buf("/repo").abs(),
|
||||
cwd: PathUri::from_abs_path(&test_abs_path("/repo")),
|
||||
shell: fake_shell_name(),
|
||||
}],
|
||||
Some("2026-02-26".to_string()),
|
||||
@@ -351,12 +376,12 @@ fn serialize_environment_context_with_multiple_selected_environments() {
|
||||
vec![
|
||||
EnvironmentContextEnvironment {
|
||||
id: "local".to_string(),
|
||||
cwd: local_cwd.abs(),
|
||||
cwd: PathUri::from_abs_path(&local_cwd.abs()),
|
||||
shell: "bash".to_string(),
|
||||
},
|
||||
EnvironmentContextEnvironment {
|
||||
id: "remote".to_string(),
|
||||
cwd: remote_cwd.abs(),
|
||||
cwd: PathUri::from_abs_path(&remote_cwd.abs()),
|
||||
shell: "bash".to_string(),
|
||||
},
|
||||
],
|
||||
@@ -396,12 +421,12 @@ fn serialize_environment_context_prefers_environment_shell_when_present() {
|
||||
vec![
|
||||
EnvironmentContextEnvironment {
|
||||
id: "local".to_string(),
|
||||
cwd: local_cwd.abs(),
|
||||
cwd: PathUri::from_abs_path(&local_cwd.abs()),
|
||||
shell: "powershell".to_string(),
|
||||
},
|
||||
EnvironmentContextEnvironment {
|
||||
id: "remote".to_string(),
|
||||
cwd: remote_cwd.abs(),
|
||||
cwd: PathUri::from_abs_path(&remote_cwd.abs()),
|
||||
shell: "cmd".to_string(),
|
||||
},
|
||||
],
|
||||
|
||||
Reference in New Issue
Block a user