mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Make environment context date and timezone configurable (#52164)
## What changed Add `include_environment_context_time`, enabled by default. Setting it to `false` omits `<current_date>` and `<timezone>` while retaining the rest of the environment context, and skips the clock read used to populate the date. ## Testing Extend configuration, session context, and external time provider tests to cover enabled and disabled time fields, context updates across turns, and current-time reminder behavior. GitOrigin-RevId: 5071ce1701c044b680624d0cdbba7e0bc2b5bbe6
This commit is contained in:
@@ -3,6 +3,7 @@ include_permissions_instructions = true
|
||||
include_apps_instructions = true
|
||||
include_collaboration_mode_instructions = true
|
||||
include_environment_context = true
|
||||
include_environment_context_time = true
|
||||
cli_auth_credentials_store = "file"
|
||||
mcp_oauth_credentials_store = "auto"
|
||||
project_doc_max_bytes = 32768
|
||||
|
||||
@@ -265,6 +265,9 @@ pub struct ConfigToml {
|
||||
/// Whether to inject the `<environment_context>` user block.
|
||||
pub include_environment_context: Option<bool>,
|
||||
|
||||
/// Whether environment context includes the current date and timezone.
|
||||
pub include_environment_context_time: Option<bool>,
|
||||
|
||||
/// Optional path to a file containing model instructions that will override
|
||||
/// the built-in instructions for the selected model. Users are STRONGLY
|
||||
/// DISCOURAGED from using this field, as deviating from the instructions
|
||||
|
||||
@@ -55,6 +55,7 @@ pub struct ConfigProfile {
|
||||
pub include_apps_instructions: Option<bool>,
|
||||
pub include_collaboration_mode_instructions: Option<bool>,
|
||||
pub include_environment_context: Option<bool>,
|
||||
pub include_environment_context_time: Option<bool>,
|
||||
pub experimental_use_unified_exec_tool: Option<bool>,
|
||||
pub tools: Option<ToolsToml>,
|
||||
pub web_search: Option<WebSearchMode>,
|
||||
|
||||
@@ -1283,6 +1283,9 @@
|
||||
"include_environment_context": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_environment_context_time": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_permissions_instructions": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -7585,6 +7588,10 @@
|
||||
"description": "Whether to inject the `<environment_context>` user block.",
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_environment_context_time": {
|
||||
"description": "Whether environment context includes the current date and timezone.",
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_permissions_instructions": {
|
||||
"description": "Whether to inject the `<permissions instructions>` developer block.",
|
||||
"type": "boolean"
|
||||
|
||||
@@ -11882,6 +11882,7 @@ async fn prompt_instruction_blocks_can_be_disabled_from_config() -> std::io::Res
|
||||
include_apps_instructions = false
|
||||
include_collaboration_mode_instructions = false
|
||||
include_environment_context = false
|
||||
include_environment_context_time = false
|
||||
|
||||
[skills]
|
||||
include_instructions = false
|
||||
@@ -11899,6 +11900,7 @@ include_instructions = false
|
||||
assert!(!config.include_collaboration_mode_instructions);
|
||||
assert!(!config.include_skill_instructions);
|
||||
assert!(!config.include_environment_context);
|
||||
assert!(!config.include_environment_context_time);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -745,6 +745,9 @@ pub struct Config {
|
||||
/// Whether to inject the `<environment_context>` user block.
|
||||
pub include_environment_context: bool,
|
||||
|
||||
/// Whether environment context includes the current date and timezone.
|
||||
pub include_environment_context_time: bool,
|
||||
|
||||
/// Compact prompt override.
|
||||
pub compact_prompt: Option<String>,
|
||||
|
||||
@@ -4047,6 +4050,7 @@ impl Config {
|
||||
.as_ref()
|
||||
.and_then(|skills| skills.max_context_tokens);
|
||||
let include_environment_context = cfg.include_environment_context.unwrap_or(true);
|
||||
let include_environment_context_time = cfg.include_environment_context_time.unwrap_or(true);
|
||||
let guardian_policy_config =
|
||||
guardian_policy_config_from_requirements(config_layer_stack.requirements_toml())
|
||||
.or_else(|| {
|
||||
@@ -4356,6 +4360,7 @@ impl Config {
|
||||
cloud_skill_enabled,
|
||||
orchestrator_mcp_enabled,
|
||||
include_environment_context,
|
||||
include_environment_context_time,
|
||||
// The config.toml omits "_mode" because it's a config file. However, "_mode"
|
||||
// is important in code to differentiate the mode from the store implementation.
|
||||
cli_auth_credentials_store_mode: match cli_auth_credentials_store {
|
||||
|
||||
@@ -61,7 +61,10 @@ impl EnvironmentsState {
|
||||
environments: environment_states(environments),
|
||||
shell_version,
|
||||
current_date,
|
||||
timezone: turn_context.timezone.clone(),
|
||||
timezone: turn_context
|
||||
.timezone
|
||||
.clone()
|
||||
.filter(|_| turn_context.config.include_environment_context_time),
|
||||
network: network_from_turn_context(turn_context),
|
||||
filesystem: environments.primary().map(|environment| {
|
||||
FileSystemContext::from_permission_profile(
|
||||
|
||||
@@ -10265,9 +10265,14 @@ async fn record_context_updates_emits_environment_item_for_cwd_changes() {
|
||||
assert!(!environment_update.contains("<environments>"));
|
||||
}
|
||||
|
||||
#[test_case::test_case(true; "with_time")]
|
||||
#[test_case::test_case(false; "without_time")]
|
||||
#[tokio::test]
|
||||
async fn record_context_updates_use_environment_permission_profile_and_workspace_roots() {
|
||||
async fn record_context_updates_use_environment_permission_profile_and_workspace_roots(
|
||||
include_time: bool,
|
||||
) {
|
||||
let (session, mut previous_context) = make_session_and_context().await;
|
||||
Arc::make_mut(&mut previous_context.config).include_environment_context_time = include_time;
|
||||
Arc::make_mut(&mut previous_context.config)
|
||||
.permissions
|
||||
.set_permission_profile(PermissionProfile::Disabled)
|
||||
@@ -10332,6 +10337,8 @@ async fn record_context_updates_use_environment_permission_profile_and_workspace
|
||||
&& environment_update.contains(workspace_root.to_string_lossy().as_ref()),
|
||||
"selected environment permissions should be visible: {environment_update}"
|
||||
);
|
||||
assert_eq!(environment_update.contains("<current_date"), include_time);
|
||||
assert_eq!(environment_update.contains("<timezone"), include_time);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -264,15 +264,18 @@ impl Session {
|
||||
));
|
||||
}
|
||||
if turn_context.config.include_environment_context {
|
||||
let current_date = self
|
||||
.read_clock_for_context(turn_context, "environment_date")
|
||||
.await?
|
||||
.map(|current_time| {
|
||||
current_time
|
||||
.with_timezone(&chrono::Local)
|
||||
.format("%Y-%m-%d")
|
||||
.to_string()
|
||||
});
|
||||
let current_date = if turn_context.config.include_environment_context_time {
|
||||
self.read_clock_for_context(turn_context, "environment_date")
|
||||
.await?
|
||||
.map(|current_time| {
|
||||
current_time
|
||||
.with_timezone(&chrono::Local)
|
||||
.format("%Y-%m-%d")
|
||||
.to_string()
|
||||
})
|
||||
} else {
|
||||
None
|
||||
};
|
||||
world_state.add_section(
|
||||
EnvironmentsState::from_turn_context_with_environments(
|
||||
turn_context,
|
||||
|
||||
@@ -148,8 +148,13 @@ fn enable_current_time_reminder(
|
||||
});
|
||||
}
|
||||
|
||||
#[test_case(true, SECOND_REMINDER; "default")]
|
||||
#[test_case(false, FIRST_REMINDER; "time_omitted")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn environment_context_uses_external_current_time_on_each_turn() -> Result<()> {
|
||||
async fn environment_context_time_follows_config(
|
||||
include_time: bool,
|
||||
expected_reminder: &str,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
@@ -163,9 +168,12 @@ async fn environment_context_uses_external_current_time_on_each_turn() -> Result
|
||||
.await;
|
||||
let time_provider = Arc::new(TestTimeProvider::default());
|
||||
let test = test_codex()
|
||||
.with_config(|config| {
|
||||
.with_config(move |config| {
|
||||
enable_current_time_reminder(config, /*interval*/ 0, CurrentTimeSource::External);
|
||||
config.include_environment_context = true;
|
||||
if !include_time {
|
||||
config.include_environment_context_time = false;
|
||||
}
|
||||
})
|
||||
.with_external_time_provider(time_provider.clone())
|
||||
.build_with_auto_env(&server)
|
||||
@@ -179,22 +187,32 @@ async fn environment_context_uses_external_current_time_on_each_turn() -> Result
|
||||
|
||||
let requests = responses.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
for (request, timestamp) in requests
|
||||
for (index, (request, timestamp)) in requests
|
||||
.iter()
|
||||
.zip([FIRST_TIME_UNIX_SECONDS, FIRST_TIME_UNIX_SECONDS + 86_400])
|
||||
.enumerate()
|
||||
{
|
||||
assert!(request.has_content_kinds(&["environments.environment_context"]));
|
||||
let current_date = DateTime::<Utc>::from_timestamp(timestamp, 0)
|
||||
.expect("test timestamp should be valid")
|
||||
.with_timezone(&Local)
|
||||
.format("%Y-%m-%d")
|
||||
.to_string();
|
||||
assert!(request.message_input_texts("user").iter().any(|text| {
|
||||
text.contains("<environment_context>")
|
||||
&& text.contains(&format!("<current_date>{current_date}</current_date>"))
|
||||
}));
|
||||
let text = request.message_input_texts("user").join("\n");
|
||||
assert_eq!(
|
||||
text.matches("<environment_context>").count(),
|
||||
if include_time { index + 1 } else { 1 }
|
||||
);
|
||||
assert_eq!(text.contains("<current_date"), include_time);
|
||||
assert_eq!(text.contains("<timezone"), include_time);
|
||||
if include_time {
|
||||
let current_date = DateTime::<Utc>::from_timestamp(timestamp, 0)
|
||||
.expect("test timestamp should be valid")
|
||||
.with_timezone(&Local)
|
||||
.format("%Y-%m-%d")
|
||||
.to_string();
|
||||
assert!(text.contains(&format!("<current_date>{current_date}</current_date>")));
|
||||
}
|
||||
}
|
||||
assert_eq!(current_time_reminders(&requests[0]), vec![SECOND_REMINDER]);
|
||||
assert_eq!(
|
||||
current_time_reminders(&requests[0]),
|
||||
vec![expected_reminder]
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -280,6 +280,7 @@ async fn new_config(
|
||||
cloud_skill_enabled: false,
|
||||
orchestrator_mcp_enabled: false,
|
||||
include_environment_context: false,
|
||||
include_environment_context_time: true,
|
||||
compact_prompt: None,
|
||||
notify: None,
|
||||
tui_notifications: TuiNotificationSettings::default(),
|
||||
|
||||
Reference in New Issue
Block a user