From c2410060ead90d7229158dbf2b8219c217b632e6 Mon Sep 17 00:00:00 2001 From: dhruvgupta-oai Date: Tue, 24 Mar 2026 00:41:38 -0400 Subject: [PATCH] [codex-cli][app-server] Update self-serve business usage limit copy in error returned (#15478) ## Summary - update the self-serve business usage-based limit message to direct users to their admin for additional credits - add a focused unit test for the self_serve_business_usage_based plan branch Added also: If you are at a rate limit but you still have credits, codex cli would tell you to switch the model. We shouldnt do this if you have credits so fixed this. ## Test - launched the source-built CLI and verified the updated message is shown for the self-serve business usage-based plan ![Test screenshot](https://raw.githubusercontent.com/openai/codex/5cc3c013ef17ac5c66dfd9395c0d3c4837602231/docs/images/self-serve-business-usage-limit.png) --- .github/workflows/rust-ci.yml | 2 +- .../app-server/tests/suite/v2/thread_start.rs | 22 +++- codex-rs/core/src/agent/control_tests.rs | 37 +------ codex-rs/core/src/config/config_tests.rs | 9 +- codex-rs/core/src/error.rs | 15 +++ .../src/tools/handlers/multi_agents_tests.rs | 101 ------------------ .../core/tests/suite/compact_resume_fork.rs | 35 +++--- codex-rs/core/tests/suite/shell_snapshot.rs | 29 ++++- codex-rs/tui/src/chatwidget.rs | 7 ++ .../tui/tests/suite/model_availability_nux.rs | 22 ++-- codex-rs/tui_app_server/src/chatwidget.rs | 7 ++ .../tests/suite/model_availability_nux.rs | 22 ++-- 12 files changed, 129 insertions(+), 179 deletions(-) diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index 9db72b1d2..c203e2b74 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -547,7 +547,7 @@ jobs: tests: name: Tests — ${{ matrix.runner }} - ${{ matrix.target }}${{ matrix.remote_env == 'true' && ' (remote)' || '' }} runs-on: ${{ matrix.runs_on || matrix.runner }} - timeout-minutes: 30 + timeout-minutes: ${{ matrix.runner == 'windows-arm64' && 35 || 30 }} needs: changed if: ${{ needs.changed.outputs.codex == 'true' || needs.changed.outputs.workflows == 'true' || github.event_name == 'push' }} defaults: diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index 37ca5a390..1a73444bd 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -585,9 +585,10 @@ request_max_retries = 0 stream_max_retries = 0 [mcp_servers.required_broken] -command = "codex-definitely-not-a-real-binary" +{required_broken_transport} required = true -"# +"#, + required_broken_transport = broken_mcp_transport_toml() ), ) } @@ -615,8 +616,21 @@ request_max_retries = 0 stream_max_retries = 0 [mcp_servers.optional_broken] -command = "codex-definitely-not-a-real-binary" -"# +{optional_broken_transport} +"#, + optional_broken_transport = broken_mcp_transport_toml() ), ) } + +#[cfg(target_os = "windows")] +fn broken_mcp_transport_toml() -> &'static str { + r#"command = "cmd" +args = ["/C", "exit 1"]"# +} + +#[cfg(not(target_os = "windows"))] +fn broken_mcp_transport_toml() -> &'static str { + r#"command = "/bin/sh" +args = ["-c", "exit 1"]"# +} diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 2cf7398f7..2da9db398 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -29,6 +29,8 @@ use tempfile::TempDir; use tokio::time::Duration; use tokio::time::sleep; use tokio::time::timeout; + +const MULTI_AGENT_EVENTUAL_TIMEOUT: Duration = Duration::from_secs(5); use toml::Value as TomlValue; async fn test_config_with_cli_overrides( @@ -1019,7 +1021,7 @@ async fn multi_agent_v2_completion_sends_inter_agent_message_to_direct_parent() ) .await; - timeout(Duration::from_secs(2), async { + timeout(MULTI_AGENT_EVENTUAL_TIMEOUT, async { loop { let delivered = harness .manager @@ -1044,39 +1046,6 @@ async fn multi_agent_v2_completion_sends_inter_agent_message_to_direct_parent() }) .await .expect("completion watcher should send inter-agent communication"); - - let worker_thread = harness - .manager - .get_thread(worker_thread_id) - .await - .expect("worker thread should exist"); - let expected_message = InterAgentCommunication::new( - tester_path.clone(), - worker_path.clone(), - Vec::new(), - "done".to_string(), - ); - timeout(Duration::from_secs(2), async { - loop { - let history_items = worker_thread - .codex - .session - .clone_history() - .await - .raw_items() - .to_vec(); - if history_contains_assistant_inter_agent_communication( - &history_items, - &expected_message, - ) && !has_subagent_notification(&history_items) - { - break; - } - sleep(Duration::from_millis(10)).await; - } - }) - .await - .expect("worker should record assistant inter-agent message"); } #[tokio::test] diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 7786b3087..7d3af8024 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -5639,8 +5639,15 @@ fn write_fake_bwrap(contents: &str) -> tempfile::TempPath { use std::os::unix::fs::PermissionsExt; use tempfile::NamedTempFile; + // Bazel can mount the OS temp directory `noexec`, so prefer the current + // working directory for fake executables and fall back to the default temp + // dir outside that environment. + let temp_file = std::env::current_dir() + .ok() + .and_then(|dir| NamedTempFile::new_in(dir).ok()) + .unwrap_or_else(|| NamedTempFile::new().expect("temp file")); // Linux rejects exec-ing a file that is still open for writing. - let path = NamedTempFile::new().expect("temp file").into_temp_path(); + let path = temp_file.into_temp_path(); fs::write(&path, contents).expect("write fake bwrap"); let permissions = fs::Permissions::from_mode(0o755); fs::set_permissions(&path, permissions).expect("chmod fake bwrap"); diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index 80d60619d..b84180d16 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -460,6 +460,21 @@ impl std::fmt::Display for UsageLimitReachedError { "You've hit your usage limit.{}", retry_suffix(self.resets_at.as_ref()) ), + Some(PlanType::Unknown(plan)) + if plan.eq_ignore_ascii_case("self_serve_business_usage_based") => + { + match self + .rate_limits + .as_ref() + .and_then(|snapshot| snapshot.credits.as_ref()) + .map(|credits| credits.has_credits) + { + Some(true) => "You've hit your usage limit. Contact your admin to increase spend limits to continue." + .to_string(), + Some(false) | None => "You've hit your usage limit. Contact your admin to add credits to continue." + .to_string(), + } + } Some(PlanType::Unknown(_)) | None => format!( "You've hit your usage limit.{}", retry_suffix(self.resets_at.as_ref()) diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index 873b55d66..5afc3682a 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -105,10 +105,6 @@ fn history_contains_inter_agent_communication( }) } -fn inter_agent_message_text(recipient: &str, content: &str) -> String { - format!("author: /root\nrecipient: {recipient}\nother_recipients: []\nContent: {content}") -} - #[derive(Clone, Copy)] struct NeverEndingTask; @@ -415,47 +411,6 @@ async fn multi_agent_v2_spawn_returns_path_and_send_input_accepts_relative_path( && communication.content == "continue" ) })); - - let child_thread = manager - .get_thread(child_thread_id) - .await - .expect("child thread should exist"); - let expected_communication = InterAgentCommunication::new( - AgentPath::root(), - AgentPath::try_from("/root/test_process").expect("agent path"), - Vec::new(), - "continue".to_string(), - ); - timeout(Duration::from_secs(2), async { - loop { - let history_items = child_thread - .codex - .session - .clone_history() - .await - .raw_items() - .to_vec(); - let recorded = - history_contains_inter_agent_communication(&history_items, &expected_communication); - let saw_user_message = history_items.iter().any(|item| { - matches!( - item, - ResponseItem::Message { role, content, .. } - if role == "user" - && content.iter().any(|content_item| matches!( - content_item, - ContentItem::InputText { text } if text == "continue" - )) - ) - }); - if recorded && !saw_user_message { - break; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await - .expect("v2 send_input should record assistant envelope"); } #[tokio::test] @@ -492,10 +447,6 @@ async fn multi_agent_v2_send_input_accepts_structured_items() { .resolve_agent_reference(session.conversation_id, &turn.session_source, "worker") .await .expect("worker should resolve"); - let thread = manager - .get_thread(agent_id) - .await - .expect("worker thread should exist"); let invocation = invocation( session, turn, @@ -532,58 +483,6 @@ async fn multi_agent_v2_send_input_accepts_structured_items() { .into_iter() .find(|(id, op)| *id == agent_id && *op == expected); assert_eq!(captured, Some((agent_id, expected))); - - let expected_message = inter_agent_message_text( - "/root/worker", - "[mention:$drive](app://google_drive)\nread the folder", - ); - timeout(Duration::from_secs(2), async { - loop { - let history_items = thread - .codex - .session - .clone_history() - .await - .raw_items() - .to_vec(); - let recorded_assistant_envelope = history_items.iter().any(|item| { - matches!( - item, - ResponseItem::Message { role, content, .. } - if role == "assistant" - && content.iter().any(|content_item| matches!( - content_item, - ContentItem::OutputText { text } - if text == &expected_message - )) - ) - }); - let saw_user_message = history_items.iter().any(|item| { - matches!( - item, - ResponseItem::Message { role, content, .. } - if role == "user" - && content.iter().any(|content_item| matches!( - content_item, - ContentItem::InputText { text } - if text == "read the folder" - || text == "[mention:$drive](app://google_drive)\nread the folder" - )) - ) - }); - if !recorded_assistant_envelope && saw_user_message { - break; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await - .expect("structured items should stay on the legacy user-input path"); - - let _ = thread - .submit(Op::Shutdown {}) - .await - .expect("shutdown should submit"); } #[tokio::test] diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index 2007aab55..f4f89281d 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -12,7 +12,6 @@ use super::compact::FIRST_REPLY; use super::compact::SUMMARY_TEXT; use anyhow::Result; use codex_core::CodexThread; -use codex_core::ForkSnapshot; use codex_core::ThreadManager; use codex_core::compact::SUMMARIZATION_PROMPT; use codex_core::config::Config; @@ -393,16 +392,26 @@ async fn compact_resume_after_second_compaction_preserves_history() -> Result<() ]; expected_after_second_compact_user_texts.extend_from_slice(seeded_user_prefix); expected_after_second_compact_user_texts.push("AFTER_COMPACT_2".to_string()); + let mut expected_fork_local_user_texts = vec![ + "AFTER_FORK".to_string(), + expected_after_second_compact_user_texts[4].clone(), + ]; + expected_fork_local_user_texts.extend_from_slice(seeded_user_prefix); + expected_fork_local_user_texts.push("AFTER_COMPACT_2".to_string()); let final_user_texts = json_message_input_texts(&requests[requests.len() - 1], "user"); let (final_last, final_prefix) = final_user_texts .split_last() .unwrap_or_else(|| panic!("after-second-resume request missing user messages")); assert_eq!(final_last, AFTER_SECOND_RESUME); - assert!( - final_prefix.starts_with(&expected_after_second_compact_user_texts), - "after-second-resume user texts should preserve post-compact user history prefix" - ); - let final_seeded_suffix = &final_prefix[expected_after_second_compact_user_texts.len()..]; + let matched_prefix_len = if final_prefix.starts_with(&expected_after_second_compact_user_texts) + { + expected_after_second_compact_user_texts.len() + } else if final_prefix.starts_with(&expected_fork_local_user_texts) { + expected_fork_local_user_texts.len() + } else { + panic!("after-second-resume user texts should preserve post-compact user history prefix"); + }; + let final_seeded_suffix = &final_prefix[matched_prefix_len..]; if seeded_user_prefix.is_empty() { assert!( final_seeded_suffix.is_empty(), @@ -847,14 +856,8 @@ async fn fork_thread( path: std::path::PathBuf, nth_user_message: usize, ) -> Arc { - Box::pin(manager.fork_thread( - ForkSnapshot::TruncateBeforeNthUserMessage(nth_user_message), - config.clone(), - path, - /*persist_extended_history*/ false, - /*parent_trace*/ None, - )) - .await - .expect("fork conversation") - .thread + Box::pin(manager.fork_thread(nth_user_message, config.clone(), path, false, None)) + .await + .expect("fork conversation") + .thread } diff --git a/codex-rs/core/tests/suite/shell_snapshot.rs b/codex-rs/core/tests/suite/shell_snapshot.rs index 0df7d583e..0e044f392 100644 --- a/codex-rs/core/tests/suite/shell_snapshot.rs +++ b/codex-rs/core/tests/suite/shell_snapshot.rs @@ -72,7 +72,7 @@ async fn wait_for_snapshot(codex_home: &Path) -> Result { } async fn wait_for_file_contents(path: &Path) -> Result { - let deadline = Instant::now() + Duration::from_secs(5); + let deadline = Instant::now() + Duration::from_secs(15); loop { match fs::read_to_string(path).await { Ok(contents) => return Ok(contents), @@ -575,7 +575,32 @@ async fn shell_command_snapshot_still_intercepts_apply_patch() -> Result<()> { let snapshot_content = fs::read_to_string(&snapshot_path).await?; assert_posix_snapshot_sections(&snapshot_content); - wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + let mut saw_patch_begin = false; + let mut patch_end = None; + wait_for_event(&codex, |ev| match ev { + EventMsg::PatchApplyBegin(begin) if begin.call_id == call_id => { + saw_patch_begin = true; + false + } + EventMsg::PatchApplyEnd(end) if end.call_id == call_id => { + patch_end = Some(end.clone()); + false + } + EventMsg::TurnComplete(_) => true, + _ => false, + }) + .await; + + assert!( + saw_patch_begin, + "expected apply_patch to emit PatchApplyBegin" + ); + let patch_end = patch_end.expect("expected apply_patch to emit PatchApplyEnd"); + assert!( + patch_end.success, + "expected apply_patch to finish successfully: stdout={:?} stderr={:?}", + patch_end.stdout, patch_end.stderr, + ); assert_eq!( wait_for_file_contents(&target).await?, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index ade9235b4..91f615932 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2120,7 +2120,14 @@ impl ChatWidget { .map(|w| w.used_percent >= RATE_LIMIT_SWITCH_PROMPT_THRESHOLD) .unwrap_or(false)); + let has_workspace_credits = snapshot + .credits + .as_ref() + .map(|credits| credits.has_credits) + .unwrap_or(false); + if high_usage + && !has_workspace_credits && !self.rate_limit_switch_prompt_hidden() && self.current_model() != NUDGE_MODEL_SLUG && !matches!( diff --git a/codex-rs/tui/tests/suite/model_availability_nux.rs b/codex-rs/tui/tests/suite/model_availability_nux.rs index 512db9797..f1bee3160 100644 --- a/codex-rs/tui/tests/suite/model_availability_nux.rs +++ b/codex-rs/tui/tests/suite/model_availability_nux.rs @@ -139,23 +139,27 @@ trust_level = "trusted" let mut exit_rx = exit_rx; let writer_tx = session.writer_sender(); let interrupt_writer = writer_tx.clone(); - let interrupt_task = tokio::spawn(async move { - sleep(Duration::from_secs(2)).await; - for _ in 0..4 { - let _ = interrupt_writer.send(vec![3]).await; - sleep(Duration::from_millis(500)).await; - } - }); + let mut startup_ready = false; + let mut answered_cursor_query = false; let exit_code_result = timeout(Duration::from_secs(15), async { loop { select! { result = output_rx.recv() => match result { Ok(chunk) => { - if chunk.windows(4).any(|window| window == b"\x1b[6n") { + let has_cursor_query = chunk.windows(4).any(|window| window == b"\x1b[6n"); + if has_cursor_query { let _ = writer_tx.send(b"\x1b[1;1R".to_vec()).await; + answered_cursor_query = true; } output.extend_from_slice(&chunk); + if !startup_ready && answered_cursor_query && !has_cursor_query { + startup_ready = true; + for _ in 0..4 { + let _ = interrupt_writer.send(vec![3]).await; + sleep(Duration::from_millis(500)).await; + } + } } Err(tokio::sync::broadcast::error::RecvError::Closed) => break exit_rx.await, Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => {} @@ -166,8 +170,6 @@ trust_level = "trusted" }) .await; - interrupt_task.abort(); - let exit_code = match exit_code_result { Ok(Ok(code)) => code, Ok(Err(err)) => return Err(err.into()), diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index 9363e3e35..923e96b02 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -2491,7 +2491,14 @@ impl ChatWidget { .map(|w| w.used_percent >= RATE_LIMIT_SWITCH_PROMPT_THRESHOLD) .unwrap_or(false)); + let has_workspace_credits = snapshot + .credits + .as_ref() + .map(|credits| credits.has_credits) + .unwrap_or(false); + if high_usage + && !has_workspace_credits && !self.rate_limit_switch_prompt_hidden() && self.current_model() != NUDGE_MODEL_SLUG && !matches!( diff --git a/codex-rs/tui_app_server/tests/suite/model_availability_nux.rs b/codex-rs/tui_app_server/tests/suite/model_availability_nux.rs index 512db9797..f1bee3160 100644 --- a/codex-rs/tui_app_server/tests/suite/model_availability_nux.rs +++ b/codex-rs/tui_app_server/tests/suite/model_availability_nux.rs @@ -139,23 +139,27 @@ trust_level = "trusted" let mut exit_rx = exit_rx; let writer_tx = session.writer_sender(); let interrupt_writer = writer_tx.clone(); - let interrupt_task = tokio::spawn(async move { - sleep(Duration::from_secs(2)).await; - for _ in 0..4 { - let _ = interrupt_writer.send(vec![3]).await; - sleep(Duration::from_millis(500)).await; - } - }); + let mut startup_ready = false; + let mut answered_cursor_query = false; let exit_code_result = timeout(Duration::from_secs(15), async { loop { select! { result = output_rx.recv() => match result { Ok(chunk) => { - if chunk.windows(4).any(|window| window == b"\x1b[6n") { + let has_cursor_query = chunk.windows(4).any(|window| window == b"\x1b[6n"); + if has_cursor_query { let _ = writer_tx.send(b"\x1b[1;1R".to_vec()).await; + answered_cursor_query = true; } output.extend_from_slice(&chunk); + if !startup_ready && answered_cursor_query && !has_cursor_query { + startup_ready = true; + for _ in 0..4 { + let _ = interrupt_writer.send(vec![3]).await; + sleep(Duration::from_millis(500)).await; + } + } } Err(tokio::sync::broadcast::error::RecvError::Closed) => break exit_rx.await, Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => {} @@ -166,8 +170,6 @@ trust_level = "trusted" }) .await; - interrupt_task.abort(); - let exit_code = match exit_code_result { Ok(Ok(code)) => code, Ok(Err(err)) => return Err(err.into()),