mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Refresh step world state for all sessions (#33665)
## Why Changes to the selected working directory can select different `AGENTS.md` instructions. These updates need to reach the model even when the deferred executor feature is disabled. ## What changed - Refresh environment readiness and `AGENTS.md` before capturing each step. - Record model-visible world-state changes for every session instead of gating them on the deferred executor feature. - Preserve the initial environment context without duplicating it. ## Testing - Cover `AGENTS.md` replacement after a working-directory change. - Verify that initial environment context remains unique with the deferred executor both enabled and disabled. GitOrigin-RevId: ea5dbe590cc049170259e7eccb6a3346fb2ed8e6
This commit is contained in:
@@ -204,8 +204,7 @@ impl Session {
|
||||
.iter()
|
||||
.chain(
|
||||
environments
|
||||
.turn_environments
|
||||
.iter()
|
||||
.turn_environments()
|
||||
.flat_map(|environment| environment.environment.selected_capability_roots()),
|
||||
)
|
||||
.enumerate()
|
||||
|
||||
@@ -2849,22 +2849,12 @@ impl Session {
|
||||
self: &Arc<Self>,
|
||||
turn_context: Arc<TurnContext>,
|
||||
) -> Arc<StepContext> {
|
||||
let deferred_executor_enabled = turn_context
|
||||
.config
|
||||
.features
|
||||
.enabled(Feature::DeferredExecutor);
|
||||
let environments = if deferred_executor_enabled {
|
||||
// Keep selections fixed for the turn while allowing their startup work to finish.
|
||||
turn_context.environments.refresh_readiness()
|
||||
} else {
|
||||
turn_context.environments.clone()
|
||||
};
|
||||
if deferred_executor_enabled {
|
||||
self.services
|
||||
.agents_md_manager
|
||||
.refresh(&turn_context.config, &environments)
|
||||
.await;
|
||||
}
|
||||
// Keep selections fixed for the turn while allowing their startup work to finish.
|
||||
let environments = turn_context.environments.refresh_readiness();
|
||||
self.services
|
||||
.agents_md_manager
|
||||
.refresh(&turn_context.config, &environments)
|
||||
.await;
|
||||
let loaded_agents_md = self.services.agents_md_manager.get_loaded().await;
|
||||
let selected_capability_roots = self
|
||||
.resolve_selected_capability_roots_for_step(&environments)
|
||||
|
||||
@@ -7793,14 +7793,13 @@ async fn deferred_environment_roots_refresh_plugin_availability() {
|
||||
.primary()
|
||||
.expect("ready local environment");
|
||||
let environments = TurnEnvironmentSnapshot {
|
||||
turn_environments: vec![TurnEnvironment::new(
|
||||
environments: vec![TurnEnvironmentState::Ready(TurnEnvironment::new(
|
||||
"executor".to_string(),
|
||||
environment,
|
||||
local_environment.cwd().clone(),
|
||||
local_environment.workspace_roots().to_vec(),
|
||||
local_environment.shell.clone(),
|
||||
)],
|
||||
starting: Vec::new(),
|
||||
))],
|
||||
};
|
||||
let resolved_roots = session
|
||||
.resolve_selected_capability_roots_for_step(&environments)
|
||||
@@ -7873,8 +7872,10 @@ async fn conflicting_ready_environment_root_ids_keep_first_location() {
|
||||
));
|
||||
}
|
||||
let environments = TurnEnvironmentSnapshot {
|
||||
turn_environments,
|
||||
starting: Vec::new(),
|
||||
environments: turn_environments
|
||||
.into_iter()
|
||||
.map(TurnEnvironmentState::Ready)
|
||||
.collect(),
|
||||
};
|
||||
|
||||
let resolved_roots = session
|
||||
|
||||
@@ -259,15 +259,9 @@ pub(crate) async fn run_turn(
|
||||
)
|
||||
.await?;
|
||||
|
||||
if turn_context
|
||||
.config
|
||||
.features
|
||||
.enabled(Feature::DeferredExecutor)
|
||||
{
|
||||
world_state = sess
|
||||
.record_step_world_state_if_changed(&world_state, step_context.as_ref())
|
||||
.await;
|
||||
}
|
||||
world_state = sess
|
||||
.record_step_world_state_if_changed(&world_state, step_context.as_ref())
|
||||
.await;
|
||||
|
||||
// Construct the input that we will send to the model.
|
||||
let sampling_request_input: Vec<ResponseItem> = async {
|
||||
|
||||
@@ -142,10 +142,6 @@ async fn apps_guidance_appears_after_background_recovery_within_a_turn() -> Resu
|
||||
.await;
|
||||
let mut builder =
|
||||
apps_enabled_builder(apps_server.chatgpt_base_url.clone()).with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::DeferredExecutor)
|
||||
.expect("test config should allow feature update");
|
||||
config
|
||||
.features
|
||||
.enable(Feature::DefaultModeRequestUserInput)
|
||||
|
||||
@@ -261,9 +261,7 @@ async fn snapshot_model_visible_layout_turn_overrides() -> Result<()> {
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
// TODO(ccunningham): Diff `user_instructions` and emit updates when AGENTS.md content changes
|
||||
// (for example after cwd changes), then update this test to assert refreshed AGENTS content.
|
||||
async fn snapshot_model_visible_layout_cwd_change_does_not_refresh_agents() -> Result<()> {
|
||||
async fn snapshot_model_visible_layout_cwd_change_refreshes_agents() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
@@ -371,18 +369,18 @@ async fn snapshot_model_visible_layout_cwd_change_does_not_refresh_agents() -> R
|
||||
assert_eq!(requests.len(), 2, "expected two requests");
|
||||
assert_eq!(
|
||||
user_instructions_wrapper_count(&requests[0]),
|
||||
0,
|
||||
"expected first request to omit the serialized user-instructions wrapper when cwd-only project docs are introduced after session init"
|
||||
1,
|
||||
"expected first request to include AGENTS.md from its selected cwd"
|
||||
);
|
||||
assert_eq!(
|
||||
user_instructions_wrapper_count(&requests[1]),
|
||||
0,
|
||||
"expected second request to keep omitting the serialized user-instructions wrapper after cwd change with the current session-scoped project doc behavior"
|
||||
2,
|
||||
"expected second request to retain the original AGENTS.md item and append its replacement"
|
||||
);
|
||||
insta::assert_snapshot!(
|
||||
"model_visible_layout_cwd_change_does_not_refresh_agents",
|
||||
"model_visible_layout_cwd_change_refreshes_agents",
|
||||
format_labeled_requests_snapshot(
|
||||
"Second turn changes cwd to a directory with different AGENTS.md; current behavior does not emit refreshed AGENTS instructions.",
|
||||
"Second turn changes cwd to a directory with different AGENTS.md and refreshes the model-visible instructions.",
|
||||
&[
|
||||
("First Request (agents_one)", &requests[0]),
|
||||
("Second Request (agents_two cwd)", &requests[1]),
|
||||
|
||||
@@ -338,32 +338,37 @@ async fn explicit_remote_shell_runs_in_remote_cwd() -> Result<()> {
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn deferred_executor_does_not_duplicate_initial_environment_context() -> Result<()> {
|
||||
let server = start_mock_server().await;
|
||||
let response_mock = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
assert!(config.features.enable(Feature::DeferredExecutor).is_ok());
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
async fn step_world_state_does_not_duplicate_initial_environment_context() -> Result<()> {
|
||||
for deferred_executor_enabled in [false, true] {
|
||||
let server = start_mock_server().await;
|
||||
let response_mock = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let mut builder = test_codex().with_config(move |config| {
|
||||
if deferred_executor_enabled {
|
||||
assert!(config.features.enable(Feature::DeferredExecutor).is_ok());
|
||||
}
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.submit_turn("report the environment").await?;
|
||||
test.submit_turn("report the environment").await?;
|
||||
|
||||
let user_context = response_mock.single_request().message_input_texts("user");
|
||||
assert_eq!(
|
||||
user_context
|
||||
.iter()
|
||||
.filter(|text| text.contains("<environment_context>"))
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
let user_context = response_mock.single_request().message_input_texts("user");
|
||||
assert_eq!(
|
||||
user_context
|
||||
.iter()
|
||||
.filter(|text| text.contains("<environment_context>"))
|
||||
.count(),
|
||||
1,
|
||||
"deferred executor enabled: {deferred_executor_enabled}",
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
+11
-5
@@ -1,22 +1,28 @@
|
||||
---
|
||||
source: core/tests/suite/model_visible_layout.rs
|
||||
expression: "format_labeled_requests_snapshot(\"Second turn changes cwd to a directory with different AGENTS.md; current behavior does not emit refreshed AGENTS instructions.\",\n&[(\"First Request (agents_one)\", &requests[0]),\n(\"Second Request (agents_two cwd)\", &requests[1]),])"
|
||||
expression: "format_labeled_requests_snapshot(\"Second turn changes cwd to a directory with different AGENTS.md and refreshes the model-visible instructions.\",\n&[(\"First Request (agents_one)\", &requests[0]),\n(\"Second Request (agents_two cwd)\", &requests[1]),])"
|
||||
---
|
||||
Scenario: Second turn changes cwd to a directory with different AGENTS.md; current behavior does not emit refreshed AGENTS instructions.
|
||||
Scenario: Second turn changes cwd to a directory with different AGENTS.md and refreshes the model-visible instructions.
|
||||
|
||||
## First Request (agents_one)
|
||||
00:message/developer[2]:
|
||||
[01] <PERMISSIONS_INSTRUCTIONS>
|
||||
[02] <SKILLS_INSTRUCTIONS>
|
||||
01:message/user:<ENVIRONMENT_CONTEXT:cwd=<CWD>>
|
||||
01:message/user[2]:
|
||||
[01] <AGENTS_MD>
|
||||
[02] <ENVIRONMENT_CONTEXT:cwd=<CWD>>
|
||||
02:message/user:first turn in agents_one
|
||||
|
||||
## Second Request (agents_two cwd)
|
||||
00:message/developer[2]:
|
||||
[01] <PERMISSIONS_INSTRUCTIONS>
|
||||
[02] <SKILLS_INSTRUCTIONS>
|
||||
01:message/user:<ENVIRONMENT_CONTEXT:cwd=<CWD>>
|
||||
01:message/user[2]:
|
||||
[01] <AGENTS_MD>
|
||||
[02] <ENVIRONMENT_CONTEXT:cwd=<CWD>>
|
||||
02:message/user:first turn in agents_one
|
||||
03:message/assistant:turn one complete
|
||||
04:message/user:<ENVIRONMENT_CONTEXT:cwd=<CWD>>
|
||||
04:message/user[2]:
|
||||
[01] <AGENTS_MD>
|
||||
[02] <ENVIRONMENT_CONTEXT:cwd=<CWD>>
|
||||
05:message/user:second turn in agents_two
|
||||
Reference in New Issue
Block a user