From a7fda7005344b754ddb3303704373ad0def1312c Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Thu, 18 Sep 2025 17:08:28 -0700 Subject: [PATCH 01/10] Use a unified shell tell to not break cache (#3814) Currently, we change the tool description according to the sandbox policy and approval policy. This breaks the cache when the user hits `/approvals`. This PR does the following: - Always use the shell with escalation parameter: - removes `create_shell_tool_for_sandbox` and always uses unified tool via `create_shell_tool` - Reject the func call when the model uses escalation parameter when it cannot. --- codex-rs/core/src/codex.rs | 127 +++++++++++++++++++++++++--- codex-rs/core/src/exec.rs | 2 +- codex-rs/core/src/openai_tools.rs | 132 ++++-------------------------- 3 files changed, 133 insertions(+), 128 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index e9f52b1c94a2..8852b25834d2 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -456,8 +456,6 @@ impl Session { client, tools_config: ToolsConfig::new(&ToolsConfigParams { model_family: &config.model_family, - approval_policy, - sandbox_policy: sandbox_policy.clone(), include_plan_tool: config.include_plan_tool, include_apply_patch_tool: config.include_apply_patch_tool, include_web_search_request: config.tools_web_search_request, @@ -1237,8 +1235,6 @@ async fn submission_loop( let tools_config = ToolsConfig::new(&ToolsConfigParams { model_family: &effective_family, - approval_policy: new_approval_policy, - sandbox_policy: new_sandbox_policy.clone(), include_plan_tool: config.include_plan_tool, include_apply_patch_tool: config.include_apply_patch_tool, include_web_search_request: config.tools_web_search_request, @@ -1325,8 +1321,6 @@ async fn submission_loop( client, tools_config: ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, - approval_policy, - sandbox_policy: sandbox_policy.clone(), include_plan_tool: config.include_plan_tool, include_apply_patch_tool: config.include_apply_patch_tool, include_web_search_request: config.tools_web_search_request, @@ -1553,8 +1547,6 @@ async fn spawn_review_thread( .unwrap_or_else(|| parent_turn_context.client.get_model_family()); let tools_config = ToolsConfig::new(&ToolsConfigParams { model_family: &review_model_family, - approval_policy: parent_turn_context.approval_policy, - sandbox_policy: parent_turn_context.sandbox_policy.clone(), include_plan_tool: false, include_apply_patch_tool: config.include_apply_patch_tool, include_web_search_request: false, @@ -2724,6 +2716,21 @@ async fn handle_container_exec_with_params( sub_id: String, call_id: String, ) -> ResponseInputItem { + if params.with_escalated_permissions.unwrap_or(false) + && !matches!(turn_context.approval_policy, AskForApproval::OnRequest) + { + return ResponseInputItem::FunctionCallOutput { + call_id, + output: FunctionCallOutputPayload { + content: format!( + "approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}", + policy = turn_context.approval_policy + ), + success: None, + }, + }; + } + // check if this was a patch, and apply it if so let apply_patch_exec = match maybe_parse_apply_patch_verified(¶ms.command, ¶ms.cwd) { MaybeApplyPatchVerified::Body(changes) => { @@ -3345,6 +3352,7 @@ mod tests { use mcp_types::ContentBlock; use mcp_types::TextContent; use pretty_assertions::assert_eq; + use serde::Deserialize; use serde_json::json; use std::path::PathBuf; use std::sync::Arc; @@ -3594,8 +3602,6 @@ mod tests { ); let tools_config = ToolsConfig::new(&ToolsConfigParams { model_family: &config.model_family, - approval_policy: config.approval_policy, - sandbox_policy: config.sandbox_policy.clone(), include_plan_tool: config.include_plan_tool, include_apply_patch_tool: config.include_apply_patch_tool, include_web_search_request: config.tools_web_search_request, @@ -3735,4 +3741,105 @@ mod tests { (rollout_items, live_history.contents()) } + + #[tokio::test] + async fn rejects_escalated_permissions_when_policy_not_on_request() { + use crate::exec::ExecParams; + use crate::protocol::AskForApproval; + use crate::protocol::SandboxPolicy; + use crate::turn_diff_tracker::TurnDiffTracker; + use std::collections::HashMap; + + let (session, mut turn_context) = make_session_and_context(); + // Ensure policy is NOT OnRequest so the early rejection path triggers + turn_context.approval_policy = AskForApproval::OnFailure; + + let params = ExecParams { + command: if cfg!(windows) { + vec![ + "cmd.exe".to_string(), + "/C".to_string(), + "echo hi".to_string(), + ] + } else { + vec![ + "/bin/sh".to_string(), + "-c".to_string(), + "echo hi".to_string(), + ] + }, + cwd: turn_context.cwd.clone(), + timeout_ms: Some(1000), + env: HashMap::new(), + with_escalated_permissions: Some(true), + justification: Some("test".to_string()), + }; + + let params2 = ExecParams { + with_escalated_permissions: Some(false), + ..params.clone() + }; + + let mut turn_diff_tracker = TurnDiffTracker::new(); + + let sub_id = "test-sub".to_string(); + let call_id = "test-call".to_string(); + + let resp = handle_container_exec_with_params( + params, + &session, + &turn_context, + &mut turn_diff_tracker, + sub_id, + call_id, + ) + .await; + + let ResponseInputItem::FunctionCallOutput { output, .. } = resp else { + panic!("expected FunctionCallOutput"); + }; + + let expected = format!( + "approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}", + policy = turn_context.approval_policy + ); + + pretty_assertions::assert_eq!(output.content, expected); + + // Now retry the same command WITHOUT escalated permissions; should succeed. + // Force DangerFullAccess to avoid platform sandbox dependencies in tests. + turn_context.sandbox_policy = SandboxPolicy::DangerFullAccess; + + let resp2 = handle_container_exec_with_params( + params2, + &session, + &turn_context, + &mut turn_diff_tracker, + "test-sub".to_string(), + "test-call-2".to_string(), + ) + .await; + + let ResponseInputItem::FunctionCallOutput { output, .. } = resp2 else { + panic!("expected FunctionCallOutput on retry"); + }; + + #[derive(Deserialize, PartialEq, Eq, Debug)] + struct ResponseExecMetadata { + exit_code: i32, + } + + #[derive(Deserialize)] + struct ResponseExecOutput { + output: String, + metadata: ResponseExecMetadata, + } + + let exec_output: ResponseExecOutput = + serde_json::from_str(&output.content).expect("valid exec output json"); + + pretty_assertions::assert_eq!(exec_output.metadata, ResponseExecMetadata { exit_code: 0 }); + assert!(exec_output.output.contains("hi")); + pretty_assertions::assert_eq!(output.success, Some(true)); + } } diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 9e11604c3958..d84bbc9fcb08 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -45,7 +45,7 @@ const AGGREGATE_BUFFER_INITIAL_CAPACITY: usize = 8 * 1024; // 8 KiB /// Aggregation still collects full output; only the live event stream is capped. pub(crate) const MAX_EXEC_OUTPUT_DELTAS_PER_CALL: usize = 10_000; -#[derive(Debug, Clone)] +#[derive(Clone, Debug)] pub struct ExecParams { pub command: Vec, pub cwd: PathBuf, diff --git a/codex-rs/core/src/openai_tools.rs b/codex-rs/core/src/openai_tools.rs index b57d1b7692cd..05b71ce6eacd 100644 --- a/codex-rs/core/src/openai_tools.rs +++ b/codex-rs/core/src/openai_tools.rs @@ -7,8 +7,6 @@ use std::collections::HashMap; use crate::model_family::ModelFamily; use crate::plan_tool::PLAN_TOOL; -use crate::protocol::AskForApproval; -use crate::protocol::SandboxPolicy; use crate::tool_apply_patch::ApplyPatchToolType; use crate::tool_apply_patch::create_apply_patch_freeform_tool; use crate::tool_apply_patch::create_apply_patch_json_tool; @@ -57,10 +55,9 @@ pub(crate) enum OpenAiTool { #[derive(Debug, Clone)] pub enum ConfigShellToolType { - DefaultShell, - ShellWithRequest { sandbox_policy: SandboxPolicy }, - LocalShell, - StreamableShell, + Default, + Local, + Streamable, } #[derive(Debug, Clone)] @@ -75,8 +72,6 @@ pub(crate) struct ToolsConfig { pub(crate) struct ToolsConfigParams<'a> { pub(crate) model_family: &'a ModelFamily, - pub(crate) approval_policy: AskForApproval, - pub(crate) sandbox_policy: SandboxPolicy, pub(crate) include_plan_tool: bool, pub(crate) include_apply_patch_tool: bool, pub(crate) include_web_search_request: bool, @@ -89,8 +84,6 @@ impl ToolsConfig { pub fn new(params: &ToolsConfigParams) -> Self { let ToolsConfigParams { model_family, - approval_policy, - sandbox_policy, include_plan_tool, include_apply_patch_tool, include_web_search_request, @@ -98,18 +91,13 @@ impl ToolsConfig { include_view_image_tool, experimental_unified_exec_tool, } = params; - let mut shell_type = if *use_streamable_shell_tool { - ConfigShellToolType::StreamableShell + let shell_type = if *use_streamable_shell_tool { + ConfigShellToolType::Streamable } else if model_family.uses_local_shell_tool { - ConfigShellToolType::LocalShell + ConfigShellToolType::Local } else { - ConfigShellToolType::DefaultShell + ConfigShellToolType::Default }; - if matches!(approval_policy, AskForApproval::OnRequest) && !use_streamable_shell_tool { - shell_type = ConfigShellToolType::ShellWithRequest { - sandbox_policy: sandbox_policy.clone(), - } - } let apply_patch_tool_type = match model_family.apply_patch_tool_type { Some(ApplyPatchToolType::Freeform) => Some(ApplyPatchToolType::Freeform), @@ -170,40 +158,6 @@ pub(crate) enum JsonSchema { }, } -fn create_shell_tool() -> OpenAiTool { - let mut properties = BTreeMap::new(); - properties.insert( - "command".to_string(), - JsonSchema::Array { - items: Box::new(JsonSchema::String { description: None }), - description: Some("The command to execute".to_string()), - }, - ); - properties.insert( - "workdir".to_string(), - JsonSchema::String { - description: Some("The working directory to execute the command in".to_string()), - }, - ); - properties.insert( - "timeout_ms".to_string(), - JsonSchema::Number { - description: Some("The timeout for the command in milliseconds".to_string()), - }, - ); - - OpenAiTool::Function(ResponsesApiTool { - name: "shell".to_string(), - description: "Runs a shell command and returns its output".to_string(), - strict: false, - parameters: JsonSchema::Object { - properties, - required: Some(vec!["command".to_string()]), - additional_properties: Some(false), - }, - }) -} - fn create_unified_exec_tool() -> OpenAiTool { let mut properties = BTreeMap::new(); properties.insert( @@ -251,7 +205,7 @@ fn create_unified_exec_tool() -> OpenAiTool { }) } -fn create_shell_tool_for_sandbox(sandbox_policy: &SandboxPolicy) -> OpenAiTool { +fn create_shell_tool() -> OpenAiTool { let mut properties = BTreeMap::new(); properties.insert( "command".to_string(), @@ -273,20 +227,18 @@ fn create_shell_tool_for_sandbox(sandbox_policy: &SandboxPolicy) -> OpenAiTool { }, ); - if !matches!(sandbox_policy, SandboxPolicy::DangerFullAccess) { - properties.insert( + properties.insert( "with_escalated_permissions".to_string(), JsonSchema::Boolean { description: Some("Whether to request escalated permissions. Set to true if command needs to be run without sandbox restrictions".to_string()), }, ); - properties.insert( + properties.insert( "justification".to_string(), JsonSchema::String { description: Some("Only set if with_escalated_permissions is true. 1-sentence explanation of why we want to run this command.".to_string()), }, ); - } OpenAiTool::Function(ResponsesApiTool { name: "shell".to_string(), @@ -537,16 +489,13 @@ pub(crate) fn get_openai_tools( tools.push(create_unified_exec_tool()); } else { match &config.shell_type { - ConfigShellToolType::DefaultShell => { + ConfigShellToolType::Default => { tools.push(create_shell_tool()); } - ConfigShellToolType::ShellWithRequest { sandbox_policy } => { - tools.push(create_shell_tool_for_sandbox(sandbox_policy)); - } - ConfigShellToolType::LocalShell => { + ConfigShellToolType::Local => { tools.push(OpenAiTool::LocalShell {}); } - ConfigShellToolType::StreamableShell => { + ConfigShellToolType::Streamable => { tools.push(OpenAiTool::Function( crate::exec_command::create_exec_command_tool_for_responses_api(), )); @@ -636,8 +585,6 @@ mod tests { .expect("codex-mini-latest should be a valid model family"); let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: true, include_apply_patch_tool: false, include_web_search_request: true, @@ -658,8 +605,6 @@ mod tests { let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: true, include_apply_patch_tool: false, include_web_search_request: true, @@ -680,8 +625,6 @@ mod tests { let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: false, include_apply_patch_tool: false, include_web_search_request: true, @@ -786,8 +729,6 @@ mod tests { let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: false, include_apply_patch_tool: false, include_web_search_request: false, @@ -864,8 +805,6 @@ mod tests { let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: false, include_apply_patch_tool: false, include_web_search_request: true, @@ -927,8 +866,6 @@ mod tests { let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: false, include_apply_patch_tool: false, include_web_search_request: true, @@ -985,8 +922,6 @@ mod tests { let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: false, include_apply_patch_tool: false, include_web_search_request: true, @@ -1046,8 +981,6 @@ mod tests { let model_family = find_family_for_model("o3").expect("o3 should be a valid model family"); let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, - approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: false, include_apply_patch_tool: false, include_web_search_request: true, @@ -1100,29 +1033,8 @@ mod tests { } #[test] - fn test_shell_tool_for_sandbox_workspace_write() { - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec!["workspace".into()], - network_access: false, - exclude_tmpdir_env_var: false, - exclude_slash_tmp: false, - }; - let tool = super::create_shell_tool_for_sandbox(&sandbox_policy); - let OpenAiTool::Function(ResponsesApiTool { - description, name, .. - }) = &tool - else { - panic!("expected function tool"); - }; - assert_eq!(name, "shell"); - - let expected = "Runs a shell command and returns its output."; - assert_eq!(description, expected); - } - - #[test] - fn test_shell_tool_for_sandbox_readonly() { - let tool = super::create_shell_tool_for_sandbox(&SandboxPolicy::ReadOnly); + fn test_shell_tool() { + let tool = super::create_shell_tool(); let OpenAiTool::Function(ResponsesApiTool { description, name, .. }) = &tool @@ -1134,18 +1046,4 @@ mod tests { let expected = "Runs a shell command and returns its output."; assert_eq!(description, expected); } - - #[test] - fn test_shell_tool_for_sandbox_danger_full_access() { - let tool = super::create_shell_tool_for_sandbox(&SandboxPolicy::DangerFullAccess); - let OpenAiTool::Function(ResponsesApiTool { - description, name, .. - }) = &tool - else { - panic!("expected function tool"); - }; - assert_eq!(name, "shell"); - - assert_eq!(description, "Runs a shell command and returns its output."); - } } From 881c7978f1362b7eb4009777d0907fdc7e81b4e3 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 18 Sep 2025 17:53:14 -0700 Subject: [PATCH 02/10] Move responses mocking helpers to a shared lib (#3878) These are generally useful --- codex-rs/Cargo.lock | 1 + codex-rs/core/tests/common/Cargo.toml | 1 + codex-rs/core/tests/common/lib.rs | 2 + codex-rs/core/tests/common/responses.rs | 100 +++++++++++ codex-rs/core/tests/suite/compact.rs | 160 ++++-------------- .../core/tests/suite/compact_resume_fork.rs | 8 +- 6 files changed, 141 insertions(+), 131 deletions(-) create mode 100644 codex-rs/core/tests/common/responses.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index e3930b507a92..73c1117af186 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1077,6 +1077,7 @@ dependencies = [ "serde_json", "tempfile", "tokio", + "wiremock", ] [[package]] diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index 9cfd20cdb442..2d4305191911 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -11,3 +11,4 @@ codex-core = { path = "../.." } serde_json = "1" tempfile = "3" tokio = { version = "1", features = ["time"] } +wiremock = "0.6" diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index 244d093e7def..95af79b22027 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -7,6 +7,8 @@ use codex_core::config::Config; use codex_core::config::ConfigOverrides; use codex_core::config::ConfigToml; +pub mod responses; + /// Returns a default `Config` whose on-disk state is confined to the provided /// temporary directory. Using a per-test directory keeps tests hermetic and /// avoids clobbering a developer’s real `~/.codex`. diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs new file mode 100644 index 000000000000..dc6f849ee774 --- /dev/null +++ b/codex-rs/core/tests/common/responses.rs @@ -0,0 +1,100 @@ +use serde_json::Value; +use wiremock::BodyPrintLimit; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::method; +use wiremock::matchers::path; + +/// Build an SSE stream body from a list of JSON events. +pub fn sse(events: Vec) -> String { + use std::fmt::Write as _; + let mut out = String::new(); + for ev in events { + let kind = ev.get("type").and_then(|v| v.as_str()).unwrap(); + writeln!(&mut out, "event: {kind}").unwrap(); + if !ev.as_object().map(|o| o.len() == 1).unwrap_or(false) { + write!(&mut out, "data: {ev}\n\n").unwrap(); + } else { + out.push('\n'); + } + } + out +} + +/// Convenience: SSE event for a completed response with a specific id. +pub fn ev_completed(id: &str) -> Value { + serde_json::json!({ + "type": "response.completed", + "response": { + "id": id, + "usage": {"input_tokens":0,"input_tokens_details":null,"output_tokens":0,"output_tokens_details":null,"total_tokens":0} + } + }) +} + +pub fn ev_completed_with_tokens(id: &str, total_tokens: u64) -> Value { + serde_json::json!({ + "type": "response.completed", + "response": { + "id": id, + "usage": { + "input_tokens": total_tokens, + "input_tokens_details": null, + "output_tokens": 0, + "output_tokens_details": null, + "total_tokens": total_tokens + } + } + }) +} + +/// Convenience: SSE event for a single assistant message output item. +pub fn ev_assistant_message(id: &str, text: &str) -> Value { + serde_json::json!({ + "type": "response.output_item.done", + "item": { + "type": "message", + "role": "assistant", + "id": id, + "content": [{"type": "output_text", "text": text}] + } + }) +} + +pub fn ev_function_call(call_id: &str, name: &str, arguments: &str) -> Value { + serde_json::json!({ + "type": "response.output_item.done", + "item": { + "type": "function_call", + "call_id": call_id, + "name": name, + "arguments": arguments + } + }) +} + +pub fn sse_response(body: String) -> ResponseTemplate { + ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw(body, "text/event-stream") +} + +pub async fn mount_sse_once(server: &MockServer, matcher: M, body: String) +where + M: wiremock::Match + Send + Sync + 'static, +{ + Mock::given(method("POST")) + .and(path("/v1/responses")) + .and(matcher) + .respond_with(sse_response(body)) + .mount(server) + .await; +} + +pub async fn start_mock_server() -> MockServer { + MockServer::builder() + .body_print_limit(BodyPrintLimit::Limited(80_000)) + .start() + .await +} diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index 8db70f355947..a58de304fbad 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -1,5 +1,3 @@ -#![expect(clippy::unwrap_used)] - use codex_core::CodexAuth; use codex_core::ConversationManager; use codex_core::ModelProviderInfo; @@ -13,12 +11,10 @@ use codex_core::protocol::RolloutItem; use codex_core::protocol::RolloutLine; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use core_test_support::load_default_config_for_test; +use core_test_support::responses; use core_test_support::wait_for_event; -use serde_json::Value; use tempfile::TempDir; -use wiremock::BodyPrintLimit; use wiremock::Mock; -use wiremock::MockServer; use wiremock::Request; use wiremock::Respond; use wiremock::ResponseTemplate; @@ -26,106 +22,16 @@ use wiremock::matchers::method; use wiremock::matchers::path; use pretty_assertions::assert_eq; +use responses::ev_assistant_message; +use responses::ev_completed; +use responses::sse; +use responses::start_mock_server; use std::sync::Arc; use std::sync::Mutex; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; - // --- Test helpers ----------------------------------------------------------- -/// Build an SSE stream body from a list of JSON events. -pub(super) fn sse(events: Vec) -> String { - use std::fmt::Write as _; - let mut out = String::new(); - for ev in events { - let kind = ev.get("type").and_then(|v| v.as_str()).unwrap(); - writeln!(&mut out, "event: {kind}").unwrap(); - if !ev.as_object().map(|o| o.len() == 1).unwrap_or(false) { - write!(&mut out, "data: {ev}\n\n").unwrap(); - } else { - out.push('\n'); - } - } - out -} - -/// Convenience: SSE event for a completed response with a specific id. -pub(super) fn ev_completed(id: &str) -> Value { - serde_json::json!({ - "type": "response.completed", - "response": { - "id": id, - "usage": {"input_tokens":0,"input_tokens_details":null,"output_tokens":0,"output_tokens_details":null,"total_tokens":0} - } - }) -} - -fn ev_completed_with_tokens(id: &str, total_tokens: u64) -> Value { - serde_json::json!({ - "type": "response.completed", - "response": { - "id": id, - "usage": { - "input_tokens": total_tokens, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": total_tokens - } - } - }) -} - -/// Convenience: SSE event for a single assistant message output item. -pub(super) fn ev_assistant_message(id: &str, text: &str) -> Value { - serde_json::json!({ - "type": "response.output_item.done", - "item": { - "type": "message", - "role": "assistant", - "id": id, - "content": [{"type": "output_text", "text": text}] - } - }) -} - -fn ev_function_call(call_id: &str, name: &str, arguments: &str) -> Value { - serde_json::json!({ - "type": "response.output_item.done", - "item": { - "type": "function_call", - "call_id": call_id, - "name": name, - "arguments": arguments - } - }) -} - -pub(super) fn sse_response(body: String) -> ResponseTemplate { - ResponseTemplate::new(200) - .insert_header("content-type", "text/event-stream") - .set_body_raw(body, "text/event-stream") -} - -pub(super) async fn mount_sse_once(server: &MockServer, matcher: M, body: String) -where - M: wiremock::Match + Send + Sync + 'static, -{ - Mock::given(method("POST")) - .and(path("/v1/responses")) - .and(matcher) - .respond_with(sse_response(body)) - .mount(server) - .await; -} - -async fn start_mock_server() -> MockServer { - MockServer::builder() - .body_print_limit(BodyPrintLimit::Limited(80_000)) - .start() - .await -} - pub(super) const FIRST_REPLY: &str = "FIRST_REPLY"; pub(super) const SUMMARY_TEXT: &str = "SUMMARY_ONLY_CONTEXT"; pub(super) const SUMMARIZE_TRIGGER: &str = "Start Summarization"; @@ -175,19 +81,19 @@ async fn summarize_context_three_requests_and_instructions() { body.contains("\"text\":\"hello world\"") && !body.contains(&format!("\"text\":\"{SUMMARIZE_TRIGGER}\"")) }; - mount_sse_once(&server, first_matcher, sse1).await; + responses::mount_sse_once(&server, first_matcher, sse1).await; let second_matcher = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); body.contains(&format!("\"text\":\"{SUMMARIZE_TRIGGER}\"")) }; - mount_sse_once(&server, second_matcher, sse2).await; + responses::mount_sse_once(&server, second_matcher, sse2).await; let third_matcher = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); body.contains(&format!("\"text\":\"{THIRD_USER_MSG}\"")) }; - mount_sse_once(&server, third_matcher, sse3).await; + responses::mount_sse_once(&server, third_matcher, sse3).await; // Build config pointing to the mock server and spawn Codex. let model_provider = ModelProviderInfo { @@ -381,17 +287,17 @@ async fn auto_compact_runs_after_token_limit_hit() { let sse1 = sse(vec![ ev_assistant_message("m1", FIRST_REPLY), - ev_completed_with_tokens("r1", 70_000), + responses::ev_completed_with_tokens("r1", 70_000), ]); let sse2 = sse(vec![ ev_assistant_message("m2", "SECOND_REPLY"), - ev_completed_with_tokens("r2", 330_000), + responses::ev_completed_with_tokens("r2", 330_000), ]); let sse3 = sse(vec![ ev_assistant_message("m3", AUTO_SUMMARY_TEXT), - ev_completed_with_tokens("r3", 200), + responses::ev_completed_with_tokens("r3", 200), ]); let first_matcher = |req: &wiremock::Request| { @@ -403,7 +309,7 @@ async fn auto_compact_runs_after_token_limit_hit() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(first_matcher) - .respond_with(sse_response(sse1)) + .respond_with(responses::sse_response(sse1)) .mount(&server) .await; @@ -416,7 +322,7 @@ async fn auto_compact_runs_after_token_limit_hit() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(second_matcher) - .respond_with(sse_response(sse2)) + .respond_with(responses::sse_response(sse2)) .mount(&server) .await; @@ -427,7 +333,7 @@ async fn auto_compact_runs_after_token_limit_hit() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(third_matcher) - .respond_with(sse_response(sse3)) + .respond_with(responses::sse_response(sse3)) .mount(&server) .await; @@ -522,17 +428,17 @@ async fn auto_compact_persists_rollout_entries() { let sse1 = sse(vec![ ev_assistant_message("m1", FIRST_REPLY), - ev_completed_with_tokens("r1", 70_000), + responses::ev_completed_with_tokens("r1", 70_000), ]); let sse2 = sse(vec![ ev_assistant_message("m2", "SECOND_REPLY"), - ev_completed_with_tokens("r2", 330_000), + responses::ev_completed_with_tokens("r2", 330_000), ]); let sse3 = sse(vec![ ev_assistant_message("m3", AUTO_SUMMARY_TEXT), - ev_completed_with_tokens("r3", 200), + responses::ev_completed_with_tokens("r3", 200), ]); let first_matcher = |req: &wiremock::Request| { @@ -544,7 +450,7 @@ async fn auto_compact_persists_rollout_entries() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(first_matcher) - .respond_with(sse_response(sse1)) + .respond_with(responses::sse_response(sse1)) .mount(&server) .await; @@ -557,7 +463,7 @@ async fn auto_compact_persists_rollout_entries() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(second_matcher) - .respond_with(sse_response(sse2)) + .respond_with(responses::sse_response(sse2)) .mount(&server) .await; @@ -568,7 +474,7 @@ async fn auto_compact_persists_rollout_entries() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(third_matcher) - .respond_with(sse_response(sse3)) + .respond_with(responses::sse_response(sse3)) .mount(&server) .await; @@ -655,17 +561,17 @@ async fn auto_compact_stops_after_failed_attempt() { let sse1 = sse(vec![ ev_assistant_message("m1", FIRST_REPLY), - ev_completed_with_tokens("r1", 500), + responses::ev_completed_with_tokens("r1", 500), ]); let sse2 = sse(vec![ ev_assistant_message("m2", SUMMARY_TEXT), - ev_completed_with_tokens("r2", 50), + responses::ev_completed_with_tokens("r2", 50), ]); let sse3 = sse(vec![ ev_assistant_message("m3", STILL_TOO_BIG_REPLY), - ev_completed_with_tokens("r3", 500), + responses::ev_completed_with_tokens("r3", 500), ]); let first_matcher = |req: &wiremock::Request| { @@ -676,7 +582,7 @@ async fn auto_compact_stops_after_failed_attempt() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(first_matcher) - .respond_with(sse_response(sse1.clone())) + .respond_with(responses::sse_response(sse1.clone())) .mount(&server) .await; @@ -687,7 +593,7 @@ async fn auto_compact_stops_after_failed_attempt() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(second_matcher) - .respond_with(sse_response(sse2.clone())) + .respond_with(responses::sse_response(sse2.clone())) .mount(&server) .await; @@ -699,7 +605,7 @@ async fn auto_compact_stops_after_failed_attempt() { Mock::given(method("POST")) .and(path("/v1/responses")) .and(third_matcher) - .respond_with(sse_response(sse3.clone())) + .respond_with(responses::sse_response(sse3.clone())) .mount(&server) .await; @@ -769,27 +675,27 @@ async fn auto_compact_allows_multiple_attempts_when_interleaved_with_other_turn_ let sse1 = sse(vec![ ev_assistant_message("m1", FIRST_REPLY), - ev_completed_with_tokens("r1", 500), + responses::ev_completed_with_tokens("r1", 500), ]); let sse2 = sse(vec![ ev_assistant_message("m2", FIRST_AUTO_SUMMARY), - ev_completed_with_tokens("r2", 50), + responses::ev_completed_with_tokens("r2", 50), ]); let sse3 = sse(vec![ - ev_function_call(DUMMY_CALL_ID, DUMMY_FUNCTION_NAME, "{}"), - ev_completed_with_tokens("r3", 150), + responses::ev_function_call(DUMMY_CALL_ID, DUMMY_FUNCTION_NAME, "{}"), + responses::ev_completed_with_tokens("r3", 150), ]); let sse4 = sse(vec![ ev_assistant_message("m4", SECOND_LARGE_REPLY), - ev_completed_with_tokens("r4", 450), + responses::ev_completed_with_tokens("r4", 450), ]); let sse5 = sse(vec![ ev_assistant_message("m5", SECOND_AUTO_SUMMARY), - ev_completed_with_tokens("r5", 60), + responses::ev_completed_with_tokens("r5", 60), ]); let sse6 = sse(vec![ ev_assistant_message("m6", FINAL_REPLY), - ev_completed_with_tokens("r6", 120), + responses::ev_completed_with_tokens("r6", 120), ]); #[derive(Clone)] diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index 1bedbcab083d..1e752826bb9f 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -10,10 +10,6 @@ use super::compact::FIRST_REPLY; use super::compact::SUMMARIZE_TRIGGER; use super::compact::SUMMARY_TEXT; -use super::compact::ev_assistant_message; -use super::compact::ev_completed; -use super::compact::mount_sse_once; -use super::compact::sse; use codex_core::CodexAuth; use codex_core::CodexConversation; use codex_core::ConversationManager; @@ -27,6 +23,10 @@ use codex_core::protocol::InputItem; use codex_core::protocol::Op; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use core_test_support::load_default_config_for_test; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::mount_sse_once; +use core_test_support::responses::sse; use core_test_support::wait_for_event; use pretty_assertions::assert_eq; use serde_json::Value; From 0db45f33499229ab3e388ac0fa3182470b1508a2 Mon Sep 17 00:00:00 2001 From: just-every-code Date: Fri, 19 Sep 2025 01:19:51 +0000 Subject: [PATCH 03/10] docs(upstream-merge): add MERGE_PLAN.md and MERGE_REPORT.md for by-bucket sync --- .github/auto/MERGE_PLAN.md | 39 ++++++++++++++++++++++++++++++++++++ .github/auto/MERGE_REPORT.md | 30 +++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 .github/auto/MERGE_PLAN.md create mode 100644 .github/auto/MERGE_REPORT.md diff --git a/.github/auto/MERGE_PLAN.md b/.github/auto/MERGE_PLAN.md new file mode 100644 index 000000000000..f93bdb333db2 --- /dev/null +++ b/.github/auto/MERGE_PLAN.md @@ -0,0 +1,39 @@ +# Upstream Merge Plan (by-bucket) + +Mode: by-bucket + +Strategy +- Fetch and merge `upstream/main` into `upstream-merge` with `--no-commit` to allow policy-driven conflict resolution. +- Apply selective reconciliation per policy buckets: + - prefer_ours: keep our fork-specific TUI/core/tooling unless upstream change is clearly beneficial and compatible. + - prefer_theirs: adopt upstream for common/exec/file-search unless it breaks build or documented behavior. + - default: favor upstream while preserving fork invariants (browser/agent/web_fetch tools, exposure gating, screenshot queuing, UA/version helpers, core re-exports). +- Purge assets matching purge globs if reintroduced by upstream. +- Validate with `scripts/upstream-merge/verify.sh` and repo `./build-fast.sh`. + +Policy +- prefer_ours_globs: + - codex-rs/tui/** + - codex-cli/** + - codex-rs/core/src/openai_tools.rs + - codex-rs/core/src/codex.rs + - codex-rs/core/src/agent_tool.rs + - codex-rs/core/src/default_client.rs + - codex-rs/protocol/src/models.rs + - .github/workflows/** + - docs/** + - AGENTS.md + - README.md + - CHANGELOG.md +- prefer_theirs_globs: + - codex-rs/common/** + - codex-rs/exec/** + - codex-rs/file-search/** +- purge_globs: + - .github/codex-cli-*.png/.jpg/.jpeg/.webp + +Checks +- Preserve public re-exports in `codex-core` (ModelClient, Prompt, ResponseEvent, ResponseStream) and the `codex_core::models` alias. +- Do not drop ICU/sys-locale deps unless unused across workspace. +- Ensure tool handler↔openai_tools parity and UA/version invariants pass verify script. + diff --git a/.github/auto/MERGE_REPORT.md b/.github/auto/MERGE_REPORT.md new file mode 100644 index 000000000000..fbb9d4024d98 --- /dev/null +++ b/.github/auto/MERGE_REPORT.md @@ -0,0 +1,30 @@ +# Upstream Merge Report + +Branch: upstream-merge +Upstream: openai/codex@main +Mode: by-bucket +Date: 2025-09-19 + +## Incorporated +- Upstream test infra additions under `codex-rs/core/tests/common` (new `responses.rs` helpers; `wiremock` dep). +- Minor derive/style tweak in `codex-rs/core/src/exec.rs` (`#[derive(Clone, Debug)]`). +- Test updates in `codex-rs/core/tests/suite` to use shared helpers. + +## Kept Ours (prefer_ours) +- `codex-rs/core/src/codex.rs`: preserved fork logic for tool registration, browser gating, UA/version helpers, and response streaming invariants. +- `codex-rs/core/src/openai_tools.rs`: preserved custom tool schemas and handler↔tool parity for `browser_*`, `agent_*`, and `web_fetch`. + +Rationale: Maintains fork invariants (tool families, gating, screenshot queue semantics, UA/version), and ensures compatibility with our TUI/tooling. + +## Dropped/Rejected +- No upstream files were explicitly dropped beyond automatic policy application; no purge-glob assets present. + +## Other Notes +- Purge globs: none present. +- Public API checks: `ModelClient`, `Prompt`, `ResponseEvent`, `ResponseStream` re-exports intact; `codex_core::models` alias intact. +- ICU/sys-locale: no removals performed. + +## Validation +- `scripts/upstream-merge/verify.sh`: PASS (build_fast=ok, api_check=ok, branding guard ok). +- `./build-fast.sh`: executed as part of verify; PASS. + From 87df3ddc70eaf0a7c2547092d428296e8dcf97cf Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 19 Sep 2025 01:28:48 +0000 Subject: [PATCH 04/10] chore(merge): enforce policy removals (images + cargo caches) --- .github/auto/MERGE_PLAN.md | 39 ------------------------------------ .github/auto/MERGE_REPORT.md | 30 --------------------------- 2 files changed, 69 deletions(-) delete mode 100644 .github/auto/MERGE_PLAN.md delete mode 100644 .github/auto/MERGE_REPORT.md diff --git a/.github/auto/MERGE_PLAN.md b/.github/auto/MERGE_PLAN.md deleted file mode 100644 index f93bdb333db2..000000000000 --- a/.github/auto/MERGE_PLAN.md +++ /dev/null @@ -1,39 +0,0 @@ -# Upstream Merge Plan (by-bucket) - -Mode: by-bucket - -Strategy -- Fetch and merge `upstream/main` into `upstream-merge` with `--no-commit` to allow policy-driven conflict resolution. -- Apply selective reconciliation per policy buckets: - - prefer_ours: keep our fork-specific TUI/core/tooling unless upstream change is clearly beneficial and compatible. - - prefer_theirs: adopt upstream for common/exec/file-search unless it breaks build or documented behavior. - - default: favor upstream while preserving fork invariants (browser/agent/web_fetch tools, exposure gating, screenshot queuing, UA/version helpers, core re-exports). -- Purge assets matching purge globs if reintroduced by upstream. -- Validate with `scripts/upstream-merge/verify.sh` and repo `./build-fast.sh`. - -Policy -- prefer_ours_globs: - - codex-rs/tui/** - - codex-cli/** - - codex-rs/core/src/openai_tools.rs - - codex-rs/core/src/codex.rs - - codex-rs/core/src/agent_tool.rs - - codex-rs/core/src/default_client.rs - - codex-rs/protocol/src/models.rs - - .github/workflows/** - - docs/** - - AGENTS.md - - README.md - - CHANGELOG.md -- prefer_theirs_globs: - - codex-rs/common/** - - codex-rs/exec/** - - codex-rs/file-search/** -- purge_globs: - - .github/codex-cli-*.png/.jpg/.jpeg/.webp - -Checks -- Preserve public re-exports in `codex-core` (ModelClient, Prompt, ResponseEvent, ResponseStream) and the `codex_core::models` alias. -- Do not drop ICU/sys-locale deps unless unused across workspace. -- Ensure tool handler↔openai_tools parity and UA/version invariants pass verify script. - diff --git a/.github/auto/MERGE_REPORT.md b/.github/auto/MERGE_REPORT.md deleted file mode 100644 index fbb9d4024d98..000000000000 --- a/.github/auto/MERGE_REPORT.md +++ /dev/null @@ -1,30 +0,0 @@ -# Upstream Merge Report - -Branch: upstream-merge -Upstream: openai/codex@main -Mode: by-bucket -Date: 2025-09-19 - -## Incorporated -- Upstream test infra additions under `codex-rs/core/tests/common` (new `responses.rs` helpers; `wiremock` dep). -- Minor derive/style tweak in `codex-rs/core/src/exec.rs` (`#[derive(Clone, Debug)]`). -- Test updates in `codex-rs/core/tests/suite` to use shared helpers. - -## Kept Ours (prefer_ours) -- `codex-rs/core/src/codex.rs`: preserved fork logic for tool registration, browser gating, UA/version helpers, and response streaming invariants. -- `codex-rs/core/src/openai_tools.rs`: preserved custom tool schemas and handler↔tool parity for `browser_*`, `agent_*`, and `web_fetch`. - -Rationale: Maintains fork invariants (tool families, gating, screenshot queue semantics, UA/version), and ensures compatibility with our TUI/tooling. - -## Dropped/Rejected -- No upstream files were explicitly dropped beyond automatic policy application; no purge-glob assets present. - -## Other Notes -- Purge globs: none present. -- Public API checks: `ModelClient`, `Prompt`, `ResponseEvent`, `ResponseStream` re-exports intact; `codex_core::models` alias intact. -- ICU/sys-locale: no removals performed. - -## Validation -- `scripts/upstream-merge/verify.sh`: PASS (build_fast=ok, api_check=ok, branding guard ok). -- `./build-fast.sh`: executed as part of verify; PASS. - From 9b18875a42c6453ff79a8ae7a658e5e615f7b353 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Fri, 19 Sep 2025 06:46:25 -0700 Subject: [PATCH 05/10] Use helpers instead of fixtures (#3888) Move to using test helper method everywhere. --- codex-rs/core/tests/common/responses.rs | 33 +++++++++ .../tests/fixtures/sse_apply_patch_add.json | 25 ------- .../sse_apply_patch_freeform_add.json | 25 ------- .../sse_apply_patch_freeform_update.json | 25 ------- .../fixtures/sse_apply_patch_update.json | 25 ------- .../fixtures/sse_response_completed.json | 16 ----- codex-rs/exec/tests/suite/apply_patch.rs | 71 ++++++++++++++----- codex-rs/exec/tests/suite/common.rs | 6 +- 8 files changed, 86 insertions(+), 140 deletions(-) delete mode 100644 codex-rs/exec/tests/fixtures/sse_apply_patch_add.json delete mode 100644 codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_add.json delete mode 100644 codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_update.json delete mode 100644 codex-rs/exec/tests/fixtures/sse_apply_patch_update.json delete mode 100644 codex-rs/exec/tests/fixtures/sse_response_completed.json diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index dc6f849ee774..2f55f17a5229 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -74,6 +74,39 @@ pub fn ev_function_call(call_id: &str, name: &str, arguments: &str) -> Value { }) } +/// Convenience: SSE event for an `apply_patch` custom tool call with raw patch +/// text. This mirrors the payload produced by the Responses API when the model +/// invokes `apply_patch` directly (before we convert it to a function call). +pub fn ev_apply_patch_custom_tool_call(call_id: &str, patch: &str) -> Value { + serde_json::json!({ + "type": "response.output_item.done", + "item": { + "type": "custom_tool_call", + "name": "apply_patch", + "input": patch, + "call_id": call_id + } + }) +} + +/// Convenience: SSE event for an `apply_patch` function call. The Responses API +/// wraps the patch content in a JSON string under the `input` key; we recreate +/// the same structure so downstream code exercises the full parsing path. +pub fn ev_apply_patch_function_call(call_id: &str, patch: &str) -> Value { + let arguments = serde_json::json!({ "input": patch }); + let arguments = serde_json::to_string(&arguments).expect("serialize apply_patch arguments"); + + serde_json::json!({ + "type": "response.output_item.done", + "item": { + "type": "function_call", + "name": "apply_patch", + "arguments": arguments, + "call_id": call_id + } + }) +} + pub fn sse_response(body: String) -> ResponseTemplate { ResponseTemplate::new(200) .insert_header("content-type", "text/event-stream") diff --git a/codex-rs/exec/tests/fixtures/sse_apply_patch_add.json b/codex-rs/exec/tests/fixtures/sse_apply_patch_add.json deleted file mode 100644 index 8d2bf261af64..000000000000 --- a/codex-rs/exec/tests/fixtures/sse_apply_patch_add.json +++ /dev/null @@ -1,25 +0,0 @@ -[ - { - "type": "response.output_item.done", - "item": { - "type": "custom_tool_call", - "name": "apply_patch", - "input": "*** Begin Patch\n*** Add File: test.md\n+Hello world\n*** End Patch", - "call_id": "__ID__" - } - }, - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -] diff --git a/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_add.json b/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_add.json deleted file mode 100644 index ce05e7d48272..000000000000 --- a/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_add.json +++ /dev/null @@ -1,25 +0,0 @@ -[ - { - "type": "response.output_item.done", - "item": { - "type": "custom_tool_call", - "name": "apply_patch", - "input": "*** Begin Patch\n*** Add File: app.py\n+class BaseClass:\n+ def method():\n+ return False\n*** End Patch", - "call_id": "__ID__" - } - }, - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -] diff --git a/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_update.json b/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_update.json deleted file mode 100644 index 8329d9628c9e..000000000000 --- a/codex-rs/exec/tests/fixtures/sse_apply_patch_freeform_update.json +++ /dev/null @@ -1,25 +0,0 @@ -[ - { - "type": "response.output_item.done", - "item": { - "type": "custom_tool_call", - "name": "apply_patch", - "input": "*** Begin Patch\n*** Update File: app.py\n@@ def method():\n- return False\n+\n+ return True\n*** End Patch", - "call_id": "__ID__" - } - }, - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -] diff --git a/codex-rs/exec/tests/fixtures/sse_apply_patch_update.json b/codex-rs/exec/tests/fixtures/sse_apply_patch_update.json deleted file mode 100644 index 79689bece35e..000000000000 --- a/codex-rs/exec/tests/fixtures/sse_apply_patch_update.json +++ /dev/null @@ -1,25 +0,0 @@ -[ - { - "type": "response.output_item.done", - "item": { - "type": "function_call", - "name": "apply_patch", - "arguments": "{\n \"input\": \"*** Begin Patch\\n*** Update File: test.md\\n@@\\n-Hello world\\n+Final text\\n*** End Patch\"\n}", - "call_id": "__ID__" - } - }, - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -] diff --git a/codex-rs/exec/tests/fixtures/sse_response_completed.json b/codex-rs/exec/tests/fixtures/sse_response_completed.json deleted file mode 100644 index 1774dc5e8452..000000000000 --- a/codex-rs/exec/tests/fixtures/sse_response_completed.json +++ /dev/null @@ -1,16 +0,0 @@ -[ - { - "type": "response.completed", - "response": { - "id": "__ID__", - "usage": { - "input_tokens": 0, - "input_tokens_details": null, - "output_tokens": 0, - "output_tokens_details": null, - "total_tokens": 0 - }, - "output": [] - } - } -] diff --git a/codex-rs/exec/tests/suite/apply_patch.rs b/codex-rs/exec/tests/suite/apply_patch.rs index 5537853b0221..489f34f9ce8f 100644 --- a/codex-rs/exec/tests/suite/apply_patch.rs +++ b/codex-rs/exec/tests/suite/apply_patch.rs @@ -1,8 +1,12 @@ -#![allow(clippy::expect_used, clippy::unwrap_used)] +#![allow(clippy::expect_used, clippy::unwrap_used, unused_imports)] use anyhow::Context; use assert_cmd::prelude::*; use codex_core::CODEX_APPLY_PATCH_ARG1; +use core_test_support::responses::ev_apply_patch_custom_tool_call; +use core_test_support::responses::ev_apply_patch_function_call; +use core_test_support::responses::ev_completed; +use core_test_support::responses::sse; use std::fs; use std::process::Command; use tempfile::tempdir; @@ -55,15 +59,28 @@ async fn test_apply_patch_tool() -> anyhow::Result<()> { let tmp_cwd = tempdir().expect("failed to create temp dir"); let tmp_path = tmp_cwd.path().to_path_buf(); - run_e2e_exec_test( - tmp_cwd.path(), - vec![ - include_str!("../fixtures/sse_apply_patch_add.json").to_string(), - include_str!("../fixtures/sse_apply_patch_update.json").to_string(), - include_str!("../fixtures/sse_response_completed.json").to_string(), - ], - ) - .await; + let add_patch = r#"*** Begin Patch +*** Add File: test.md ++Hello world +*** End Patch"#; + let update_patch = r#"*** Begin Patch +*** Update File: test.md +@@ +-Hello world ++Final text +*** End Patch"#; + let response_streams = vec![ + sse(vec![ + ev_apply_patch_custom_tool_call("request_0", add_patch), + ev_completed("request_0"), + ]), + sse(vec![ + ev_apply_patch_function_call("request_1", update_patch), + ev_completed("request_1"), + ]), + sse(vec![ev_completed("request_2")]), + ]; + run_e2e_exec_test(tmp_cwd.path(), response_streams).await; let final_path = tmp_path.join("test.md"); let contents = std::fs::read_to_string(&final_path) @@ -86,15 +103,31 @@ async fn test_apply_patch_freeform_tool() -> anyhow::Result<()> { } let tmp_cwd = tempdir().expect("failed to create temp dir"); - run_e2e_exec_test( - tmp_cwd.path(), - vec![ - include_str!("../fixtures/sse_apply_patch_freeform_add.json").to_string(), - include_str!("../fixtures/sse_apply_patch_freeform_update.json").to_string(), - include_str!("../fixtures/sse_response_completed.json").to_string(), - ], - ) - .await; + let freeform_add_patch = r#"*** Begin Patch +*** Add File: app.py ++class BaseClass: ++ def method(): ++ return False +*** End Patch"#; + let freeform_update_patch = r#"*** Begin Patch +*** Update File: app.py +@@ def method(): +- return False ++ ++ return True +*** End Patch"#; + let response_streams = vec![ + sse(vec![ + ev_apply_patch_custom_tool_call("request_0", freeform_add_patch), + ev_completed("request_0"), + ]), + sse(vec![ + ev_apply_patch_custom_tool_call("request_1", freeform_update_patch), + ev_completed("request_1"), + ]), + sse(vec![ev_completed("request_2")]), + ]; + run_e2e_exec_test(tmp_cwd.path(), response_streams).await; // Verify final file contents let final_path = tmp_cwd.path().join("app.py"); diff --git a/codex-rs/exec/tests/suite/common.rs b/codex-rs/exec/tests/suite/common.rs index 4a3719aabaf1..19de9a3ef66d 100644 --- a/codex-rs/exec/tests/suite/common.rs +++ b/codex-rs/exec/tests/suite/common.rs @@ -4,7 +4,6 @@ use anyhow::Context; use assert_cmd::prelude::*; -use core_test_support::load_sse_fixture_with_id_from_str; use std::path::Path; use std::process::Command; use std::sync::atomic::AtomicUsize; @@ -27,10 +26,7 @@ impl Respond for SeqResponder { match self.responses.get(call_num) { Some(body) => wiremock::ResponseTemplate::new(200) .insert_header("content-type", "text/event-stream") - .set_body_raw( - load_sse_fixture_with_id_from_str(body, &format!("request_{call_num}")), - "text/event-stream", - ), + .set_body_string(body.clone()), None => panic!("no response for {call_num}"), } } From ff389dc52f257d60d1978d34938dc7ab3cefdd15 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Fri, 19 Sep 2025 12:08:04 -0700 Subject: [PATCH 06/10] fix alignment in slash command popup (#3937) --- codex-rs/tui/src/bottom_pane/command_popup.rs | 77 ++++++++----------- .../src/bottom_pane/selection_popup_common.rs | 27 ++++++- ..._chat_composer__tests__slash_popup_mo.snap | 4 +- ..._list_selection_spacing_with_subtitle.snap | 4 +- ...st_selection_spacing_without_subtitle.snap | 4 +- 5 files changed, 60 insertions(+), 56 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index 8de266f81904..a492a2e17ef1 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -95,32 +95,10 @@ impl CommandPopup { /// Determine the preferred height of the popup for a given width. /// Accounts for wrapped descriptions so that long tooltips don't overflow. pub(crate) fn calculate_required_height(&self, width: u16) -> u16 { - use super::selection_popup_common::GenericDisplayRow; use super::selection_popup_common::measure_rows_height; - let matches = self.filtered(); - let rows_all: Vec = if matches.is_empty() { - Vec::new() - } else { - matches - .into_iter() - .map(|(item, indices, _)| match item { - CommandItem::Builtin(cmd) => GenericDisplayRow { - name: format!("/{}", cmd.command()), - match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()), - is_current: false, - description: Some(cmd.description().to_string()), - }, - CommandItem::UserPrompt(i) => GenericDisplayRow { - name: format!("/{}", self.prompts[i].name), - match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()), - is_current: false, - description: Some("send saved prompt".to_string()), - }, - }) - .collect() - }; + let rows = self.rows_from_matches(self.filtered()); - measure_rows_height(&rows_all, &self.state, MAX_POPUP_ROWS, width) + measure_rows_height(&rows, &self.state, MAX_POPUP_ROWS, width) } /// Compute fuzzy-filtered matches over built-in commands and user prompts, @@ -172,6 +150,32 @@ impl CommandPopup { self.filtered().into_iter().map(|(c, _, _)| c).collect() } + fn rows_from_matches( + &self, + matches: Vec<(CommandItem, Option>, i32)>, + ) -> Vec { + matches + .into_iter() + .map(|(item, indices, _)| { + let (name, description) = match item { + CommandItem::Builtin(cmd) => { + (format!("/{}", cmd.command()), cmd.description().to_string()) + } + CommandItem::UserPrompt(i) => ( + format!("/{}", self.prompts[i].name), + "send saved prompt".to_string(), + ), + }; + GenericDisplayRow { + name, + match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()), + is_current: false, + description: Some(description), + } + }) + .collect() + } + /// Move the selection cursor one step up. pub(crate) fn move_up(&mut self) { let len = self.filtered_items().len(); @@ -198,32 +202,11 @@ impl CommandPopup { impl WidgetRef for CommandPopup { fn render_ref(&self, area: Rect, buf: &mut Buffer) { - let matches = self.filtered(); - let rows_all: Vec = if matches.is_empty() { - Vec::new() - } else { - matches - .into_iter() - .map(|(item, indices, _)| match item { - CommandItem::Builtin(cmd) => GenericDisplayRow { - name: format!("/{}", cmd.command()), - match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()), - is_current: false, - description: Some(cmd.description().to_string()), - }, - CommandItem::UserPrompt(i) => GenericDisplayRow { - name: format!("/{}", self.prompts[i].name), - match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()), - is_current: false, - description: Some("send saved prompt".to_string()), - }, - }) - .collect() - }; + let rows = self.rows_from_matches(self.filtered()); render_rows( area, buf, - &rows_all, + &rows, &self.state, MAX_POPUP_ROWS, false, diff --git a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs index 61a26f93418f..684924a44a05 100644 --- a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs +++ b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs @@ -15,6 +15,7 @@ use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; use super::scroll_state::ScrollState; +use crate::ui_consts::LIVE_PREFIX_COLS; /// A generic representation of a display row for selection popups. pub(crate) struct GenericDisplayRow { @@ -99,14 +100,34 @@ pub(crate) fn render_rows( .border_style(Style::default().add_modifier(Modifier::DIM)); block.render(area, buf); - // Content renders to the right of the border. + // Content renders to the right of the border with the same live prefix + // padding used by the composer so the popup aligns with the input text. + let prefix_cols = LIVE_PREFIX_COLS; let content_area = Rect { - x: area.x.saturating_add(1), + x: area.x.saturating_add(prefix_cols), y: area.y, - width: area.width.saturating_sub(1), + width: area.width.saturating_sub(prefix_cols), height: area.height, }; + // Clear the padding column(s) so stale characters never peek between the + // border and the popup contents. + let padding_cols = prefix_cols.saturating_sub(1); + if padding_cols > 0 { + let pad_start = area.x.saturating_add(1); + let pad_end = pad_start + .saturating_add(padding_cols) + .min(area.x.saturating_add(area.width)); + let pad_bottom = area.y.saturating_add(area.height); + for x in pad_start..pad_end { + for y in area.y..pad_bottom { + if let Some(cell) = buf.cell_mut((x, y)) { + cell.set_symbol(" "); + } + } + } + } + if rows_all.is_empty() { if content_area.height > 0 { let para = Paragraph::new(Line::from(empty_message.dim().italic())); diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap index f908fb6144cb..9c667c0d0868 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap @@ -4,5 +4,5 @@ expression: terminal.backend() --- "▌ /mo " "▌ " -"▌/model choose what model and reasoning effort to use " -"▌/mention mention a file " +"▌ /model choose what model and reasoning effort to use " +"▌ /mention mention a file " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_with_subtitle.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_with_subtitle.snap index 65606ed7d06c..ced53b7ce6e1 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_with_subtitle.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_with_subtitle.snap @@ -5,7 +5,7 @@ expression: render_lines(&view) ▌ Select Approval Mode ▌ Switch between Codex approval presets ▌ -▌> 1. Read Only (current) Codex can read files -▌ 2. Full Access Codex can edit files +▌ > 1. Read Only (current) Codex can read files +▌ 2. Full Access Codex can edit files Press Enter to confirm or Esc to go back diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_without_subtitle.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_without_subtitle.snap index b42a5f8c6b03..b9858a430706 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_without_subtitle.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_spacing_without_subtitle.snap @@ -4,7 +4,7 @@ expression: render_lines(&view) --- ▌ Select Approval Mode ▌ -▌> 1. Read Only (current) Codex can read files -▌ 2. Full Access Codex can edit files +▌ > 1. Read Only (current) Codex can read files +▌ 2. Full Access Codex can edit files Press Enter to confirm or Esc to go back From ad0c2b4db302cdc89b9c52e33ec780dae85cb46a Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Fri, 19 Sep 2025 14:22:58 -0700 Subject: [PATCH 07/10] don't clear screen on startup (#3925) --- codex-rs/tui/src/tui.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 6d7b9e810106..cacbc7165c3c 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -14,7 +14,7 @@ use std::time::Instant; use crossterm::Command; use crossterm::SynchronizedUpdate; -use crossterm::cursor; +#[cfg(unix)] use crossterm::cursor::MoveTo; use crossterm::event::DisableBracketedPaste; use crossterm::event::DisableFocusChange; @@ -27,7 +27,6 @@ use crossterm::event::PopKeyboardEnhancementFlags; use crossterm::event::PushKeyboardEnhancementFlags; use crossterm::terminal::EnterAlternateScreen; use crossterm::terminal::LeaveAlternateScreen; -use crossterm::terminal::ScrollUp; use ratatui::backend::Backend; use ratatui::backend::CrosstermBackend; use ratatui::crossterm::execute; @@ -127,15 +126,6 @@ pub fn init() -> Result { set_panic_hook(); - // Instead of clearing the screen (which can drop scrollback in some terminals), - // scroll existing lines up until the cursor reaches the top, then start at (0, 0). - if let Ok((_x, y)) = cursor::position() - && y > 0 - { - execute!(stdout(), ScrollUp(y))?; - } - execute!(stdout(), MoveTo(0, 0))?; - let backend = CrosstermBackend::new(stdout()); let tui = CustomTerminal::with_options(backend)?; Ok(tui) From 42d335deb878c2b558e663bd3f6a2d52c578db05 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Fri, 19 Sep 2025 14:38:36 -0700 Subject: [PATCH 08/10] Cache keyboard enhancement detection before event streams (#3950) Hopefully fixes incorrectly showing ^J instead of Shift+Enter in the key hints occasionally. --- codex-rs/tui/src/app.rs | 3 +-- codex-rs/tui/src/tui.rs | 11 +++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index bd9150cad0a0..f6b735b10bd6 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -22,7 +22,6 @@ use color_eyre::eyre::WrapErr; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; -use crossterm::terminal::supports_keyboard_enhancement; use ratatui::style::Stylize; use ratatui::text::Line; use std::path::PathBuf; @@ -85,7 +84,7 @@ impl App { let conversation_manager = Arc::new(ConversationManager::new(auth_manager.clone())); - let enhanced_keys_supported = supports_keyboard_enhancement().unwrap_or(false); + let enhanced_keys_supported = tui.enhanced_keys_supported(); let chat_widget = match resume_selection { ResumeSelection::StartFresh | ResumeSelection::Exit => { diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index cacbc7165c3c..eeabec4e04c5 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -27,6 +27,7 @@ use crossterm::event::PopKeyboardEnhancementFlags; use crossterm::event::PushKeyboardEnhancementFlags; use crossterm::terminal::EnterAlternateScreen; use crossterm::terminal::LeaveAlternateScreen; +use crossterm::terminal::supports_keyboard_enhancement; use ratatui::backend::Backend; use ratatui::backend::CrosstermBackend; use ratatui::crossterm::execute; @@ -160,6 +161,7 @@ pub struct Tui { alt_screen_active: Arc, // True when terminal/tab is focused; updated internally from crossterm events terminal_focused: Arc, + enhanced_keys_supported: bool, } #[cfg(unix)] @@ -266,6 +268,10 @@ impl Tui { } }); + // Detect keyboard enhancement support before any EventStream is created so the + // crossterm poller can acquire its lock without contention. + let enhanced_keys_supported = supports_keyboard_enhancement().unwrap_or(false); + Self { frame_schedule_tx, draw_tx, @@ -278,6 +284,7 @@ impl Tui { suspend_cursor_y: Arc::new(AtomicU16::new(0)), alt_screen_active: Arc::new(AtomicBool::new(false)), terminal_focused: Arc::new(AtomicBool::new(true)), + enhanced_keys_supported, } } @@ -287,6 +294,10 @@ impl Tui { } } + pub fn enhanced_keys_supported(&self) -> bool { + self.enhanced_keys_supported + } + pub fn event_stream(&self) -> Pin + Send + 'static>> { use tokio_stream::StreamExt; let mut crossterm_events = crossterm::event::EventStream::new(); From 04504d8218465326ed19f967e773b29b152e916b Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Sat, 20 Sep 2025 21:26:16 -0700 Subject: [PATCH 09/10] Forward Rate limits to the UI (#3965) We currently get information about rate limits in the response headers. We want to forward them to the clients to have better transparency. UI/UX plans have been discussed and this information is needed. --- codex-rs/core/src/chat_completions.rs | 3 + codex-rs/core/src/client.rs | 43 ++++++++++++ codex-rs/core/src/client_common.rs | 2 + codex-rs/core/src/codex.rs | 46 +++++++++---- codex-rs/core/tests/suite/client.rs | 95 +++++++++++++++++++++++++++ codex-rs/protocol/src/protocol.rs | 15 +++++ 6 files changed, 192 insertions(+), 12 deletions(-) diff --git a/codex-rs/core/src/chat_completions.rs b/codex-rs/core/src/chat_completions.rs index fc8602de8ebe..f666cc119f4f 100644 --- a/codex-rs/core/src/chat_completions.rs +++ b/codex-rs/core/src/chat_completions.rs @@ -716,6 +716,9 @@ where // Not an assistant message – forward immediately. return Poll::Ready(Some(Ok(ResponseEvent::OutputItemDone(item)))); } + Poll::Ready(Some(Ok(ResponseEvent::RateLimits(snapshot)))) => { + return Poll::Ready(Some(Ok(ResponseEvent::RateLimits(snapshot)))); + } Poll::Ready(Some(Ok(ResponseEvent::Completed { response_id, token_usage, diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 055c3afa870e..57848d388b4a 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -11,6 +11,7 @@ use eventsource_stream::Eventsource; use futures::prelude::*; use regex_lite::Regex; use reqwest::StatusCode; +use reqwest::header::HeaderMap; use serde::Deserialize; use serde::Serialize; use serde_json::Value; @@ -40,6 +41,7 @@ use crate::model_provider_info::ModelProviderInfo; use crate::model_provider_info::WireApi; use crate::openai_model_info::get_model_info; use crate::openai_tools::create_tools_json_for_responses_api; +use crate::protocol::RateLimitSnapshotEvent; use crate::protocol::TokenUsage; use crate::token_data::PlanType; use crate::util::backoff; @@ -274,6 +276,15 @@ impl ModelClient { Ok(resp) if resp.status().is_success() => { let (tx_event, rx_event) = mpsc::channel::>(1600); + if let Some(snapshot) = parse_rate_limit_snapshot(resp.headers()) + && tx_event + .send(Ok(ResponseEvent::RateLimits(snapshot))) + .await + .is_err() + { + debug!("receiver dropped rate limit snapshot event"); + } + // spawn task to process SSE let stream = resp.bytes_stream().map_err(CodexErr::Reqwest); tokio::spawn(process_sse( @@ -473,6 +484,38 @@ fn attach_item_ids(payload_json: &mut Value, original_items: &[ResponseItem]) { } } +fn parse_rate_limit_snapshot(headers: &HeaderMap) -> Option { + let primary_used_percent = parse_header_f64(headers, "x-codex-primary-used-percent")?; + let weekly_used_percent = parse_header_f64(headers, "x-codex-protection-used-percent")?; + let primary_to_weekly_ratio_percent = + parse_header_f64(headers, "x-codex-primary-over-protection-limit-percent")?; + let primary_window_minutes = parse_header_u64(headers, "x-codex-primary-window-minutes")?; + let weekly_window_minutes = parse_header_u64(headers, "x-codex-protection-window-minutes")?; + + Some(RateLimitSnapshotEvent { + primary_used_percent, + weekly_used_percent, + primary_to_weekly_ratio_percent, + primary_window_minutes, + weekly_window_minutes, + }) +} + +fn parse_header_f64(headers: &HeaderMap, name: &str) -> Option { + parse_header_str(headers, name)? + .parse::() + .ok() + .filter(|v| v.is_finite()) +} + +fn parse_header_u64(headers: &HeaderMap, name: &str) -> Option { + parse_header_str(headers, name)?.parse::().ok() +} + +fn parse_header_str<'a>(headers: &'a HeaderMap, name: &str) -> Option<&'a str> { + headers.get(name)?.to_str().ok() +} + async fn process_sse( stream: S, tx_event: mpsc::Sender>, diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index 3a5eb5b16b7c..15bfb5d4001a 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -1,6 +1,7 @@ use crate::error::Result; use crate::model_family::ModelFamily; use crate::openai_tools::OpenAiTool; +use crate::protocol::RateLimitSnapshotEvent; use crate::protocol::TokenUsage; use codex_apply_patch::APPLY_PATCH_TOOL_INSTRUCTIONS; use codex_protocol::config_types::ReasoningEffort as ReasoningEffortConfig; @@ -78,6 +79,7 @@ pub enum ResponseEvent { WebSearchCallBegin { call_id: String, }, + RateLimits(RateLimitSnapshotEvent), } #[derive(Debug, Serialize)] diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 8852b25834d2..05ef09377e87 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -98,6 +98,7 @@ use crate::protocol::ListCustomPromptsResponseEvent; use crate::protocol::Op; use crate::protocol::PatchApplyBeginEvent; use crate::protocol::PatchApplyEndEvent; +use crate::protocol::RateLimitSnapshotEvent; use crate::protocol::ReviewDecision; use crate::protocol::ReviewOutputEvent; use crate::protocol::SandboxPolicy; @@ -105,6 +106,7 @@ use crate::protocol::SessionConfiguredEvent; use crate::protocol::StreamErrorEvent; use crate::protocol::Submission; use crate::protocol::TaskCompleteEvent; +use crate::protocol::TokenCountEvent; use crate::protocol::TokenUsage; use crate::protocol::TokenUsageInfo; use crate::protocol::TurnDiffEvent; @@ -257,6 +259,7 @@ struct State { pending_input: Vec, history: ConversationHistory, token_info: Option, + latest_rate_limits: Option, } /// Context for an initialized model agent @@ -738,16 +741,30 @@ impl Session { async fn update_token_usage_info( &self, turn_context: &TurnContext, - token_usage: &Option, - ) -> Option { + token_usage: Option<&TokenUsage>, + ) { let mut state = self.state.lock().await; - let info = TokenUsageInfo::new_or_append( - &state.token_info, - token_usage, - turn_context.client.get_model_context_window(), - ); - state.token_info = info.clone(); - info + if let Some(token_usage) = token_usage { + let info = TokenUsageInfo::new_or_append( + &state.token_info, + &Some(token_usage.clone()), + turn_context.client.get_model_context_window(), + ); + state.token_info = info; + } + } + + async fn update_rate_limits(&self, new_rate_limits: RateLimitSnapshotEvent) { + let mut state = self.state.lock().await; + state.latest_rate_limits = Some(new_rate_limits); + } + + async fn get_token_count_event(&self) -> TokenCountEvent { + let state = self.state.lock().await; + TokenCountEvent { + info: state.token_info.clone(), + rate_limits: state.latest_rate_limits.clone(), + } } /// Record a user input item to conversation history and also persist a @@ -2136,17 +2153,22 @@ async fn try_run_turn( }) .await; } + ResponseEvent::RateLimits(snapshot) => { + // Update internal state with latest rate limits, but defer sending until + // token usage is available to avoid duplicate TokenCount events. + sess.update_rate_limits(snapshot).await; + } ResponseEvent::Completed { response_id: _, token_usage, } => { - let info = sess - .update_token_usage_info(turn_context, &token_usage) + sess.update_token_usage_info(turn_context, token_usage.as_ref()) .await; + let token_event = sess.get_token_count_event().await; let _ = sess .send_event(Event { id: sub_id.to_string(), - msg: EventMsg::TokenCount(crate::protocol::TokenCountEvent { info }), + msg: EventMsg::TokenCount(token_event), }) .await; diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index cfc6f5f40c41..d0ae608cb02b 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -22,6 +22,7 @@ use codex_protocol::models::ReasoningItemReasoningSummary; use codex_protocol::models::WebSearchAction; use core_test_support::load_default_config_for_test; use core_test_support::load_sse_fixture_with_id; +use core_test_support::responses; use core_test_support::wait_for_event; use futures::StreamExt; use serde_json::json; @@ -776,6 +777,100 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() { assert_eq!(body["input"][5]["id"].as_str(), Some("custom-tool-id")); } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn token_count_includes_rate_limits_snapshot() { + let server = MockServer::start().await; + + let sse_body = responses::sse(vec![responses::ev_completed_with_tokens("resp_rate", 123)]); + + let response = ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .insert_header("x-codex-primary-used-percent", "12.5") + .insert_header("x-codex-protection-used-percent", "40.0") + .insert_header("x-codex-primary-over-protection-limit-percent", "75.0") + .insert_header("x-codex-primary-window-minutes", "10") + .insert_header("x-codex-protection-window-minutes", "60") + .set_body_raw(sse_body, "text/event-stream"); + + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with(response) + .expect(1) + .mount(&server) + .await; + + let mut provider = built_in_model_providers()["openai"].clone(); + provider.base_url = Some(format!("{}/v1", server.uri())); + + let home = TempDir::new().unwrap(); + let mut config = load_default_config_for_test(&home); + config.model_provider = provider; + + let conversation_manager = ConversationManager::with_auth(CodexAuth::from_api_key("test")); + let codex = conversation_manager + .new_conversation(config) + .await + .expect("create conversation") + .conversation; + + codex + .submit(Op::UserInput { + items: vec![InputItem::Text { + text: "hello".into(), + }], + }) + .await + .unwrap(); + + let token_event = wait_for_event(&codex, |msg| matches!(msg, EventMsg::TokenCount(_))).await; + let final_payload = match token_event { + EventMsg::TokenCount(ev) => ev, + _ => unreachable!(), + }; + // Assert full JSON for the final token count event (usage + rate limits) + let final_json = serde_json::to_value(&final_payload).unwrap(); + pretty_assertions::assert_eq!( + final_json, + json!({ + "info": { + "total_token_usage": { + "input_tokens": 123, + "cached_input_tokens": 0, + "output_tokens": 0, + "reasoning_output_tokens": 0, + "total_tokens": 123 + }, + "last_token_usage": { + "input_tokens": 123, + "cached_input_tokens": 0, + "output_tokens": 0, + "reasoning_output_tokens": 0, + "total_tokens": 123 + }, + // Default model is gpt-5 in tests → 272000 context window + "model_context_window": 272000 + }, + "rate_limits": { + "primary_used_percent": 12.5, + "weekly_used_percent": 40.0, + "primary_to_weekly_ratio_percent": 75.0, + "primary_window_minutes": 10, + "weekly_window_minutes": 60 + } + }) + ); + let usage = final_payload + .info + .expect("token usage info should be recorded after completion"); + assert_eq!(usage.total_token_usage.total_tokens, 123); + let final_snapshot = final_payload + .rate_limits + .expect("latest rate limit snapshot should be retained"); + assert_eq!(final_snapshot.primary_used_percent, 12.5); + + wait_for_event(&codex, |msg| matches!(msg, EventMsg::TaskComplete(_))).await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn azure_overrides_assign_properties_used_for_responses_url() { let existing_env_var_with_random_value = if cfg!(windows) { "USERNAME" } else { "USER" }; diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 8009ac9bf3fd..edcdcbebf832 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -589,6 +589,21 @@ impl TokenUsageInfo { #[derive(Debug, Clone, Deserialize, Serialize, TS)] pub struct TokenCountEvent { pub info: Option, + pub rate_limits: Option, +} + +#[derive(Debug, Clone, Deserialize, Serialize, TS)] +pub struct RateLimitSnapshotEvent { + /// Percentage (0-100) of the primary window that has been consumed. + pub primary_used_percent: f64, + /// Percentage (0-100) of the protection window that has been consumed. + pub weekly_used_percent: f64, + /// Size of the primary window relative to weekly (0-100). + pub primary_to_weekly_ratio_percent: f64, + /// Rolling window duration for the primary limit, in minutes. + pub primary_window_minutes: u64, + /// Rolling window duration for the weekly limit, in minutes. + pub weekly_window_minutes: u64, } // Includes prompts, tools and space to call compact. From a4ebd069e566e91b5ad53f58ab337d0edbacc59e Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Sun, 21 Sep 2025 10:20:49 -0700 Subject: [PATCH 10/10] Tui: Rate limits (#3977) ### /limits: show rate limits graph image ### Warning on close to rate limits: image Based on #3965 --- codex-rs/tui/src/chatwidget.rs | 75 ++- ...chatwidget__tests__limits_placeholder.snap | 10 + ...twidget__tests__limits_snapshot_basic.snap | 29 + ...sts__limits_snapshot_hourly_remaining.snap | 29 + ...t__tests__limits_snapshot_mixed_usage.snap | 29 + ...__tests__limits_snapshot_weekly_heavy.snap | 29 + codex-rs/tui/src/chatwidget/tests.rs | 157 ++++++ codex-rs/tui/src/history_cell.rs | 45 ++ codex-rs/tui/src/lib.rs | 1 + codex-rs/tui/src/rate_limits_view.rs | 504 ++++++++++++++++++ codex-rs/tui/src/slash_command.rs | 3 + 11 files changed, 910 insertions(+), 1 deletion(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_placeholder.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_basic.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_hourly_remaining.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_mixed_usage.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_weekly_heavy.snap create mode 100644 codex-rs/tui/src/rate_limits_view.rs diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index e024fd0fab81..cb8acd04fb59 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -28,6 +28,7 @@ use codex_core::protocol::McpToolCallBeginEvent; use codex_core::protocol::McpToolCallEndEvent; use codex_core::protocol::Op; use codex_core::protocol::PatchApplyBeginEvent; +use codex_core::protocol::RateLimitSnapshotEvent; use codex_core::protocol::ReviewRequest; use codex_core::protocol::StreamErrorEvent; use codex_core::protocol::TaskCompleteEvent; @@ -103,6 +104,42 @@ struct RunningCommand { parsed_cmd: Vec, } +const RATE_LIMIT_WARNING_THRESHOLDS: [f64; 3] = [50.0, 75.0, 90.0]; + +#[derive(Default)] +struct RateLimitWarningState { + weekly_index: usize, + hourly_index: usize, +} + +impl RateLimitWarningState { + fn take_warnings(&mut self, weekly_used_percent: f64, hourly_used_percent: f64) -> Vec { + let mut warnings = Vec::new(); + + while self.weekly_index < RATE_LIMIT_WARNING_THRESHOLDS.len() + && weekly_used_percent >= RATE_LIMIT_WARNING_THRESHOLDS[self.weekly_index] + { + let threshold = RATE_LIMIT_WARNING_THRESHOLDS[self.weekly_index]; + warnings.push(format!( + "Weekly usage exceeded {threshold:.0}% of the limit. Run /limits for detailed usage." + )); + self.weekly_index += 1; + } + + while self.hourly_index < RATE_LIMIT_WARNING_THRESHOLDS.len() + && hourly_used_percent >= RATE_LIMIT_WARNING_THRESHOLDS[self.hourly_index] + { + let threshold = RATE_LIMIT_WARNING_THRESHOLDS[self.hourly_index]; + warnings.push(format!( + "Hourly usage exceeded {threshold:.0}% of the limit. Run /limits for detailed usage." + )); + self.hourly_index += 1; + } + + warnings + } +} + /// Common initialization parameters shared by all `ChatWidget` constructors. pub(crate) struct ChatWidgetInit { pub(crate) config: Config, @@ -124,6 +161,8 @@ pub(crate) struct ChatWidget { session_header: SessionHeader, initial_user_message: Option, token_info: Option, + rate_limit_snapshot: Option, + rate_limit_warnings: RateLimitWarningState, // Stream lifecycle controller stream: StreamController, running_commands: HashMap, @@ -285,6 +324,21 @@ impl ChatWidget { self.bottom_pane.set_token_usage(info.clone()); self.token_info = info; } + + fn on_rate_limit_snapshot(&mut self, snapshot: Option) { + if let Some(snapshot) = snapshot { + let warnings = self + .rate_limit_warnings + .take_warnings(snapshot.weekly_used_percent, snapshot.primary_used_percent); + self.rate_limit_snapshot = Some(snapshot); + if !warnings.is_empty() { + for warning in warnings { + self.add_to_history(history_cell::new_warning_event(warning)); + } + self.request_redraw(); + } + } + } /// Finalize any active exec as failed and stop/clear running UI state. fn finalize_turn(&mut self) { // Ensure any spinner is replaced by a red ✗ and flushed into history. @@ -699,6 +753,8 @@ impl ChatWidget { initial_images, ), token_info: None, + rate_limit_snapshot: None, + rate_limit_warnings: RateLimitWarningState::default(), stream: StreamController::new(config), running_commands: HashMap::new(), task_complete_pending: false, @@ -756,6 +812,8 @@ impl ChatWidget { initial_images, ), token_info: None, + rate_limit_snapshot: None, + rate_limit_warnings: RateLimitWarningState::default(), stream: StreamController::new(config), running_commands: HashMap::new(), task_complete_pending: false, @@ -929,6 +987,9 @@ impl ChatWidget { SlashCommand::Status => { self.add_status_output(); } + SlashCommand::Limits => { + self.add_limits_output(); + } SlashCommand::Mcp => { self.add_mcp_output(); } @@ -1106,7 +1167,10 @@ impl ChatWidget { EventMsg::TaskComplete(TaskCompleteEvent { last_agent_message }) => { self.on_task_complete(last_agent_message) } - EventMsg::TokenCount(ev) => self.set_token_info(ev.info), + EventMsg::TokenCount(ev) => { + self.set_token_info(ev.info); + self.on_rate_limit_snapshot(ev.rate_limits); + } EventMsg::Error(ErrorEvent { message }) => self.on_error(message), EventMsg::TurnAborted(ev) => match ev.reason { TurnAbortReason::Interrupted => { @@ -1282,6 +1346,15 @@ impl ChatWidget { self.request_redraw(); } + pub(crate) fn add_limits_output(&mut self) { + if let Some(snapshot) = &self.rate_limit_snapshot { + self.add_to_history(history_cell::new_limits_output(snapshot)); + } else { + self.add_to_history(history_cell::new_limits_unavailable()); + } + self.request_redraw(); + } + pub(crate) fn add_status_output(&mut self) { let default_usage; let usage_ref = if let Some(ti) = &self.token_info { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_placeholder.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_placeholder.snap new file mode 100644 index 000000000000..fa37b2201ff7 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_placeholder.snap @@ -0,0 +1,10 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: visual +--- +[magenta]/limits[/] + +[bold]Rate limit usage snapshot[/] +[dim] Tip: run `/limits` right after Codex replies for freshest numbers.[/] + Real usage data is not available yet. +[dim] Send a message to Codex, then run /limits again.[/] diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_basic.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_basic.snap new file mode 100644 index 000000000000..ced446683758 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_basic.snap @@ -0,0 +1,29 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: visual +--- +[magenta]/limits[/] + +[bold]Rate limit usage snapshot[/] +[dim] Tip: run `/limits` right after Codex replies for freshest numbers.[/] + • Hourly limit[dim] (≈5 hours window)[/]: [dark-gray+bold]30.0% used[/] + • Weekly limit[dim] (≈1 week window)[/]: [dark-gray+bold]60.0% used[/] +[green] Within current limits[/] + +[dim] ╭─────────────────────────────────────────────────╮[/] + [dim]│[/][dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/][dim]│[/] + [dim]│[/][dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/][dim]│[/] + [dim]│[/][dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/][dim]│[/] + [dim]│[/][dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/][dim]│[/] + [dim]│[/][dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/][dim]│[/] + [dim]│[/][dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/][dim]│[/] + [dim]│[/][green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/][dim]│[/] + [dim]│[/][green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/][dim]│[/] + [dim]│[/][green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] +[dim] ╰─────────────────────────────────────────────────╯[/] + +[bold]Legend[/] + • [dark-gray+bold]Dark gray[/] = weekly usage so far + • [green+bold]Green[/] = hourly capacity still available + • [bold]Default[/] = weekly capacity beyond the hourly window diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_hourly_remaining.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_hourly_remaining.snap new file mode 100644 index 000000000000..defc5f213c3b --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_hourly_remaining.snap @@ -0,0 +1,29 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: visual +--- +[magenta]/limits[/] + +[bold]Rate limit usage snapshot[/] +[dim] Tip: run `/limits` right after Codex replies for freshest numbers.[/] + • Hourly limit[dim] (≈5 hours window)[/]: [dark-gray+bold]0.0% used[/] + • Weekly limit[dim] (≈1 week window)[/]: [dark-gray+bold]20.0% used[/] +[green] Within current limits[/] + +[dim] ╭─────────────────────────────────────────────────╮[/] + [dim]│[/][dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/][dim]│[/] + [dim]│[/][dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/][dim]│[/] + [dim]│[/][green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/][dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] +[dim] ╰─────────────────────────────────────────────────╯[/] + +[bold]Legend[/] + • [dark-gray+bold]Dark gray[/] = weekly usage so far + • [green+bold]Green[/] = hourly capacity still available + • [bold]Default[/] = weekly capacity beyond the hourly window diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_mixed_usage.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_mixed_usage.snap new file mode 100644 index 000000000000..86c82d9247bf --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_mixed_usage.snap @@ -0,0 +1,29 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: visual +--- +[magenta]/limits[/] + +[bold]Rate limit usage snapshot[/] +[dim] Tip: run `/limits` right after Codex replies for freshest numbers.[/] + • Hourly limit[dim] (≈5 hours window)[/]: [dark-gray+bold]20.0% used[/] + • Weekly limit[dim] (≈1 week window)[/]: [dark-gray+bold]20.0% used[/] +[green] Within current limits[/] + +[dim] ╭─────────────────────────────────────────────────╮[/] + [dim]│[/][dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/][dim]│[/] + [dim]│[/][dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/] [dark-gray](>_)[/][dim]│[/] + [dim]│[/][green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] [green](>_)[/] (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] +[dim] ╰─────────────────────────────────────────────────╯[/] + +[bold]Legend[/] + • [dark-gray+bold]Dark gray[/] = weekly usage so far + • [green+bold]Green[/] = hourly capacity still available + • [bold]Default[/] = weekly capacity beyond the hourly window diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_weekly_heavy.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_weekly_heavy.snap new file mode 100644 index 000000000000..a1650545b61e --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__limits_snapshot_weekly_heavy.snap @@ -0,0 +1,29 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: visual +--- +[magenta]/limits[/] + +[bold]Rate limit usage snapshot[/] +[dim] Tip: run `/limits` right after Codex replies for freshest numbers.[/] + • Hourly limit[dim] (≈5 hours window)[/]: [dark-gray+bold]98.0% used[/] + • Weekly limit[dim] (≈1 week window)[/]: [dark-gray+bold]0.0% used[/] +[green] Within current limits[/] + +[dim] ╭─────────────────────────────────────────────────╮[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] + [dim]│[/](>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_) (>_)[dim]│[/] +[dim] ╰─────────────────────────────────────────────────╯[/] + +[bold]Legend[/] + • [dark-gray+bold]Dark gray[/] = weekly usage so far + • [green+bold]Green[/] = hourly capacity still available + • [bold]Default[/] = weekly capacity beyond the hourly window diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index ffe3f3f70768..7427bb44140a 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -25,6 +25,7 @@ use codex_core::protocol::InputMessageKind; use codex_core::protocol::Op; use codex_core::protocol::PatchApplyBeginEvent; use codex_core::protocol::PatchApplyEndEvent; +use codex_core::protocol::RateLimitSnapshotEvent; use codex_core::protocol::ReviewCodeLocation; use codex_core::protocol::ReviewFinding; use codex_core::protocol::ReviewLineRange; @@ -39,6 +40,8 @@ use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; use insta::assert_snapshot; use pretty_assertions::assert_eq; +use ratatui::style::Color; +use ratatui::style::Modifier; use std::fs::File; use std::io::BufRead; use std::io::BufReader; @@ -320,6 +323,8 @@ fn make_chatwidget_manual() -> ( session_header: SessionHeader::new(cfg.model.clone()), initial_user_message: None, token_info: None, + rate_limit_snapshot: None, + rate_limit_warnings: RateLimitWarningState::default(), stream: StreamController::new(cfg), running_commands: HashMap::new(), task_complete_pending: false, @@ -375,6 +380,158 @@ fn lines_to_single_string(lines: &[ratatui::text::Line<'static>]) -> String { s } +fn styled_lines_to_string(lines: &[ratatui::text::Line<'static>]) -> String { + let mut out = String::new(); + for line in lines { + for span in &line.spans { + let mut tags: Vec<&str> = Vec::new(); + if let Some(color) = span.style.fg { + let name = match color { + Color::Black => "black", + Color::Blue => "blue", + Color::Cyan => "cyan", + Color::DarkGray => "dark-gray", + Color::Gray => "gray", + Color::Green => "green", + Color::LightBlue => "light-blue", + Color::LightCyan => "light-cyan", + Color::LightGreen => "light-green", + Color::LightMagenta => "light-magenta", + Color::LightRed => "light-red", + Color::LightYellow => "light-yellow", + Color::Magenta => "magenta", + Color::Red => "red", + Color::Rgb(_, _, _) => "rgb", + Color::Indexed(_) => "indexed", + Color::Reset => "reset", + Color::Yellow => "yellow", + Color::White => "white", + }; + tags.push(name); + } + let modifiers = span.style.add_modifier; + if modifiers.contains(Modifier::BOLD) { + tags.push("bold"); + } + if modifiers.contains(Modifier::DIM) { + tags.push("dim"); + } + if modifiers.contains(Modifier::ITALIC) { + tags.push("italic"); + } + if modifiers.contains(Modifier::UNDERLINED) { + tags.push("underlined"); + } + if !tags.is_empty() { + out.push('['); + out.push_str(&tags.join("+")); + out.push(']'); + } + out.push_str(&span.content); + if !tags.is_empty() { + out.push_str("[/]"); + } + } + out.push('\n'); + } + out +} + +fn sample_rate_limit_snapshot( + primary_used_percent: f64, + weekly_used_percent: f64, + ratio_percent: f64, +) -> RateLimitSnapshotEvent { + RateLimitSnapshotEvent { + primary_used_percent, + weekly_used_percent, + primary_to_weekly_ratio_percent: ratio_percent, + primary_window_minutes: 300, + weekly_window_minutes: 10_080, + } +} + +fn capture_limits_snapshot(snapshot: Option) -> String { + let lines = match snapshot { + Some(ref snapshot) => history_cell::new_limits_output(snapshot).display_lines(80), + None => history_cell::new_limits_unavailable().display_lines(80), + }; + styled_lines_to_string(&lines) +} + +#[test] +fn limits_placeholder() { + let visual = capture_limits_snapshot(None); + assert_snapshot!(visual); +} + +#[test] +fn limits_snapshot_basic() { + let visual = capture_limits_snapshot(Some(sample_rate_limit_snapshot(30.0, 60.0, 40.0))); + assert_snapshot!(visual); +} + +#[test] +fn limits_snapshot_hourly_remaining() { + let visual = capture_limits_snapshot(Some(sample_rate_limit_snapshot(0.0, 20.0, 10.0))); + assert_snapshot!(visual); +} + +#[test] +fn limits_snapshot_mixed_usage() { + let visual = capture_limits_snapshot(Some(sample_rate_limit_snapshot(20.0, 20.0, 10.0))); + assert_snapshot!(visual); +} + +#[test] +fn limits_snapshot_weekly_heavy() { + let visual = capture_limits_snapshot(Some(sample_rate_limit_snapshot(98.0, 0.0, 10.0))); + assert_snapshot!(visual); +} + +#[test] +fn rate_limit_warnings_emit_thresholds() { + let mut state = RateLimitWarningState::default(); + let mut warnings: Vec = Vec::new(); + + warnings.extend(state.take_warnings(10.0, 55.0)); + warnings.extend(state.take_warnings(55.0, 10.0)); + warnings.extend(state.take_warnings(10.0, 80.0)); + warnings.extend(state.take_warnings(80.0, 10.0)); + warnings.extend(state.take_warnings(10.0, 95.0)); + warnings.extend(state.take_warnings(95.0, 10.0)); + + assert_eq!( + warnings.len(), + 6, + "expected one warning per threshold per limit" + ); + assert!( + warnings + .iter() + .any(|w| w.contains("Hourly usage exceeded 50%")), + "expected hourly 50% warning" + ); + assert!( + warnings + .iter() + .any(|w| w.contains("Weekly usage exceeded 50%")), + "expected weekly 50% warning" + ); + assert!( + warnings + .iter() + .any(|w| w.contains("Hourly usage exceeded 90%")), + "expected hourly 90% warning" + ); + assert!( + warnings + .iter() + .any(|w| w.contains("Weekly usage exceeded 90%")), + "expected weekly 90% warning" + ); +} + // (removed experimental resize snapshot test) #[test] diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index a99a4dd836f0..24e6b2844fae 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -2,6 +2,9 @@ use crate::diff_render::create_diff_summary; use crate::exec_command::relativize_to_home; use crate::exec_command::strip_bash_lc_and_escape; use crate::markdown::append_markdown; +use crate::rate_limits_view::DEFAULT_GRID_CONFIG; +use crate::rate_limits_view::LimitsView; +use crate::rate_limits_view::build_limits_view; use crate::render::line_utils::line_to_static; use crate::render::line_utils::prefix_lines; use crate::render::line_utils::push_owned_lines; @@ -24,6 +27,7 @@ use codex_core::plan_tool::UpdatePlanArgs; use codex_core::project_doc::discover_project_doc_paths; use codex_core::protocol::FileChange; use codex_core::protocol::McpInvocation; +use codex_core::protocol::RateLimitSnapshotEvent; use codex_core::protocol::SandboxPolicy; use codex_core::protocol::SessionConfiguredEvent; use codex_core::protocol::TokenUsage; @@ -221,6 +225,20 @@ impl HistoryCell for PlainHistoryCell { } } +#[derive(Debug)] +pub(crate) struct LimitsHistoryCell { + display: LimitsView, +} + +impl HistoryCell for LimitsHistoryCell { + fn display_lines(&self, width: u16) -> Vec> { + let mut lines = self.display.summary_lines.clone(); + lines.extend(self.display.gauge_lines(width)); + lines.extend(self.display.legend_lines.clone()); + lines + } +} + #[derive(Debug)] pub(crate) struct TranscriptOnlyHistoryCell { lines: Vec>, @@ -1075,6 +1093,33 @@ pub(crate) fn new_completed_mcp_tool_call( Box::new(PlainHistoryCell { lines }) } +pub(crate) fn new_limits_output(snapshot: &RateLimitSnapshotEvent) -> LimitsHistoryCell { + LimitsHistoryCell { + display: build_limits_view(snapshot, DEFAULT_GRID_CONFIG), + } +} + +pub(crate) fn new_limits_unavailable() -> PlainHistoryCell { + PlainHistoryCell { + lines: vec![ + "/limits".magenta().into(), + "".into(), + vec!["Rate limit usage snapshot".bold()].into(), + vec![" Tip: run `/limits` right after Codex replies for freshest numbers.".dim()] + .into(), + vec![" Real usage data is not available yet.".into()].into(), + vec![" Send a message to Codex, then run /limits again.".dim()].into(), + ], + } +} + +#[allow(clippy::disallowed_methods)] +pub(crate) fn new_warning_event(message: String) -> PlainHistoryCell { + PlainHistoryCell { + lines: vec![vec![format!("⚠ {message}").yellow()].into()], + } +} + pub(crate) fn new_status_output( config: &Config, usage: &TokenUsage, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 308e563ded42..92b42732c3d3 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -55,6 +55,7 @@ mod markdown_stream; mod new_model_popup; pub mod onboarding; mod pager_overlay; +mod rate_limits_view; mod render; mod resume_picker; mod session_log; diff --git a/codex-rs/tui/src/rate_limits_view.rs b/codex-rs/tui/src/rate_limits_view.rs new file mode 100644 index 000000000000..72fc6e976398 --- /dev/null +++ b/codex-rs/tui/src/rate_limits_view.rs @@ -0,0 +1,504 @@ +use codex_core::protocol::RateLimitSnapshotEvent; +use ratatui::prelude::*; +use ratatui::style::Stylize; + +/// Aggregated output used by the `/limits` command. +/// It contains the rendered summary lines, optional legend, +/// and the precomputed gauge state when one can be shown. +#[derive(Debug)] +pub(crate) struct LimitsView { + pub(crate) summary_lines: Vec>, + pub(crate) legend_lines: Vec>, + grid_state: Option, + grid: GridConfig, +} + +impl LimitsView { + /// Render the gauge for the provided width if the data supports it. + pub(crate) fn gauge_lines(&self, width: u16) -> Vec> { + match self.grid_state { + Some(state) => render_limit_grid(state, self.grid, width), + None => Vec::new(), + } + } +} + +/// Configuration for the simple grid gauge rendered by `/limits`. +#[derive(Clone, Copy, Debug)] +pub(crate) struct GridConfig { + pub(crate) weekly_slots: usize, + pub(crate) logo: &'static str, +} + +/// Default gauge configuration used by the TUI. +pub(crate) const DEFAULT_GRID_CONFIG: GridConfig = GridConfig { + weekly_slots: 100, + logo: "(>_)", +}; + +/// Build the lines and optional gauge used by the `/limits` view. +pub(crate) fn build_limits_view( + snapshot: &RateLimitSnapshotEvent, + grid_config: GridConfig, +) -> LimitsView { + let metrics = RateLimitMetrics::from_snapshot(snapshot); + let grid_state = extract_capacity_fraction(snapshot) + .and_then(|fraction| compute_grid_state(&metrics, fraction)) + .map(|state| scale_grid_state(state, grid_config)); + + LimitsView { + summary_lines: build_summary_lines(&metrics), + legend_lines: build_legend_lines(grid_state.is_some()), + grid_state, + grid: grid_config, + } +} + +#[derive(Debug)] +struct RateLimitMetrics { + hourly_used: f64, + weekly_used: f64, + hourly_remaining: f64, + weekly_remaining: f64, + hourly_window_label: String, + weekly_window_label: String, + hourly_reset_hint: String, + weekly_reset_hint: String, +} + +impl RateLimitMetrics { + fn from_snapshot(snapshot: &RateLimitSnapshotEvent) -> Self { + let hourly_used = snapshot.primary_used_percent.clamp(0.0, 100.0); + let weekly_used = snapshot.weekly_used_percent.clamp(0.0, 100.0); + Self { + hourly_used, + weekly_used, + hourly_remaining: (100.0 - hourly_used).max(0.0), + weekly_remaining: (100.0 - weekly_used).max(0.0), + hourly_window_label: format_window_label(Some(snapshot.primary_window_minutes)), + weekly_window_label: format_window_label(Some(snapshot.weekly_window_minutes)), + hourly_reset_hint: format_reset_hint(Some(snapshot.primary_window_minutes)), + weekly_reset_hint: format_reset_hint(Some(snapshot.weekly_window_minutes)), + } + } + + fn hourly_exhausted(&self) -> bool { + self.hourly_remaining <= 0.0 + } + + fn weekly_exhausted(&self) -> bool { + self.weekly_remaining <= 0.0 + } +} + +fn format_window_label(minutes: Option) -> String { + approximate_duration(minutes) + .map(|(value, unit)| format!("≈{value} {} window", pluralize_unit(unit, value))) + .unwrap_or_else(|| "window unknown".to_string()) +} + +fn format_reset_hint(minutes: Option) -> String { + approximate_duration(minutes) + .map(|(value, unit)| format!("≈{value} {}", pluralize_unit(unit, value))) + .unwrap_or_else(|| "unknown".to_string()) +} + +fn approximate_duration(minutes: Option) -> Option<(u64, DurationUnit)> { + let minutes = minutes?; + if minutes == 0 { + return Some((1, DurationUnit::Minute)); + } + if minutes < 60 { + return Some((minutes, DurationUnit::Minute)); + } + if minutes < 1_440 { + let hours = ((minutes as f64) / 60.0).round().max(1.0) as u64; + return Some((hours, DurationUnit::Hour)); + } + let days = ((minutes as f64) / 1_440.0).round().max(1.0) as u64; + if days >= 7 { + let weeks = ((days as f64) / 7.0).round().max(1.0) as u64; + Some((weeks, DurationUnit::Week)) + } else { + Some((days, DurationUnit::Day)) + } +} + +fn pluralize_unit(unit: DurationUnit, value: u64) -> String { + match unit { + DurationUnit::Minute => { + if value == 1 { + "minute".to_string() + } else { + "minutes".to_string() + } + } + DurationUnit::Hour => { + if value == 1 { + "hour".to_string() + } else { + "hours".to_string() + } + } + DurationUnit::Day => { + if value == 1 { + "day".to_string() + } else { + "days".to_string() + } + } + DurationUnit::Week => { + if value == 1 { + "week".to_string() + } else { + "weeks".to_string() + } + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum DurationUnit { + Minute, + Hour, + Day, + Week, +} + +#[derive(Clone, Copy, Debug)] +struct GridState { + weekly_used_ratio: f64, + hourly_remaining_ratio: f64, +} + +fn build_summary_lines(metrics: &RateLimitMetrics) -> Vec> { + let mut lines: Vec> = vec![ + "/limits".magenta().into(), + "".into(), + vec!["Rate limit usage snapshot".bold()].into(), + vec![" Tip: run `/limits` right after Codex replies for freshest numbers.".dim()].into(), + build_usage_line( + " • Hourly limit", + &metrics.hourly_window_label, + metrics.hourly_used, + ), + build_usage_line( + " • Weekly limit", + &metrics.weekly_window_label, + metrics.weekly_used, + ), + ]; + lines.push(build_status_line(metrics)); + lines +} + +fn build_usage_line(label: &str, window_label: &str, used_percent: f64) -> Line<'static> { + Line::from(vec![ + label.to_string().into(), + format!(" ({window_label})").dim(), + ": ".into(), + format!("{used_percent:.1}% used").dark_gray().bold(), + ]) +} + +fn build_status_line(metrics: &RateLimitMetrics) -> Line<'static> { + let mut spans: Vec> = Vec::new(); + if metrics.weekly_exhausted() || metrics.hourly_exhausted() { + spans.push(" Rate limited: ".into()); + let reason = match (metrics.hourly_exhausted(), metrics.weekly_exhausted()) { + (true, true) => "weekly and hourly windows exhausted", + (true, false) => "hourly window exhausted", + (false, true) => "weekly window exhausted", + (false, false) => unreachable!(), + }; + spans.push(reason.red()); + if metrics.hourly_exhausted() { + spans.push(" — hourly resets in ".into()); + spans.push(metrics.hourly_reset_hint.clone().dim()); + } + if metrics.weekly_exhausted() { + spans.push(" — weekly resets in ".into()); + spans.push(metrics.weekly_reset_hint.clone().dim()); + } + } else { + spans.push(" Within current limits".green()); + } + Line::from(spans) +} + +fn build_legend_lines(show_gauge: bool) -> Vec> { + if !show_gauge { + return Vec::new(); + } + vec![ + vec!["Legend".bold()].into(), + vec![ + " • ".into(), + "Dark gray".dark_gray().bold(), + " = weekly usage so far".into(), + ] + .into(), + vec![ + " • ".into(), + "Green".green().bold(), + " = hourly capacity still available".into(), + ] + .into(), + vec![ + " • ".into(), + "Default".bold(), + " = weekly capacity beyond the hourly window".into(), + ] + .into(), + ] +} + +fn extract_capacity_fraction(snapshot: &RateLimitSnapshotEvent) -> Option { + let ratio = snapshot.primary_to_weekly_ratio_percent; + if ratio.is_finite() { + Some((ratio / 100.0).clamp(0.0, 1.0)) + } else { + None + } +} + +fn compute_grid_state(metrics: &RateLimitMetrics, capacity_fraction: f64) -> Option { + if capacity_fraction <= 0.0 { + return None; + } + + let weekly_used_ratio = (metrics.weekly_used / 100.0).clamp(0.0, 1.0); + let weekly_remaining_ratio = (1.0 - weekly_used_ratio).max(0.0); + + let hourly_used_ratio = (metrics.hourly_used / 100.0).clamp(0.0, 1.0); + let hourly_used_within_capacity = + (hourly_used_ratio * capacity_fraction).min(capacity_fraction); + let hourly_remaining_within_capacity = + (capacity_fraction - hourly_used_within_capacity).max(0.0); + + let hourly_remaining_ratio = hourly_remaining_within_capacity.min(weekly_remaining_ratio); + + Some(GridState { + weekly_used_ratio, + hourly_remaining_ratio, + }) +} + +fn scale_grid_state(state: GridState, grid: GridConfig) -> GridState { + if grid.weekly_slots == 0 { + return GridState { + weekly_used_ratio: 0.0, + hourly_remaining_ratio: 0.0, + }; + } + state +} + +/// Convert the grid state to rendered lines for the TUI. +fn render_limit_grid(state: GridState, grid_config: GridConfig, width: u16) -> Vec> { + GridLayout::new(grid_config, width) + .map(|layout| layout.render(state)) + .unwrap_or_default() +} + +/// Precomputed layout information for the usage grid. +struct GridLayout { + size: usize, + inner_width: usize, + config: GridConfig, +} + +impl GridLayout { + const MIN_SIDE: usize = 4; + const MAX_SIDE: usize = 12; + const PREFIX: &'static str = " "; + + fn new(config: GridConfig, width: u16) -> Option { + if config.weekly_slots == 0 || config.logo.is_empty() { + return None; + } + let cell_width = config.logo.chars().count(); + if cell_width == 0 { + return None; + } + + let available_inner = width.saturating_sub((Self::PREFIX.len() + 2) as u16) as usize; + if available_inner == 0 { + return None; + } + + let base_side = (config.weekly_slots as f64) + .sqrt() + .round() + .clamp(1.0, Self::MAX_SIDE as f64) as usize; + let width_limited_side = + ((available_inner + 1) / (cell_width + 1)).clamp(1, Self::MAX_SIDE); + + let mut side = base_side.min(width_limited_side); + if width_limited_side >= Self::MIN_SIDE { + side = side.max(Self::MIN_SIDE.min(width_limited_side)); + } + let side = side.clamp(1, Self::MAX_SIDE); + if side == 0 { + return None; + } + + let inner_width = side * cell_width + side.saturating_sub(1); + Some(Self { + size: side, + inner_width, + config, + }) + } + + /// Render the grid into styled lines for the history cell. + fn render(&self, state: GridState) -> Vec> { + let counts = self.cell_counts(state); + let mut lines = Vec::new(); + lines.push("".into()); + lines.push(self.render_border('╭', '╮')); + + let mut cell_index = 0isize; + for _ in 0..self.size { + let mut spans: Vec> = Vec::new(); + spans.push(Self::PREFIX.into()); + spans.push("│".dim()); + + for col in 0..self.size { + if col > 0 { + spans.push(" ".into()); + } + let span = if cell_index < counts.dark_cells { + self.config.logo.dark_gray() + } else if cell_index < counts.dark_cells + counts.green_cells { + self.config.logo.green() + } else { + self.config.logo.into() + }; + spans.push(span); + cell_index += 1; + } + + spans.push("│".dim()); + lines.push(Line::from(spans)); + } + + lines.push(self.render_border('╰', '╯')); + lines.push("".into()); + + if counts.white_cells == 0 { + lines.push(vec![" (No unused weekly capacity remaining)".dim()].into()); + lines.push("".into()); + } + + lines + } + + fn render_border(&self, left: char, right: char) -> Line<'static> { + let mut text = String::from(Self::PREFIX); + text.push(left); + text.push_str(&"─".repeat(self.inner_width)); + text.push(right); + vec![Span::from(text).dim()].into() + } + + /// Translate usage ratios into the number of coloured cells. + fn cell_counts(&self, state: GridState) -> GridCellCounts { + let total_cells = self.size * self.size; + let mut dark_cells = (state.weekly_used_ratio * total_cells as f64).round() as isize; + dark_cells = dark_cells.clamp(0, total_cells as isize); + let mut green_cells = (state.hourly_remaining_ratio * total_cells as f64).round() as isize; + if dark_cells + green_cells > total_cells as isize { + green_cells = (total_cells as isize - dark_cells).max(0); + } + let white_cells = (total_cells as isize - dark_cells - green_cells).max(0); + + GridCellCounts { + dark_cells, + green_cells, + white_cells, + } + } +} + +/// Number of weekly (dark), hourly (green) and unused (default) cells. +struct GridCellCounts { + dark_cells: isize, + green_cells: isize, + white_cells: isize, +} + +#[cfg(test)] +mod tests { + use super::*; + + fn snapshot() -> RateLimitSnapshotEvent { + RateLimitSnapshotEvent { + primary_used_percent: 30.0, + weekly_used_percent: 60.0, + primary_to_weekly_ratio_percent: 40.0, + primary_window_minutes: 300, + weekly_window_minutes: 10_080, + } + } + + #[test] + fn approximate_duration_handles_hours_and_weeks() { + assert_eq!( + approximate_duration(Some(299)), + Some((5, DurationUnit::Hour)) + ); + assert_eq!( + approximate_duration(Some(10_080)), + Some((1, DurationUnit::Week)) + ); + assert_eq!( + approximate_duration(Some(90)), + Some((2, DurationUnit::Hour)) + ); + } + + #[test] + fn build_display_constructs_summary_and_gauge() { + let display = build_limits_view(&snapshot(), DEFAULT_GRID_CONFIG); + assert!(display.summary_lines.iter().any(|line| { + line.spans + .iter() + .any(|span| span.content.contains("Weekly limit")) + })); + assert!(display.summary_lines.iter().any(|line| { + line.spans + .iter() + .any(|span| span.content.contains("Hourly limit")) + })); + assert!(!display.gauge_lines(80).is_empty()); + } + + #[test] + fn hourly_and_weekly_percentages_are_not_swapped() { + let display = build_limits_view(&snapshot(), DEFAULT_GRID_CONFIG); + let summary = display + .summary_lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + + assert!(summary.contains("Hourly limit (≈5 hours window): 30.0% used")); + assert!(summary.contains("Weekly limit (≈1 week window): 60.0% used")); + } + + #[test] + fn build_display_without_ratio_skips_gauge() { + let mut s = snapshot(); + s.primary_to_weekly_ratio_percent = f64::NAN; + let display = build_limits_view(&s, DEFAULT_GRID_CONFIG); + assert!(display.gauge_lines(80).is_empty()); + assert!(display.legend_lines.is_empty()); + } +} diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 433c0a6d7f94..4d4c806727bf 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -21,6 +21,7 @@ pub enum SlashCommand { Diff, Mention, Status, + Limits, Mcp, Logout, Quit, @@ -40,6 +41,7 @@ impl SlashCommand { SlashCommand::Diff => "show git diff (including untracked files)", SlashCommand::Mention => "mention a file", SlashCommand::Status => "show current session configuration and token usage", + SlashCommand::Limits => "visualize weekly and hourly rate limits", SlashCommand::Model => "choose what model and reasoning effort to use", SlashCommand::Approvals => "choose what Codex can do without approval", SlashCommand::Mcp => "list configured MCP tools", @@ -68,6 +70,7 @@ impl SlashCommand { SlashCommand::Diff | SlashCommand::Mention | SlashCommand::Status + | SlashCommand::Limits | SlashCommand::Mcp | SlashCommand::Quit => true,