Do not append items on override turn context (#10354)

This commit is contained in:
pakrym-oai 2026-02-01 18:51:26 -08:00 committed by GitHub
parent 8b95d3e082
commit 03fcd12e77
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 288 additions and 101 deletions

View file

@ -490,7 +490,7 @@ pub(crate) struct TurnContext {
pub(crate) developer_instructions: Option<String>,
pub(crate) compact_prompt: Option<String>,
pub(crate) user_instructions: Option<String>,
pub(crate) collaboration_mode_kind: ModeKind,
pub(crate) collaboration_mode: CollaborationMode,
pub(crate) personality: Option<Personality>,
pub(crate) approval_policy: AskForApproval,
pub(crate) sandbox_policy: SandboxPolicy,
@ -692,7 +692,7 @@ impl Session {
developer_instructions: session_configuration.developer_instructions.clone(),
compact_prompt: session_configuration.compact_prompt.clone(),
user_instructions: session_configuration.user_instructions.clone(),
collaboration_mode_kind: session_configuration.collaboration_mode.mode,
collaboration_mode: session_configuration.collaboration_mode.clone(),
personality: session_configuration.personality,
approval_policy: session_configuration.approval_policy.value(),
sandbox_policy: session_configuration.sandbox_policy.get().clone(),
@ -1356,16 +1356,14 @@ impl Session {
fn build_collaboration_mode_update_item(
&self,
previous_collaboration_mode: &CollaborationMode,
next_collaboration_mode: Option<&CollaborationMode>,
previous: Option<&Arc<TurnContext>>,
next: &TurnContext,
) -> Option<ResponseItem> {
if let Some(next_mode) = next_collaboration_mode {
if previous_collaboration_mode == next_mode {
return None;
}
let prev = previous?;
if prev.collaboration_mode != next.collaboration_mode {
// If the next mode has empty developer instructions, this returns None and we emit no
// update, so prior collaboration instructions remain in the prompt history.
Some(DeveloperInstructions::from_collaboration_mode(next_mode)?.into())
Some(DeveloperInstructions::from_collaboration_mode(&next.collaboration_mode)?.into())
} else {
None
}
@ -1375,8 +1373,6 @@ impl Session {
&self,
previous_context: Option<&Arc<TurnContext>>,
current_context: &TurnContext,
previous_collaboration_mode: &CollaborationMode,
next_collaboration_mode: Option<&CollaborationMode>,
) -> Vec<ResponseItem> {
let mut update_items = Vec::new();
if let Some(env_item) =
@ -1389,10 +1385,9 @@ impl Session {
{
update_items.push(permissions_item);
}
if let Some(collaboration_mode_item) = self.build_collaboration_mode_update_item(
previous_collaboration_mode,
next_collaboration_mode,
) {
if let Some(collaboration_mode_item) =
self.build_collaboration_mode_update_item(previous_context, current_context)
{
update_items.push(collaboration_mode_item);
}
if let Some(personality_item) =
@ -2573,18 +2568,6 @@ mod handlers {
sub_id: String,
updates: SessionSettingsUpdate,
) {
let previous_context = sess
.new_default_turn_with_sub_id(sess.next_internal_sub_id())
.await;
let previous_collaboration_mode = sess
.state
.lock()
.await
.session_configuration
.collaboration_mode
.clone();
let next_collaboration_mode = updates.collaboration_mode.clone();
if let Err(err) = sess.update_settings(updates).await {
sess.send_event_raw(Event {
id: sub_id,
@ -2594,24 +2577,6 @@ mod handlers {
}),
})
.await;
return;
}
let initial_context_seeded = sess.state.lock().await.initial_context_seeded;
if !initial_context_seeded {
return;
}
let current_context = sess.new_default_turn_with_sub_id(sub_id).await;
let update_items = sess.build_settings_update_items(
Some(&previous_context),
&current_context,
&previous_collaboration_mode,
next_collaboration_mode.as_ref(),
);
if !update_items.is_empty() {
sess.record_conversation_items(&current_context, &update_items)
.await;
}
}
@ -2671,14 +2636,6 @@ mod handlers {
_ => unreachable!(),
};
let previous_collaboration_mode = sess
.state
.lock()
.await
.session_configuration
.collaboration_mode
.clone();
let next_collaboration_mode = updates.collaboration_mode.clone();
let Ok(current_context) = sess.new_turn_with_sub_id(sub_id, updates).await else {
// new_turn_with_sub_id already emits the error event.
return;
@ -2691,12 +2648,8 @@ mod handlers {
// Attempt to inject input into current task
if let Err(items) = sess.inject_input(items).await {
sess.seed_initial_context_if_needed(&current_context).await;
let update_items = sess.build_settings_update_items(
previous_context.as_ref(),
&current_context,
&previous_collaboration_mode,
next_collaboration_mode.as_ref(),
);
let update_items =
sess.build_settings_update_items(previous_context.as_ref(), &current_context);
if !update_items.is_empty() {
sess.record_conversation_items(&current_context, &update_items)
.await;
@ -3211,7 +3164,7 @@ async fn spawn_review_thread(
developer_instructions: None,
user_instructions: None,
compact_prompt: parent_turn_context.compact_prompt.clone(),
collaboration_mode_kind: parent_turn_context.collaboration_mode_kind,
collaboration_mode: parent_turn_context.collaboration_mode.clone(),
personality: parent_turn_context.personality,
approval_policy: parent_turn_context.approval_policy,
sandbox_policy: parent_turn_context.sandbox_policy.clone(),
@ -3326,7 +3279,7 @@ pub(crate) async fn run_turn(
let total_usage_tokens = sess.get_total_token_usage().await;
let event = EventMsg::TurnStarted(TurnStartedEvent {
model_context_window: turn_context.client.get_model_context_window(),
collaboration_mode_kind: turn_context.collaboration_mode_kind,
collaboration_mode_kind: turn_context.collaboration_mode.mode,
});
sess.send_event(&turn_context, event).await;
if total_usage_tokens >= auto_compact_limit {
@ -4239,7 +4192,7 @@ async fn try_run_sampling_request(
let mut last_agent_message: Option<String> = None;
let mut active_item: Option<TurnItem> = None;
let mut should_emit_turn_diff = false;
let plan_mode = turn_context.collaboration_mode_kind == ModeKind::Plan;
let plan_mode = turn_context.collaboration_mode.mode == ModeKind::Plan;
let mut plan_mode_state = plan_mode.then(|| PlanModeStreamState::new(&turn_context.sub_id));
let receiving_span = trace_span!("receiving_stream");
let outcome: CodexResult<SamplingRequestResult> = loop {

View file

@ -61,7 +61,7 @@ pub(crate) async fn run_compact_task(
) {
let start_event = EventMsg::TurnStarted(TurnStartedEvent {
model_context_window: turn_context.client.get_model_context_window(),
collaboration_mode_kind: turn_context.collaboration_mode_kind,
collaboration_mode_kind: turn_context.collaboration_mode.mode,
});
sess.send_event(&turn_context, start_event).await;
run_compact_task_inner(sess.clone(), turn_context, input).await;

View file

@ -22,7 +22,7 @@ pub(crate) async fn run_inline_remote_auto_compact_task(
pub(crate) async fn run_remote_compact_task(sess: Arc<Session>, turn_context: Arc<TurnContext>) {
let start_event = EventMsg::TurnStarted(TurnStartedEvent {
model_context_window: turn_context.client.get_model_context_window(),
collaboration_mode_kind: turn_context.collaboration_mode_kind,
collaboration_mode_kind: turn_context.collaboration_mode.mode,
});
sess.send_event(&turn_context, start_event).await;

View file

@ -48,7 +48,7 @@ pub(crate) async fn handle_output_item_done(
previously_active_item: Option<TurnItem>,
) -> Result<OutputItemResult> {
let mut output = OutputItemResult::default();
let plan_mode = ctx.turn_context.collaboration_mode_kind == ModeKind::Plan;
let plan_mode = ctx.turn_context.collaboration_mode.mode == ModeKind::Plan;
match ToolRouter::build_tool_call(ctx.sess.as_ref(), item.clone()).await {
// The model emitted a tool call; log it, persist the item immediately, and queue the tool execution.

View file

@ -67,7 +67,7 @@ impl SessionTask for UserShellCommandTask {
let event = EventMsg::TurnStarted(TurnStartedEvent {
model_context_window: turn_context.client.get_model_context_window(),
collaboration_mode_kind: turn_context.collaboration_mode_kind,
collaboration_mode_kind: turn_context.collaboration_mode.mode,
});
let session = session.clone_session();
session.send_event(turn_context.as_ref(), event).await;

View file

@ -104,7 +104,7 @@ pub(crate) async fn handle_update_plan(
arguments: String,
_call_id: String,
) -> Result<String, FunctionCallError> {
if turn_context.collaboration_mode_kind == ModeKind::Plan {
if turn_context.collaboration_mode.mode == ModeKind::Plan {
return Err(FunctionCallError::RespondToModel(
"update_plan is a TODO/checklist tool and is not allowed in Plan mode".to_string(),
));

View file

@ -22,9 +22,12 @@ fn sse_completed(id: &str) -> String {
sse(vec![ev_response_created(id), ev_completed(id)])
}
fn collab_mode_with_instructions(instructions: Option<&str>) -> CollaborationMode {
fn collab_mode_with_mode_and_instructions(
mode: ModeKind,
instructions: Option<&str>,
) -> CollaborationMode {
CollaborationMode {
mode: ModeKind::Custom,
mode,
settings: Settings {
model: "gpt-5.1".to_string(),
reasoning_effort: None,
@ -33,6 +36,10 @@ fn collab_mode_with_instructions(instructions: Option<&str>) -> CollaborationMod
}
}
fn collab_mode_with_instructions(instructions: Option<&str>) -> CollaborationMode {
collab_mode_with_mode_and_instructions(ModeKind::Custom, instructions)
}
fn developer_texts(input: &[Value]) -> Vec<String> {
input
.iter()
@ -171,7 +178,7 @@ async fn collaboration_instructions_added_on_user_turn() -> Result<()> {
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn override_then_user_turn_uses_updated_collaboration_instructions() -> Result<()> {
async fn override_then_next_turn_uses_updated_collaboration_instructions() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@ -196,20 +203,12 @@ async fn override_then_user_turn_uses_updated_collaboration_instructions() -> Re
.await?;
test.codex
.submit(Op::UserTurn {
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "hello".into(),
text_elements: Vec::new(),
}],
cwd: test.config.cwd.clone(),
approval_policy: test.config.approval_policy.value(),
sandbox_policy: test.config.sandbox_policy.get().clone(),
model: test.session_configured.model.clone(),
effort: None,
summary: test.config.model_reasoning_summary,
collaboration_mode: None,
final_output_json_schema: None,
personality: None,
})
.await?;
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
@ -272,7 +271,7 @@ async fn user_turn_overrides_collaboration_instructions_after_override() -> Resu
let dev_texts = developer_texts(&input);
let base_text = collab_xml(base_text);
let turn_text = collab_xml(turn_text);
assert_eq!(count_exact(&dev_texts, &base_text), 1);
assert_eq!(count_exact(&dev_texts, &base_text), 0);
assert_eq!(count_exact(&dev_texts, &turn_text), 1);
Ok(())
@ -419,6 +418,159 @@ async fn collaboration_mode_update_noop_does_not_append() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn collaboration_mode_update_emits_new_instruction_message_when_mode_changes() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let _req1 = mount_sse_once(&server, sse_completed("resp-1")).await;
let req2 = mount_sse_once(&server, sse_completed("resp-2")).await;
let test = test_codex().build(&server).await?;
let code_text = "code mode instructions";
let plan_text = "plan mode instructions";
test.codex
.submit(Op::OverrideTurnContext {
cwd: None,
approval_policy: None,
sandbox_policy: None,
windows_sandbox_level: None,
model: None,
effort: None,
summary: None,
collaboration_mode: Some(collab_mode_with_mode_and_instructions(
ModeKind::Code,
Some(code_text),
)),
personality: None,
})
.await?;
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "hello 1".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
})
.await?;
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
test.codex
.submit(Op::OverrideTurnContext {
cwd: None,
approval_policy: None,
sandbox_policy: None,
windows_sandbox_level: None,
model: None,
effort: None,
summary: None,
collaboration_mode: Some(collab_mode_with_mode_and_instructions(
ModeKind::Plan,
Some(plan_text),
)),
personality: None,
})
.await?;
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "hello 2".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
})
.await?;
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
let input = req2.single_request().input();
let dev_texts = developer_texts(&input);
let code_text = collab_xml(code_text);
let plan_text = collab_xml(plan_text);
assert_eq!(count_exact(&dev_texts, &code_text), 1);
assert_eq!(count_exact(&dev_texts, &plan_text), 1);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn collaboration_mode_update_noop_does_not_append_when_mode_is_unchanged() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let _req1 = mount_sse_once(&server, sse_completed("resp-1")).await;
let req2 = mount_sse_once(&server, sse_completed("resp-2")).await;
let test = test_codex().build(&server).await?;
let collab_text = "mode-stable instructions";
test.codex
.submit(Op::OverrideTurnContext {
cwd: None,
approval_policy: None,
sandbox_policy: None,
windows_sandbox_level: None,
model: None,
effort: None,
summary: None,
collaboration_mode: Some(collab_mode_with_mode_and_instructions(
ModeKind::Code,
Some(collab_text),
)),
personality: None,
})
.await?;
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "hello 1".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
})
.await?;
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
test.codex
.submit(Op::OverrideTurnContext {
cwd: None,
approval_policy: None,
sandbox_policy: None,
windows_sandbox_level: None,
model: None,
effort: None,
summary: None,
collaboration_mode: Some(collab_mode_with_mode_and_instructions(
ModeKind::Code,
Some(collab_text),
)),
personality: None,
})
.await?;
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "hello 2".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
})
.await?;
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
let input = req2.single_request().input();
let dev_texts = developer_texts(&input);
let collab_text = collab_xml(collab_text);
assert_eq!(count_exact(&dev_texts, &collab_text), 1);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn resume_replays_collaboration_instructions() -> Result<()> {
skip_if_no_network!(Ok(()));

View file

@ -18,7 +18,6 @@ use core_test_support::skip_if_no_network;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
use std::path::Path;
use std::time::Duration;
use tempfile::TempDir;
@ -104,7 +103,7 @@ fn rollout_environment_texts(text: &str) -> Vec<String> {
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn override_turn_context_records_permissions_update() -> Result<()> {
async fn override_turn_context_without_user_turn_does_not_record_permissions_update() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@ -138,19 +137,15 @@ async fn override_turn_context_records_permissions_update() -> Result<()> {
.filter(|text| text.contains("`approval_policy`"))
.collect();
assert!(
approval_texts
.iter()
.any(|text| text.contains("`approval_policy` is `never`")),
"expected updated approval policy instructions in rollout"
approval_texts.is_empty(),
"did not expect permissions updates before a new user turn: {approval_texts:?}"
);
let unique: HashSet<&String> = approval_texts.iter().copied().collect();
assert_eq!(unique.len(), 2);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn override_turn_context_records_environment_update() -> Result<()> {
async fn override_turn_context_without_user_turn_does_not_record_environment_update() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@ -177,17 +172,16 @@ async fn override_turn_context_records_environment_update() -> Result<()> {
let rollout_path = test.codex.rollout_path().expect("rollout path");
let rollout_text = read_rollout_text(&rollout_path).await?;
let env_texts = rollout_environment_texts(&rollout_text);
let new_cwd_text = new_cwd.path().display().to_string();
assert!(
env_texts.iter().any(|text| text.contains(&new_cwd_text)),
"expected environment update with new cwd in rollout"
env_texts.is_empty(),
"did not expect environment updates before a new user turn: {env_texts:?}"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn override_turn_context_records_collaboration_update() -> Result<()> {
async fn override_turn_context_without_user_turn_does_not_record_collaboration_update() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@ -220,7 +214,7 @@ async fn override_turn_context_records_collaboration_update() -> Result<()> {
.iter()
.filter(|text| text.as_str() == collab_text.as_str())
.count();
assert_eq!(collab_count, 1);
assert_eq!(collab_count, 0);
Ok(())
}

View file

@ -136,7 +136,7 @@ async fn permissions_message_added_on_override_change() -> Result<()> {
let permissions_2 = permissions_texts(input2);
assert_eq!(permissions_1.len(), 1);
assert_eq!(permissions_2.len(), 3);
assert_eq!(permissions_2.len(), 2);
let unique = permissions_2.into_iter().collect::<HashSet<String>>();
assert_eq!(unique.len(), 2);
@ -267,7 +267,7 @@ async fn resume_replays_permissions_messages() -> Result<()> {
let body3 = req3.single_request().body_json();
let input = body3["input"].as_array().expect("input array");
let permissions = permissions_texts(input);
assert_eq!(permissions.len(), 4);
assert_eq!(permissions.len(), 3);
let unique = permissions.into_iter().collect::<HashSet<String>>();
assert_eq!(unique.len(), 2);
@ -337,7 +337,7 @@ async fn resume_and_fork_append_permissions_messages() -> Result<()> {
let body2 = req2.single_request().body_json();
let input2 = body2["input"].as_array().expect("input array");
let permissions_base = permissions_texts(input2);
assert_eq!(permissions_base.len(), 3);
assert_eq!(permissions_base.len(), 2);
builder = builder.with_config(|config| {
config.approval_policy = Constrained::allow_any(AskForApproval::UnlessTrusted);

View file

@ -272,6 +272,97 @@ async fn user_turn_personality_some_adds_update_message() -> anyhow::Result<()>
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn user_turn_personality_same_value_does_not_add_update_message() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let resp_mock = mount_sse_sequence(
&server,
vec![sse_completed("resp-1"), sse_completed("resp-2")],
)
.await;
let mut builder = test_codex()
.with_model("exp-codex-personality")
.with_config(|config| {
config.features.disable(Feature::RemoteModels);
config.features.enable(Feature::Personality);
config.personality = Some(Personality::Pragmatic);
});
let test = builder.build(&server).await?;
test.codex
.submit(Op::UserTurn {
items: vec![UserInput::Text {
text: "hello".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
cwd: test.cwd_path().to_path_buf(),
approval_policy: test.config.approval_policy.value(),
sandbox_policy: SandboxPolicy::ReadOnly,
model: test.session_configured.model.clone(),
effort: test.config.model_reasoning_effort,
summary: ReasoningSummary::Auto,
collaboration_mode: None,
personality: None,
})
.await?;
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
test.codex
.submit(Op::OverrideTurnContext {
cwd: None,
approval_policy: None,
sandbox_policy: None,
windows_sandbox_level: None,
model: None,
effort: None,
summary: None,
collaboration_mode: None,
personality: Some(Personality::Pragmatic),
})
.await?;
test.codex
.submit(Op::UserTurn {
items: vec![UserInput::Text {
text: "hello".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
cwd: test.cwd_path().to_path_buf(),
approval_policy: test.config.approval_policy.value(),
sandbox_policy: SandboxPolicy::ReadOnly,
model: test.session_configured.model.clone(),
effort: test.config.model_reasoning_effort,
summary: ReasoningSummary::Auto,
collaboration_mode: None,
personality: None,
})
.await?;
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
let requests = resp_mock.requests();
assert_eq!(requests.len(), 2, "expected two requests");
let request = requests
.last()
.expect("expected second request after personality override");
let developer_texts = request.message_input_texts("developer");
let personality_text = developer_texts
.iter()
.find(|text| text.contains("<personality_spec>"));
assert!(
personality_text.is_none(),
"expected no personality preamble for unchanged personality, got {personality_text:?}"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn instructions_uses_base_if_feature_disabled() -> anyhow::Result<()> {
let codex_home = TempDir::new().expect("create temp dir");

View file

@ -388,17 +388,14 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() -> an
});
let expected_permissions_msg = body1["input"][0].clone();
let body1_input = body1["input"].as_array().expect("input array");
// After overriding the turn context, emit two updated permissions messages.
// After overriding the turn context, emit one updated permissions message.
let expected_permissions_msg_2 = body2["input"][body1_input.len()].clone();
let expected_permissions_msg_3 = body2["input"][body1_input.len() + 1].clone();
assert_ne!(
expected_permissions_msg_2, expected_permissions_msg,
"expected updated permissions message after override"
);
assert_eq!(expected_permissions_msg_2, expected_permissions_msg_3);
let mut expected_body2 = body1_input.to_vec();
expected_body2.push(expected_permissions_msg_2);
expected_body2.push(expected_permissions_msg_3);
expected_body2.push(expected_user_message_2);
assert_eq!(body2["input"], serde_json::Value::Array(expected_body2));