From f6a7da4ac3760b8442e9c104612bb1478ccfd9b9 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 2 Dec 2025 12:46:26 -0800 Subject: [PATCH 1/5] fix: drop lock once it is no longer needed (#7500) I noticed this while doing a post-commit review of https://github.com/openai/codex/pull/7467. --- codex-rs/core/src/mcp_connection_manager.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index e8c289b573..11a90f77a8 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -151,8 +151,10 @@ impl ElicitationRequestManager { let server_name = server_name.clone(); async move { let (tx, rx) = oneshot::channel(); - let mut lock = elicitation_requests.lock().await; - lock.insert((server_name.clone(), id.clone()), tx); + { + let mut lock = elicitation_requests.lock().await; + lock.insert((server_name.clone(), id.clone()), tx); + } let _ = tx_event .send(Event { id: "mcp_elicitation_request".to_string(), From 5ebdc9af1ba4db1ce7d1d34f65bf66dabe45e78e Mon Sep 17 00:00:00 2001 From: zhao-oai Date: Tue, 2 Dec 2025 16:23:24 -0500 Subject: [PATCH 2/5] persisting credits if new snapshot does not contain credit info (#7490) in response to incoming changes to responses headers where the header may sometimes not contain credits info (no longer forcing a credit check) --- codex-rs/core/src/codex.rs | 72 ++++++++++++++++++++++++++++ codex-rs/core/src/state/session.rs | 16 ++++++- codex-rs/tui/src/chatwidget.rs | 15 +++++- codex-rs/tui/src/chatwidget/tests.rs | 48 +++++++++++++++++++ 4 files changed, 149 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index ef0a237f75..8b1dfafd21 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2481,7 +2481,10 @@ mod tests { use crate::tools::format_exec_output_str; use crate::protocol::CompactedItem; + use crate::protocol::CreditsSnapshot; use crate::protocol::InitialHistory; + use crate::protocol::RateLimitSnapshot; + use crate::protocol::RateLimitWindow; use crate::protocol::ResumedHistory; use crate::state::TaskKind; use crate::tasks::SessionTask; @@ -2551,6 +2554,75 @@ mod tests { assert_eq!(expected, actual); } + #[test] + fn set_rate_limits_retains_previous_credits() { + let codex_home = tempfile::tempdir().expect("create temp dir"); + let config = Config::load_from_base_config_with_overrides( + ConfigToml::default(), + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + ) + .expect("load default test config"); + let config = Arc::new(config); + let session_configuration = SessionConfiguration { + provider: config.model_provider.clone(), + model: config.model.clone(), + model_reasoning_effort: config.model_reasoning_effort, + model_reasoning_summary: config.model_reasoning_summary, + developer_instructions: config.developer_instructions.clone(), + user_instructions: config.user_instructions.clone(), + base_instructions: config.base_instructions.clone(), + compact_prompt: config.compact_prompt.clone(), + approval_policy: config.approval_policy, + sandbox_policy: config.sandbox_policy.clone(), + cwd: config.cwd.clone(), + original_config_do_not_use: Arc::clone(&config), + features: Features::default(), + exec_policy: Arc::new(ExecPolicy::empty()), + session_source: SessionSource::Exec, + }; + + let mut state = SessionState::new(session_configuration); + let initial = RateLimitSnapshot { + primary: Some(RateLimitWindow { + used_percent: 10.0, + window_minutes: Some(15), + resets_at: Some(1_700), + }), + secondary: None, + credits: Some(CreditsSnapshot { + has_credits: true, + unlimited: false, + balance: Some("10.00".to_string()), + }), + }; + state.set_rate_limits(initial.clone()); + + let update = RateLimitSnapshot { + primary: Some(RateLimitWindow { + used_percent: 40.0, + window_minutes: Some(30), + resets_at: Some(1_800), + }), + secondary: Some(RateLimitWindow { + used_percent: 5.0, + window_minutes: Some(60), + resets_at: Some(1_900), + }), + credits: None, + }; + state.set_rate_limits(update.clone()); + + assert_eq!( + state.latest_rate_limits, + Some(RateLimitSnapshot { + primary: update.primary.clone(), + secondary: update.secondary, + credits: initial.credits, + }) + ); + } + #[test] fn prefers_structured_content_when_present() { let ctr = CallToolResult { diff --git a/codex-rs/core/src/state/session.rs b/codex-rs/core/src/state/session.rs index caebac6b86..8c739c9243 100644 --- a/codex-rs/core/src/state/session.rs +++ b/codex-rs/core/src/state/session.rs @@ -62,7 +62,10 @@ impl SessionState { } pub(crate) fn set_rate_limits(&mut self, snapshot: RateLimitSnapshot) { - self.latest_rate_limits = Some(snapshot); + self.latest_rate_limits = Some(merge_rate_limit_credits( + self.latest_rate_limits.as_ref(), + snapshot, + )); } pub(crate) fn token_info_and_rate_limits( @@ -79,3 +82,14 @@ impl SessionState { self.history.get_total_token_usage() } } + +// Sometimes new snapshots don't include credits +fn merge_rate_limit_credits( + previous: Option<&RateLimitSnapshot>, + mut snapshot: RateLimitSnapshot, +) -> RateLimitSnapshot { + if snapshot.credits.is_none() { + snapshot.credits = previous.and_then(|prior| prior.credits.clone()); + } + snapshot +} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 8bb1e78e24..79a1843130 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -20,6 +20,7 @@ use codex_core::protocol::AgentReasoningRawContentDeltaEvent; use codex_core::protocol::AgentReasoningRawContentEvent; use codex_core::protocol::ApplyPatchApprovalRequestEvent; use codex_core::protocol::BackgroundEventEvent; +use codex_core::protocol::CreditsSnapshot; use codex_core::protocol::DeprecationNoticeEvent; use codex_core::protocol::ErrorEvent; use codex_core::protocol::Event; @@ -558,7 +559,19 @@ impl ChatWidget { } pub(crate) fn on_rate_limit_snapshot(&mut self, snapshot: Option) { - if let Some(snapshot) = snapshot { + if let Some(mut snapshot) = snapshot { + if snapshot.credits.is_none() { + snapshot.credits = self + .rate_limit_snapshot + .as_ref() + .and_then(|display| display.credits.as_ref()) + .map(|credits| CreditsSnapshot { + has_credits: credits.has_credits, + unlimited: credits.unlimited, + balance: credits.balance.clone(), + }); + } + let warnings = self.rate_limit_warnings.take_warnings( snapshot .secondary diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index eb65e3a5dd..890e8bbe1d 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -18,6 +18,7 @@ use codex_core::protocol::AgentReasoningDeltaEvent; use codex_core::protocol::AgentReasoningEvent; use codex_core::protocol::ApplyPatchApprovalRequestEvent; use codex_core::protocol::BackgroundEventEvent; +use codex_core::protocol::CreditsSnapshot; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::ExecApprovalRequestEvent; @@ -521,6 +522,53 @@ fn test_rate_limit_warnings_monthly() { ); } +#[test] +fn rate_limit_snapshot_keeps_prior_credits_when_missing_from_headers() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + + chat.on_rate_limit_snapshot(Some(RateLimitSnapshot { + primary: None, + secondary: None, + credits: Some(CreditsSnapshot { + has_credits: true, + unlimited: false, + balance: Some("17.5".to_string()), + }), + })); + let initial_balance = chat + .rate_limit_snapshot + .as_ref() + .and_then(|snapshot| snapshot.credits.as_ref()) + .and_then(|credits| credits.balance.as_deref()); + assert_eq!(initial_balance, Some("17.5")); + + chat.on_rate_limit_snapshot(Some(RateLimitSnapshot { + primary: Some(RateLimitWindow { + used_percent: 80.0, + window_minutes: Some(60), + resets_at: Some(123), + }), + secondary: None, + credits: None, + })); + + let display = chat + .rate_limit_snapshot + .as_ref() + .expect("rate limits should be cached"); + let credits = display + .credits + .as_ref() + .expect("credits should persist when headers omit them"); + + assert_eq!(credits.balance.as_deref(), Some("17.5")); + assert!(!credits.unlimited); + assert_eq!( + display.primary.as_ref().map(|window| window.used_percent), + Some(80.0) + ); +} + #[test] fn rate_limit_switch_prompt_skips_when_on_lower_cost_model() { let (mut chat, _, _) = make_chatwidget_manual(); From 77c457121ecd40d50ae7d92b87b233dd9be927f1 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Tue, 2 Dec 2025 13:39:10 -0800 Subject: [PATCH 3/5] fix: remove serde(flatten) annotation for TurnError (#7499) The problem with using `serde(flatten)` on Turn status is that it conditionally serializes the `error` field, which is not the pattern we want in API v2 where all fields on an object should always be returned. ``` #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct Turn { pub id: String, /// Only populated on a `thread/resume` response. /// For all other responses and notifications returning a Turn, /// the items field will be an empty list. pub items: Vec, #[serde(flatten)] pub status: TurnStatus, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(tag = "status", rename_all = "camelCase")] #[ts(tag = "status", export_to = "v2/")] pub enum TurnStatus { Completed, Interrupted, Failed { error: TurnError }, InProgress, } ``` serializes to: ``` { "id": "turn-123", "items": [], "status": "completed" } { "id": "turn-123", "items": [], "status": "failed", "error": { "message": "Tool timeout", "codexErrorInfo": null } } ``` Instead we want: ``` { "id": "turn-123", "items": [], "status": "completed", "error": null } { "id": "turn-123", "items": [], "status": "failed", "error": { "message": "Tool timeout", "codexErrorInfo": null } } ``` --- .../src/protocol/thread_history.rs | 4 ++ .../app-server-protocol/src/protocol/v2.rs | 9 ++-- codex-rs/app-server-test-client/src/main.rs | 4 +- .../app-server/src/bespoke_event_handling.rs | 54 ++++++++++--------- .../app-server/src/codex_message_processor.rs | 2 + 5 files changed, 42 insertions(+), 31 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/thread_history.rs b/codex-rs/app-server-protocol/src/protocol/thread_history.rs index 04cd1190bc..ba1e6261cc 100644 --- a/codex-rs/app-server-protocol/src/protocol/thread_history.rs +++ b/codex-rs/app-server-protocol/src/protocol/thread_history.rs @@ -1,5 +1,6 @@ use crate::protocol::v2::ThreadItem; use crate::protocol::v2::Turn; +use crate::protocol::v2::TurnError; use crate::protocol::v2::TurnStatus; use crate::protocol::v2::UserInput; use codex_protocol::protocol::AgentReasoningEvent; @@ -142,6 +143,7 @@ impl ThreadHistoryBuilder { PendingTurn { id: self.next_turn_id(), items: Vec::new(), + error: None, status: TurnStatus::Completed, } } @@ -190,6 +192,7 @@ impl ThreadHistoryBuilder { struct PendingTurn { id: String, items: Vec, + error: Option, status: TurnStatus, } @@ -198,6 +201,7 @@ impl From for Turn { Self { id: value.id, items: value.items, + error: value.error, status: value.status, } } diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 680b5745fe..0e2be70c93 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -877,8 +877,9 @@ pub struct Turn { /// For all other responses and notifications returning a Turn, /// the items field will be an empty list. pub items: Vec, - #[serde(flatten)] pub status: TurnStatus, + /// Only populated when the Turn's status is failed. + pub error: Option, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, Error)] @@ -900,12 +901,12 @@ pub struct ErrorNotification { } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] -#[serde(tag = "status", rename_all = "camelCase")] -#[ts(tag = "status", export_to = "v2/")] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] pub enum TurnStatus { Completed, Interrupted, - Failed { error: TurnError }, + Failed, InProgress, } diff --git a/codex-rs/app-server-test-client/src/main.rs b/codex-rs/app-server-test-client/src/main.rs index f75f3f98a8..8c2a38e46c 100644 --- a/codex-rs/app-server-test-client/src/main.rs +++ b/codex-rs/app-server-test-client/src/main.rs @@ -563,7 +563,9 @@ impl CodexClient { ServerNotification::TurnCompleted(payload) => { if payload.turn.id == turn_id { println!("\n< turn/completed notification: {:?}", payload.turn.status); - if let TurnStatus::Failed { error } = &payload.turn.status { + if payload.turn.status == TurnStatus::Failed + && let Some(error) = payload.turn.error + { println!("[turn error] {}", error.message); } break; diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 00757c8a94..b4dd16b9a6 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -718,6 +718,7 @@ async fn emit_turn_completed_with_status( conversation_id: ConversationId, event_turn_id: String, status: TurnStatus, + error: Option, outgoing: &OutgoingMessageSender, ) { let notification = TurnCompletedNotification { @@ -725,6 +726,7 @@ async fn emit_turn_completed_with_status( turn: Turn { id: event_turn_id, items: vec![], + error, status, }, }; @@ -813,13 +815,12 @@ async fn handle_turn_complete( ) { let turn_summary = find_and_remove_turn_summary(conversation_id, turn_summary_store).await; - let status = if let Some(error) = turn_summary.last_error { - TurnStatus::Failed { error } - } else { - TurnStatus::Completed + let (status, error) = match turn_summary.last_error { + Some(error) => (TurnStatus::Failed, Some(error)), + None => (TurnStatus::Completed, None), }; - emit_turn_completed_with_status(conversation_id, event_turn_id, status, outgoing).await; + emit_turn_completed_with_status(conversation_id, event_turn_id, status, error, outgoing).await; } async fn handle_turn_interrupted( @@ -834,6 +835,7 @@ async fn handle_turn_interrupted( conversation_id, event_turn_id, TurnStatus::Interrupted, + None, outgoing, ) .await; @@ -1306,6 +1308,7 @@ mod tests { OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => { assert_eq!(n.turn.id, event_turn_id); assert_eq!(n.turn.status, TurnStatus::Completed); + assert_eq!(n.turn.error, None); } other => bail!("unexpected message: {other:?}"), } @@ -1346,6 +1349,7 @@ mod tests { OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => { assert_eq!(n.turn.id, event_turn_id); assert_eq!(n.turn.status, TurnStatus::Interrupted); + assert_eq!(n.turn.error, None); } other => bail!("unexpected message: {other:?}"), } @@ -1385,14 +1389,13 @@ mod tests { match msg { OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => { assert_eq!(n.turn.id, event_turn_id); + assert_eq!(n.turn.status, TurnStatus::Failed); assert_eq!( - n.turn.status, - TurnStatus::Failed { - error: TurnError { - message: "bad".to_string(), - codex_error_info: Some(V2CodexErrorInfo::Other), - } - } + n.turn.error, + Some(TurnError { + message: "bad".to_string(), + codex_error_info: Some(V2CodexErrorInfo::Other), + }) ); } other => bail!("unexpected message: {other:?}"), @@ -1653,14 +1656,13 @@ mod tests { match msg { OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => { assert_eq!(n.turn.id, a_turn1); + assert_eq!(n.turn.status, TurnStatus::Failed); assert_eq!( - n.turn.status, - TurnStatus::Failed { - error: TurnError { - message: "a1".to_string(), - codex_error_info: Some(V2CodexErrorInfo::BadRequest), - } - } + n.turn.error, + Some(TurnError { + message: "a1".to_string(), + codex_error_info: Some(V2CodexErrorInfo::BadRequest), + }) ); } other => bail!("unexpected message: {other:?}"), @@ -1674,14 +1676,13 @@ mod tests { match msg { OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => { assert_eq!(n.turn.id, b_turn1); + assert_eq!(n.turn.status, TurnStatus::Failed); assert_eq!( - n.turn.status, - TurnStatus::Failed { - error: TurnError { - message: "b1".to_string(), - codex_error_info: None, - } - } + n.turn.error, + Some(TurnError { + message: "b1".to_string(), + codex_error_info: None, + }) ); } other => bail!("unexpected message: {other:?}"), @@ -1696,6 +1697,7 @@ mod tests { OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => { assert_eq!(n.turn.id, a_turn2); assert_eq!(n.turn.status, TurnStatus::Completed); + assert_eq!(n.turn.error, None); } other => bail!("unexpected message: {other:?}"), } diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 9c718309ed..cd8aa9b704 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -2455,6 +2455,7 @@ impl CodexMessageProcessor { let turn = Turn { id: turn_id.clone(), items: vec![], + error: None, status: TurnStatus::InProgress, }; @@ -2496,6 +2497,7 @@ impl CodexMessageProcessor { Turn { id: turn_id, items, + error: None, status: TurnStatus::InProgress, } } From 4d4778ec1cbc93ff2b992afc4a4214bbffa26c2a Mon Sep 17 00:00:00 2001 From: liam Date: Tue, 2 Dec 2025 17:01:05 -0500 Subject: [PATCH 4/5] Trim `history.jsonl` when `history.max_bytes` is set (#6242) This PR honors the `history.max_bytes` configuration parameter by trimming `history.jsonl` whenever it grows past the configured limit. While appending new entries we retain the newest record, drop the oldest lines to stay within the byte budget, and serialize the compacted file back to disk under the same lock to keep writers safe. --- codex-rs/core/src/config/types.rs | 4 +- codex-rs/core/src/message_history.rs | 266 +++++++++++++++++++++++++-- docs/config.md | 127 +++++++------ docs/example-config.md | 2 +- 4 files changed, 319 insertions(+), 80 deletions(-) diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 869ec82973..c47a570969 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -256,8 +256,8 @@ pub struct History { /// If true, history entries will not be written to disk. pub persistence: HistoryPersistence, - /// If set, the maximum size of the history file in bytes. - /// TODO(mbolin): Not currently honored. + /// If set, the maximum size of the history file in bytes. The oldest entries + /// are dropped once the file exceeds this limit. pub max_bytes: Option, } diff --git a/codex-rs/core/src/message_history.rs b/codex-rs/core/src/message_history.rs index 387641c310..e46dd93067 100644 --- a/codex-rs/core/src/message_history.rs +++ b/codex-rs/core/src/message_history.rs @@ -16,7 +16,12 @@ use std::fs::File; use std::fs::OpenOptions; +use std::io::BufRead; +use std::io::BufReader; +use std::io::Read; use std::io::Result; +use std::io::Seek; +use std::io::SeekFrom; use std::io::Write; use std::path::Path; use std::path::PathBuf; @@ -40,6 +45,9 @@ use std::os::unix::fs::PermissionsExt; /// Filename that stores the message history inside `~/.codex`. const HISTORY_FILENAME: &str = "history.jsonl"; +/// When history exceeds the hard cap, trim it down to this fraction of `max_bytes`. +const HISTORY_SOFT_CAP_RATIO: f64 = 0.8; + const MAX_RETRIES: usize = 10; const RETRY_SLEEP: Duration = Duration::from_millis(100); @@ -98,11 +106,12 @@ pub(crate) async fn append_entry( .map_err(|e| std::io::Error::other(format!("failed to serialise history entry: {e}")))?; line.push('\n'); - // Open in append-only mode. + // Open the history file for read/write access (append-only on Unix). let mut options = OpenOptions::new(); - options.append(true).read(true).create(true); + options.read(true).write(true).create(true); #[cfg(unix)] { + options.append(true); options.mode(0o600); } @@ -111,6 +120,8 @@ pub(crate) async fn append_entry( // Ensure permissions. ensure_owner_only_permissions(&history_file).await?; + let history_max_bytes = config.history.max_bytes; + // Perform a blocking write under an advisory write lock using std::fs. tokio::task::spawn_blocking(move || -> Result<()> { // Retry a few times to avoid indefinite blocking when contended. @@ -118,8 +129,12 @@ pub(crate) async fn append_entry( match history_file.try_lock() { Ok(()) => { // While holding the exclusive lock, write the full line. + // We do not open the file with `append(true)` on Windows, so ensure the + // cursor is positioned at the end before writing. + history_file.seek(SeekFrom::End(0))?; history_file.write_all(line.as_bytes())?; history_file.flush()?; + enforce_history_limit(&mut history_file, history_max_bytes)?; return Ok(()); } Err(std::fs::TryLockError::WouldBlock) => { @@ -139,6 +154,93 @@ pub(crate) async fn append_entry( Ok(()) } +/// Trim the history file to honor `max_bytes`, dropping the oldest lines while holding +/// the write lock so the newest entry is always retained. When the file exceeds the +/// hard cap, it rewrites the remaining tail to a soft cap to avoid trimming again +/// immediately on the next write. +fn enforce_history_limit(file: &mut File, max_bytes: Option) -> Result<()> { + let Some(max_bytes) = max_bytes else { + return Ok(()); + }; + + if max_bytes == 0 { + return Ok(()); + } + + let max_bytes = match u64::try_from(max_bytes) { + Ok(value) => value, + Err(_) => return Ok(()), + }; + + let mut current_len = file.metadata()?.len(); + + if current_len <= max_bytes { + return Ok(()); + } + + let mut reader_file = file.try_clone()?; + reader_file.seek(SeekFrom::Start(0))?; + + let mut buf_reader = BufReader::new(reader_file); + let mut line_lengths = Vec::new(); + let mut line_buf = String::new(); + + loop { + line_buf.clear(); + + let bytes = buf_reader.read_line(&mut line_buf)?; + + if bytes == 0 { + break; + } + + line_lengths.push(bytes as u64); + } + + if line_lengths.is_empty() { + return Ok(()); + } + + let last_index = line_lengths.len() - 1; + let trim_target = trim_target_bytes(max_bytes, line_lengths[last_index]); + + let mut drop_bytes = 0u64; + let mut idx = 0usize; + + while current_len > trim_target && idx < last_index { + current_len = current_len.saturating_sub(line_lengths[idx]); + drop_bytes += line_lengths[idx]; + idx += 1; + } + + if drop_bytes == 0 { + return Ok(()); + } + + let mut reader = buf_reader.into_inner(); + reader.seek(SeekFrom::Start(drop_bytes))?; + + let capacity = usize::try_from(current_len).unwrap_or(0); + let mut tail = Vec::with_capacity(capacity); + + reader.read_to_end(&mut tail)?; + + file.set_len(0)?; + file.seek(SeekFrom::Start(0))?; + file.write_all(&tail)?; + file.flush()?; + + Ok(()) +} + +fn trim_target_bytes(max_bytes: u64, newest_entry_len: u64) -> u64 { + let soft_cap_bytes = ((max_bytes as f64) * HISTORY_SOFT_CAP_RATIO) + .floor() + .clamp(1.0, max_bytes as f64) as u64; + + soft_cap_bytes.max(newest_entry_len) +} + /// Asynchronously fetch the history file's *identifier* (inode on Unix) and /// the current number of entries by counting newline characters. pub(crate) async fn history_metadata(config: &Config) -> (u64, usize) { @@ -154,7 +256,6 @@ pub(crate) async fn history_metadata(config: &Config) -> (u64, usize) { /// /// Note this function is not async because it uses a sync advisory file /// locking API. -#[cfg(any(unix, windows))] pub(crate) fn lookup(log_id: u64, offset: usize, config: &Config) -> Option { let path = history_filepath(config); lookup_history_entry(&path, log_id, offset) @@ -211,7 +312,6 @@ async fn history_metadata_for_file(path: &Path) -> (u64, usize) { (log_id, count) } -#[cfg(any(unix, windows))] fn lookup_history_entry(path: &Path, log_id: u64, offset: usize) -> Option { use std::io::BufRead; use std::io::BufReader; @@ -281,23 +381,30 @@ fn lookup_history_entry(path: &Path, log_id: u64, offset: usize) -> Option Option { - #[cfg(unix)] - { - use std::os::unix::fs::MetadataExt; - Some(metadata.ino()) - } - - #[cfg(windows)] - { - use std::os::windows::fs::MetadataExt; - Some(metadata.creation_time()) - } + use std::os::unix::fs::MetadataExt; + Some(metadata.ino()) } -#[cfg(all(test, any(unix, windows)))] +#[cfg(windows)] +fn history_log_id(metadata: &std::fs::Metadata) -> Option { + use std::os::windows::fs::MetadataExt; + Some(metadata.creation_time()) +} + +#[cfg(not(any(unix, windows)))] +fn history_log_id(_metadata: &std::fs::Metadata) -> Option { + None +} + +#[cfg(test)] mod tests { use super::*; + use crate::config::Config; + use crate::config::ConfigOverrides; + use crate::config::ConfigToml; + use codex_protocol::ConversationId; use pretty_assertions::assert_eq; use std::fs::File; use std::io::Write; @@ -381,4 +488,131 @@ mod tests { lookup_history_entry(&history_path, log_id, 1).expect("lookup appended history entry"); assert_eq!(fetched, appended); } + + #[tokio::test] + async fn append_entry_trims_history_when_beyond_max_bytes() { + let codex_home = TempDir::new().expect("create temp dir"); + + let mut config = Config::load_from_base_config_with_overrides( + ConfigToml::default(), + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + ) + .expect("load config"); + + let conversation_id = ConversationId::new(); + + let entry_one = "a".repeat(200); + let entry_two = "b".repeat(200); + + let history_path = codex_home.path().join("history.jsonl"); + + append_entry(&entry_one, &conversation_id, &config) + .await + .expect("write first entry"); + + let first_len = std::fs::metadata(&history_path).expect("metadata").len(); + let limit_bytes = first_len + 10; + + config.history.max_bytes = + Some(usize::try_from(limit_bytes).expect("limit should fit into usize")); + + append_entry(&entry_two, &conversation_id, &config) + .await + .expect("write second entry"); + + let contents = std::fs::read_to_string(&history_path).expect("read history"); + + let entries = contents + .lines() + .map(|line| serde_json::from_str::(line).expect("parse entry")) + .collect::>(); + + assert_eq!( + entries.len(), + 1, + "only one entry left because entry_one should be evicted" + ); + assert_eq!(entries[0].text, entry_two); + assert!(std::fs::metadata(&history_path).expect("metadata").len() <= limit_bytes); + } + + #[tokio::test] + async fn append_entry_trims_history_to_soft_cap() { + let codex_home = TempDir::new().expect("create temp dir"); + + let mut config = Config::load_from_base_config_with_overrides( + ConfigToml::default(), + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + ) + .expect("load config"); + + let conversation_id = ConversationId::new(); + + let short_entry = "a".repeat(200); + let long_entry = "b".repeat(400); + + let history_path = codex_home.path().join("history.jsonl"); + + append_entry(&short_entry, &conversation_id, &config) + .await + .expect("write first entry"); + + let short_entry_len = std::fs::metadata(&history_path).expect("metadata").len(); + + append_entry(&long_entry, &conversation_id, &config) + .await + .expect("write second entry"); + + let two_entry_len = std::fs::metadata(&history_path).expect("metadata").len(); + + let long_entry_len = two_entry_len + .checked_sub(short_entry_len) + .expect("second entry length should be larger than first entry length"); + + config.history.max_bytes = Some( + usize::try_from((2 * long_entry_len) + (short_entry_len / 2)) + .expect("max bytes should fit into usize"), + ); + + append_entry(&long_entry, &conversation_id, &config) + .await + .expect("write third entry"); + + let contents = std::fs::read_to_string(&history_path).expect("read history"); + + let entries = contents + .lines() + .map(|line| serde_json::from_str::(line).expect("parse entry")) + .collect::>(); + + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].text, long_entry); + + let pruned_len = std::fs::metadata(&history_path).expect("metadata").len() as u64; + let max_bytes = config + .history + .max_bytes + .expect("max bytes should be configured") as u64; + + assert!(pruned_len <= max_bytes); + + let soft_cap_bytes = ((max_bytes as f64) * HISTORY_SOFT_CAP_RATIO) + .floor() + .clamp(1.0, max_bytes as f64) as u64; + let len_without_first = 2 * long_entry_len; + + assert!( + len_without_first <= max_bytes, + "dropping only the first entry would satisfy the hard cap" + ); + assert!( + len_without_first > soft_cap_bytes, + "soft cap should require more aggressive trimming than the hard cap" + ); + + assert_eq!(pruned_len, long_entry_len); + assert!(pruned_len <= soft_cap_bytes.max(long_entry_len)); + } } diff --git a/docs/config.md b/docs/config.md index a3e6ed266b..0f0761a301 100644 --- a/docs/config.md +++ b/docs/config.md @@ -841,6 +841,11 @@ To disable this behavior, configure `[history]` as follows: persistence = "none" # "save-all" is the default value ``` +To cap the size of `history.jsonl`, set `history.max_bytes` to a positive byte +count. When the file grows beyond the limit, Codex removes the oldest entries, +compacting the file down to roughly 80% of the hard cap while keeping the newest +record intact. Omitting the option—or setting it to `0`—disables pruning. + ### file_opener Identifies the editor/URI scheme to use for hyperlinking citations in model output. If set, citations to files in the model output will be hyperlinked using the specified URI scheme so they can be ctrl/cmd-clicked from the terminal to open them. @@ -933,64 +938,64 @@ Valid values: ## Config reference -| Key | Type / Values | Notes | -| ------------------------------------------------ | ----------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------- | -| `model` | string | Model to use (e.g., `gpt-5.1-codex-max`). | -| `model_provider` | string | Provider id from `model_providers` (default: `openai`). | -| `model_context_window` | number | Context window tokens. | -| `tool_output_token_limit` | number | Token budget for stored function/tool outputs in history (default: 2,560 tokens). | -| `approval_policy` | `untrusted` \| `on-failure` \| `on-request` \| `never` | When to prompt for approval. | -| `sandbox_mode` | `read-only` \| `workspace-write` \| `danger-full-access` | OS sandbox policy. | -| `sandbox_workspace_write.writable_roots` | array | Extra writable roots in workspace‑write. | -| `sandbox_workspace_write.network_access` | boolean | Allow network in workspace‑write (default: false). | -| `sandbox_workspace_write.exclude_tmpdir_env_var` | boolean | Exclude `$TMPDIR` from writable roots (default: false). | -| `sandbox_workspace_write.exclude_slash_tmp` | boolean | Exclude `/tmp` from writable roots (default: false). | -| `notify` | array | External program for notifications. | -| `tui.animations` | boolean | Enable terminal animations (welcome screen, shimmer, spinner). Defaults to true; set to `false` to disable visual motion. | -| `instructions` | string | Currently ignored; use `experimental_instructions_file` or `AGENTS.md`. | -| `features.` | boolean | See [feature flags](#feature-flags) for details | -| `mcp_servers..command` | string | MCP server launcher command (stdio servers only). | -| `mcp_servers..args` | array | MCP server args (stdio servers only). | -| `mcp_servers..env` | map | MCP server env vars (stdio servers only). | -| `mcp_servers..url` | string | MCP server url (streamable http servers only). | -| `mcp_servers..bearer_token_env_var` | string | environment variable containing a bearer token to use for auth (streamable http servers only). | -| `mcp_servers..enabled` | boolean | When false, Codex skips starting the server (default: true). | -| `mcp_servers..startup_timeout_sec` | number | Startup timeout in seconds (default: 10). Timeout is applied both for initializing MCP server and initially listing tools. | -| `mcp_servers..tool_timeout_sec` | number | Per-tool timeout in seconds (default: 60). Accepts fractional values; omit to use the default. | -| `mcp_servers..enabled_tools` | array | Restrict the server to the listed tool names. | -| `mcp_servers..disabled_tools` | array | Remove the listed tool names after applying `enabled_tools`, if any. | -| `model_providers..name` | string | Display name. | -| `model_providers..base_url` | string | API base URL. | -| `model_providers..env_key` | string | Env var for API key. | -| `model_providers..wire_api` | `chat` \| `responses` | Protocol used (default: `chat`). | -| `model_providers..query_params` | map | Extra query params (e.g., Azure `api-version`). | -| `model_providers..http_headers` | map | Additional static headers. | -| `model_providers..env_http_headers` | map | Headers sourced from env vars. | -| `model_providers..request_max_retries` | number | Per‑provider HTTP retry count (default: 4). | -| `model_providers..stream_max_retries` | number | SSE stream retry count (default: 5). | -| `model_providers..stream_idle_timeout_ms` | number | SSE idle timeout (ms) (default: 300000). | -| `project_doc_max_bytes` | number | Max bytes to read from `AGENTS.md`. | -| `profile` | string | Active profile name. | -| `profiles..*` | various | Profile‑scoped overrides of the same keys. | -| `history.persistence` | `save-all` \| `none` | History file persistence (default: `save-all`). | -| `history.max_bytes` | number | Currently ignored (not enforced). | -| `file_opener` | `vscode` \| `vscode-insiders` \| `windsurf` \| `cursor` \| `none` | URI scheme for clickable citations (default: `vscode`). | -| `tui` | table | TUI‑specific options. | -| `tui.notifications` | boolean \| array | Enable desktop notifications in the tui (default: true). | -| `hide_agent_reasoning` | boolean | Hide model reasoning events. | -| `check_for_update_on_startup` | boolean | Check for Codex updates on startup (default: true). Set to `false` only if updates are centrally managed. | -| `show_raw_agent_reasoning` | boolean | Show raw reasoning (when available). | -| `model_reasoning_effort` | `minimal` \| `low` \| `medium` \| `high` | Responses API reasoning effort. | -| `model_reasoning_summary` | `auto` \| `concise` \| `detailed` \| `none` | Reasoning summaries. | -| `model_verbosity` | `low` \| `medium` \| `high` | GPT‑5 text verbosity (Responses API). | -| `model_supports_reasoning_summaries` | boolean | Force‑enable reasoning summaries. | -| `model_reasoning_summary_format` | `none` \| `experimental` | Force reasoning summary format. | -| `chatgpt_base_url` | string | Base URL for ChatGPT auth flow. | -| `experimental_instructions_file` | string (path) | Replace built‑in instructions (experimental). | -| `experimental_use_exec_command_tool` | boolean | Use experimental exec command tool. | -| `projects..trust_level` | string | Mark project/worktree as trusted (only `"trusted"` is recognized). | -| `tools.web_search` | boolean | Enable web search tool (deprecated) (default: false). | -| `tools.view_image` | boolean | Enable or disable the `view_image` tool so Codex can attach local image files from the workspace (default: true). | -| `forced_login_method` | `chatgpt` \| `api` | Only allow Codex to be used with ChatGPT or API keys. | -| `forced_chatgpt_workspace_id` | string (uuid) | Only allow Codex to be used with the specified ChatGPT workspace. | -| `cli_auth_credentials_store` | `file` \| `keyring` \| `auto` | Where to store CLI login credentials (default: `file`). | +| Key | Type / Values | Notes | +| ------------------------------------------------ | ----------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------- | +| `model` | string | Model to use (e.g., `gpt-5.1-codex-max`). | +| `model_provider` | string | Provider id from `model_providers` (default: `openai`). | +| `model_context_window` | number | Context window tokens. | +| `tool_output_token_limit` | number | Token budget for stored function/tool outputs in history (default: 2,560 tokens). | +| `approval_policy` | `untrusted` \| `on-failure` \| `on-request` \| `never` | When to prompt for approval. | +| `sandbox_mode` | `read-only` \| `workspace-write` \| `danger-full-access` | OS sandbox policy. | +| `sandbox_workspace_write.writable_roots` | array | Extra writable roots in workspace‑write. | +| `sandbox_workspace_write.network_access` | boolean | Allow network in workspace‑write (default: false). | +| `sandbox_workspace_write.exclude_tmpdir_env_var` | boolean | Exclude `$TMPDIR` from writable roots (default: false). | +| `sandbox_workspace_write.exclude_slash_tmp` | boolean | Exclude `/tmp` from writable roots (default: false). | +| `notify` | array | External program for notifications. | +| `tui.animations` | boolean | Enable terminal animations (welcome screen, shimmer, spinner). Defaults to true; set to `false` to disable visual motion. | +| `instructions` | string | Currently ignored; use `experimental_instructions_file` or `AGENTS.md`. | +| `features.` | boolean | See [feature flags](#feature-flags) for details | +| `mcp_servers..command` | string | MCP server launcher command (stdio servers only). | +| `mcp_servers..args` | array | MCP server args (stdio servers only). | +| `mcp_servers..env` | map | MCP server env vars (stdio servers only). | +| `mcp_servers..url` | string | MCP server url (streamable http servers only). | +| `mcp_servers..bearer_token_env_var` | string | environment variable containing a bearer token to use for auth (streamable http servers only). | +| `mcp_servers..enabled` | boolean | When false, Codex skips starting the server (default: true). | +| `mcp_servers..startup_timeout_sec` | number | Startup timeout in seconds (default: 10). Timeout is applied both for initializing MCP server and initially listing tools. | +| `mcp_servers..tool_timeout_sec` | number | Per-tool timeout in seconds (default: 60). Accepts fractional values; omit to use the default. | +| `mcp_servers..enabled_tools` | array | Restrict the server to the listed tool names. | +| `mcp_servers..disabled_tools` | array | Remove the listed tool names after applying `enabled_tools`, if any. | +| `model_providers..name` | string | Display name. | +| `model_providers..base_url` | string | API base URL. | +| `model_providers..env_key` | string | Env var for API key. | +| `model_providers..wire_api` | `chat` \| `responses` | Protocol used (default: `chat`). | +| `model_providers..query_params` | map | Extra query params (e.g., Azure `api-version`). | +| `model_providers..http_headers` | map | Additional static headers. | +| `model_providers..env_http_headers` | map | Headers sourced from env vars. | +| `model_providers..request_max_retries` | number | Per‑provider HTTP retry count (default: 4). | +| `model_providers..stream_max_retries` | number | SSE stream retry count (default: 5). | +| `model_providers..stream_idle_timeout_ms` | number | SSE idle timeout (ms) (default: 300000). | +| `project_doc_max_bytes` | number | Max bytes to read from `AGENTS.md`. | +| `profile` | string | Active profile name. | +| `profiles..*` | various | Profile‑scoped overrides of the same keys. | +| `history.persistence` | `save-all` \| `none` | History file persistence (default: `save-all`). | +| `history.max_bytes` | number | Maximum size of `history.jsonl` in bytes; when exceeded, history is compacted to ~80% of this limit by dropping oldest entries. | +| `file_opener` | `vscode` \| `vscode-insiders` \| `windsurf` \| `cursor` \| `none` | URI scheme for clickable citations (default: `vscode`). | +| `tui` | table | TUI‑specific options. | +| `tui.notifications` | boolean \| array | Enable desktop notifications in the tui (default: true). | +| `hide_agent_reasoning` | boolean | Hide model reasoning events. | +| `check_for_update_on_startup` | boolean | Check for Codex updates on startup (default: true). Set to `false` only if updates are centrally managed. | +| `show_raw_agent_reasoning` | boolean | Show raw reasoning (when available). | +| `model_reasoning_effort` | `minimal` \| `low` \| `medium` \| `high` | Responses API reasoning effort. | +| `model_reasoning_summary` | `auto` \| `concise` \| `detailed` \| `none` | Reasoning summaries. | +| `model_verbosity` | `low` \| `medium` \| `high` | GPT‑5 text verbosity (Responses API). | +| `model_supports_reasoning_summaries` | boolean | Force‑enable reasoning summaries. | +| `model_reasoning_summary_format` | `none` \| `experimental` | Force reasoning summary format. | +| `chatgpt_base_url` | string | Base URL for ChatGPT auth flow. | +| `experimental_instructions_file` | string (path) | Replace built‑in instructions (experimental). | +| `experimental_use_exec_command_tool` | boolean | Use experimental exec command tool. | +| `projects..trust_level` | string | Mark project/worktree as trusted (only `"trusted"` is recognized). | +| `tools.web_search` | boolean | Enable web search tool (deprecated) (default: false). | +| `tools.view_image` | boolean | Enable or disable the `view_image` tool so Codex can attach local image files from the workspace (default: true). | +| `forced_login_method` | `chatgpt` \| `api` | Only allow Codex to be used with ChatGPT or API keys. | +| `forced_chatgpt_workspace_id` | string (uuid) | Only allow Codex to be used with the specified ChatGPT workspace. | +| `cli_auth_credentials_store` | `file` \| `keyring` \| `auto` | Where to store CLI login credentials (default: `file`). | diff --git a/docs/example-config.md b/docs/example-config.md index cc53efa612..6061dc8830 100644 --- a/docs/example-config.md +++ b/docs/example-config.md @@ -124,7 +124,7 @@ experimental_use_profile = false [history] # save-all (default) | none persistence = "save-all" -# Maximum bytes for history file (currently not enforced). Example: 5242880 +# Maximum bytes for history file; oldest entries are trimmed when exceeded. Example: 5242880 # max_bytes = 0 # URI scheme for clickable citations: vscode (default) | vscode-insiders | windsurf | cursor | none From 9d6f9738b9ab92881271e6283475bb628be8d7ee Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 2 Dec 2025 14:31:34 -0800 Subject: [PATCH 5/5] chore: make create_approval_requirement_for_command an async fn --- codex-rs/core/src/exec_policy.rs | 23 +++++++++++-------- codex-rs/core/src/tools/handlers/shell.rs | 17 ++++++++------ .../core/src/unified_exec/session_manager.rs | 16 +++++++------ 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 15e591648d..602e5d679e 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -115,7 +115,7 @@ fn evaluate_with_policy( } } -pub(crate) fn create_approval_requirement_for_command( +pub(crate) async fn create_approval_requirement_for_command( policy: &Policy, command: &[String], approval_policy: AskForApproval, @@ -296,8 +296,8 @@ prefix_rule(pattern=["rm"], decision="forbidden") ); } - #[test] - fn approval_requirement_prefers_execpolicy_match() { + #[tokio::test] + async fn approval_requirement_prefers_execpolicy_match() { let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; let mut parser = PolicyParser::new(); parser @@ -312,7 +312,8 @@ prefix_rule(pattern=["rm"], decision="forbidden") AskForApproval::OnRequest, &SandboxPolicy::DangerFullAccess, SandboxPermissions::UseDefault, - ); + ) + .await; assert_eq!( requirement, @@ -322,8 +323,8 @@ prefix_rule(pattern=["rm"], decision="forbidden") ); } - #[test] - fn approval_requirement_respects_approval_policy() { + #[tokio::test] + async fn approval_requirement_respects_approval_policy() { let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; let mut parser = PolicyParser::new(); parser @@ -338,7 +339,8 @@ prefix_rule(pattern=["rm"], decision="forbidden") AskForApproval::Never, &SandboxPolicy::DangerFullAccess, SandboxPermissions::UseDefault, - ); + ) + .await; assert_eq!( requirement, @@ -348,8 +350,8 @@ prefix_rule(pattern=["rm"], decision="forbidden") ); } - #[test] - fn approval_requirement_falls_back_to_heuristics() { + #[tokio::test] + async fn approval_requirement_falls_back_to_heuristics() { let command = vec!["python".to_string()]; let empty_policy = Policy::empty(); @@ -359,7 +361,8 @@ prefix_rule(pattern=["rm"], decision="forbidden") AskForApproval::UnlessTrusted, &SandboxPolicy::ReadOnly, SandboxPermissions::UseDefault, - ); + ) + .await; assert_eq!( requirement, diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 736da34491..d1b7d3144c 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -231,6 +231,15 @@ impl ShellHandler { let event_ctx = ToolEventCtx::new(session.as_ref(), turn.as_ref(), &call_id, None); emitter.begin(event_ctx).await; + let approval_requirement = create_approval_requirement_for_command( + &turn.exec_policy, + &exec_params.command, + turn.approval_policy, + &turn.sandbox_policy, + SandboxPermissions::from(exec_params.with_escalated_permissions.unwrap_or(false)), + ) + .await; + let req = ShellRequest { command: exec_params.command.clone(), cwd: exec_params.cwd.clone(), @@ -238,13 +247,7 @@ impl ShellHandler { env: exec_params.env.clone(), with_escalated_permissions: exec_params.with_escalated_permissions, justification: exec_params.justification.clone(), - approval_requirement: create_approval_requirement_for_command( - &turn.exec_policy, - &exec_params.command, - turn.approval_policy, - &turn.sandbox_policy, - SandboxPermissions::from(exec_params.with_escalated_permissions.unwrap_or(false)), - ), + approval_requirement, }; let mut orchestrator = ToolOrchestrator::new(); let mut runtime = ShellRuntime::new(); diff --git a/codex-rs/core/src/unified_exec/session_manager.rs b/codex-rs/core/src/unified_exec/session_manager.rs index 37a12bf2df..72c02cdb99 100644 --- a/codex-rs/core/src/unified_exec/session_manager.rs +++ b/codex-rs/core/src/unified_exec/session_manager.rs @@ -554,19 +554,21 @@ impl UnifiedExecSessionManager { let env = apply_unified_exec_env(create_env(&context.turn.shell_environment_policy)); let mut orchestrator = ToolOrchestrator::new(); let mut runtime = UnifiedExecRuntime::new(self); + let approval_requirement = create_approval_requirement_for_command( + &context.turn.exec_policy, + command, + context.turn.approval_policy, + &context.turn.sandbox_policy, + SandboxPermissions::from(with_escalated_permissions.unwrap_or(false)), + ) + .await; let req = UnifiedExecToolRequest::new( command.to_vec(), cwd, env, with_escalated_permissions, justification, - create_approval_requirement_for_command( - &context.turn.exec_policy, - command, - context.turn.approval_policy, - &context.turn.sandbox_policy, - SandboxPermissions::from(with_escalated_permissions.unwrap_or(false)), - ), + approval_requirement, ); let tool_ctx = ToolCtx { session: context.session.as_ref(),