Skip to content

Commit 15b5eb3

Browse files
fix(core) Support changing /approvals before conversation (#6836)
## Summary Setting `/approvals` before the start of a conversation was not updating the environment_context for a conversation. Not sure exactly when this problem was introduced, but this should reduce model confusion dramatically. ## Testing - [x] Added unit test to reproduce bug, confirmed fix with update - [x] Tested locally
1 parent 3e9e1d9 commit 15b5eb3

File tree

2 files changed

+88
-1
lines changed

2 files changed

+88
-1
lines changed

codex-rs/core/src/codex.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1335,7 +1335,10 @@ impl Session {
13351335
}
13361336

13371337
async fn submission_loop(sess: Arc<Session>, config: Arc<Config>, rx_sub: Receiver<Submission>) {
1338-
let mut previous_context: Option<Arc<TurnContext>> = None;
1338+
// Seed with context in case there is an OverrideTurnContext first.
1339+
let mut previous_context: Option<Arc<TurnContext>> =
1340+
Some(sess.new_turn(SessionSettingsUpdate::default()).await);
1341+
13391342
// To break out of this loop, send Op::Shutdown.
13401343
while let Ok(sub) = rx_sub.recv().await {
13411344
debug!(?sub, "Submission");

codex-rs/core/tests/suite/prompt_caching.rs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use codex_core::features::Feature;
44
use codex_core::model_family::find_family_for_model;
55
use codex_core::protocol::AskForApproval;
6+
use codex_core::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG;
67
use codex_core::protocol::EventMsg;
78
use codex_core::protocol::Op;
89
use codex_core::protocol::SandboxPolicy;
@@ -372,6 +373,89 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() -> an
372373
Ok(())
373374
}
374375

376+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
377+
async fn override_before_first_turn_emits_environment_context() -> anyhow::Result<()> {
378+
skip_if_no_network!(Ok(()));
379+
380+
let server = start_mock_server().await;
381+
let req = mount_sse_once(&server, sse_completed("resp-1")).await;
382+
383+
let TestCodex { codex, .. } = test_codex().build(&server).await?;
384+
385+
codex
386+
.submit(Op::OverrideTurnContext {
387+
cwd: None,
388+
approval_policy: Some(AskForApproval::Never),
389+
sandbox_policy: None,
390+
model: None,
391+
effort: None,
392+
summary: None,
393+
})
394+
.await?;
395+
396+
codex
397+
.submit(Op::UserInput {
398+
items: vec![UserInput::Text {
399+
text: "first message".into(),
400+
}],
401+
})
402+
.await?;
403+
404+
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
405+
406+
let body = req.single_request().body_json();
407+
let input = body["input"]
408+
.as_array()
409+
.expect("input array must be present");
410+
assert!(
411+
!input.is_empty(),
412+
"expected at least environment context and user message"
413+
);
414+
415+
let env_msg = &input[1];
416+
let env_text = env_msg["content"][0]["text"]
417+
.as_str()
418+
.expect("environment context text");
419+
assert!(
420+
env_text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG),
421+
"second entry should be environment context, got: {env_text}"
422+
);
423+
assert!(
424+
env_text.contains("<approval_policy>never</approval_policy>"),
425+
"environment context should reflect overridden approval policy: {env_text}"
426+
);
427+
428+
let env_count = input
429+
.iter()
430+
.filter(|msg| {
431+
msg["content"]
432+
.as_array()
433+
.and_then(|content| {
434+
content.iter().find(|item| {
435+
item["type"].as_str() == Some("input_text")
436+
&& item["text"]
437+
.as_str()
438+
.map(|text| text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG))
439+
.unwrap_or(false)
440+
})
441+
})
442+
.is_some()
443+
})
444+
.count();
445+
assert_eq!(
446+
env_count, 2,
447+
"environment context should appear exactly twice, found {env_count}"
448+
);
449+
450+
let user_msg = &input[2];
451+
let user_text = user_msg["content"][0]["text"]
452+
.as_str()
453+
.expect("user message text");
454+
assert_eq!(user_text, "first message");
455+
456+
Ok(())
457+
}
458+
375459
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
376460
async fn per_turn_overrides_keep_cached_prefix_and_key_constant() -> anyhow::Result<()> {
377461
skip_if_no_network!(Ok(()));

0 commit comments

Comments
 (0)