From 7aa46ab5fc8538d564058a25614eac4c292482e4 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Wed, 29 Oct 2025 17:47:55 -0700 Subject: [PATCH 1/2] ignore agent message deltas for the review mode (#5937) The deltas produce the whole json output. ignore them. --- codex-rs/core/src/tasks/review.rs | 10 ++-- codex-rs/core/tests/suite/review.rs | 79 +++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/codex-rs/core/src/tasks/review.rs b/codex-rs/core/src/tasks/review.rs index 08576cad99..286eefa6f5 100644 --- a/codex-rs/core/src/tasks/review.rs +++ b/codex-rs/core/src/tasks/review.rs @@ -4,6 +4,8 @@ use async_trait::async_trait; use codex_protocol::items::TurnItem; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; +use codex_protocol::protocol::AgentMessageContentDeltaEvent; +use codex_protocol::protocol::AgentMessageDeltaEvent; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExitedReviewModeEvent; @@ -111,13 +113,15 @@ async fn process_review_events( } prev_agent_message = Some(event); } - // Suppress ItemCompleted for assistant messages: forwarding it would - // trigger legacy AgentMessage via as_legacy_events(), which this + // Suppress ItemCompleted only for assistant messages: forwarding it + // would trigger legacy AgentMessage via as_legacy_events(), which this // review flow intentionally hides in favor of structured output. EventMsg::ItemCompleted(ItemCompletedEvent { item: TurnItem::AgentMessage(_), .. - }) => {} + }) + | EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { .. }) + | EventMsg::AgentMessageContentDelta(AgentMessageContentDeltaEvent { .. }) => {} EventMsg::TaskComplete(task_complete) => { // Parse review output from the last agent message (if present). let out = task_complete diff --git a/codex-rs/core/tests/suite/review.rs b/codex-rs/core/tests/suite/review.rs index fdf7669d1d..093fd99268 100644 --- a/codex-rs/core/tests/suite/review.rs +++ b/codex-rs/core/tests/suite/review.rs @@ -204,6 +204,85 @@ async fn review_op_with_plain_text_emits_review_fallback() { server.verify().await; } +/// Ensure review flow suppresses assistant-specific streaming/completion events: +/// - AgentMessageContentDelta +/// - AgentMessageDelta (legacy) +/// - ItemCompleted for TurnItem::AgentMessage +// Windows CI only: bump to 4 workers to prevent SSE/event starvation and test timeouts. +#[cfg_attr(windows, tokio::test(flavor = "multi_thread", worker_threads = 4))] +#[cfg_attr(not(windows), tokio::test(flavor = "multi_thread", worker_threads = 2))] +async fn review_filters_agent_message_related_events() { + skip_if_no_network!(); + + // Stream simulating a typing assistant message with deltas and finalization. + let sse_raw = r#"[ + {"type":"response.output_item.added", "item":{ + "type":"message", "role":"assistant", "id":"msg-1", + "content":[{"type":"output_text","text":""}] + }}, + {"type":"response.output_text.delta", "delta":"Hi"}, + {"type":"response.output_text.delta", "delta":" there"}, + {"type":"response.output_item.done", "item":{ + "type":"message", "role":"assistant", "id":"msg-1", + "content":[{"type":"output_text","text":"Hi there"}] + }}, + {"type":"response.completed", "response": {"id": "__ID__"}} + ]"#; + let server = start_responses_server_with_sse(sse_raw, 1).await; + let codex_home = TempDir::new().unwrap(); + let codex = new_conversation_for_server(&server, &codex_home, |_| {}).await; + + codex + .submit(Op::Review { + review_request: ReviewRequest { + prompt: "Filter streaming events".to_string(), + user_facing_hint: "Filter streaming events".to_string(), + }, + }) + .await + .unwrap(); + + let mut saw_entered = false; + let mut saw_exited = false; + + // Drain until TaskComplete; assert filtered events never surface. + wait_for_event_with_timeout( + &codex, + |event| match event { + EventMsg::TaskComplete(_) => true, + EventMsg::EnteredReviewMode(_) => { + saw_entered = true; + false + } + EventMsg::ExitedReviewMode(_) => { + saw_exited = true; + false + } + // The following must be filtered by review flow + EventMsg::AgentMessageContentDelta(_) => { + panic!("unexpected AgentMessageContentDelta surfaced during review") + } + EventMsg::AgentMessageDelta(_) => { + panic!("unexpected AgentMessageDelta surfaced during review") + } + EventMsg::ItemCompleted(ev) => match &ev.item { + codex_protocol::items::TurnItem::AgentMessage(_) => { + panic!( + "unexpected ItemCompleted for TurnItem::AgentMessage surfaced during review" + ) + } + _ => false, + }, + _ => false, + }, + tokio::time::Duration::from_secs(5), + ) + .await; + assert!(saw_entered && saw_exited, "missing review lifecycle events"); + + server.verify().await; +} + /// When the model returns structured JSON in a review, ensure no AgentMessage /// is emitted; the UI consumes the structured result via ExitedReviewMode. // Windows CI only: bump to 4 workers to prevent SSE/event starvation and test timeouts. From b34efde2f3bdf562e712174b97260d9a5794b356 Mon Sep 17 00:00:00 2001 From: zhao-oai Date: Wed, 29 Oct 2025 18:10:41 -0700 Subject: [PATCH 2/2] asdf (#5940) . --- codex-rs/core/src/error.rs | 32 +++++++++++++++---- codex-rs/tui/src/status/card.rs | 25 +++++++++++---- ...tatus_snapshot_includes_monthly_limit.snap | 3 ++ ...s_snapshot_includes_reasoning_details.snap | 3 ++ ...s_snapshot_shows_empty_limits_message.snap | 3 ++ ...snapshot_shows_missing_limits_message.snap | 5 ++- ...s_snapshot_shows_stale_limits_message.snap | 3 ++ ...snapshot_truncates_in_narrow_terminal.snap | 4 +++ 8 files changed, 64 insertions(+), 14 deletions(-) diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index 0901e426ad..0446c24fb1 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -253,7 +253,7 @@ impl std::fmt::Display for UsageLimitReachedError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let message = match self.plan_type.as_ref() { Some(PlanType::Known(KnownPlan::Plus)) => format!( - "You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing){}", + "You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing), visit chatgpt.com/codex/settings/usage to purchase more credits{}", retry_suffix_after_or(self.resets_at.as_ref()) ), Some(PlanType::Known(KnownPlan::Team)) | Some(PlanType::Known(KnownPlan::Business)) => { @@ -266,8 +266,11 @@ impl std::fmt::Display for UsageLimitReachedError { "You've hit your usage limit. Upgrade to Plus to continue using Codex (https://openai.com/chatgpt/pricing)." .to_string() } - Some(PlanType::Known(KnownPlan::Pro)) - | Some(PlanType::Known(KnownPlan::Enterprise)) + Some(PlanType::Known(KnownPlan::Pro)) => format!( + "You've hit your usage limit. Visit chatgpt.com/codex/settings/usage to purchase more credits{}", + retry_suffix_after_or(self.resets_at.as_ref()) + ), + Some(PlanType::Known(KnownPlan::Enterprise)) | Some(PlanType::Known(KnownPlan::Edu)) => format!( "You've hit your usage limit.{}", retry_suffix(self.resets_at.as_ref()) @@ -467,7 +470,7 @@ mod tests { }; assert_eq!( err.to_string(), - "You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing) or try again later." + "You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing), visit chatgpt.com/codex/settings/usage to purchase more credits or try again later." ); } @@ -597,7 +600,7 @@ mod tests { #[test] fn usage_limit_reached_error_formats_default_for_other_plans() { let err = UsageLimitReachedError { - plan_type: Some(PlanType::Known(KnownPlan::Pro)), + plan_type: Some(PlanType::Known(KnownPlan::Enterprise)), resets_at: None, rate_limits: Some(rate_limit_snapshot()), }; @@ -607,6 +610,23 @@ mod tests { ); } + #[test] + fn usage_limit_reached_error_formats_pro_plan_with_reset() { + let base = Utc.with_ymd_and_hms(2024, 1, 1, 0, 0, 0).unwrap(); + let resets_at = base + ChronoDuration::hours(1); + with_now_override(base, move || { + let err = UsageLimitReachedError { + plan_type: Some(PlanType::Known(KnownPlan::Pro)), + resets_at: Some(resets_at), + rate_limits: Some(rate_limit_snapshot()), + }; + assert_eq!( + err.to_string(), + "You've hit your usage limit. Visit chatgpt.com/codex/settings/usage to purchase more credits or try again in 1 hour." + ); + }); + } + #[test] fn usage_limit_reached_includes_minutes_when_available() { let base = Utc.with_ymd_and_hms(2024, 1, 1, 0, 0, 0).unwrap(); @@ -636,7 +656,7 @@ mod tests { }; assert_eq!( err.to_string(), - "You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing) or try again in 3 hours 32 minutes." + "You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing), visit chatgpt.com/codex/settings/usage to purchase more credits or try again in 3 hours 32 minutes." ); }); } diff --git a/codex-rs/tui/src/status/card.rs b/codex-rs/tui/src/status/card.rs index 9333509700..fdf6629a0a 100644 --- a/codex-rs/tui/src/status/card.rs +++ b/codex-rs/tui/src/status/card.rs @@ -31,6 +31,8 @@ use super::rate_limits::StatusRateLimitRow; use super::rate_limits::compose_rate_limit_data; use super::rate_limits::format_status_limit_summary; use super::rate_limits::render_status_limit_progress_bar; +use crate::wrapping::RtOptions; +use crate::wrapping::word_wrap_lines; #[derive(Debug, Clone)] struct StatusContextWindowData { @@ -195,13 +197,7 @@ impl StatusHistoryCell { lines } StatusRateLimitData::Missing => { - vec![formatter.line( - "Limits", - vec![ - Span::from("visit ").dim(), - "chatgpt.com/codex/settings/usage".cyan().underlined(), - ], - )] + vec![formatter.line("Limits", vec![Span::from("data not available yet").dim()])] } } } @@ -315,6 +311,21 @@ impl HistoryCell for StatusHistoryCell { let formatter = FieldFormatter::from_labels(labels.iter().map(String::as_str)); let value_width = formatter.value_width(available_inner_width); + let note_first_line = Line::from(vec![ + Span::from("Visit ").cyan(), + "chatgpt.com/codex/settings/usage".cyan().underlined(), + Span::from(" for up-to-date").cyan(), + ]); + let note_second_line = Line::from(vec![ + Span::from("information on rate limits and credits").cyan(), + ]); + let note_lines = word_wrap_lines( + [note_first_line, note_second_line], + RtOptions::new(available_inner_width), + ); + lines.extend(note_lines); + lines.push(Line::from(Vec::>::new())); + let mut model_spans = vec![Span::from(self.model_name.clone())]; if !self.model_details.is_empty() { model_spans.push(Span::from(" (").dim()); diff --git a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_includes_monthly_limit.snap b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_includes_monthly_limit.snap index cf3f492eef..cf45f3f810 100644 --- a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_includes_monthly_limit.snap +++ b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_includes_monthly_limit.snap @@ -7,6 +7,9 @@ expression: sanitized ╭────────────────────────────────────────────────────────────────────────────╮ │ >_ OpenAI Codex (v0.0.0) │ │ │ +│ Visit chatgpt.com/codex/settings/usage for up-to-date │ +│ information on rate limits and credits │ +│ │ │ Model: gpt-5-codex (reasoning none, summaries auto) │ │ Directory: [[workspace]] │ │ Approval: on-request │ diff --git a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_includes_reasoning_details.snap b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_includes_reasoning_details.snap index 505375eae5..e38a1c7c1b 100644 --- a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_includes_reasoning_details.snap +++ b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_includes_reasoning_details.snap @@ -7,6 +7,9 @@ expression: sanitized ╭─────────────────────────────────────────────────────────────────────╮ │ >_ OpenAI Codex (v0.0.0) │ │ │ +│ Visit chatgpt.com/codex/settings/usage for up-to-date │ +│ information on rate limits and credits │ +│ │ │ Model: gpt-5-codex (reasoning high, summaries detailed) │ │ Directory: [[workspace]] │ │ Approval: on-request │ diff --git a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_empty_limits_message.snap b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_empty_limits_message.snap index 472df543d6..82ed8fc0b9 100644 --- a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_empty_limits_message.snap +++ b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_empty_limits_message.snap @@ -7,6 +7,9 @@ expression: sanitized ╭─────────────────────────────────────────────────────────────────╮ │ >_ OpenAI Codex (v0.0.0) │ │ │ +│ Visit chatgpt.com/codex/settings/usage for up-to-date │ +│ information on rate limits and credits │ +│ │ │ Model: gpt-5-codex (reasoning none, summaries auto) │ │ Directory: [[workspace]] │ │ Approval: on-request │ diff --git a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_missing_limits_message.snap b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_missing_limits_message.snap index 9997006357..82ed8fc0b9 100644 --- a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_missing_limits_message.snap +++ b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_missing_limits_message.snap @@ -7,6 +7,9 @@ expression: sanitized ╭─────────────────────────────────────────────────────────────────╮ │ >_ OpenAI Codex (v0.0.0) │ │ │ +│ Visit chatgpt.com/codex/settings/usage for up-to-date │ +│ information on rate limits and credits │ +│ │ │ Model: gpt-5-codex (reasoning none, summaries auto) │ │ Directory: [[workspace]] │ │ Approval: on-request │ @@ -15,5 +18,5 @@ expression: sanitized │ │ │ Token usage: 750 total (500 input + 250 output) │ │ Context window: 100% left (750 used / 272K) │ -│ Limits: visit chatgpt.com/codex/settings/usage │ +│ Limits: data not available yet │ ╰─────────────────────────────────────────────────────────────────╯ diff --git a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_stale_limits_message.snap b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_stale_limits_message.snap index bd07335210..bd18719700 100644 --- a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_stale_limits_message.snap +++ b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_stale_limits_message.snap @@ -7,6 +7,9 @@ expression: sanitized ╭─────────────────────────────────────────────────────────────────────╮ │ >_ OpenAI Codex (v0.0.0) │ │ │ +│ Visit chatgpt.com/codex/settings/usage for up-to-date │ +│ information on rate limits and credits │ +│ │ │ Model: gpt-5-codex (reasoning none, summaries auto) │ │ Directory: [[workspace]] │ │ Approval: on-request │ diff --git a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_truncates_in_narrow_terminal.snap b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_truncates_in_narrow_terminal.snap index b7c595340d..64e9271df7 100644 --- a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_truncates_in_narrow_terminal.snap +++ b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_truncates_in_narrow_terminal.snap @@ -7,6 +7,10 @@ expression: sanitized ╭────────────────────────────────────────────╮ │ >_ OpenAI Codex (v0.0.0) │ │ │ +│ Visit chatgpt.com/codex/settings/usage for │ +│ up-to-date │ +│ information on rate limits and credits │ +│ │ │ Model: gpt-5-codex (reasoning │ │ Directory: [[workspace]] │ │ Approval: on-request │