Skip to content

Commit d2b2a6d

Browse files
[prompt] xml-format EnvironmentContext (#2272)
## Summary Before we land #2243, let's start printing environment_context in our preferred format. This struct will evolve over time with new information, xml gives us a balance of human readable without too much parsing, llm readable, and extensible. Also moves us over to an Option-based struct, so we can easily provide diffs to the model. ## Testing - [x] Updated tests to reflect new format
1 parent 74683ba commit d2b2a6d

File tree

3 files changed

+85
-57
lines changed

3 files changed

+85
-57
lines changed

codex-rs/core/src/codex.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -508,10 +508,10 @@ impl Session {
508508
conversation_items.push(Prompt::format_user_instructions_message(user_instructions));
509509
}
510510
conversation_items.push(ResponseItem::from(EnvironmentContext::new(
511-
turn_context.cwd.to_path_buf(),
512-
turn_context.approval_policy,
513-
turn_context.sandbox_policy.clone(),
514-
sess.user_shell.clone(),
511+
Some(turn_context.cwd.clone()),
512+
Some(turn_context.approval_policy),
513+
Some(turn_context.sandbox_policy.clone()),
514+
Some(sess.user_shell.clone()),
515515
)));
516516
sess.record_conversation_items(&conversation_items).await;
517517

@@ -1068,10 +1068,11 @@ async fn submission_loop(
10681068
turn_context = Arc::new(new_turn_context);
10691069
if cwd.is_some() || approval_policy.is_some() || sandbox_policy.is_some() {
10701070
sess.record_conversation_items(&[ResponseItem::from(EnvironmentContext::new(
1071-
new_cwd,
1072-
new_approval_policy,
1073-
new_sandbox_policy,
1074-
sess.user_shell.clone(),
1071+
cwd,
1072+
approval_policy,
1073+
sandbox_policy,
1074+
// Shell is not configurable from turn to turn
1075+
None,
10751076
))])
10761077
.await;
10771078
}

codex-rs/core/src/environment_context.rs

Lines changed: 61 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,10 @@ use crate::protocol::AskForApproval;
88
use crate::protocol::SandboxPolicy;
99
use crate::shell::Shell;
1010
use codex_protocol::config_types::SandboxMode;
11-
use std::fmt::Display;
1211
use std::path::PathBuf;
1312

1413
/// wraps environment context message in a tag for the model to parse more easily.
15-
pub(crate) const ENVIRONMENT_CONTEXT_START: &str = "<environment_context>\n";
14+
pub(crate) const ENVIRONMENT_CONTEXT_START: &str = "<environment_context>";
1615
pub(crate) const ENVIRONMENT_CONTEXT_END: &str = "</environment_context>";
1716

1817
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, DeriveDisplay)]
@@ -25,58 +24,87 @@ pub enum NetworkAccess {
2524
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
2625
#[serde(rename = "environment_context", rename_all = "snake_case")]
2726
pub(crate) struct EnvironmentContext {
28-
pub cwd: PathBuf,
29-
pub approval_policy: AskForApproval,
30-
pub sandbox_mode: SandboxMode,
31-
pub network_access: NetworkAccess,
32-
pub shell: Shell,
27+
pub cwd: Option<PathBuf>,
28+
pub approval_policy: Option<AskForApproval>,
29+
pub sandbox_mode: Option<SandboxMode>,
30+
pub network_access: Option<NetworkAccess>,
31+
pub shell: Option<Shell>,
3332
}
3433

3534
impl EnvironmentContext {
3635
pub fn new(
37-
cwd: PathBuf,
38-
approval_policy: AskForApproval,
39-
sandbox_policy: SandboxPolicy,
40-
shell: Shell,
36+
cwd: Option<PathBuf>,
37+
approval_policy: Option<AskForApproval>,
38+
sandbox_policy: Option<SandboxPolicy>,
39+
shell: Option<Shell>,
4140
) -> Self {
4241
Self {
4342
cwd,
4443
approval_policy,
4544
sandbox_mode: match sandbox_policy {
46-
SandboxPolicy::DangerFullAccess => SandboxMode::DangerFullAccess,
47-
SandboxPolicy::ReadOnly => SandboxMode::ReadOnly,
48-
SandboxPolicy::WorkspaceWrite { .. } => SandboxMode::WorkspaceWrite,
45+
Some(SandboxPolicy::DangerFullAccess) => Some(SandboxMode::DangerFullAccess),
46+
Some(SandboxPolicy::ReadOnly) => Some(SandboxMode::ReadOnly),
47+
Some(SandboxPolicy::WorkspaceWrite { .. }) => Some(SandboxMode::WorkspaceWrite),
48+
None => None,
4949
},
5050
network_access: match sandbox_policy {
51-
SandboxPolicy::DangerFullAccess => NetworkAccess::Enabled,
52-
SandboxPolicy::ReadOnly => NetworkAccess::Restricted,
53-
SandboxPolicy::WorkspaceWrite { network_access, .. } => {
51+
Some(SandboxPolicy::DangerFullAccess) => Some(NetworkAccess::Enabled),
52+
Some(SandboxPolicy::ReadOnly) => Some(NetworkAccess::Restricted),
53+
Some(SandboxPolicy::WorkspaceWrite { network_access, .. }) => {
5454
if network_access {
55-
NetworkAccess::Enabled
55+
Some(NetworkAccess::Enabled)
5656
} else {
57-
NetworkAccess::Restricted
57+
Some(NetworkAccess::Restricted)
5858
}
5959
}
60+
None => None,
6061
},
6162
shell,
6263
}
6364
}
6465
}
6566

66-
impl Display for EnvironmentContext {
67-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
68-
writeln!(
69-
f,
70-
"Current working directory: {}",
71-
self.cwd.to_string_lossy()
72-
)?;
73-
writeln!(f, "Approval policy: {}", self.approval_policy)?;
74-
writeln!(f, "Sandbox mode: {}", self.sandbox_mode)?;
75-
writeln!(f, "Network access: {}", self.network_access)?;
76-
if let Some(shell_name) = self.shell.name() {
77-
writeln!(f, "Shell: {shell_name}")?;
67+
impl EnvironmentContext {
68+
/// Serializes the environment context to XML. Libraries like `quick-xml`
69+
/// require custom macros to handle Enums with newtypes, so we just do it
70+
/// manually, to keep things simple. Output looks like:
71+
///
72+
/// ```xml
73+
/// <environment_context>
74+
/// <cwd>...</cwd>
75+
/// <approval_policy>...</approval_policy>
76+
/// <sandbox_mode>...</sandbox_mode>
77+
/// <network_access>...</network_access>
78+
/// <shell>...</shell>
79+
/// </environment_context>
80+
/// ```
81+
pub fn serialize_to_xml(self) -> String {
82+
let mut lines = vec![ENVIRONMENT_CONTEXT_START.to_string()];
83+
if let Some(cwd) = self.cwd {
84+
lines.push(format!(" <cwd>{}</cwd>", cwd.to_string_lossy()));
85+
}
86+
if let Some(approval_policy) = self.approval_policy {
87+
lines.push(format!(
88+
" <approval_policy>{}</approval_policy>",
89+
approval_policy
90+
));
91+
}
92+
if let Some(sandbox_mode) = self.sandbox_mode {
93+
lines.push(format!(" <sandbox_mode>{}</sandbox_mode>", sandbox_mode));
94+
}
95+
if let Some(network_access) = self.network_access {
96+
lines.push(format!(
97+
" <network_access>{}</network_access>",
98+
network_access
99+
));
100+
}
101+
if let Some(shell) = self.shell
102+
&& let Some(shell_name) = shell.name()
103+
{
104+
lines.push(format!(" <shell>{}</shell>", shell_name));
78105
}
79-
Ok(())
106+
lines.push(ENVIRONMENT_CONTEXT_END.to_string());
107+
lines.join("\n")
80108
}
81109
}
82110

@@ -86,7 +114,7 @@ impl From<EnvironmentContext> for ResponseItem {
86114
id: None,
87115
role: "user".to_string(),
88116
content: vec![ContentItem::InputText {
89-
text: format!("{ENVIRONMENT_CONTEXT_START}{ec}{ENVIRONMENT_CONTEXT_END}"),
117+
text: ec.serialize_to_xml(),
90118
}],
91119
}
92120
}

codex-rs/core/tests/prompt_caching.rs

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,15 @@ async fn prefixes_context_and_instructions_once_and_consistently_across_requests
8989
let shell = default_user_shell().await;
9090

9191
let expected_env_text = format!(
92-
"<environment_context>\nCurrent working directory: {}\nApproval policy: on-request\nSandbox mode: read-only\nNetwork access: restricted\n{}</environment_context>",
92+
r#"<environment_context>
93+
<cwd>{}</cwd>
94+
<approval_policy>on-request</approval_policy>
95+
<sandbox_mode>read-only</sandbox_mode>
96+
<network_access>restricted</network_access>
97+
{}</environment_context>"#,
9398
cwd.path().to_string_lossy(),
9499
match shell.name() {
95-
Some(name) => format!("Shell: {name}\n"),
100+
Some(name) => format!(" <shell>{}</shell>\n", name),
96101
None => String::new(),
97102
}
98103
);
@@ -190,12 +195,10 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() {
190195
.unwrap();
191196
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
192197

193-
// Change everything about the turn context.
194-
let new_cwd = TempDir::new().unwrap();
195198
let writable = TempDir::new().unwrap();
196199
codex
197200
.submit(Op::OverrideTurnContext {
198-
cwd: Some(new_cwd.path().to_path_buf()),
201+
cwd: None,
199202
approval_policy: Some(AskForApproval::Never),
200203
sandbox_policy: Some(SandboxPolicy::WorkspaceWrite {
201204
writable_roots: vec![writable.path().to_path_buf()],
@@ -227,7 +230,6 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() {
227230

228231
let body1 = requests[0].body_json::<serde_json::Value>().unwrap();
229232
let body2 = requests[1].body_json::<serde_json::Value>().unwrap();
230-
231233
// prompt_cache_key should remain constant across overrides
232234
assert_eq!(
233235
body1["prompt_cache_key"], body2["prompt_cache_key"],
@@ -243,16 +245,13 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() {
243245
"content": [ { "type": "input_text", "text": "hello 2" } ]
244246
});
245247
// After overriding the turn context, the environment context should be emitted again
246-
// reflecting the new cwd, approval policy and sandbox settings.
247-
let shell = default_user_shell().await;
248-
let expected_env_text_2 = format!(
249-
"<environment_context>\nCurrent working directory: {}\nApproval policy: never\nSandbox mode: workspace-write\nNetwork access: enabled\n{}</environment_context>",
250-
new_cwd.path().to_string_lossy(),
251-
match shell.name() {
252-
Some(name) => format!("Shell: {name}\n"),
253-
None => String::new(),
254-
}
255-
);
248+
// reflecting the new approval policy and sandbox settings. Omit cwd because it did
249+
// not change.
250+
let expected_env_text_2 = r#"<environment_context>
251+
<approval_policy>never</approval_policy>
252+
<sandbox_mode>workspace-write</sandbox_mode>
253+
<network_access>enabled</network_access>
254+
</environment_context>"#;
256255
let expected_env_msg_2 = serde_json::json!({
257256
"type": "message",
258257
"id": serde_json::Value::Null,

0 commit comments

Comments
 (0)