diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index c1f2bc2841ee..b9fcc969b388 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1283,6 +1283,7 @@ dependencies = [ "serde_json", "shlex", "starlark", + "tempfile", "thiserror 2.0.17", ] @@ -1614,6 +1615,7 @@ dependencies = [ "textwrap 0.16.2", "tokio", "tokio-stream", + "tokio-util", "toml", "tracing", "tracing-appender", @@ -6599,6 +6601,7 @@ dependencies = [ "futures-sink", "futures-util", "pin-project-lite", + "slab", "tokio", ] diff --git a/codex-rs/app-server-protocol/src/protocol/mappers.rs b/codex-rs/app-server-protocol/src/protocol/mappers.rs index 802b3fb23a18..f708c1fa8558 100644 --- a/codex-rs/app-server-protocol/src/protocol/mappers.rs +++ b/codex-rs/app-server-protocol/src/protocol/mappers.rs @@ -5,7 +5,9 @@ impl From for v2::CommandExecParams { fn from(value: v1::ExecOneOffCommandParams) -> Self { Self { command: value.command, - timeout_ms: value.timeout_ms, + timeout_ms: value + .timeout_ms + .map(|timeout| i64::try_from(timeout).unwrap_or(60_000)), cwd: value.cwd, sandbox_policy: value.sandbox_policy.map(std::convert::Into::into), } 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 04cd1190bc0d..ba1e6261cc65 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 37726074202e..0e2be70c932e 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -637,7 +637,8 @@ pub struct FeedbackUploadResponse { #[ts(export_to = "v2/")] pub struct CommandExecParams { pub command: Vec, - pub timeout_ms: Option, + #[ts(type = "number | null")] + pub timeout_ms: Option, pub cwd: Option, pub sandbox_policy: Option, } @@ -785,6 +786,7 @@ pub struct Thread { /// Model provider used for this thread (for example, 'openai'). pub model_provider: String, /// Unix timestamp (in seconds) when the thread was created. + #[ts(type = "number")] pub created_at: i64, /// [UNSTABLE] Path to the thread on disk. pub path: PathBuf, @@ -875,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)] @@ -898,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, } @@ -1072,6 +1075,7 @@ pub enum ThreadItem { /// The command's exit code. exit_code: Option, /// The duration of the command execution in milliseconds. + #[ts(type = "number | null")] duration_ms: Option, }, #[serde(rename_all = "camelCase")] @@ -1338,6 +1342,7 @@ pub struct ReasoningSummaryTextDeltaNotification { pub turn_id: String, pub item_id: String, pub delta: String, + #[ts(type = "number")] pub summary_index: i64, } @@ -1348,6 +1353,7 @@ pub struct ReasoningSummaryPartAddedNotification { pub thread_id: String, pub turn_id: String, pub item_id: String, + #[ts(type = "number")] pub summary_index: i64, } @@ -1359,6 +1365,7 @@ pub struct ReasoningTextDeltaNotification { pub turn_id: String, pub item_id: String, pub delta: String, + #[ts(type = "number")] pub content_index: i64, } @@ -1493,7 +1500,9 @@ impl From for RateLimitSnapshot { #[ts(export_to = "v2/")] pub struct RateLimitWindow { pub used_percent: i32, + #[ts(type = "number | null")] pub window_duration_mins: Option, + #[ts(type = "number | null")] pub resets_at: Option, } diff --git a/codex-rs/app-server-test-client/src/main.rs b/codex-rs/app-server-test-client/src/main.rs index f75f3f98a8c4..8c2a38e46c9d 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/README.md b/codex-rs/app-server/README.md index 4155f78df93e..4e94a2c1331e 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1,6 +1,6 @@ # codex-app-server -`codex app-server` is the interface Codex uses to power rich interfaces such as the [Codex VS Code extension](https://marketplace.visualstudio.com/items?itemName=openai.chatgpt). The message schema is currently unstable, but those who wish to build experimental UIs on top of Codex may find it valuable. +`codex app-server` is the interface Codex uses to power rich interfaces such as the [Codex VS Code extension](https://marketplace.visualstudio.com/items?itemName=openai.chatgpt). ## Table of Contents - [Protocol](#protocol) diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 00757c8a94e0..b4dd16b9a618 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 95d53c720230..cd8aa9b7042d 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -1152,7 +1152,9 @@ impl CodexMessageProcessor { let cwd = params.cwd.unwrap_or_else(|| self.config.cwd.clone()); let env = create_env(&self.config.shell_environment_policy); - let timeout_ms = params.timeout_ms; + let timeout_ms = params + .timeout_ms + .and_then(|timeout_ms| u64::try_from(timeout_ms).ok()); let exec_params = ExecParams { command: params.command, cwd, @@ -2453,6 +2455,7 @@ impl CodexMessageProcessor { let turn = Turn { id: turn_id.clone(), items: vec![], + error: None, status: TurnStatus::InProgress, }; @@ -2494,6 +2497,7 @@ impl CodexMessageProcessor { Turn { id: turn_id, items, + error: None, status: TurnStatus::InProgress, } } diff --git a/codex-rs/core/src/api_bridge.rs b/codex-rs/core/src/api_bridge.rs index b9f802ae66c9..79fd67d65016 100644 --- a/codex-rs/core/src/api_bridge.rs +++ b/codex-rs/core/src/api_bridge.rs @@ -33,12 +33,20 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr { headers, body, } => { - if status == http::StatusCode::INTERNAL_SERVER_ERROR { + let body_text = body.unwrap_or_default(); + + if status == http::StatusCode::BAD_REQUEST { + if body_text + .contains("The image data you provided does not represent a valid image") + { + CodexErr::InvalidImageRequest() + } else { + CodexErr::InvalidRequest(body_text) + } + } else if status == http::StatusCode::INTERNAL_SERVER_ERROR { CodexErr::InternalServerError } else if status == http::StatusCode::TOO_MANY_REQUESTS { - if let Some(body) = body - && let Ok(err) = serde_json::from_str::(&body) - { + if let Ok(err) = serde_json::from_str::(&body_text) { if err.error.error_type.as_deref() == Some("usage_limit_reached") { let rate_limits = headers.as_ref().and_then(parse_rate_limit); let resets_at = err @@ -62,7 +70,7 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr { } else { CodexErr::UnexpectedStatus(UnexpectedResponseError { status, - body: body.unwrap_or_default(), + body: body_text, request_id: extract_request_id(headers.as_ref()), }) } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 8ae83d003436..f76cd7de7446 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -976,6 +976,7 @@ impl Session { .read() .await .resolve_elicitation(server_name, id, response) + .await } /// Records input items: always append to conversation history and @@ -2038,6 +2039,13 @@ pub(crate) async fn run_task( // Aborted turn is reported via a different event. break; } + Err(CodexErr::InvalidImageRequest()) => { + let mut state = sess.state.lock().await; + error_or_panic( + "Invalid image detected, replacing it in the last turn to prevent poisoning", + ); + state.history.replace_last_turn_images("Invalid image"); + } Err(e) => { info!("Turn error: {e:#}"); let event = EventMsg::Error(e.to_error_event(None)); @@ -2145,6 +2153,8 @@ async fn run_turn( } Err(CodexErr::UsageNotIncluded) => return Err(CodexErr::UsageNotIncluded), Err(e @ CodexErr::QuotaExceeded) => return Err(e), + Err(e @ CodexErr::InvalidImageRequest()) => return Err(e), + Err(e @ CodexErr::InvalidRequest(_)) => return Err(e), Err(e @ CodexErr::RefreshTokenFailed(_)) => return Err(e), Err(e) => { // Use the configured provider-specific stream retry budget. @@ -2480,7 +2490,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; @@ -2550,6 +2563,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/config/mod.rs b/codex-rs/core/src/config/mod.rs index 44cc651a0c25..5e768ba9d446 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -34,6 +34,7 @@ use crate::project_doc::DEFAULT_PROJECT_DOC_FILENAME; use crate::project_doc::LOCAL_PROJECT_DOC_FILENAME; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; +use crate::util::resolve_path; use codex_app_server_protocol::Tools; use codex_app_server_protocol::UserSavedConfig; use codex_protocol::config_types::ForcedLoginMethod; @@ -1016,15 +1017,8 @@ impl Config { let additional_writable_roots: Vec = additional_writable_roots .into_iter() .map(|path| { - let absolute = if path.is_absolute() { - path - } else { - resolved_cwd.join(path) - }; - match canonicalize(&absolute) { - Ok(canonical) => canonical, - Err(_) => absolute, - } + let absolute = resolve_path(&resolved_cwd, &path); + canonicalize(&absolute).unwrap_or(absolute) }) .collect(); let active_project = cfg @@ -1299,11 +1293,7 @@ impl Config { return Ok(None); }; - let full_path = if p.is_relative() { - cwd.join(p) - } else { - p.to_path_buf() - }; + let full_path = resolve_path(cwd, p); let contents = std::fs::read_to_string(&full_path).map_err(|e| { std::io::Error::new( diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 869ec829734b..c47a5709692d 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/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index 7277f0b0c631..b9a9c58f63d3 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -5,6 +5,8 @@ use crate::truncate::approx_token_count; use crate::truncate::approx_tokens_from_byte_count; use crate::truncate::truncate_function_output_items_with_policy; use crate::truncate::truncate_text; +use codex_protocol::models::ContentItem; +use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::TokenUsage; @@ -118,6 +120,37 @@ impl ContextManager { self.items = items; } + pub(crate) fn replace_last_turn_images(&mut self, placeholder: &str) { + let Some(last_item) = self.items.last_mut() else { + return; + }; + + match last_item { + ResponseItem::Message { role, content, .. } if role == "user" => { + for item in content.iter_mut() { + if matches!(item, ContentItem::InputImage { .. }) { + *item = ContentItem::InputText { + text: placeholder.to_string(), + }; + } + } + } + ResponseItem::FunctionCallOutput { output, .. } => { + let Some(content_items) = output.content_items.as_mut() else { + return; + }; + for item in content_items.iter_mut() { + if matches!(item, FunctionCallOutputContentItem::InputImage { .. }) { + *item = FunctionCallOutputContentItem::InputText { + text: placeholder.to_string(), + }; + } + } + } + _ => {} + } + } + pub(crate) fn update_token_info( &mut self, usage: &TokenUsage, diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index 9130b40e1e7d..a25261d649f3 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -103,6 +103,14 @@ pub enum CodexErr { #[error("{0}")] UnexpectedStatus(UnexpectedResponseError), + /// Invalid request. + #[error("{0}")] + InvalidRequest(String), + + /// Invalid image. + #[error("Image poisoning")] + InvalidImageRequest(), + #[error("{0}")] UsageLimitReached(UsageLimitReachedError), diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 15e591648d31..602e5d679ec2 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/git_info.rs b/codex-rs/core/src/git_info.rs index 4284ff12632f..065f1280a493 100644 --- a/codex-rs/core/src/git_info.rs +++ b/codex-rs/core/src/git_info.rs @@ -2,6 +2,7 @@ use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; +use crate::util::resolve_path; use codex_app_server_protocol::GitSha; use codex_protocol::protocol::GitInfo; use futures::future::join_all; @@ -548,11 +549,7 @@ pub fn resolve_root_git_project_for_trust(cwd: &Path) -> Option { .trim() .to_string(); - let git_dir_path_raw = if Path::new(&git_dir_s).is_absolute() { - PathBuf::from(&git_dir_s) - } else { - base.join(&git_dir_s) - }; + let git_dir_path_raw = resolve_path(base, &PathBuf::from(&git_dir_s)); // Normalize to handle macOS /var vs /private/var and resolve ".." segments. let git_dir_path = std::fs::canonicalize(&git_dir_path_raw).unwrap_or(git_dir_path_raw); diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 22cb84e2c956..11a90f77a81c 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -12,7 +12,6 @@ use std::env; use std::ffi::OsString; use std::path::PathBuf; use std::sync::Arc; -use std::sync::Mutex; use std::time::Duration; use crate::mcp::auth::McpAuthStatusEntry; @@ -55,6 +54,7 @@ use serde::Serialize; use serde_json::json; use sha1::Digest; use sha1::Sha1; +use tokio::sync::Mutex; use tokio::sync::oneshot; use tokio::task::JoinSet; use tokio_util::sync::CancellationToken; @@ -128,7 +128,7 @@ struct ElicitationRequestManager { } impl ElicitationRequestManager { - fn resolve( + async fn resolve( &self, server_name: String, id: RequestId, @@ -136,7 +136,7 @@ impl ElicitationRequestManager { ) -> Result<()> { self.requests .lock() - .map_err(|e| anyhow!("failed to lock elicitation requests: {e:?}"))? + .await .remove(&(server_name, id)) .ok_or_else(|| anyhow!("elicitation request not found"))? .send(response) @@ -151,7 +151,8 @@ impl ElicitationRequestManager { let server_name = server_name.clone(); async move { let (tx, rx) = oneshot::channel(); - if let Ok(mut lock) = elicitation_requests.lock() { + { + let mut lock = elicitation_requests.lock().await; lock.insert((server_name.clone(), id.clone()), tx); } let _ = tx_event @@ -365,13 +366,15 @@ impl McpConnectionManager { .context("failed to get client") } - pub fn resolve_elicitation( + pub async fn resolve_elicitation( &self, server_name: String, id: RequestId, response: ElicitationResponse, ) -> Result<()> { - self.elicitation_requests.resolve(server_name, id, response) + self.elicitation_requests + .resolve(server_name, id, response) + .await } /// Returns a single map that contains all tools. Each key is the diff --git a/codex-rs/core/src/message_history.rs b/codex-rs/core/src/message_history.rs index 387641c310d5..e46dd9306754 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()) - } + use std::os::unix::fs::MetadataExt; + Some(metadata.ino()) +} - #[cfg(windows)] - { - use std::os::windows::fs::MetadataExt; - Some(metadata.creation_time()) - } +#[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(all(test, any(unix, windows)))] +#[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/codex-rs/core/src/openai_model_info.rs b/codex-rs/core/src/openai_model_info.rs index 0ae3f03eb5bc..96f3ed77cbe0 100644 --- a/codex-rs/core/src/openai_model_info.rs +++ b/codex-rs/core/src/openai_model_info.rs @@ -76,6 +76,8 @@ pub(crate) fn get_model_info(model_family: &ModelFamily) -> Option { _ if slug.starts_with("codex-") => Some(ModelInfo::new(CONTEXT_WINDOW_272K)), + _ if slug.starts_with("exp-") => Some(ModelInfo::new(CONTEXT_WINDOW_272K)), + _ => None, } } diff --git a/codex-rs/core/src/rollout/list.rs b/codex-rs/core/src/rollout/list.rs index 781e3fe372ac..e2ef0e883c6a 100644 --- a/codex-rs/core/src/rollout/list.rs +++ b/codex-rs/core/src/rollout/list.rs @@ -9,6 +9,7 @@ use std::sync::atomic::AtomicBool; use time::OffsetDateTime; use time::PrimitiveDateTime; use time::format_description::FormatItem; +use time::format_description::well_known::Rfc3339; use time::macros::format_description; use uuid::Uuid; @@ -39,18 +40,15 @@ pub struct ConversationItem { pub path: PathBuf, /// First up to `HEAD_RECORD_LIMIT` JSONL records parsed as JSON (includes meta line). pub head: Vec, - /// Last up to `TAIL_RECORD_LIMIT` JSONL response records parsed as JSON. - pub tail: Vec, /// RFC3339 timestamp string for when the session was created, if available. pub created_at: Option, - /// RFC3339 timestamp string for the most recent response in the tail, if available. + /// RFC3339 timestamp string for the most recent update (from file mtime). pub updated_at: Option, } #[derive(Default)] struct HeadTailSummary { head: Vec, - tail: Vec, saw_session_meta: bool, saw_user_event: bool, source: Option, @@ -62,7 +60,6 @@ struct HeadTailSummary { /// Hard cap to bound worst‑case work per request. const MAX_SCAN_FILES: usize = 10000; const HEAD_RECORD_LIMIT: usize = 10; -const TAIL_RECORD_LIMIT: usize = 10; /// Pagination cursor identifying a file by timestamp and UUID. #[derive(Debug, Clone, PartialEq, Eq)] @@ -141,13 +138,6 @@ pub(crate) async fn get_conversations( Ok(result) } -/// Load the full contents of a single conversation session file at `path`. -/// Returns the entire file contents as a String. -#[allow(dead_code)] -pub(crate) async fn get_conversation(path: &Path) -> io::Result { - tokio::fs::read_to_string(path).await -} - /// Load conversation file paths from disk using directory traversal. /// /// Directory layout: `~/.codex/sessions/YYYY/MM/DD/rollout-YYYY-MM-DDThh-mm-ss-.jsonl` @@ -212,9 +202,8 @@ async fn traverse_directories_for_paths( more_matches_available = true; break 'outer; } - // Read head and simultaneously detect message events within the same - // first N JSONL records to avoid a second file read. - let summary = read_head_and_tail(&path, HEAD_RECORD_LIMIT, TAIL_RECORD_LIMIT) + // Read head and detect message events; stop once meta + user are found. + let summary = read_head_summary(&path, HEAD_RECORD_LIMIT) .await .unwrap_or_default(); if !allowed_sources.is_empty() @@ -233,16 +222,19 @@ async fn traverse_directories_for_paths( if summary.saw_session_meta && summary.saw_user_event { let HeadTailSummary { head, - tail, created_at, mut updated_at, .. } = summary; - updated_at = updated_at.or_else(|| created_at.clone()); + if updated_at.is_none() { + updated_at = file_modified_rfc3339(&path) + .await + .unwrap_or(None) + .or_else(|| created_at.clone()); + } items.push(ConversationItem { path, head, - tail, created_at, updated_at, }); @@ -384,11 +376,7 @@ impl<'a> ProviderMatcher<'a> { } } -async fn read_head_and_tail( - path: &Path, - head_limit: usize, - tail_limit: usize, -) -> io::Result { +async fn read_head_summary(path: &Path, head_limit: usize) -> io::Result { use tokio::io::AsyncBufReadExt; let file = tokio::fs::File::open(path).await?; @@ -441,107 +429,30 @@ async fn read_head_and_tail( } } } - } - if tail_limit != 0 { - let (tail, updated_at) = read_tail_records(path, tail_limit).await?; - summary.tail = tail; - summary.updated_at = updated_at; + if summary.saw_session_meta && summary.saw_user_event { + break; + } } + Ok(summary) } /// Read up to `HEAD_RECORD_LIMIT` records from the start of the rollout file at `path`. /// This should be enough to produce a summary including the session meta line. pub async fn read_head_for_summary(path: &Path) -> io::Result> { - let summary = read_head_and_tail(path, HEAD_RECORD_LIMIT, 0).await?; + let summary = read_head_summary(path, HEAD_RECORD_LIMIT).await?; Ok(summary.head) } -async fn read_tail_records( - path: &Path, - max_records: usize, -) -> io::Result<(Vec, Option)> { - use std::io::SeekFrom; - use tokio::io::AsyncReadExt; - use tokio::io::AsyncSeekExt; - - if max_records == 0 { - return Ok((Vec::new(), None)); - } - - const CHUNK_SIZE: usize = 8192; - - let mut file = tokio::fs::File::open(path).await?; - let mut pos = file.seek(SeekFrom::End(0)).await?; - if pos == 0 { - return Ok((Vec::new(), None)); - } - - let mut buffer: Vec = Vec::new(); - let mut latest_timestamp: Option = None; - - loop { - let slice_start = match (pos > 0, buffer.iter().position(|&b| b == b'\n')) { - (true, Some(idx)) => idx + 1, - _ => 0, - }; - let (tail, newest_ts) = collect_last_response_values(&buffer[slice_start..], max_records); - if latest_timestamp.is_none() { - latest_timestamp = newest_ts.clone(); - } - if tail.len() >= max_records || pos == 0 { - return Ok((tail, latest_timestamp.or(newest_ts))); - } - - let read_size = CHUNK_SIZE.min(pos as usize); - if read_size == 0 { - return Ok((tail, latest_timestamp.or(newest_ts))); - } - pos -= read_size as u64; - file.seek(SeekFrom::Start(pos)).await?; - let mut chunk = vec![0; read_size]; - file.read_exact(&mut chunk).await?; - chunk.extend_from_slice(&buffer); - buffer = chunk; - } -} - -fn collect_last_response_values( - buffer: &[u8], - max_records: usize, -) -> (Vec, Option) { - use std::borrow::Cow; - - if buffer.is_empty() || max_records == 0 { - return (Vec::new(), None); - } - - let text: Cow<'_, str> = String::from_utf8_lossy(buffer); - let mut collected_rev: Vec = Vec::new(); - let mut latest_timestamp: Option = None; - for line in text.lines().rev() { - let trimmed = line.trim(); - if trimmed.is_empty() { - continue; - } - let parsed: serde_json::Result = serde_json::from_str(trimmed); - let Ok(rollout_line) = parsed else { continue }; - let RolloutLine { timestamp, item } = rollout_line; - if let RolloutItem::ResponseItem(item) = item - && let Ok(val) = serde_json::to_value(&item) - { - if latest_timestamp.is_none() { - latest_timestamp = Some(timestamp.clone()); - } - collected_rev.push(val); - if collected_rev.len() == max_records { - break; - } - } - } - collected_rev.reverse(); - (collected_rev, latest_timestamp) +async fn file_modified_rfc3339(path: &Path) -> io::Result> { + let meta = tokio::fs::metadata(path).await?; + let modified = meta.modified().ok(); + let Some(modified) = modified else { + return Ok(None); + }; + let dt = OffsetDateTime::from(modified); + Ok(dt.format(&Rfc3339).ok()) } /// Locate a recorded conversation rollout file by its UUID string using the existing diff --git a/codex-rs/core/src/rollout/tests.rs b/codex-rs/core/src/rollout/tests.rs index f367782b12ce..1df3659ba0f6 100644 --- a/codex-rs/core/src/rollout/tests.rs +++ b/codex-rs/core/src/rollout/tests.rs @@ -16,13 +16,11 @@ use crate::rollout::INTERACTIVE_SESSION_SOURCES; use crate::rollout::list::ConversationItem; use crate::rollout::list::ConversationsPage; use crate::rollout::list::Cursor; -use crate::rollout::list::get_conversation; use crate::rollout::list::get_conversations; use anyhow::Result; use codex_protocol::ConversationId; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; -use codex_protocol::protocol::CompactedItem; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::RolloutLine; @@ -226,28 +224,28 @@ async fn test_list_conversations_latest_first() { "model_provider": "test-provider", })]; + let updated_times: Vec> = + page.items.iter().map(|i| i.updated_at.clone()).collect(); + let expected = ConversationsPage { items: vec![ ConversationItem { path: p1, head: head_3, - tail: Vec::new(), created_at: Some("2025-01-03T12-00-00".into()), - updated_at: Some("2025-01-03T12-00-00".into()), + updated_at: updated_times.first().cloned().flatten(), }, ConversationItem { path: p2, head: head_2, - tail: Vec::new(), created_at: Some("2025-01-02T12-00-00".into()), - updated_at: Some("2025-01-02T12-00-00".into()), + updated_at: updated_times.get(1).cloned().flatten(), }, ConversationItem { path: p3, head: head_1, - tail: Vec::new(), created_at: Some("2025-01-01T12-00-00".into()), - updated_at: Some("2025-01-01T12-00-00".into()), + updated_at: updated_times.get(2).cloned().flatten(), }, ], next_cursor: None, @@ -355,6 +353,8 @@ async fn test_pagination_cursor() { "source": "vscode", "model_provider": "test-provider", })]; + let updated_page1: Vec> = + page1.items.iter().map(|i| i.updated_at.clone()).collect(); let expected_cursor1: Cursor = serde_json::from_str(&format!("\"2025-03-04T09-00-00|{u4}\"")).unwrap(); let expected_page1 = ConversationsPage { @@ -362,16 +362,14 @@ async fn test_pagination_cursor() { ConversationItem { path: p5, head: head_5, - tail: Vec::new(), created_at: Some("2025-03-05T09-00-00".into()), - updated_at: Some("2025-03-05T09-00-00".into()), + updated_at: updated_page1.first().cloned().flatten(), }, ConversationItem { path: p4, head: head_4, - tail: Vec::new(), created_at: Some("2025-03-04T09-00-00".into()), - updated_at: Some("2025-03-04T09-00-00".into()), + updated_at: updated_page1.get(1).cloned().flatten(), }, ], next_cursor: Some(expected_cursor1.clone()), @@ -422,6 +420,8 @@ async fn test_pagination_cursor() { "source": "vscode", "model_provider": "test-provider", })]; + let updated_page2: Vec> = + page2.items.iter().map(|i| i.updated_at.clone()).collect(); let expected_cursor2: Cursor = serde_json::from_str(&format!("\"2025-03-02T09-00-00|{u2}\"")).unwrap(); let expected_page2 = ConversationsPage { @@ -429,16 +429,14 @@ async fn test_pagination_cursor() { ConversationItem { path: p3, head: head_3, - tail: Vec::new(), created_at: Some("2025-03-03T09-00-00".into()), - updated_at: Some("2025-03-03T09-00-00".into()), + updated_at: updated_page2.first().cloned().flatten(), }, ConversationItem { path: p2, head: head_2, - tail: Vec::new(), created_at: Some("2025-03-02T09-00-00".into()), - updated_at: Some("2025-03-02T09-00-00".into()), + updated_at: updated_page2.get(1).cloned().flatten(), }, ], next_cursor: Some(expected_cursor2.clone()), @@ -473,13 +471,14 @@ async fn test_pagination_cursor() { "source": "vscode", "model_provider": "test-provider", })]; + let updated_page3: Vec> = + page3.items.iter().map(|i| i.updated_at.clone()).collect(); let expected_page3 = ConversationsPage { items: vec![ConversationItem { path: p1, head: head_1, - tail: Vec::new(), created_at: Some("2025-03-01T09-00-00".into()), - updated_at: Some("2025-03-01T09-00-00".into()), + updated_at: updated_page3.first().cloned().flatten(), }], next_cursor: None, num_scanned_files: 5, // scanned 05, 04 (anchor), 03, 02 (anchor), 01 @@ -510,7 +509,7 @@ async fn test_get_conversation_contents() { .unwrap(); let path = &page.items[0].path; - let content = get_conversation(path).await.unwrap(); + let content = tokio::fs::read_to_string(path).await.unwrap(); // Page equality (single item) let expected_path = home @@ -533,9 +532,8 @@ async fn test_get_conversation_contents() { items: vec![ConversationItem { path: expected_path, head: expected_head, - tail: Vec::new(), created_at: Some(ts.into()), - updated_at: Some(ts.into()), + updated_at: page.items[0].updated_at.clone(), }], next_cursor: None, num_scanned_files: 1, @@ -570,7 +568,7 @@ async fn test_get_conversation_contents() { } #[tokio::test] -async fn test_tail_includes_last_response_items() -> Result<()> { +async fn test_updated_at_uses_file_mtime() -> Result<()> { let temp = TempDir::new().unwrap(); let home = temp.path(); @@ -636,229 +634,16 @@ async fn test_tail_includes_last_response_items() -> Result<()> { ) .await?; let item = page.items.first().expect("conversation item"); - let tail_len = item.tail.len(); - assert_eq!(tail_len, 10usize.min(total_messages)); - - let expected: Vec = (total_messages - tail_len..total_messages) - .map(|idx| { - serde_json::json!({ - "type": "message", - "role": "assistant", - "content": [ - { - "type": "output_text", - "text": format!("reply-{idx}"), - } - ], - }) - }) - .collect(); - - assert_eq!(item.tail, expected); assert_eq!(item.created_at.as_deref(), Some(ts)); - let expected_updated = format!("{ts}-{last:02}", last = total_messages - 1); - assert_eq!(item.updated_at.as_deref(), Some(expected_updated.as_str())); - - Ok(()) -} - -#[tokio::test] -async fn test_tail_handles_short_sessions() -> Result<()> { - let temp = TempDir::new().unwrap(); - let home = temp.path(); - - let ts = "2025-06-02T08-30-00"; - let uuid = Uuid::from_u128(7); - let day_dir = home.join("sessions").join("2025").join("06").join("02"); - fs::create_dir_all(&day_dir)?; - let file_path = day_dir.join(format!("rollout-{ts}-{uuid}.jsonl")); - let mut file = File::create(&file_path)?; - - let conversation_id = ConversationId::from_string(&uuid.to_string())?; - let meta_line = RolloutLine { - timestamp: ts.to_string(), - item: RolloutItem::SessionMeta(SessionMetaLine { - meta: SessionMeta { - id: conversation_id, - timestamp: ts.to_string(), - instructions: None, - cwd: ".".into(), - originator: "test_originator".into(), - cli_version: "test_version".into(), - source: SessionSource::VSCode, - model_provider: Some("test-provider".into()), - }, - git: None, - }), - }; - writeln!(file, "{}", serde_json::to_string(&meta_line)?)?; - - let user_event_line = RolloutLine { - timestamp: ts.to_string(), - item: RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent { - message: "hi".into(), - images: None, - })), - }; - writeln!(file, "{}", serde_json::to_string(&user_event_line)?)?; - - for idx in 0..3 { - let response_line = RolloutLine { - timestamp: format!("{ts}-{idx:02}"), - item: RolloutItem::ResponseItem(ResponseItem::Message { - id: None, - role: "assistant".into(), - content: vec![ContentItem::OutputText { - text: format!("short-{idx}"), - }], - }), - }; - writeln!(file, "{}", serde_json::to_string(&response_line)?)?; - } - drop(file); - - let provider_filter = provider_vec(&[TEST_PROVIDER]); - let page = get_conversations( - home, - 1, - None, - INTERACTIVE_SESSION_SOURCES, - Some(provider_filter.as_slice()), - TEST_PROVIDER, - ) - .await?; - let tail = &page.items.first().expect("conversation item").tail; - - assert_eq!(tail.len(), 3); - - let expected: Vec = (0..3) - .map(|idx| { - serde_json::json!({ - "type": "message", - "role": "assistant", - "content": [ - { - "type": "output_text", - "text": format!("short-{idx}"), - } - ], - }) - }) - .collect(); - - assert_eq!(tail, &expected); - let expected_updated = format!("{ts}-{last:02}", last = 2); - assert_eq!( - page.items[0].updated_at.as_deref(), - Some(expected_updated.as_str()) - ); - - Ok(()) -} - -#[tokio::test] -async fn test_tail_skips_trailing_non_responses() -> Result<()> { - let temp = TempDir::new().unwrap(); - let home = temp.path(); - - let ts = "2025-06-03T10-00-00"; - let uuid = Uuid::from_u128(11); - let day_dir = home.join("sessions").join("2025").join("06").join("03"); - fs::create_dir_all(&day_dir)?; - let file_path = day_dir.join(format!("rollout-{ts}-{uuid}.jsonl")); - let mut file = File::create(&file_path)?; - - let conversation_id = ConversationId::from_string(&uuid.to_string())?; - let meta_line = RolloutLine { - timestamp: ts.to_string(), - item: RolloutItem::SessionMeta(SessionMetaLine { - meta: SessionMeta { - id: conversation_id, - timestamp: ts.to_string(), - instructions: None, - cwd: ".".into(), - originator: "test_originator".into(), - cli_version: "test_version".into(), - source: SessionSource::VSCode, - model_provider: Some("test-provider".into()), - }, - git: None, - }), - }; - writeln!(file, "{}", serde_json::to_string(&meta_line)?)?; - - let user_event_line = RolloutLine { - timestamp: ts.to_string(), - item: RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent { - message: "hello".into(), - images: None, - })), - }; - writeln!(file, "{}", serde_json::to_string(&user_event_line)?)?; - - for idx in 0..4 { - let response_line = RolloutLine { - timestamp: format!("{ts}-{idx:02}"), - item: RolloutItem::ResponseItem(ResponseItem::Message { - id: None, - role: "assistant".into(), - content: vec![ContentItem::OutputText { - text: format!("response-{idx}"), - }], - }), - }; - writeln!(file, "{}", serde_json::to_string(&response_line)?)?; - } - - let compacted_line = RolloutLine { - timestamp: format!("{ts}-compacted"), - item: RolloutItem::Compacted(CompactedItem { - message: "compacted".into(), - replacement_history: None, - }), - }; - writeln!(file, "{}", serde_json::to_string(&compacted_line)?)?; - - let shutdown_event = RolloutLine { - timestamp: format!("{ts}-shutdown"), - item: RolloutItem::EventMsg(EventMsg::ShutdownComplete), - }; - writeln!(file, "{}", serde_json::to_string(&shutdown_event)?)?; - drop(file); - - let provider_filter = provider_vec(&[TEST_PROVIDER]); - let page = get_conversations( - home, - 1, - None, - INTERACTIVE_SESSION_SOURCES, - Some(provider_filter.as_slice()), - TEST_PROVIDER, - ) - .await?; - let tail = &page.items.first().expect("conversation item").tail; - - let expected: Vec = (0..4) - .map(|idx| { - serde_json::json!({ - "type": "message", - "role": "assistant", - "content": [ - { - "type": "output_text", - "text": format!("response-{idx}"), - } - ], - }) - }) - .collect(); - - assert_eq!(tail, &expected); - let expected_updated = format!("{ts}-{last:02}", last = 3); - assert_eq!( - page.items[0].updated_at.as_deref(), - Some(expected_updated.as_str()) - ); + let updated = item + .updated_at + .as_deref() + .and_then(|s| chrono::DateTime::parse_from_rfc3339(s).ok()) + .map(|dt| dt.with_timezone(&chrono::Utc)) + .expect("updated_at set from file mtime"); + let now = chrono::Utc::now(); + let age = now - updated; + assert!(age.num_seconds().abs() < 30); Ok(()) } @@ -913,22 +698,22 @@ async fn test_stable_ordering_same_second_pagination() { "model_provider": "test-provider", })] }; + let updated_page1: Vec> = + page1.items.iter().map(|i| i.updated_at.clone()).collect(); let expected_cursor1: Cursor = serde_json::from_str(&format!("\"{ts}|{u2}\"")).unwrap(); let expected_page1 = ConversationsPage { items: vec![ ConversationItem { path: p3, head: head(u3), - tail: Vec::new(), created_at: Some(ts.to_string()), - updated_at: Some(ts.to_string()), + updated_at: updated_page1.first().cloned().flatten(), }, ConversationItem { path: p2, head: head(u2), - tail: Vec::new(), created_at: Some(ts.to_string()), - updated_at: Some(ts.to_string()), + updated_at: updated_page1.get(1).cloned().flatten(), }, ], next_cursor: Some(expected_cursor1.clone()), @@ -953,13 +738,14 @@ async fn test_stable_ordering_same_second_pagination() { .join("07") .join("01") .join(format!("rollout-2025-07-01T00-00-00-{u1}.jsonl")); + let updated_page2: Vec> = + page2.items.iter().map(|i| i.updated_at.clone()).collect(); let expected_page2 = ConversationsPage { items: vec![ConversationItem { path: p1, head: head(u1), - tail: Vec::new(), created_at: Some(ts.to_string()), - updated_at: Some(ts.to_string()), + updated_at: updated_page2.first().cloned().flatten(), }], next_cursor: None, num_scanned_files: 3, // scanned u3, u2 (anchor), u1 diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index 1e81981fd10e..e82e1074536f 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -6,6 +6,7 @@ use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::ApplyPatchFileChange; use crate::exec::SandboxType; +use crate::util::resolve_path; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; @@ -150,11 +151,7 @@ fn is_write_patch_constrained_to_writable_paths( // and roots are converted to absolute, normalized forms before the // prefix check. let is_path_writable = |p: &PathBuf| { - let abs = if p.is_absolute() { - p.clone() - } else { - cwd.join(p) - }; + let abs = resolve_path(cwd, p); let abs = match normalize(&abs) { Some(v) => v, None => return false, diff --git a/codex-rs/core/src/state/session.rs b/codex-rs/core/src/state/session.rs index caebac6b86c1..8c739c924357 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/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 2109f1d2c87f..4a28619c760e 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -1,4 +1,5 @@ use std::collections::BTreeMap; +use std::path::Path; use crate::apply_patch; use crate::apply_patch::InternalApplyPatchInvocation; @@ -7,7 +8,10 @@ use crate::client_common::tools::FreeformTool; use crate::client_common::tools::FreeformToolFormat; use crate::client_common::tools::ResponsesApiTool; use crate::client_common::tools::ToolSpec; +use crate::codex::Session; +use crate::codex::TurnContext; use crate::function_tool::FunctionCallError; +use crate::tools::context::SharedTurnDiffTracker; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; @@ -164,6 +168,86 @@ pub enum ApplyPatchToolType { Function, } +#[allow(clippy::too_many_arguments)] +pub(crate) async fn intercept_apply_patch( + command: &[String], + cwd: &Path, + timeout_ms: Option, + session: &Session, + turn: &TurnContext, + tracker: Option<&SharedTurnDiffTracker>, + call_id: &str, + tool_name: &str, +) -> Result, FunctionCallError> { + match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd) { + codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => { + session + .record_model_warning( + format!("apply_patch was requested via {tool_name}. Use the apply_patch tool instead of exec_command."), + turn, + ) + .await; + match apply_patch::apply_patch(session, turn, call_id, changes).await { + InternalApplyPatchInvocation::Output(item) => { + let content = item?; + Ok(Some(ToolOutput::Function { + content, + content_items: None, + success: Some(true), + })) + } + InternalApplyPatchInvocation::DelegateToExec(apply) => { + let emitter = ToolEmitter::apply_patch( + convert_apply_patch_to_protocol(&apply.action), + !apply.user_explicitly_approved_this_action, + ); + let event_ctx = + ToolEventCtx::new(session, turn, call_id, tracker.as_ref().copied()); + emitter.begin(event_ctx).await; + + let req = ApplyPatchRequest { + patch: apply.action.patch.clone(), + cwd: apply.action.cwd.clone(), + timeout_ms, + user_explicitly_approved: apply.user_explicitly_approved_this_action, + codex_exe: turn.codex_linux_sandbox_exe.clone(), + }; + + let mut orchestrator = ToolOrchestrator::new(); + let mut runtime = ApplyPatchRuntime::new(); + let tool_ctx = ToolCtx { + session, + turn, + call_id: call_id.to_string(), + tool_name: tool_name.to_string(), + }; + let out = orchestrator + .run(&mut runtime, &req, &tool_ctx, turn, turn.approval_policy) + .await; + let event_ctx = + ToolEventCtx::new(session, turn, call_id, tracker.as_ref().copied()); + let content = emitter.finish(event_ctx, out).await?; + Ok(Some(ToolOutput::Function { + content, + content_items: None, + success: Some(true), + })) + } + } + } + codex_apply_patch::MaybeApplyPatchVerified::CorrectnessError(parse_error) => { + Err(FunctionCallError::RespondToModel(format!( + "apply_patch verification failed: {parse_error}" + ))) + } + codex_apply_patch::MaybeApplyPatchVerified::ShellParseError(error) => { + tracing::trace!("Failed to parse apply_patch input, {error:?}"); + Ok(None) + } + codex_apply_patch::MaybeApplyPatchVerified::NotApplyPatch => Ok(None), + } +} + /// Returns a custom tool that can be used to edit files. Well-suited for GPT-5 models /// https://platform.openai.com/docs/guides/function-calling#custom-tools pub(crate) fn create_apply_patch_freeform_tool() -> ToolSpec { diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 99c822fa565d..d1b7d3144cc5 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -3,9 +3,6 @@ use codex_protocol::models::ShellCommandToolCallParams; use codex_protocol::models::ShellToolCallParams; use std::sync::Arc; -use crate::apply_patch; -use crate::apply_patch::InternalApplyPatchInvocation; -use crate::apply_patch::convert_apply_patch_to_protocol; use crate::codex::TurnContext; use crate::exec::ExecParams; use crate::exec_env::create_env; @@ -19,11 +16,10 @@ use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; +use crate::tools::handlers::apply_patch::intercept_apply_patch; use crate::tools::orchestrator::ToolOrchestrator; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; -use crate::tools::runtimes::apply_patch::ApplyPatchRequest; -use crate::tools::runtimes::apply_patch::ApplyPatchRuntime; use crate::tools::runtimes::shell::ShellRequest; use crate::tools::runtimes::shell::ShellRuntime; use crate::tools::sandboxing::ToolCtx; @@ -210,81 +206,19 @@ impl ShellHandler { } // Intercept apply_patch if present. - match codex_apply_patch::maybe_parse_apply_patch_verified( + if let Some(output) = intercept_apply_patch( &exec_params.command, &exec_params.cwd, - ) { - codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => { - match apply_patch::apply_patch(session.as_ref(), turn.as_ref(), &call_id, changes) - .await - { - InternalApplyPatchInvocation::Output(item) => { - // Programmatic apply_patch path; return its result. - let content = item?; - return Ok(ToolOutput::Function { - content, - content_items: None, - success: Some(true), - }); - } - InternalApplyPatchInvocation::DelegateToExec(apply) => { - let emitter = ToolEmitter::apply_patch( - convert_apply_patch_to_protocol(&apply.action), - !apply.user_explicitly_approved_this_action, - ); - let event_ctx = ToolEventCtx::new( - session.as_ref(), - turn.as_ref(), - &call_id, - Some(&tracker), - ); - emitter.begin(event_ctx).await; - - let req = ApplyPatchRequest { - patch: apply.action.patch.clone(), - cwd: apply.action.cwd.clone(), - timeout_ms: exec_params.expiration.timeout_ms(), - user_explicitly_approved: apply.user_explicitly_approved_this_action, - codex_exe: turn.codex_linux_sandbox_exe.clone(), - }; - let mut orchestrator = ToolOrchestrator::new(); - let mut runtime = ApplyPatchRuntime::new(); - let tool_ctx = ToolCtx { - session: session.as_ref(), - turn: turn.as_ref(), - call_id: call_id.clone(), - tool_name: tool_name.to_string(), - }; - let out = orchestrator - .run(&mut runtime, &req, &tool_ctx, &turn, turn.approval_policy) - .await; - let event_ctx = ToolEventCtx::new( - session.as_ref(), - turn.as_ref(), - &call_id, - Some(&tracker), - ); - let content = emitter.finish(event_ctx, out).await?; - return Ok(ToolOutput::Function { - content, - content_items: None, - success: Some(true), - }); - } - } - } - codex_apply_patch::MaybeApplyPatchVerified::CorrectnessError(parse_error) => { - return Err(FunctionCallError::RespondToModel(format!( - "apply_patch verification failed: {parse_error}" - ))); - } - codex_apply_patch::MaybeApplyPatchVerified::ShellParseError(error) => { - tracing::trace!("Failed to parse shell command, {error:?}"); - // Fall through to regular shell execution. - } - codex_apply_patch::MaybeApplyPatchVerified::NotApplyPatch => { - // Fall through to regular shell execution. - } + exec_params.expiration.timeout_ms(), + session.as_ref(), + turn.as_ref(), + Some(&tracker), + &call_id, + tool_name, + ) + .await? + { + return Ok(output); } let source = ExecCommandSource::Agent; @@ -297,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(), @@ -304,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/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 0c4f0a031b67..4c943c6285e6 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -6,6 +6,7 @@ use crate::protocol::EventMsg; use crate::protocol::ExecCommandOutputDeltaEvent; use crate::protocol::ExecCommandSource; use crate::protocol::ExecOutputStream; +use crate::shell::default_user_shell; use crate::shell::get_shell_by_model_provided_path; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; @@ -13,6 +14,7 @@ use crate::tools::context::ToolPayload; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; use crate::tools::events::ToolEventStage; +use crate::tools::handlers::apply_patch::intercept_apply_patch; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use crate::unified_exec::ExecCommandRequest; @@ -30,8 +32,8 @@ struct ExecCommandArgs { cmd: String, #[serde(default)] workdir: Option, - #[serde(default = "default_shell")] - shell: String, + #[serde(default)] + shell: Option, #[serde(default = "default_login")] login: bool, #[serde(default = "default_exec_yield_time_ms")] @@ -64,10 +66,6 @@ fn default_write_stdin_yield_time_ms() -> u64 { 250 } -fn default_shell() -> String { - "/bin/bash".to_string() -} - fn default_login() -> bool { true } @@ -103,6 +101,7 @@ impl ToolHandler for UnifiedExecHandler { let ToolInvocation { session, turn, + tracker, call_id, tool_name, payload, @@ -153,12 +152,26 @@ impl ToolHandler for UnifiedExecHandler { ))); } - let workdir = workdir - .as_deref() - .filter(|value| !value.is_empty()) - .map(PathBuf::from); + let workdir = workdir.filter(|value| !value.is_empty()); + + let workdir = workdir.map(|dir| context.turn.resolve_path(Some(dir))); let cwd = workdir.clone().unwrap_or_else(|| context.turn.cwd.clone()); + if let Some(output) = intercept_apply_patch( + &command, + &cwd, + Some(yield_time_ms), + context.session.as_ref(), + context.turn.as_ref(), + Some(&tracker), + &context.call_id, + tool_name.as_str(), + ) + .await? + { + return Ok(output); + } + let event_ctx = ToolEventCtx::new( context.session.as_ref(), context.turn.as_ref(), @@ -241,7 +254,12 @@ impl ToolHandler for UnifiedExecHandler { } fn get_command(args: &ExecCommandArgs) -> Vec { - let shell = get_shell_by_model_provided_path(&PathBuf::from(args.shell.clone())); + let shell = if let Some(shell_str) = &args.shell { + get_shell_by_model_provided_path(&PathBuf::from(shell_str)) + } else { + default_user_shell() + }; + shell.derive_exec_args(&args.cmd, args.login) } @@ -273,3 +291,65 @@ fn format_response(response: &UnifiedExecResponse) -> String { sections.join("\n") } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_get_command_uses_default_shell_when_unspecified() { + let json = r#"{"cmd": "echo hello"}"#; + + let args: ExecCommandArgs = + serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + + assert!(args.shell.is_none()); + + let command = get_command(&args); + + assert_eq!(command.len(), 3); + assert_eq!(command[2], "echo hello"); + } + + #[test] + fn test_get_command_respects_explicit_bash_shell() { + let json = r#"{"cmd": "echo hello", "shell": "/bin/bash"}"#; + + let args: ExecCommandArgs = + serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + + assert_eq!(args.shell.as_deref(), Some("/bin/bash")); + + let command = get_command(&args); + + assert_eq!(command[2], "echo hello"); + } + + #[test] + fn test_get_command_respects_explicit_powershell_shell() { + let json = r#"{"cmd": "echo hello", "shell": "powershell"}"#; + + let args: ExecCommandArgs = + serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + + assert_eq!(args.shell.as_deref(), Some("powershell")); + + let command = get_command(&args); + + assert_eq!(command[2], "echo hello"); + } + + #[test] + fn test_get_command_respects_explicit_cmd_shell() { + let json = r#"{"cmd": "echo hello", "shell": "cmd"}"#; + + let args: ExecCommandArgs = + serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + + assert_eq!(args.shell.as_deref(), Some("cmd")); + + let command = get_command(&args); + + assert_eq!(command[2], "echo hello"); + } +} diff --git a/codex-rs/core/src/unified_exec/session_manager.rs b/codex-rs/core/src/unified_exec/session_manager.rs index 37a12bf2df5c..72c02cdb9994 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(), diff --git a/codex-rs/core/src/util.rs b/codex-rs/core/src/util.rs index 0bce5b443957..5304a89ac9f7 100644 --- a/codex-rs/core/src/util.rs +++ b/codex-rs/core/src/util.rs @@ -1,3 +1,5 @@ +use std::path::Path; +use std::path::PathBuf; use std::time::Duration; use rand::Rng; @@ -14,11 +16,11 @@ pub(crate) fn backoff(attempt: u64) -> Duration { Duration::from_millis((base as f64 * jitter) as u64) } -pub(crate) fn error_or_panic(message: String) { +pub(crate) fn error_or_panic(message: impl std::string::ToString) { if cfg!(debug_assertions) || env!("CARGO_PKG_VERSION").contains("alpha") { - panic!("{message}"); + panic!("{}", message.to_string()); } else { - error!("{message}"); + error!("{}", message.to_string()); } } @@ -37,6 +39,14 @@ pub(crate) fn try_parse_error_message(text: &str) -> String { text.to_string() } +pub fn resolve_path(base: &Path, path: &PathBuf) -> PathBuf { + if path.is_absolute() { + path.clone() + } else { + base.join(path) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index 932ddb51a3c3..a8209b5139b6 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -518,6 +518,32 @@ pub fn sse_response(body: String) -> ResponseTemplate { .set_body_raw(body, "text/event-stream") } +pub async fn mount_response_once(server: &MockServer, response: ResponseTemplate) -> ResponseMock { + let (mock, response_mock) = base_mock(); + mock.respond_with(response) + .up_to_n_times(1) + .mount(server) + .await; + response_mock +} + +pub async fn mount_response_once_match( + server: &MockServer, + matcher: M, + response: ResponseTemplate, +) -> ResponseMock +where + M: wiremock::Match + Send + Sync + 'static, +{ + let (mock, response_mock) = base_mock(); + mock.and(matcher) + .respond_with(response) + .up_to_n_times(1) + .mount(server) + .await; + response_mock +} + fn base_mock() -> (MockBuilder, ResponseMock) { let response_mock = ResponseMock::new(); let mock = Mock::given(method("POST")) diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index 63964b241597..dc7bdb6b1087 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -1,5 +1,7 @@ #![cfg(not(target_os = "windows"))] use std::collections::HashMap; +use std::ffi::OsStr; +use std::fs; use std::sync::OnceLock; use anyhow::Context; @@ -23,6 +25,7 @@ use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; use core_test_support::skip_if_sandbox; use core_test_support::test_codex::TestCodex; +use core_test_support::test_codex::TestCodexHarness; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use core_test_support::wait_for_event_match; @@ -148,6 +151,130 @@ fn collect_tool_outputs(bodies: &[Value]) -> Result Result<()> { + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + + let builder = test_codex().with_config(|config| { + config.include_apply_patch_tool = true; + config.use_experimental_unified_exec_tool = true; + config.features.enable(Feature::UnifiedExec); + }); + let harness = TestCodexHarness::with_builder(builder).await?; + + let patch = + "*** Begin Patch\n*** Add File: uexec_apply.txt\n+hello from unified exec\n*** End Patch"; + let command = format!("apply_patch <<'EOF'\n{patch}\nEOF\n"); + let call_id = "uexec-apply-patch"; + let args = json!({ + "cmd": command, + "yield_time_ms": 250, + }); + + let responses = vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ]; + mount_sse_sequence(harness.server(), responses).await; + + let test = harness.test(); + let codex = test.codex.clone(); + let cwd = test.cwd_path().to_path_buf(); + let session_model = test.session_configured.model.clone(); + + codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "apply patch via unified exec".into(), + }], + final_output_json_schema: None, + cwd, + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: session_model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + let mut saw_patch_begin = false; + let mut patch_end = None; + let mut saw_exec_begin = false; + let mut saw_exec_end = false; + wait_for_event(&codex, |event| match event { + EventMsg::PatchApplyBegin(begin) if begin.call_id == call_id => { + saw_patch_begin = true; + assert!( + begin + .changes + .keys() + .any(|path| path.file_name() == Some(OsStr::new("uexec_apply.txt"))), + "expected apply_patch changes to target uexec_apply.txt", + ); + false + } + EventMsg::PatchApplyEnd(end) if end.call_id == call_id => { + patch_end = Some(end.clone()); + false + } + EventMsg::ExecCommandBegin(event) if event.call_id == call_id => { + saw_exec_begin = true; + false + } + EventMsg::ExecCommandEnd(event) if event.call_id == call_id => { + saw_exec_end = true; + false + } + EventMsg::TaskComplete(_) => true, + _ => false, + }) + .await; + + assert!( + saw_patch_begin, + "expected apply_patch to emit PatchApplyBegin" + ); + let patch_end = patch_end.expect("expected apply_patch to emit PatchApplyEnd"); + assert!( + patch_end.success, + "expected apply_patch to finish successfully: stdout={:?} stderr={:?}", + patch_end.stdout, patch_end.stderr, + ); + assert!( + !saw_exec_begin, + "apply_patch should be intercepted before exec_command begin" + ); + assert!( + !saw_exec_end, + "apply_patch should not emit exec_command end events" + ); + + let output = harness.function_call_stdout(call_id).await; + assert!( + output.contains("Success. Updated the following files:"), + "expected apply_patch output, got: {output:?}" + ); + assert!( + output.contains("A uexec_apply.txt"), + "expected apply_patch file summary, got: {output:?}" + ); + assert_eq!( + fs::read_to_string(harness.path("uexec_apply.txt"))?, + "hello from unified exec\n" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn unified_exec_emits_exec_command_begin_event() -> Result<()> { skip_if_no_network!(Ok(())); @@ -168,6 +295,7 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> { let call_id = "uexec-begin-event"; let args = json!({ + "shell": "bash".to_string(), "cmd": "/bin/echo hello unified exec".to_string(), "yield_time_ms": 250, }); @@ -209,15 +337,85 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> { }) .await; + assert_command(&begin_event.command, "-lc", "/bin/echo hello unified exec"); + + assert_eq!(begin_event.cwd, cwd.path()); + + wait_for_event(&codex, |event| matches!(event, EventMsg::TaskComplete(_))).await; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn unified_exec_resolves_relative_workdir() -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + + let server = start_mock_server().await; + + let mut builder = test_codex().with_model("gpt-5").with_config(|config| { + config.use_experimental_unified_exec_tool = true; + config.features.enable(Feature::UnifiedExec); + }); + let TestCodex { + codex, + cwd, + session_configured, + .. + } = builder.build(&server).await?; + + let workdir_rel = std::path::PathBuf::from("uexec_relative_workdir"); + std::fs::create_dir_all(cwd.path().join(&workdir_rel))?; + + let call_id = "uexec-workdir-relative"; + let args = json!({ + "cmd": "pwd", + "yield_time_ms": 250, + "workdir": workdir_rel.to_string_lossy().to_string(), + }); + + let responses = vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "finished"), + ev_completed("resp-2"), + ]), + ]; + mount_sse_sequence(&server, responses).await; + + let session_model = session_configured.model.clone(); + + codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "run relative workdir test".into(), + }], + final_output_json_schema: None, + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: session_model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + let begin_event = wait_for_event_match(&codex, |msg| match msg { + EventMsg::ExecCommandBegin(event) if event.call_id == call_id => Some(event.clone()), + _ => None, + }) + .await; + assert_eq!( - begin_event.command, - vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "/bin/echo hello unified exec".to_string() - ] + begin_event.cwd, + cwd.path().join(workdir_rel), + "exec_command cwd should resolve relative workdir against turn cwd", ); - assert_eq!(begin_event.cwd, cwd.path()); wait_for_event(&codex, |event| matches!(event, EventMsg::TaskComplete(_))).await; @@ -579,6 +777,7 @@ async fn unified_exec_emits_begin_for_write_stdin() -> Result<()> { let open_call_id = "uexec-open-for-begin"; let open_args = json!({ + "shell": "bash".to_string(), "cmd": "bash -i".to_string(), "yield_time_ms": 200, }); @@ -640,14 +839,7 @@ async fn unified_exec_emits_begin_for_write_stdin() -> Result<()> { }) .await; - assert_eq!( - begin_event.command, - vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "bash -i".to_string() - ] - ); + assert_command(&begin_event.command, "-lc", "bash -i"); assert_eq!( begin_event.interaction_input, Some("echo hello".to_string()) @@ -681,6 +873,7 @@ async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()> let open_call_id = "uexec-open-session"; let open_args = json!({ + "shell": "bash".to_string(), "cmd": "bash -i".to_string(), "yield_time_ms": 250, }); @@ -756,14 +949,9 @@ async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()> .iter() .find(|ev| ev.call_id == open_call_id) .expect("missing exec_command begin"); - assert_eq!( - open_event.command, - vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "bash -i".to_string() - ] - ); + + assert_command(&open_event.command, "-lc", "bash -i"); + assert!( open_event.interaction_input.is_none(), "startup begin events should not include interaction input" @@ -774,14 +962,9 @@ async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()> .iter() .find(|ev| ev.call_id == poll_call_id) .expect("missing write_stdin begin"); - assert_eq!( - poll_event.command, - vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "bash -i".to_string() - ] - ); + + assert_command(&poll_event.command, "-lc", "bash -i"); + assert!( poll_event.interaction_input.is_none(), "poll begin events should omit interaction input" @@ -1918,3 +2101,17 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { Ok(()) } + +fn assert_command(command: &[String], expected_args: &str, expected_cmd: &str) { + assert_eq!(command.len(), 3); + let shell_path = &command[0]; + assert!( + shell_path == "/bin/bash" + || shell_path == "/usr/bin/bash" + || shell_path == "/usr/local/bin/bash" + || shell_path.ends_with("/bash"), + "unexpected bash path: {shell_path}" + ); + assert_eq!(command[1], expected_args); + assert_eq!(command[2], expected_cmd); +} diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 394c5c2f1497..6c0f6dcc80c0 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -474,3 +474,82 @@ async fn view_image_tool_errors_when_file_missing() -> anyhow::Result<()> { Ok(()) } + +#[cfg(not(debug_assertions))] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn replaces_invalid_local_image_after_bad_request() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + + const INVALID_IMAGE_ERROR: &str = + "The image data you provided does not represent a valid image"; + + let invalid_image_mock = responses::mount_response_once_match( + &server, + body_string_contains("\"input_image\""), + ResponseTemplate::new(400) + .insert_header("content-type", "text/plain") + .set_body_string(INVALID_IMAGE_ERROR), + ) + .await; + + let success_response = sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]); + + let completion_mock = responses::mount_sse_once(&server, success_response).await; + + let TestCodex { + codex, + cwd, + session_configured, + .. + } = test_codex().build(&server).await?; + + let rel_path = "assets/poisoned.png"; + let abs_path = cwd.path().join(rel_path); + if let Some(parent) = abs_path.parent() { + std::fs::create_dir_all(parent)?; + } + let image = ImageBuffer::from_pixel(1024, 512, Rgba([10u8, 20, 30, 255])); + image.save(&abs_path)?; + + let session_model = session_configured.model.clone(); + + codex + .submit(Op::UserTurn { + items: vec![UserInput::LocalImage { + path: abs_path.clone(), + }], + final_output_json_schema: None, + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: session_model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + wait_for_event(&codex, |event| matches!(event, EventMsg::TaskComplete(_))).await; + + let first_body = invalid_image_mock.single_request().body_json(); + assert!( + find_image_message(&first_body).is_some(), + "initial request should include the uploaded image" + ); + + let second_request = completion_mock.single_request(); + let second_body = second_request.body_json(); + assert!( + find_image_message(&second_body).is_none(), + "second request should replace the invalid image" + ); + let user_texts = second_request.message_input_texts("user"); + assert!(user_texts.iter().any(|text| text == "Invalid image")); + + Ok(()) +} diff --git a/codex-rs/exec-server/src/posix.rs b/codex-rs/exec-server/src/posix.rs index b4dd0fbf404c..3a4a4b952510 100644 --- a/codex-rs/exec-server/src/posix.rs +++ b/codex-rs/exec-server/src/posix.rs @@ -78,6 +78,7 @@ mod stopwatch; const CODEX_EXECVE_WRAPPER_EXE_NAME: &str = "codex-execve-wrapper"; #[derive(Parser)] +#[clap(version)] struct McpServerCli { /// Executable to delegate execve(2) calls to in Bash. #[arg(long = "execve")] diff --git a/codex-rs/execpolicy/Cargo.toml b/codex-rs/execpolicy/Cargo.toml index 77278bb1188f..890c23faa799 100644 --- a/codex-rs/execpolicy/Cargo.toml +++ b/codex-rs/execpolicy/Cargo.toml @@ -28,3 +28,4 @@ thiserror = { workspace = true } [dev-dependencies] pretty_assertions = { workspace = true } +tempfile = { workspace = true } diff --git a/codex-rs/execpolicy/src/amend.rs b/codex-rs/execpolicy/src/amend.rs new file mode 100644 index 000000000000..36a02cebf971 --- /dev/null +++ b/codex-rs/execpolicy/src/amend.rs @@ -0,0 +1,226 @@ +use std::fs::OpenOptions; +use std::io::Read; +use std::io::Seek; +use std::io::SeekFrom; +use std::io::Write; +use std::path::Path; +use std::path::PathBuf; + +use serde_json; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum AmendError { + #[error("prefix rule requires at least one token")] + EmptyPrefix, + #[error("policy path has no parent: {path}")] + MissingParent { path: PathBuf }, + #[error("failed to create policy directory {dir}: {source}")] + CreatePolicyDir { + dir: PathBuf, + source: std::io::Error, + }, + #[error("failed to format prefix tokens: {source}")] + SerializePrefix { source: serde_json::Error }, + #[error("failed to open policy file {path}: {source}")] + OpenPolicyFile { + path: PathBuf, + source: std::io::Error, + }, + #[error("failed to write to policy file {path}: {source}")] + WritePolicyFile { + path: PathBuf, + source: std::io::Error, + }, + #[error("failed to lock policy file {path}: {source}")] + LockPolicyFile { + path: PathBuf, + source: std::io::Error, + }, + #[error("failed to seek policy file {path}: {source}")] + SeekPolicyFile { + path: PathBuf, + source: std::io::Error, + }, + #[error("failed to read policy file {path}: {source}")] + ReadPolicyFile { + path: PathBuf, + source: std::io::Error, + }, + #[error("failed to read metadata for policy file {path}: {source}")] + PolicyMetadata { + path: PathBuf, + source: std::io::Error, + }, +} + +/// Note this thread uses advisory file locking and performs blocking I/O, so it should be used with +/// [`tokio::task::spawn_blocking`] when called from an async context. +pub fn blocking_append_allow_prefix_rule( + policy_path: &Path, + prefix: &[String], +) -> Result<(), AmendError> { + if prefix.is_empty() { + return Err(AmendError::EmptyPrefix); + } + + let tokens = prefix + .iter() + .map(serde_json::to_string) + .collect::, _>>() + .map_err(|source| AmendError::SerializePrefix { source })?; + let pattern = format!("[{}]", tokens.join(", ")); + let rule = format!(r#"prefix_rule(pattern={pattern}, decision="allow")"#); + + let dir = policy_path + .parent() + .ok_or_else(|| AmendError::MissingParent { + path: policy_path.to_path_buf(), + })?; + match std::fs::create_dir(dir) { + Ok(()) => {} + Err(ref source) if source.kind() == std::io::ErrorKind::AlreadyExists => {} + Err(source) => { + return Err(AmendError::CreatePolicyDir { + dir: dir.to_path_buf(), + source, + }); + } + } + append_locked_line(policy_path, &rule) +} + +fn append_locked_line(policy_path: &Path, line: &str) -> Result<(), AmendError> { + let mut file = OpenOptions::new() + .create(true) + .read(true) + .append(true) + .open(policy_path) + .map_err(|source| AmendError::OpenPolicyFile { + path: policy_path.to_path_buf(), + source, + })?; + file.lock().map_err(|source| AmendError::LockPolicyFile { + path: policy_path.to_path_buf(), + source, + })?; + + let len = file + .metadata() + .map_err(|source| AmendError::PolicyMetadata { + path: policy_path.to_path_buf(), + source, + })? + .len(); + + // Ensure file ends in a newline before appending. + if len > 0 { + file.seek(SeekFrom::End(-1)) + .map_err(|source| AmendError::SeekPolicyFile { + path: policy_path.to_path_buf(), + source, + })?; + let mut last = [0; 1]; + file.read_exact(&mut last) + .map_err(|source| AmendError::ReadPolicyFile { + path: policy_path.to_path_buf(), + source, + })?; + + if last[0] != b'\n' { + file.write_all(b"\n") + .map_err(|source| AmendError::WritePolicyFile { + path: policy_path.to_path_buf(), + source, + })?; + } + } + + file.write_all(format!("{line}\n").as_bytes()) + .map_err(|source| AmendError::WritePolicyFile { + path: policy_path.to_path_buf(), + source, + })?; + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use tempfile::tempdir; + + #[test] + fn appends_rule_and_creates_directories() { + let tmp = tempdir().expect("create temp dir"); + let policy_path = tmp.path().join("policy").join("default.codexpolicy"); + + blocking_append_allow_prefix_rule( + &policy_path, + &[String::from("echo"), String::from("Hello, world!")], + ) + .expect("append rule"); + + let contents = + std::fs::read_to_string(&policy_path).expect("default.codexpolicy should exist"); + assert_eq!( + contents, + r#"prefix_rule(pattern=["echo", "Hello, world!"], decision="allow") +"# + ); + } + + #[test] + fn appends_rule_without_duplicate_newline() { + let tmp = tempdir().expect("create temp dir"); + let policy_path = tmp.path().join("policy").join("default.codexpolicy"); + std::fs::create_dir_all(policy_path.parent().unwrap()).expect("create policy dir"); + std::fs::write( + &policy_path, + r#"prefix_rule(pattern=["ls"], decision="allow") +"#, + ) + .expect("write seed rule"); + + blocking_append_allow_prefix_rule( + &policy_path, + &[String::from("echo"), String::from("Hello, world!")], + ) + .expect("append rule"); + + let contents = std::fs::read_to_string(&policy_path).expect("read policy"); + assert_eq!( + contents, + r#"prefix_rule(pattern=["ls"], decision="allow") +prefix_rule(pattern=["echo", "Hello, world!"], decision="allow") +"# + ); + } + + #[test] + fn inserts_newline_when_missing_before_append() { + let tmp = tempdir().expect("create temp dir"); + let policy_path = tmp.path().join("policy").join("default.codexpolicy"); + std::fs::create_dir_all(policy_path.parent().unwrap()).expect("create policy dir"); + std::fs::write( + &policy_path, + r#"prefix_rule(pattern=["ls"], decision="allow")"#, + ) + .expect("write seed rule without newline"); + + blocking_append_allow_prefix_rule( + &policy_path, + &[String::from("echo"), String::from("Hello, world!")], + ) + .expect("append rule"); + + let contents = std::fs::read_to_string(&policy_path).expect("read policy"); + assert_eq!( + contents, + r#"prefix_rule(pattern=["ls"], decision="allow") +prefix_rule(pattern=["echo", "Hello, world!"], decision="allow") +"# + ); + } +} diff --git a/codex-rs/execpolicy/src/lib.rs b/codex-rs/execpolicy/src/lib.rs index b459caea1680..31062f1cb649 100644 --- a/codex-rs/execpolicy/src/lib.rs +++ b/codex-rs/execpolicy/src/lib.rs @@ -1,3 +1,4 @@ +pub mod amend; pub mod decision; pub mod error; pub mod execpolicycheck; @@ -5,6 +6,8 @@ pub mod parser; pub mod policy; pub mod rule; +pub use amend::AmendError; +pub use amend::blocking_append_allow_prefix_rule; pub use decision::Decision; pub use error::Error; pub use error::Result; diff --git a/codex-rs/execpolicy/src/policy.rs b/codex-rs/execpolicy/src/policy.rs index e048fce1ff2e..10858c9fadde 100644 --- a/codex-rs/execpolicy/src/policy.rs +++ b/codex-rs/execpolicy/src/policy.rs @@ -1,9 +1,15 @@ use crate::decision::Decision; +use crate::error::Error; +use crate::error::Result; +use crate::rule::PatternToken; +use crate::rule::PrefixPattern; +use crate::rule::PrefixRule; use crate::rule::RuleMatch; use crate::rule::RuleRef; use multimap::MultiMap; use serde::Deserialize; use serde::Serialize; +use std::sync::Arc; #[derive(Clone, Debug)] pub struct Policy { @@ -23,6 +29,27 @@ impl Policy { &self.rules_by_program } + pub fn add_prefix_rule(&mut self, prefix: &[String], decision: Decision) -> Result<()> { + let (first_token, rest) = prefix + .split_first() + .ok_or_else(|| Error::InvalidPattern("prefix cannot be empty".to_string()))?; + + let rule: RuleRef = Arc::new(PrefixRule { + pattern: PrefixPattern { + first: Arc::from(first_token.as_str()), + rest: rest + .iter() + .map(|token| PatternToken::Single(token.clone())) + .collect::>() + .into(), + }, + decision, + }); + + self.rules_by_program.insert(first_token.clone(), rule); + Ok(()) + } + pub fn check(&self, cmd: &[String]) -> Evaluation { let rules = match cmd.first() { Some(first) => match self.rules_by_program.get_vec(first) { diff --git a/codex-rs/execpolicy/tests/basic.rs b/codex-rs/execpolicy/tests/basic.rs index e4189caa21ff..9a7ec58b1e9e 100644 --- a/codex-rs/execpolicy/tests/basic.rs +++ b/codex-rs/execpolicy/tests/basic.rs @@ -1,8 +1,12 @@ use std::any::Any; use std::sync::Arc; +use anyhow::Context; +use anyhow::Result; use codex_execpolicy::Decision; +use codex_execpolicy::Error; use codex_execpolicy::Evaluation; +use codex_execpolicy::Policy; use codex_execpolicy::PolicyParser; use codex_execpolicy::RuleMatch; use codex_execpolicy::RuleRef; @@ -35,16 +39,14 @@ fn rule_snapshots(rules: &[RuleRef]) -> Vec { } #[test] -fn basic_match() { +fn basic_match() -> Result<()> { let policy_src = r#" prefix_rule( pattern = ["git", "status"], ) "#; let mut parser = PolicyParser::new(); - parser - .parse("test.codexpolicy", policy_src) - .expect("parse policy"); + parser.parse("test.codexpolicy", policy_src)?; let policy = parser.build(); let cmd = tokens(&["git", "status"]); let evaluation = policy.check(&cmd); @@ -58,10 +60,54 @@ prefix_rule( }, evaluation ); + Ok(()) } #[test] -fn parses_multiple_policy_files() { +fn add_prefix_rule_extends_policy() -> Result<()> { + let mut policy = Policy::empty(); + policy.add_prefix_rule(&tokens(&["ls", "-l"]), Decision::Prompt)?; + + let rules = rule_snapshots(policy.rules().get_vec("ls").context("missing ls rules")?); + assert_eq!( + vec![RuleSnapshot::Prefix(PrefixRule { + pattern: PrefixPattern { + first: Arc::from("ls"), + rest: vec![PatternToken::Single(String::from("-l"))].into(), + }, + decision: Decision::Prompt, + })], + rules + ); + + let evaluation = policy.check(&tokens(&["ls", "-l", "/tmp"])); + assert_eq!( + Evaluation::Match { + decision: Decision::Prompt, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: tokens(&["ls", "-l"]), + decision: Decision::Prompt, + }], + }, + evaluation + ); + Ok(()) +} + +#[test] +fn add_prefix_rule_rejects_empty_prefix() -> Result<()> { + let mut policy = Policy::empty(); + let result = policy.add_prefix_rule(&[], Decision::Allow); + + match result.unwrap_err() { + Error::InvalidPattern(message) => assert_eq!(message, "prefix cannot be empty"), + other => panic!("expected InvalidPattern(..), got {other:?}"), + } + Ok(()) +} + +#[test] +fn parses_multiple_policy_files() -> Result<()> { let first_policy = r#" prefix_rule( pattern = ["git"], @@ -75,15 +121,11 @@ prefix_rule( ) "#; let mut parser = PolicyParser::new(); - parser - .parse("first.codexpolicy", first_policy) - .expect("parse policy"); - parser - .parse("second.codexpolicy", second_policy) - .expect("parse policy"); + parser.parse("first.codexpolicy", first_policy)?; + parser.parse("second.codexpolicy", second_policy)?; let policy = parser.build(); - let git_rules = rule_snapshots(policy.rules().get_vec("git").expect("git rules")); + let git_rules = rule_snapshots(policy.rules().get_vec("git").context("missing git rules")?); assert_eq!( vec![ RuleSnapshot::Prefix(PrefixRule { @@ -133,23 +175,27 @@ prefix_rule( }, commit_eval ); + Ok(()) } #[test] -fn only_first_token_alias_expands_to_multiple_rules() { +fn only_first_token_alias_expands_to_multiple_rules() -> Result<()> { let policy_src = r#" prefix_rule( pattern = [["bash", "sh"], ["-c", "-l"]], ) "#; let mut parser = PolicyParser::new(); - parser - .parse("test.codexpolicy", policy_src) - .expect("parse policy"); + parser.parse("test.codexpolicy", policy_src)?; let policy = parser.build(); - let bash_rules = rule_snapshots(policy.rules().get_vec("bash").expect("bash rules")); - let sh_rules = rule_snapshots(policy.rules().get_vec("sh").expect("sh rules")); + let bash_rules = rule_snapshots( + policy + .rules() + .get_vec("bash") + .context("missing bash rules")?, + ); + let sh_rules = rule_snapshots(policy.rules().get_vec("sh").context("missing sh rules")?); assert_eq!( vec![RuleSnapshot::Prefix(PrefixRule { pattern: PrefixPattern { @@ -194,22 +240,21 @@ prefix_rule( }, sh_eval ); + Ok(()) } #[test] -fn tail_aliases_are_not_cartesian_expanded() { +fn tail_aliases_are_not_cartesian_expanded() -> Result<()> { let policy_src = r#" prefix_rule( pattern = ["npm", ["i", "install"], ["--legacy-peer-deps", "--no-save"]], ) "#; let mut parser = PolicyParser::new(); - parser - .parse("test.codexpolicy", policy_src) - .expect("parse policy"); + parser.parse("test.codexpolicy", policy_src)?; let policy = parser.build(); - let rules = rule_snapshots(policy.rules().get_vec("npm").expect("npm rules")); + let rules = rule_snapshots(policy.rules().get_vec("npm").context("missing npm rules")?); assert_eq!( vec![RuleSnapshot::Prefix(PrefixRule { pattern: PrefixPattern { @@ -251,10 +296,11 @@ prefix_rule( }, npm_install ); + Ok(()) } #[test] -fn match_and_not_match_examples_are_enforced() { +fn match_and_not_match_examples_are_enforced() -> Result<()> { let policy_src = r#" prefix_rule( pattern = ["git", "status"], @@ -266,9 +312,7 @@ prefix_rule( ) "#; let mut parser = PolicyParser::new(); - parser - .parse("test.codexpolicy", policy_src) - .expect("parse policy"); + parser.parse("test.codexpolicy", policy_src)?; let policy = parser.build(); let match_eval = policy.check(&tokens(&["git", "status"])); assert_eq!( @@ -289,10 +333,11 @@ prefix_rule( "status", ])); assert_eq!(Evaluation::NoMatch {}, no_match_eval); + Ok(()) } #[test] -fn strictest_decision_wins_across_matches() { +fn strictest_decision_wins_across_matches() -> Result<()> { let policy_src = r#" prefix_rule( pattern = ["git"], @@ -304,9 +349,7 @@ prefix_rule( ) "#; let mut parser = PolicyParser::new(); - parser - .parse("test.codexpolicy", policy_src) - .expect("parse policy"); + parser.parse("test.codexpolicy", policy_src)?; let policy = parser.build(); let commit = policy.check(&tokens(&["git", "commit", "-m", "hi"])); @@ -326,10 +369,11 @@ prefix_rule( }, commit ); + Ok(()) } #[test] -fn strictest_decision_across_multiple_commands() { +fn strictest_decision_across_multiple_commands() -> Result<()> { let policy_src = r#" prefix_rule( pattern = ["git"], @@ -341,9 +385,7 @@ prefix_rule( ) "#; let mut parser = PolicyParser::new(); - parser - .parse("test.codexpolicy", policy_src) - .expect("parse policy"); + parser.parse("test.codexpolicy", policy_src)?; let policy = parser.build(); let commands = vec![ @@ -372,4 +414,5 @@ prefix_rule( }, evaluation ); + Ok(()) } diff --git a/codex-rs/login/src/device_code_auth.rs b/codex-rs/login/src/device_code_auth.rs index acaf30ba0ca9..a121de7ebd4e 100644 --- a/codex-rs/login/src/device_code_auth.rs +++ b/codex-rs/login/src/device_code_auth.rs @@ -8,11 +8,10 @@ use std::time::Instant; use crate::pkce::PkceCodes; use crate::server::ServerOptions; -use std::io::Write; -use std::io::{self}; +use std::io; -const ANSI_YELLOW: &str = "\x1b[93m"; -const ANSI_BOLD: &str = "\x1b[1m"; +const ANSI_BLUE: &str = "\x1b[94m"; +const ANSI_GRAY: &str = "\x1b[90m"; const ANSI_RESET: &str = "\x1b[0m"; #[derive(Deserialize)] @@ -138,14 +137,16 @@ async fn poll_for_token( } } -fn print_colored_warning_device_code() { - let mut stdout = io::stdout().lock(); - let _ = write!( - stdout, - "{ANSI_YELLOW}{ANSI_BOLD}Only use device code authentication when browser login is not available.{ANSI_RESET}{ANSI_YELLOW}\n\ -{ANSI_BOLD}Keep the code secret; do not share it.{ANSI_RESET}{ANSI_RESET}\n\n" +fn print_device_code_prompt(code: &str) { + println!( + "\nWelcome to Codex [v{ANSI_GRAY}{version}{ANSI_RESET}]\n{ANSI_GRAY}OpenAI's command-line coding agent{ANSI_RESET}\n\ +\nFollow these steps to sign in with ChatGPT using device code authorization:\n\ +\n1. Open this link in your browser\n {ANSI_BLUE}https://auth.openai.com/codex/device{ANSI_RESET}\n\ +\n2. Enter this one-time code {ANSI_GRAY}(expires in 15 minutes){ANSI_RESET}\n {ANSI_BLUE}{code}{ANSI_RESET}\n\ +\n{ANSI_GRAY}Device codes are a common phishing target. Never share this code.{ANSI_RESET}\n", + version = env!("CARGO_PKG_VERSION"), + code = code ); - let _ = stdout.flush(); } /// Full device code login flow. @@ -153,13 +154,9 @@ pub async fn run_device_code_login(opts: ServerOptions) -> std::io::Result<()> { let client = reqwest::Client::new(); let base_url = opts.issuer.trim_end_matches('/'); let api_base_url = format!("{}/api/accounts", opts.issuer.trim_end_matches('/')); - print_colored_warning_device_code(); let uc = request_user_code(&client, &api_base_url, &opts.client_id).await?; - println!( - "To authenticate:\n 1. Open in your browser: {ANSI_BOLD}https://auth.openai.com/codex/device{ANSI_RESET}\n 2. Enter the one-time code below within 15 minutes:\n\n {ANSI_BOLD}{}{ANSI_RESET}\n", - uc.user_code - ); + print_device_code_prompt(&uc.user_code); let code_resp = poll_for_token( &client, diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index d9906b2f01c6..be4f5aead70b 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -91,6 +91,7 @@ unicode-width = { workspace = true } url = { workspace = true } codex-windows-sandbox = { workspace = true } +tokio-util = { workspace = true, features = ["time"] } [target.'cfg(unix)'.dependencies] libc = { workspace = true } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 6b42e5134c93..f78e544ea37b 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -101,6 +101,7 @@ pub(crate) struct ChatComposer { dismissed_file_popup_token: Option, current_file_query: Option, pending_pastes: Vec<(String, String)>, + large_paste_counters: HashMap, has_focus: bool, attached_images: Vec, placeholder_text: String, @@ -113,6 +114,7 @@ pub(crate) struct ChatComposer { footer_mode: FooterMode, footer_hint_override: Option>, context_window_percent: Option, + context_window_used_tokens: Option, } /// Popup state – at most one can be visible at any time. @@ -146,6 +148,7 @@ impl ChatComposer { dismissed_file_popup_token: None, current_file_query: None, pending_pastes: Vec::new(), + large_paste_counters: HashMap::new(), has_focus: has_input_focus, attached_images: Vec::new(), placeholder_text, @@ -156,6 +159,7 @@ impl ChatComposer { footer_mode: FooterMode::ShortcutSummary, footer_hint_override: None, context_window_percent: None, + context_window_used_tokens: None, }; // Apply configuration via the setter to keep side-effects centralized. this.set_disable_paste_burst(disable_paste_burst); @@ -220,7 +224,7 @@ impl ChatComposer { pub fn handle_paste(&mut self, pasted: String) -> bool { let char_count = pasted.chars().count(); if char_count > LARGE_PASTE_CHAR_THRESHOLD { - let placeholder = format!("[Pasted Content {char_count} chars]"); + let placeholder = self.next_large_paste_placeholder(char_count); self.textarea.insert_element(&placeholder); self.pending_pastes.push((placeholder, pasted)); } else if char_count > 1 && self.handle_paste_image_path(pasted.clone()) { @@ -360,6 +364,17 @@ impl ChatComposer { self.set_has_focus(has_focus); } + fn next_large_paste_placeholder(&mut self, char_count: usize) -> String { + let base = format!("[Pasted Content {char_count} chars]"); + let next_suffix = self.large_paste_counters.entry(char_count).or_insert(0); + *next_suffix += 1; + if *next_suffix == 1 { + base + } else { + format!("{base} #{next_suffix}") + } + } + pub(crate) fn insert_str(&mut self, text: &str) { self.textarea.insert_str(text); self.sync_command_popup(); @@ -1387,6 +1402,7 @@ impl ChatComposer { use_shift_enter_hint: self.use_shift_enter_hint, is_task_running: self.is_task_running, context_window_percent: self.context_window_percent, + context_window_used_tokens: self.context_window_used_tokens, } } @@ -1517,10 +1533,13 @@ impl ChatComposer { self.is_task_running = running; } - pub(crate) fn set_context_window_percent(&mut self, percent: Option) { - if self.context_window_percent != percent { - self.context_window_percent = percent; + pub(crate) fn set_context_window(&mut self, percent: Option, used_tokens: Option) { + if self.context_window_percent == percent && self.context_window_used_tokens == used_tokens + { + return; } + self.context_window_percent = percent; + self.context_window_used_tokens = used_tokens; } pub(crate) fn set_esc_backtrack_hint(&mut self, show: bool) { @@ -2665,6 +2684,83 @@ mod tests { ); } + #[test] + fn deleting_duplicate_length_pastes_removes_only_target() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + let paste = "x".repeat(LARGE_PASTE_CHAR_THRESHOLD + 4); + let placeholder_base = format!("[Pasted Content {} chars]", paste.chars().count()); + let placeholder_second = format!("{placeholder_base} #2"); + + composer.handle_paste(paste.clone()); + composer.handle_paste(paste.clone()); + assert_eq!( + composer.textarea.text(), + format!("{placeholder_base}{placeholder_second}") + ); + assert_eq!(composer.pending_pastes.len(), 2); + + composer.textarea.set_cursor(composer.textarea.text().len()); + composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE)); + + assert_eq!(composer.textarea.text(), placeholder_base); + assert_eq!(composer.pending_pastes.len(), 1); + assert_eq!(composer.pending_pastes[0].0, placeholder_base); + assert_eq!(composer.pending_pastes[0].1, paste); + } + + #[test] + fn large_paste_numbering_does_not_reuse_after_deletion() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + let paste = "x".repeat(LARGE_PASTE_CHAR_THRESHOLD + 4); + let base = format!("[Pasted Content {} chars]", paste.chars().count()); + let second = format!("{base} #2"); + let third = format!("{base} #3"); + + composer.handle_paste(paste.clone()); + composer.handle_paste(paste.clone()); + assert_eq!(composer.textarea.text(), format!("{base}{second}")); + + composer.textarea.set_cursor(base.len()); + composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE)); + assert_eq!(composer.textarea.text(), second); + assert_eq!(composer.pending_pastes.len(), 1); + assert_eq!(composer.pending_pastes[0].0, second); + + composer.textarea.set_cursor(composer.textarea.text().len()); + composer.handle_paste(paste); + + assert_eq!(composer.textarea.text(), format!("{second}{third}")); + assert_eq!(composer.pending_pastes.len(), 2); + assert_eq!(composer.pending_pastes[0].0, second); + assert_eq!(composer.pending_pastes[1].0, third); + } + #[test] fn test_partial_placeholder_deletion() { use crossterm::event::KeyCode; diff --git a/codex-rs/tui/src/bottom_pane/footer.rs b/codex-rs/tui/src/bottom_pane/footer.rs index 79d7c60fa708..11a97c783b36 100644 --- a/codex-rs/tui/src/bottom_pane/footer.rs +++ b/codex-rs/tui/src/bottom_pane/footer.rs @@ -1,6 +1,7 @@ use crate::key_hint; use crate::key_hint::KeyBinding; use crate::render::line_utils::prefix_lines; +use crate::status::format_tokens_compact; use crate::ui_consts::FOOTER_INDENT_COLS; use crossterm::event::KeyCode; use ratatui::buffer::Buffer; @@ -18,6 +19,7 @@ pub(crate) struct FooterProps { pub(crate) use_shift_enter_hint: bool, pub(crate) is_task_running: bool, pub(crate) context_window_percent: Option, + pub(crate) context_window_used_tokens: Option, } #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -81,7 +83,10 @@ fn footer_lines(props: FooterProps) -> Vec> { is_task_running: props.is_task_running, })], FooterMode::ShortcutSummary => { - let mut line = context_window_line(props.context_window_percent); + let mut line = context_window_line( + props.context_window_percent, + props.context_window_used_tokens, + ); line.push_span(" · ".dim()); line.extend(vec![ key_hint::plain(KeyCode::Char('?')).into(), @@ -94,7 +99,10 @@ fn footer_lines(props: FooterProps) -> Vec> { esc_backtrack_hint: props.esc_backtrack_hint, }), FooterMode::EscHint => vec![esc_hint_line(props.esc_backtrack_hint)], - FooterMode::ContextOnly => vec![context_window_line(props.context_window_percent)], + FooterMode::ContextOnly => vec![context_window_line( + props.context_window_percent, + props.context_window_used_tokens, + )], } } @@ -221,9 +229,18 @@ fn build_columns(entries: Vec>) -> Vec> { .collect() } -fn context_window_line(percent: Option) -> Line<'static> { - let percent = percent.unwrap_or(100).clamp(0, 100); - Line::from(vec![Span::from(format!("{percent}% context left")).dim()]) +fn context_window_line(percent: Option, used_tokens: Option) -> Line<'static> { + if let Some(percent) = percent { + let percent = percent.clamp(0, 100); + return Line::from(vec![Span::from(format!("{percent}% context left")).dim()]); + } + + if let Some(tokens) = used_tokens { + let used_fmt = format_tokens_compact(tokens); + return Line::from(vec![Span::from(format!("{used_fmt} used")).dim()]); + } + + Line::from(vec![Span::from("100% context left").dim()]) } #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -400,6 +417,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, context_window_percent: None, + context_window_used_tokens: None, }, ); @@ -411,6 +429,7 @@ mod tests { use_shift_enter_hint: true, is_task_running: false, context_window_percent: None, + context_window_used_tokens: None, }, ); @@ -422,6 +441,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, context_window_percent: None, + context_window_used_tokens: None, }, ); @@ -433,6 +453,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: true, context_window_percent: None, + context_window_used_tokens: None, }, ); @@ -444,6 +465,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, context_window_percent: None, + context_window_used_tokens: None, }, ); @@ -455,6 +477,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, context_window_percent: None, + context_window_used_tokens: None, }, ); @@ -466,6 +489,19 @@ mod tests { use_shift_enter_hint: false, is_task_running: true, context_window_percent: Some(72), + context_window_used_tokens: None, + }, + ); + + snapshot_footer( + "footer_context_tokens_used", + FooterProps { + mode: FooterMode::ShortcutSummary, + esc_backtrack_hint: false, + use_shift_enter_hint: false, + is_task_running: false, + context_window_percent: None, + context_window_used_tokens: Some(123_456), }, ); } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 5dbfb210b24d..b4255fd97914 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -76,6 +76,7 @@ pub(crate) struct BottomPane { /// Queued user messages to show above the composer while a turn is running. queued_user_messages: QueuedUserMessages, context_window_percent: Option, + context_window_used_tokens: Option, } pub(crate) struct BottomPaneParams { @@ -118,6 +119,7 @@ impl BottomPane { esc_backtrack_hint: false, animations_enabled, context_window_percent: None, + context_window_used_tokens: None, } } @@ -130,6 +132,11 @@ impl BottomPane { self.context_window_percent } + #[cfg(test)] + pub(crate) fn context_window_used_tokens(&self) -> Option { + self.context_window_used_tokens + } + fn active_view(&self) -> Option<&dyn BottomPaneView> { self.view_stack.last().map(std::convert::AsRef::as_ref) } @@ -344,13 +351,16 @@ impl BottomPane { } } - pub(crate) fn set_context_window_percent(&mut self, percent: Option) { - if self.context_window_percent == percent { + pub(crate) fn set_context_window(&mut self, percent: Option, used_tokens: Option) { + if self.context_window_percent == percent && self.context_window_used_tokens == used_tokens + { return; } self.context_window_percent = percent; - self.composer.set_context_window_percent(percent); + self.context_window_used_tokens = used_tokens; + self.composer + .set_context_window(percent, self.context_window_used_tokens); self.request_redraw(); } diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_tokens_used.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_tokens_used.snap new file mode 100644 index 000000000000..a77ca5565b68 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_context_tokens_used.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/bottom_pane/footer.rs +expression: terminal.backend() +--- +" 123K used · ? for shortcuts " diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 9ff5139a04a5..68721784459d 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; @@ -515,7 +516,7 @@ impl ChatWidget { match info { Some(info) => self.apply_token_info(info), None => { - self.bottom_pane.set_context_window_percent(None); + self.bottom_pane.set_context_window(None, None); self.token_info = None; } } @@ -523,7 +524,8 @@ impl ChatWidget { fn apply_token_info(&mut self, info: TokenUsageInfo) { let percent = self.context_remaining_percent(&info); - self.bottom_pane.set_context_window_percent(percent); + let used_tokens = self.context_used_tokens(&info, percent.is_some()); + self.bottom_pane.set_context_window(percent, used_tokens); self.token_info = Some(info); } @@ -536,12 +538,20 @@ impl ChatWidget { }) } + fn context_used_tokens(&self, info: &TokenUsageInfo, percent_known: bool) -> Option { + if percent_known { + return None; + } + + Some(info.total_token_usage.tokens_in_context_window()) + } + fn restore_pre_review_token_info(&mut self) { if let Some(saved) = self.pre_review_token_info.take() { match saved { Some(info) => self.apply_token_info(info), None => { - self.bottom_pane.set_context_window_percent(None); + self.bottom_pane.set_context_window(None, None); self.token_info = None; } } @@ -549,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 @@ -1361,7 +1383,9 @@ impl ChatWidget { modifiers, kind: KeyEventKind::Press, .. - } if modifiers.contains(KeyModifiers::CONTROL) && c.eq_ignore_ascii_case(&'v') => { + } if modifiers.intersects(KeyModifiers::CONTROL | KeyModifiers::ALT) + && c.eq_ignore_ascii_case(&'v') => + { match paste_image_to_temp_png() { Ok((path, info)) => { self.attach_image( diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 53910916c12b..890e8bbe1d20 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; @@ -297,6 +298,41 @@ fn token_count_none_resets_context_indicator() { assert_eq!(chat.bottom_pane.context_window_percent(), None); } +#[test] +fn context_indicator_shows_used_tokens_when_window_unknown() { + let (mut chat, _rx, _ops) = make_chatwidget_manual(); + + chat.config.model_context_window = None; + let auto_compact_limit = 200_000; + chat.config.model_auto_compact_token_limit = Some(auto_compact_limit); + + // No model window, so the indicator should fall back to showing tokens used. + let total_tokens = 106_000; + let token_usage = TokenUsage { + total_tokens, + ..TokenUsage::default() + }; + let token_info = TokenUsageInfo { + total_token_usage: token_usage.clone(), + last_token_usage: token_usage, + model_context_window: None, + }; + + chat.handle_codex_event(Event { + id: "token-usage".into(), + msg: EventMsg::TokenCount(TokenCountEvent { + info: Some(token_info), + rate_limits: None, + }), + }); + + assert_eq!(chat.bottom_pane.context_window_percent(), None); + assert_eq!( + chat.bottom_pane.context_window_used_tokens(), + Some(total_tokens) + ); +} + #[cfg_attr( target_os = "macos", ignore = "system configuration APIs are blocked under macOS seatbelt" @@ -486,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(); diff --git a/codex-rs/tui/src/resume_picker.rs b/codex-rs/tui/src/resume_picker.rs index 6a21496d3483..1cc9624ec31e 100644 --- a/codex-rs/tui/src/resume_picker.rs +++ b/codex-rs/tui/src/resume_picker.rs @@ -1075,7 +1075,6 @@ mod tests { ConversationItem { path: PathBuf::from(path), head: head_with_ts_and_user_text(ts, &[preview]), - tail: Vec::new(), created_at: Some(ts.to_string()), updated_at: Some(ts.to_string()), } @@ -1150,14 +1149,12 @@ mod tests { let a = ConversationItem { path: PathBuf::from("/tmp/a.jsonl"), head: head_with_ts_and_user_text("2025-01-01T00:00:00Z", &["A"]), - tail: Vec::new(), created_at: Some("2025-01-01T00:00:00Z".into()), updated_at: Some("2025-01-01T00:00:00Z".into()), }; let b = ConversationItem { path: PathBuf::from("/tmp/b.jsonl"), head: head_with_ts_and_user_text("2025-01-02T00:00:00Z", &["B"]), - tail: Vec::new(), created_at: Some("2025-01-02T00:00:00Z".into()), updated_at: Some("2025-01-02T00:00:00Z".into()), }; @@ -1171,21 +1168,9 @@ mod tests { #[test] fn row_uses_tail_timestamp_for_updated_at() { let head = head_with_ts_and_user_text("2025-01-01T00:00:00Z", &["Hello"]); - let tail = vec![json!({ - "timestamp": "2025-01-01T01:00:00Z", - "type": "message", - "role": "assistant", - "content": [ - { - "type": "output_text", - "text": "hi", - } - ], - })]; let item = ConversationItem { path: PathBuf::from("/tmp/a.jsonl"), head, - tail, created_at: Some("2025-01-01T00:00:00Z".into()), updated_at: Some("2025-01-01T01:00:00Z".into()), }; diff --git a/codex-rs/tui/src/status/mod.rs b/codex-rs/tui/src/status/mod.rs index eccb6b72b5a7..89c8daefdcf0 100644 --- a/codex-rs/tui/src/status/mod.rs +++ b/codex-rs/tui/src/status/mod.rs @@ -5,6 +5,7 @@ mod helpers; mod rate_limits; pub(crate) use card::new_status_output; +pub(crate) use helpers::format_tokens_compact; pub(crate) use rate_limits::RateLimitSnapshotDisplay; pub(crate) use rate_limits::rate_limit_snapshot_display; diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 7cbf252e7fc2..5502b8335691 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -9,8 +9,6 @@ use std::pin::Pin; use std::sync::Arc; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; -use std::time::Duration; -use std::time::Instant; use crossterm::Command; use crossterm::SynchronizedUpdate; @@ -32,10 +30,13 @@ use ratatui::crossterm::execute; use ratatui::crossterm::terminal::disable_raw_mode; use ratatui::crossterm::terminal::enable_raw_mode; use ratatui::layout::Offset; +use ratatui::layout::Rect; use ratatui::text::Line; use tokio::select; +use tokio::sync::broadcast; use tokio_stream::Stream; +pub use self::frame_requester::FrameRequester; use crate::custom_terminal; use crate::custom_terminal::Terminal as CustomTerminal; #[cfg(unix)] @@ -43,6 +44,7 @@ use crate::tui::job_control::SUSPEND_KEY; #[cfg(unix)] use crate::tui::job_control::SuspendContext; +mod frame_requester; #[cfg(unix)] mod job_control; @@ -159,8 +161,8 @@ pub enum TuiEvent { } pub struct Tui { - frame_schedule_tx: tokio::sync::mpsc::UnboundedSender, - draw_tx: tokio::sync::broadcast::Sender<()>, + frame_requester: FrameRequester, + draw_tx: broadcast::Sender<()>, pub(crate) terminal: Terminal, pending_history_lines: Vec>, alt_saved_viewport: Option, @@ -173,36 +175,10 @@ pub struct Tui { enhanced_keys_supported: bool, } -#[derive(Clone, Debug)] -pub struct FrameRequester { - frame_schedule_tx: tokio::sync::mpsc::UnboundedSender, -} -impl FrameRequester { - pub fn schedule_frame(&self) { - let _ = self.frame_schedule_tx.send(Instant::now()); - } - pub fn schedule_frame_in(&self, dur: Duration) { - let _ = self.frame_schedule_tx.send(Instant::now() + dur); - } -} - -#[cfg(test)] -impl FrameRequester { - /// Create a no-op frame requester for tests. - pub(crate) fn test_dummy() -> Self { - let (tx, _rx) = tokio::sync::mpsc::unbounded_channel(); - FrameRequester { - frame_schedule_tx: tx, - } - } -} - impl Tui { pub fn new(terminal: Terminal) -> Self { - let (frame_schedule_tx, frame_schedule_rx) = tokio::sync::mpsc::unbounded_channel(); - let (draw_tx, _) = tokio::sync::broadcast::channel(1); - - spawn_frame_scheduler(frame_schedule_rx, draw_tx.clone()); + let (draw_tx, _) = broadcast::channel(1); + let frame_requester = FrameRequester::new(draw_tx.clone()); // Detect keyboard enhancement support before any EventStream is created so the // crossterm poller can acquire its lock without contention. @@ -212,7 +188,7 @@ impl Tui { let _ = crate::terminal_palette::default_colors(); Self { - frame_schedule_tx, + frame_requester, draw_tx, terminal, pending_history_lines: vec![], @@ -226,9 +202,7 @@ impl Tui { } pub fn frame_requester(&self) -> FrameRequester { - FrameRequester { - frame_schedule_tx: self.frame_schedule_tx.clone(), - } + self.frame_requester.clone() } pub fn enhanced_keys_supported(&self) -> bool { @@ -362,34 +336,14 @@ impl Tui { // Precompute any viewport updates that need a cursor-position query before entering // the synchronized update, to avoid racing with the event reader. - let mut pending_viewport_area: Option = None; - { - let terminal = &mut self.terminal; - let screen_size = terminal.size()?; - let last_known_screen_size = terminal.last_known_screen_size; - if screen_size != last_known_screen_size - && let Ok(cursor_pos) = terminal.get_cursor_position() - { - let last_known_cursor_pos = terminal.last_known_cursor_pos; - // If we resized AND the cursor moved, we adjust the viewport area to keep the - // cursor in the same position. This is a heuristic that seems to work well - // at least in iTerm2. - if cursor_pos.y != last_known_cursor_pos.y { - let cursor_delta = cursor_pos.y as i32 - last_known_cursor_pos.y as i32; - let new_viewport_area = terminal.viewport_area.offset(Offset { - x: 0, - y: cursor_delta, - }); - pending_viewport_area = Some(new_viewport_area); - } - } - } + let mut pending_viewport_area = self.pending_viewport_area()?; stdout().sync_update(|_| { #[cfg(unix)] if let Some(prepared) = prepared_resume.take() { prepared.apply(&mut self.terminal)?; } + let terminal = &mut self.terminal; if let Some(new_area) = pending_viewport_area.take() { terminal.set_viewport_area(new_area); @@ -440,51 +394,28 @@ impl Tui { }) })? } -} -/// Spawn background scheduler to coalesce frame requests and emit draws at deadlines. -fn spawn_frame_scheduler( - frame_schedule_rx: tokio::sync::mpsc::UnboundedReceiver, - draw_tx: tokio::sync::broadcast::Sender<()>, -) { - tokio::spawn(async move { - use tokio::select; - use tokio::time::Instant as TokioInstant; - use tokio::time::sleep_until; - - let mut rx = frame_schedule_rx; - let mut next_deadline: Option = None; - - loop { - let target = next_deadline - .unwrap_or_else(|| Instant::now() + Duration::from_secs(60 * 60 * 24 * 365)); - let sleep_fut = sleep_until(TokioInstant::from_std(target)); - tokio::pin!(sleep_fut); - - select! { - recv = rx.recv() => { - match recv { - Some(at) => { - if next_deadline.is_none_or(|cur| at < cur) { - next_deadline = Some(at); - } - // Do not send a draw immediately here. By continuing the loop, - // we recompute the sleep target so the draw fires once via the - // sleep branch, coalescing multiple requests into a single draw. - continue; - } - None => break, - } - } - _ = &mut sleep_fut => { - if next_deadline.is_some() { - next_deadline = None; - let _ = draw_tx.send(()); - } - } + fn pending_viewport_area(&mut self) -> Result> { + let terminal = &mut self.terminal; + let screen_size = terminal.size()?; + let last_known_screen_size = terminal.last_known_screen_size; + if screen_size != last_known_screen_size + && let Ok(cursor_pos) = terminal.get_cursor_position() + { + let last_known_cursor_pos = terminal.last_known_cursor_pos; + // If we resized AND the cursor moved, we adjust the viewport area to keep the + // cursor in the same position. This is a heuristic that seems to work well + // at least in iTerm2. + if cursor_pos.y != last_known_cursor_pos.y { + let offset = Offset { + x: 0, + y: cursor_pos.y as i32 - last_known_cursor_pos.y as i32, + }; + return Ok(Some(terminal.viewport_area.offset(offset))); } } - }); + Ok(None) + } } /// Command that emits an OSC 9 desktop notification with a message. diff --git a/codex-rs/tui/src/tui/frame_requester.rs b/codex-rs/tui/src/tui/frame_requester.rs new file mode 100644 index 000000000000..4f7886aa22c4 --- /dev/null +++ b/codex-rs/tui/src/tui/frame_requester.rs @@ -0,0 +1,249 @@ +//! Frame draw scheduling utilities for the TUI. +//! +//! This module exposes [`FrameRequester`], a lightweight handle that widgets and +//! background tasks can clone to request future redraws of the TUI. +//! +//! Internally it spawns a [`FrameScheduler`] task that coalesces many requests +//! into a single notification on a broadcast channel used by the main TUI event +//! loop. This keeps animations and status updates smooth without redrawing more +//! often than necessary. +//! +//! This follows the actor-style design from +//! [“Actors with Tokio”](https://ryhl.io/blog/actors-with-tokio/), with a +//! dedicated scheduler task and lightweight request handles. + +use std::time::Duration; +use std::time::Instant; + +use tokio::sync::broadcast; +use tokio::sync::mpsc; + +/// A requester for scheduling future frame draws on the TUI event loop. +/// +/// This is the handler side of an actor/handler pair with `FrameScheduler`, which coalesces +/// multiple frame requests into a single draw operation. +/// +/// Clones of this type can be freely shared across tasks to make it possible to trigger frame draws +/// from anywhere in the TUI code. +#[derive(Clone, Debug)] +pub struct FrameRequester { + frame_schedule_tx: mpsc::UnboundedSender, +} + +impl FrameRequester { + /// Create a new FrameRequester and spawn its associated FrameScheduler task. + /// + /// The provided `draw_tx` is used to notify the TUI event loop of scheduled draws. + pub fn new(draw_tx: broadcast::Sender<()>) -> Self { + let (tx, rx) = mpsc::unbounded_channel(); + let scheduler = FrameScheduler::new(rx, draw_tx); + tokio::spawn(scheduler.run()); + Self { + frame_schedule_tx: tx, + } + } + + /// Schedule a frame draw as soon as possible. + pub fn schedule_frame(&self) { + let _ = self.frame_schedule_tx.send(Instant::now()); + } + + /// Schedule a frame draw to occur after the specified duration. + pub fn schedule_frame_in(&self, dur: Duration) { + let _ = self.frame_schedule_tx.send(Instant::now() + dur); + } +} + +#[cfg(test)] +impl FrameRequester { + /// Create a no-op frame requester for tests. + pub(crate) fn test_dummy() -> Self { + let (tx, _rx) = mpsc::unbounded_channel(); + FrameRequester { + frame_schedule_tx: tx, + } + } +} + +/// A scheduler for coalescing frame draw requests and notifying the TUI event loop. +/// +/// This type is internal to `FrameRequester` and is spawned as a task to handle scheduling logic. +struct FrameScheduler { + receiver: mpsc::UnboundedReceiver, + draw_tx: broadcast::Sender<()>, +} + +impl FrameScheduler { + /// Create a new FrameScheduler with the provided receiver and draw notification sender. + fn new(receiver: mpsc::UnboundedReceiver, draw_tx: broadcast::Sender<()>) -> Self { + Self { receiver, draw_tx } + } + + /// Run the scheduling loop, coalescing frame requests and notifying the TUI event loop. + /// + /// This method runs indefinitely until all senders are dropped. A single draw notification + /// is sent for multiple requests scheduled before the next draw deadline. + async fn run(mut self) { + const ONE_YEAR: Duration = Duration::from_secs(60 * 60 * 24 * 365); + let mut next_deadline: Option = None; + loop { + let target = next_deadline.unwrap_or_else(|| Instant::now() + ONE_YEAR); + let deadline = tokio::time::sleep_until(target.into()); + tokio::pin!(deadline); + + tokio::select! { + draw_at = self.receiver.recv() => { + let Some(draw_at) = draw_at else { + // All senders dropped; exit the scheduler. + break + }; + next_deadline = Some(next_deadline.map_or(draw_at, |cur| cur.min(draw_at))); + + // Do not send a draw immediately here. By continuing the loop, + // we recompute the sleep target so the draw fires once via the + // sleep branch, coalescing multiple requests into a single draw. + continue; + } + _ = &mut deadline => { + if next_deadline.is_some() { + next_deadline = None; + let _ = self.draw_tx.send(()); + } + } + } + } + } +} +#[cfg(test)] +mod tests { + use super::*; + use tokio::time; + use tokio_util::time::FutureExt; + + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn test_schedule_frame_immediate_triggers_once() { + let (draw_tx, mut draw_rx) = broadcast::channel(16); + let requester = FrameRequester::new(draw_tx); + + requester.schedule_frame(); + + // Advance time minimally to let the scheduler process and hit the deadline == now. + time::advance(Duration::from_millis(1)).await; + + // First draw should arrive. + let first = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for first draw"); + assert!(first.is_ok(), "broadcast closed unexpectedly"); + + // No second draw should arrive. + let second = draw_rx.recv().timeout(Duration::from_millis(20)).await; + assert!(second.is_err(), "unexpected extra draw received"); + } + + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn test_schedule_frame_in_triggers_at_delay() { + let (draw_tx, mut draw_rx) = broadcast::channel(16); + let requester = FrameRequester::new(draw_tx); + + requester.schedule_frame_in(Duration::from_millis(50)); + + // Advance less than the delay: no draw yet. + time::advance(Duration::from_millis(30)).await; + let early = draw_rx.recv().timeout(Duration::from_millis(10)).await; + assert!(early.is_err(), "draw fired too early"); + + // Advance past the deadline: one draw should fire. + time::advance(Duration::from_millis(25)).await; + let first = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for scheduled draw"); + assert!(first.is_ok(), "broadcast closed unexpectedly"); + + // No second draw should arrive. + let second = draw_rx.recv().timeout(Duration::from_millis(20)).await; + assert!(second.is_err(), "unexpected extra draw received"); + } + + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn test_coalesces_multiple_requests_into_single_draw() { + let (draw_tx, mut draw_rx) = broadcast::channel(16); + let requester = FrameRequester::new(draw_tx); + + // Schedule multiple immediate requests close together. + requester.schedule_frame(); + requester.schedule_frame(); + requester.schedule_frame(); + + // Allow the scheduler to process and hit the coalesced deadline. + time::advance(Duration::from_millis(1)).await; + + // Expect only a single draw notification despite three requests. + let first = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for coalesced draw"); + assert!(first.is_ok(), "broadcast closed unexpectedly"); + + // No additional draw should be sent for the same coalesced batch. + let second = draw_rx.recv().timeout(Duration::from_millis(20)).await; + assert!(second.is_err(), "unexpected extra draw received"); + } + + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn test_coalesces_mixed_immediate_and_delayed_requests() { + let (draw_tx, mut draw_rx) = broadcast::channel(16); + let requester = FrameRequester::new(draw_tx); + + // Schedule a delayed draw and then an immediate one; should coalesce and fire at the earliest (immediate). + requester.schedule_frame_in(Duration::from_millis(100)); + requester.schedule_frame(); + + time::advance(Duration::from_millis(1)).await; + + let first = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for coalesced immediate draw"); + assert!(first.is_ok(), "broadcast closed unexpectedly"); + + // The later delayed request should have been coalesced into the earlier one; no second draw. + let second = draw_rx.recv().timeout(Duration::from_millis(120)).await; + assert!(second.is_err(), "unexpected extra draw received"); + } + + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn test_multiple_delayed_requests_coalesce_to_earliest() { + let (draw_tx, mut draw_rx) = broadcast::channel(16); + let requester = FrameRequester::new(draw_tx); + + // Schedule multiple delayed draws; they should coalesce to the earliest (10ms). + requester.schedule_frame_in(Duration::from_millis(100)); + requester.schedule_frame_in(Duration::from_millis(20)); + requester.schedule_frame_in(Duration::from_millis(120)); + + // Advance to just before the earliest deadline: no draw yet. + time::advance(Duration::from_millis(10)).await; + let early = draw_rx.recv().timeout(Duration::from_millis(10)).await; + assert!(early.is_err(), "draw fired too early"); + + // Advance past the earliest deadline: one draw should fire. + time::advance(Duration::from_millis(20)).await; + let first = draw_rx + .recv() + .timeout(Duration::from_millis(50)) + .await + .expect("timed out waiting for earliest coalesced draw"); + assert!(first.is_ok(), "broadcast closed unexpectedly"); + + // No additional draw should fire for the later delayed requests. + let second = draw_rx.recv().timeout(Duration::from_millis(120)).await; + assert!(second.is_err(), "unexpected extra draw received"); + } +} diff --git a/codex-rs/utils/git/src/ghost_commits.rs b/codex-rs/utils/git/src/ghost_commits.rs index 01987bb5e602..6a3eec4894fe 100644 --- a/codex-rs/utils/git/src/ghost_commits.rs +++ b/codex-rs/utils/git/src/ghost_commits.rs @@ -3,6 +3,7 @@ use std::collections::HashSet; use std::ffi::OsString; use std::fs; use std::io; +use std::path::Component; use std::path::Path; use std::path::PathBuf; @@ -24,6 +25,24 @@ use crate::operations::run_git_for_stdout_all; const DEFAULT_COMMIT_MESSAGE: &str = "codex snapshot"; /// Default threshold that triggers a warning about large untracked directories. const LARGE_UNTRACKED_WARNING_THRESHOLD: usize = 200; +/// Directories that should always be ignored when capturing ghost snapshots, +/// even if they are not listed in .gitignore. +/// +/// These are typically large dependency or build trees that are not useful +/// for undo and can cause snapshots to grow without bound. +const DEFAULT_IGNORED_DIR_NAMES: &[&str] = &[ + "node_modules", + ".venv", + "venv", + "env", + ".env", + "dist", + "build", + ".pytest_cache", + ".mypy_cache", + ".cache", + ".tox", +]; /// Options to control ghost commit creation. pub struct CreateGhostCommitOptions<'a> { @@ -334,12 +353,11 @@ fn capture_existing_untracked( repo_prefix: Option<&Path>, ) -> Result { // Ask git for the zero-delimited porcelain status so we can enumerate - // every untracked or ignored path (including ones filtered by prefix). + // every untracked path (including ones filtered by prefix). let mut args = vec![ OsString::from("status"), OsString::from("--porcelain=2"), OsString::from("-z"), - OsString::from("--ignored=matching"), OsString::from("--untracked-files=all"), ]; if let Some(prefix) = repo_prefix { @@ -373,6 +391,9 @@ fn capture_existing_untracked( } let normalized = normalize_relative_path(Path::new(path_part))?; + if should_ignore_for_snapshot(&normalized) { + continue; + } let absolute = repo_root.join(&normalized); let is_dir = absolute.is_dir(); if is_dir { @@ -385,6 +406,19 @@ fn capture_existing_untracked( Ok(snapshot) } +fn should_ignore_for_snapshot(path: &Path) -> bool { + path.components().any(|component| { + if let Component::Normal(name) = component + && let Some(name_str) = name.to_str() + { + return DEFAULT_IGNORED_DIR_NAMES + .iter() + .any(|ignored| ignored == &name_str); + } + false + }) +} + /// Removes untracked files and directories that were not present when the snapshot was captured. fn remove_new_untracked( repo_root: &Path, @@ -480,6 +514,7 @@ mod tests { use assert_matches::assert_matches; use pretty_assertions::assert_eq; use std::process::Command; + use walkdir::WalkDir; /// Runs a git command in the test repository and asserts success. fn run_git_in(repo_path: &Path, args: &[&str]) { @@ -621,6 +656,168 @@ mod tests { Ok(()) } + #[test] + fn snapshot_ignores_default_ignored_directories() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + + std::fs::write(repo.join("tracked.txt"), "contents\n")?; + run_git_in(repo, &["add", "tracked.txt"]); + run_git_in( + repo, + &[ + "-c", + "user.name=Tester", + "-c", + "user.email=test@example.com", + "commit", + "-m", + "initial", + ], + ); + + let node_modules = repo.join("node_modules"); + std::fs::create_dir_all(node_modules.join("@scope/package/src"))?; + for idx in 0..50 { + let file = node_modules.join(format!("file-{idx}.js")); + std::fs::write(file, "console.log('ignored');\n")?; + } + std::fs::write( + node_modules.join("@scope/package/src/index.js"), + "console.log('nested ignored');\n", + )?; + + let venv = repo.join(".venv"); + std::fs::create_dir_all(venv.join("lib/python/site-packages"))?; + std::fs::write( + venv.join("lib/python/site-packages/pkg.py"), + "print('ignored')\n", + )?; + + let (ghost, report) = + create_ghost_commit_with_report(&CreateGhostCommitOptions::new(repo))?; + assert!(ghost.parent().is_some()); + + for file in ghost.preexisting_untracked_files() { + let components = file.components().collect::>(); + let mut has_default_ignored_component = false; + for component in components { + if let Component::Normal(name) = component + && let Some(name_str) = name.to_str() + && DEFAULT_IGNORED_DIR_NAMES + .iter() + .any(|ignored| ignored == &name_str) + { + has_default_ignored_component = true; + break; + } + } + assert!( + !has_default_ignored_component, + "unexpected default-ignored file captured: {file:?}" + ); + } + + for dir in ghost.preexisting_untracked_dirs() { + let components = dir.components().collect::>(); + let mut has_default_ignored_component = false; + for component in components { + if let Component::Normal(name) = component + && let Some(name_str) = name.to_str() + && DEFAULT_IGNORED_DIR_NAMES + .iter() + .any(|ignored| ignored == &name_str) + { + has_default_ignored_component = true; + break; + } + } + assert!( + !has_default_ignored_component, + "unexpected default-ignored dir captured: {dir:?}" + ); + } + + for entry in &report.large_untracked_dirs { + let components = entry.path.components().collect::>(); + let mut has_default_ignored_component = false; + for component in components { + if let Component::Normal(name) = component + && let Some(name_str) = name.to_str() + && DEFAULT_IGNORED_DIR_NAMES + .iter() + .any(|ignored| ignored == &name_str) + { + has_default_ignored_component = true; + break; + } + } + assert!( + !has_default_ignored_component, + "unexpected default-ignored dir in large_untracked_dirs: {:?}", + entry.path + ); + } + + Ok(()) + } + + #[test] + fn restore_preserves_default_ignored_directories() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + + std::fs::write(repo.join("tracked.txt"), "snapshot version\n")?; + run_git_in(repo, &["add", "tracked.txt"]); + run_git_in( + repo, + &[ + "-c", + "user.name=Tester", + "-c", + "user.email=test@example.com", + "commit", + "-m", + "initial", + ], + ); + + let node_modules = repo.join("node_modules"); + std::fs::create_dir_all(node_modules.join("pkg"))?; + std::fs::write( + node_modules.join("pkg/index.js"), + "console.log('before');\n", + )?; + + let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo))?; + + std::fs::write(repo.join("tracked.txt"), "snapshot delta\n")?; + std::fs::write(node_modules.join("pkg/index.js"), "console.log('after');\n")?; + std::fs::write(node_modules.join("pkg/extra.js"), "console.log('extra');\n")?; + std::fs::write(repo.join("temp.txt"), "new file\n")?; + + restore_ghost_commit(repo, &ghost)?; + + let tracked_after = std::fs::read_to_string(repo.join("tracked.txt"))?; + assert_eq!(tracked_after, "snapshot version\n"); + + let node_modules_exists = node_modules.exists(); + assert!(node_modules_exists); + + let files_under_node_modules: Vec<_> = WalkDir::new(&node_modules) + .into_iter() + .filter_map(Result::ok) + .filter(|entry| entry.file_type().is_file()) + .collect(); + assert!(!files_under_node_modules.is_empty()); + + assert!(!repo.join("temp.txt").exists()); + + Ok(()) + } + #[test] fn create_snapshot_reports_nested_large_untracked_dirs_under_tracked_parent() -> Result<(), GitToolingError> { @@ -880,8 +1077,8 @@ mod tests { } #[test] - /// Restoring removes ignored directories created after the snapshot. - fn restore_removes_new_ignored_directory() -> Result<(), GitToolingError> { + /// Restoring leaves ignored directories created after the snapshot untouched. + fn restore_preserves_new_ignored_directory() -> Result<(), GitToolingError> { let temp = tempfile::tempdir()?; let repo = temp.path(); init_test_repo(repo); @@ -910,7 +1107,124 @@ mod tests { restore_ghost_commit(repo, &ghost)?; - assert!(!vscode.exists()); + assert!(vscode.exists()); + let settings_after = std::fs::read_to_string(vscode.join("settings.json"))?; + assert_eq!(settings_after, "{\n \"after\": true\n}\n"); + + Ok(()) + } + + #[test] + /// Restoring leaves ignored files created after the snapshot untouched. + fn restore_preserves_new_ignored_file() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + + std::fs::write(repo.join(".gitignore"), "ignored.txt\n")?; + std::fs::write(repo.join("tracked.txt"), "snapshot version\n")?; + run_git_in(repo, &["add", ".gitignore", "tracked.txt"]); + run_git_in( + repo, + &[ + "-c", + "user.name=Tester", + "-c", + "user.email=test@example.com", + "commit", + "-m", + "initial", + ], + ); + + let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo))?; + + let ignored = repo.join("ignored.txt"); + std::fs::write(&ignored, "created later\n")?; + + restore_ghost_commit(repo, &ghost)?; + + assert!(ignored.exists()); + let contents = std::fs::read_to_string(&ignored)?; + assert_eq!(contents, "created later\n"); + + Ok(()) + } + + #[test] + /// Restoring keeps deleted ignored files deleted when they were absent before the snapshot. + fn restore_respects_removed_ignored_file() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + + std::fs::write(repo.join(".gitignore"), "ignored.txt\n")?; + std::fs::write(repo.join("tracked.txt"), "snapshot version\n")?; + let ignored = repo.join("ignored.txt"); + std::fs::write(&ignored, "initial state\n")?; + run_git_in(repo, &["add", ".gitignore", "tracked.txt"]); + run_git_in( + repo, + &[ + "-c", + "user.name=Tester", + "-c", + "user.email=test@example.com", + "commit", + "-m", + "initial", + ], + ); + + let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo))?; + + std::fs::remove_file(&ignored)?; + + restore_ghost_commit(repo, &ghost)?; + + assert!(!ignored.exists()); + + Ok(()) + } + + #[test] + /// Restoring leaves files matched by glob ignores intact. + fn restore_preserves_ignored_glob_matches() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + + std::fs::write(repo.join(".gitignore"), "dummy-dir/*.txt\n")?; + std::fs::write(repo.join("tracked.txt"), "snapshot version\n")?; + run_git_in(repo, &["add", ".gitignore", "tracked.txt"]); + run_git_in( + repo, + &[ + "-c", + "user.name=Tester", + "-c", + "user.email=test@example.com", + "commit", + "-m", + "initial", + ], + ); + + let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo))?; + + let dummy_dir = repo.join("dummy-dir"); + std::fs::create_dir_all(&dummy_dir)?; + let file1 = dummy_dir.join("file1.txt"); + let file2 = dummy_dir.join("file2.txt"); + std::fs::write(&file1, "first\n")?; + std::fs::write(&file2, "second\n")?; + + restore_ghost_commit(repo, &ghost)?; + + assert!(file1.exists()); + assert!(file2.exists()); + assert_eq!(std::fs::read_to_string(file1)?, "first\n"); + assert_eq!(std::fs::read_to_string(file2)?, "second\n"); Ok(()) } diff --git a/docs/config.md b/docs/config.md index a3e6ed266b01..0f0761a30125 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 cc53efa612b9..6061dc883011 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 diff --git a/shell-tool-mcp/src/index.ts b/shell-tool-mcp/src/index.ts index 2ce58462f46d..9199a5a2766d 100644 --- a/shell-tool-mcp/src/index.ts +++ b/shell-tool-mcp/src/index.ts @@ -8,14 +8,9 @@ import { resolveBashPath } from "./bashSelection"; import { readOsRelease } from "./osRelease"; import { resolveTargetTriple } from "./platform"; -const scriptPath = process.argv[1] - ? path.resolve(process.argv[1]) - : process.cwd(); -const __dirname = path.dirname(scriptPath); - async function main(): Promise { const targetTriple = resolveTargetTriple(process.platform, process.arch); - const vendorRoot = path.join(__dirname, "..", "vendor"); + const vendorRoot = path.resolve(__dirname, "..", "vendor"); const targetRoot = path.join(vendorRoot, targetTriple); const execveWrapperPath = path.join(targetRoot, "codex-execve-wrapper"); const serverPath = path.join(targetRoot, "codex-exec-mcp-server");