From fd9dee82a8232a8ba646a41a81dc98dc683720aa Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Mon, 11 May 2026 18:53:59 -0700 Subject: [PATCH] Filter legacy warning messages during compaction --- codex-rs/core/src/compact_remote.rs | 5 +- codex-rs/core/src/compact_tests.rs | 56 +++++++++++++++++++ .../src/context/contextual_user_message.rs | 15 +++++ ...legacy_apply_patch_exec_command_warning.rs | 21 +++++++ .../context/legacy_model_mismatch_warning.rs | 21 +++++++ ...gacy_unified_exec_process_limit_warning.rs | 21 +++++++ codex-rs/core/src/context/mod.rs | 6 ++ codex-rs/core/src/session/mod.rs | 18 ------ codex-rs/core/src/session/tests.rs | 28 ---------- .../core/src/tools/handlers/apply_patch.rs | 8 --- codex-rs/core/src/unified_exec/mod.rs | 3 - .../core/src/unified_exec/process_manager.rs | 15 +---- .../tests/suite/safety_check_downgrade.rs | 32 +---------- 13 files changed, 146 insertions(+), 103 deletions(-) create mode 100644 codex-rs/core/src/context/legacy_apply_patch_exec_command_warning.rs create mode 100644 codex-rs/core/src/context/legacy_model_mismatch_warning.rs create mode 100644 codex-rs/core/src/context/legacy_unified_exec_process_limit_warning.rs diff --git a/codex-rs/core/src/compact_remote.rs b/codex-rs/core/src/compact_remote.rs index 35b8a01fc32f..cc31d50b1326 100644 --- a/codex-rs/core/src/compact_remote.rs +++ b/codex-rs/core/src/compact_remote.rs @@ -287,8 +287,9 @@ pub(crate) async fn process_compacted_history( /// /// This intentionally keeps: /// - `assistant` messages (future remote compaction models may emit them) -/// - `user`-role warnings and compaction-generated summary messages because -/// they parse as `TurnItem::UserMessage`. +/// - `user`-role warnings that parse as `TurnItem::UserMessage` and compaction-generated summary +/// messages. Legacy warning fragments are filtered by `parse_turn_item` before they reach this +/// check. fn should_keep_compacted_history_item(item: &ResponseItem) -> bool { match item { ResponseItem::Message { role, .. } if role == "developer" => false, diff --git a/codex-rs/core/src/compact_tests.rs b/codex-rs/core/src/compact_tests.rs index def82b129854..69bd95f2df0a 100644 --- a/codex-rs/core/src/compact_tests.rs +++ b/codex-rs/core/src/compact_tests.rs @@ -23,6 +23,17 @@ async fn process_compacted_history_with_test_session( (refreshed, initial_context) } +fn user_message(text: &str) -> ResponseItem { + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: text.to_string(), + }], + phase: None, + } +} + #[test] fn content_items_to_text_joins_non_empty_segments() { let items = vec![ @@ -120,6 +131,26 @@ do things assert_eq!(vec!["real user message".to_string()], collected); } +#[test] +fn collect_user_messages_filters_legacy_warnings() { + let items = vec![ + user_message( + "Warning: The maximum number of unified exec processes you can keep open is 60 and you currently have 61 processes open. Reuse older processes or close them to prevent automatic pruning of old processes", + ), + user_message( + "Warning: apply_patch was requested via exec_command. Use the apply_patch tool instead of exec_command.", + ), + user_message( + "Warning: Your account was flagged for potentially high-risk cyber activity and this request was routed to gpt-5.2 as a fallback. To regain access to gpt-5.3-codex, apply for trusted access: https://chatgpt.com/cyber or learn more: https://developers.openai.com/codex/concepts/cyber-safety", + ), + user_message("real user message"), + ]; + + let collected = collect_user_messages(&items); + + assert_eq!(vec!["real user message".to_string()], collected); +} + #[test] fn build_token_limited_compacted_history_truncates_overlong_user_messages() { // Use a small truncation limit so the test remains fast while still validating @@ -351,6 +382,31 @@ keep me updated assert_eq!(refreshed, expected); } +#[tokio::test] +async fn process_compacted_history_drops_legacy_warnings() { + let latest_user = user_message("latest user"); + let compacted_history = vec![ + user_message( + "Warning: The maximum number of unified exec processes you can keep open is 60 and you currently have 61 processes open. Reuse older processes or close them to prevent automatic pruning of old processes", + ), + user_message( + "Warning: apply_patch was requested via exec_command. Use the apply_patch tool instead of exec_command.", + ), + user_message( + "Warning: Your account was flagged for potentially high-risk cyber activity and this request was routed to gpt-5.2 as a fallback. To regain access to gpt-5.3-codex, apply for trusted access: https://chatgpt.com/cyber or learn more: https://developers.openai.com/codex/concepts/cyber-safety", + ), + latest_user.clone(), + ]; + let (refreshed, initial_context) = process_compacted_history_with_test_session( + compacted_history, + /*previous_turn_settings*/ None, + ) + .await; + let mut expected = initial_context; + expected.push(latest_user); + assert_eq!(refreshed, expected); +} + #[tokio::test] async fn process_compacted_history_inserts_context_before_last_real_user_message_only() { let compacted_history = vec![ diff --git a/codex-rs/core/src/context/contextual_user_message.rs b/codex-rs/core/src/context/contextual_user_message.rs index f34a7e78cd44..c7ada2317f0a 100644 --- a/codex-rs/core/src/context/contextual_user_message.rs +++ b/codex-rs/core/src/context/contextual_user_message.rs @@ -6,6 +6,9 @@ use super::EnvironmentContext; use super::FragmentRegistration; use super::FragmentRegistrationProxy; use super::GoalContext; +use super::LegacyApplyPatchExecCommandWarning; +use super::LegacyModelMismatchWarning; +use super::LegacyUnifiedExecProcessLimitWarning; use super::SkillInstructions; use super::SubagentNotification; use super::TurnAborted; @@ -26,6 +29,15 @@ static SUBAGENT_NOTIFICATION_REGISTRATION: FragmentRegistrationProxy = FragmentRegistrationProxy::new(); +static LEGACY_UNIFIED_EXEC_PROCESS_LIMIT_WARNING_REGISTRATION: FragmentRegistrationProxy< + LegacyUnifiedExecProcessLimitWarning, +> = FragmentRegistrationProxy::new(); +static LEGACY_APPLY_PATCH_EXEC_COMMAND_WARNING_REGISTRATION: FragmentRegistrationProxy< + LegacyApplyPatchExecCommandWarning, +> = FragmentRegistrationProxy::new(); +static LEGACY_MODEL_MISMATCH_WARNING_REGISTRATION: FragmentRegistrationProxy< + LegacyModelMismatchWarning, +> = FragmentRegistrationProxy::new(); static CONTEXTUAL_USER_FRAGMENTS: &[&dyn FragmentRegistration] = &[ &USER_INSTRUCTIONS_REGISTRATION, @@ -35,6 +47,9 @@ static CONTEXTUAL_USER_FRAGMENTS: &[&dyn FragmentRegistration] = &[ &TURN_ABORTED_REGISTRATION, &SUBAGENT_NOTIFICATION_REGISTRATION, &GOAL_CONTEXT_REGISTRATION, + &LEGACY_UNIFIED_EXEC_PROCESS_LIMIT_WARNING_REGISTRATION, + &LEGACY_APPLY_PATCH_EXEC_COMMAND_WARNING_REGISTRATION, + &LEGACY_MODEL_MISMATCH_WARNING_REGISTRATION, ]; fn is_standard_contextual_user_text(text: &str) -> bool { diff --git a/codex-rs/core/src/context/legacy_apply_patch_exec_command_warning.rs b/codex-rs/core/src/context/legacy_apply_patch_exec_command_warning.rs new file mode 100644 index 000000000000..d8fd7da8e9b8 --- /dev/null +++ b/codex-rs/core/src/context/legacy_apply_patch_exec_command_warning.rs @@ -0,0 +1,21 @@ +use super::ContextualUserFragment; + +// This warning is not produced anymore but fragment definition is used to filter messaged from old sessions +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct LegacyApplyPatchExecCommandWarning; + +impl ContextualUserFragment for LegacyApplyPatchExecCommandWarning { + const ROLE: &'static str = "user"; + const START_MARKER: &'static str = ""; + const END_MARKER: &'static str = ""; + + fn matches_text(text: &str) -> bool { + let trimmed = text.trim(); + trimmed.starts_with("Warning: apply_patch was requested via ") + && trimmed.ends_with("Use the apply_patch tool instead of exec_command.") + } + + fn body(&self) -> String { + String::new() + } +} diff --git a/codex-rs/core/src/context/legacy_model_mismatch_warning.rs b/codex-rs/core/src/context/legacy_model_mismatch_warning.rs new file mode 100644 index 000000000000..a6afa9d7d1f3 --- /dev/null +++ b/codex-rs/core/src/context/legacy_model_mismatch_warning.rs @@ -0,0 +1,21 @@ +use super::ContextualUserFragment; + +// This warning is not produced anymore but fragment definition is used to filter messaged from old sessions +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct LegacyModelMismatchWarning; + +impl ContextualUserFragment for LegacyModelMismatchWarning { + const ROLE: &'static str = "user"; + const START_MARKER: &'static str = ""; + const END_MARKER: &'static str = ""; + + fn matches_text(text: &str) -> bool { + text.trim().starts_with( + "Warning: Your account was flagged for potentially high-risk cyber activity", + ) + } + + fn body(&self) -> String { + String::new() + } +} diff --git a/codex-rs/core/src/context/legacy_unified_exec_process_limit_warning.rs b/codex-rs/core/src/context/legacy_unified_exec_process_limit_warning.rs new file mode 100644 index 000000000000..463aeaa8aa27 --- /dev/null +++ b/codex-rs/core/src/context/legacy_unified_exec_process_limit_warning.rs @@ -0,0 +1,21 @@ +use super::ContextualUserFragment; + +// This warning is not produced anymore but fragment definition is used to filter messaged from old sessions +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct LegacyUnifiedExecProcessLimitWarning; + +impl ContextualUserFragment for LegacyUnifiedExecProcessLimitWarning { + const ROLE: &'static str = "user"; + const START_MARKER: &'static str = ""; + const END_MARKER: &'static str = ""; + + fn matches_text(text: &str) -> bool { + text.trim().starts_with( + "Warning: The maximum number of unified exec processes you can keep open is", + ) + } + + fn body(&self) -> String { + String::new() + } +} diff --git a/codex-rs/core/src/context/mod.rs b/codex-rs/core/src/context/mod.rs index f530adb4d964..bd8902536872 100644 --- a/codex-rs/core/src/context/mod.rs +++ b/codex-rs/core/src/context/mod.rs @@ -12,6 +12,9 @@ mod goal_context; mod guardian_followup_review_reminder; mod hook_additional_context; mod image_generation_instructions; +mod legacy_apply_patch_exec_command_warning; +mod legacy_model_mismatch_warning; +mod legacy_unified_exec_process_limit_warning; mod model_switch_instructions; mod network_rule_saved; mod permissions_instructions; @@ -41,6 +44,9 @@ pub(crate) use goal_context::GoalContext; pub(crate) use guardian_followup_review_reminder::GuardianFollowupReviewReminder; pub(crate) use hook_additional_context::HookAdditionalContext; pub(crate) use image_generation_instructions::ImageGenerationInstructions; +pub(crate) use legacy_apply_patch_exec_command_warning::LegacyApplyPatchExecCommandWarning; +pub(crate) use legacy_model_mismatch_warning::LegacyModelMismatchWarning; +pub(crate) use legacy_unified_exec_process_limit_warning::LegacyUnifiedExecProcessLimitWarning; pub(crate) use model_switch_instructions::ModelSwitchInstructions; pub(crate) use network_rule_saved::NetworkRuleSaved; pub use permissions_instructions::PermissionsInstructions; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 127f03496acf..ea0196a23a49 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -2436,22 +2436,6 @@ impl Session { state.record_items(items.iter(), turn_context.truncation_policy); } - pub(crate) async fn record_model_warning(&self, message: impl Into, ctx: &TurnContext) { - self.services - .session_telemetry - .counter("codex.model_warning", /*inc*/ 1, &[]); - let item = ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ContentItem::InputText { - text: format!("Warning: {}", message.into()), - }], - phase: None, - }; - - self.record_conversation_items(ctx, &[item]).await; - } - async fn maybe_warn_on_server_model_mismatch( self: &Arc, turn_context: &Arc, @@ -2488,8 +2472,6 @@ impl Session { }), ) .await; - self.record_model_warning(warning_message, turn_context) - .await; true } diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 94570785f8b1..705139cfd810 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -5797,34 +5797,6 @@ async fn refresh_mcp_servers_is_deferred_until_next_turn() { assert!(!new_token.is_cancelled()); } -#[tokio::test] -async fn record_model_warning_appends_user_message() { - let (mut session, turn_context) = make_session_and_context().await; - let features = Features::with_defaults().into(); - session.features = features; - - session - .record_model_warning("too many unified exec processes", &turn_context) - .await; - - let history = session.clone_history().await; - let history_items = history.raw_items(); - let last = history_items.last().expect("warning recorded"); - - match last { - ResponseItem::Message { role, content, .. } => { - assert_eq!(role, "user"); - assert_eq!( - content, - &vec![ContentItem::InputText { - text: "Warning: too many unified exec processes".to_string(), - }] - ); - } - other => panic!("expected user message, got {other:?}"), - } -} - #[tokio::test] async fn spawn_task_does_not_update_previous_turn_settings_for_non_run_turn_tasks() { let (sess, tc, _rx) = make_session_and_context_with_rx().await; diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 211862aa2d2b..43f799391b79 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -490,14 +490,6 @@ pub(crate) async fn intercept_apply_patch( .await { codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => { - session - .record_model_warning( - format!( - "apply_patch was requested via {tool_name}. Use the apply_patch tool instead of exec_command." - ), - turn.as_ref(), - ) - .await; let (approval_keys, effective_additional_permissions, file_system_sandbox_policy) = effective_patch_permissions(session.as_ref(), turn.as_ref(), &changes, cwd).await; match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes) diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 9b74baf64ca6..3f548150f76f 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -68,9 +68,6 @@ pub(crate) const UNIFIED_EXEC_OUTPUT_MAX_BYTES: usize = 1024 * 1024; // 1 MiB pub(crate) const UNIFIED_EXEC_OUTPUT_MAX_TOKENS: usize = UNIFIED_EXEC_OUTPUT_MAX_BYTES / 4; pub(crate) const MAX_UNIFIED_EXEC_PROCESSES: usize = 64; -// Send a warning message to the models when it reaches this number of processes. -pub(crate) const WARNING_UNIFIED_EXEC_PROCESSES: usize = 60; - pub(crate) struct UnifiedExecContext { pub session: Arc, pub turn: Arc, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 73afca2da8b2..64cb74f603de 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -37,7 +37,6 @@ use crate::unified_exec::ProcessStore; use crate::unified_exec::UnifiedExecContext; use crate::unified_exec::UnifiedExecError; use crate::unified_exec::UnifiedExecProcessManager; -use crate::unified_exec::WARNING_UNIFIED_EXEC_PROCESSES; use crate::unified_exec::WriteStdinRequest; use crate::unified_exec::async_watcher::emit_exec_end_for_unified_exec; use crate::unified_exec::async_watcher::emit_failed_exec_end_for_unified_exec; @@ -827,11 +826,11 @@ impl UnifiedExecProcessManager { session: Arc::downgrade(&context.session), last_used: started_at, }; - let (number_processes, pruned_entry) = { + let pruned_entry = { let mut store = self.process_store.lock().await; let pruned_entry = Self::prune_processes_if_needed(&mut store); store.processes.insert(process_id, entry); - (store.processes.len(), pruned_entry) + pruned_entry }; // prune_processes_if_needed runs while holding process_store; do async // network-approval cleanup only after dropping that lock. @@ -840,16 +839,6 @@ impl UnifiedExecProcessManager { pruned_entry.process.terminate(); } - if number_processes >= WARNING_UNIFIED_EXEC_PROCESSES { - context - .session - .record_model_warning( - format!("The maximum number of unified exec processes you can keep open is {WARNING_UNIFIED_EXEC_PROCESSES} and you currently have {number_processes} processes open. Reuse older processes or close them to prevent automatic pruning of old processes"), - &context.turn - ) - .await; - }; - spawn_exit_watcher( Arc::clone(&process), Arc::clone(&context.session), diff --git a/codex-rs/core/tests/suite/safety_check_downgrade.rs b/codex-rs/core/tests/suite/safety_check_downgrade.rs index eb70dafe5f1e..c937d49ad48b 100644 --- a/codex-rs/core/tests/suite/safety_check_downgrade.rs +++ b/codex-rs/core/tests/suite/safety_check_downgrade.rs @@ -59,7 +59,7 @@ fn disabled_text_turn(test: &TestCodex, text: &str) -> Op { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn openai_model_header_mismatch_emits_warning_event_and_warning_item() -> Result<()> { +async fn openai_model_header_mismatch_emits_warning_event() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -92,36 +92,6 @@ async fn openai_model_header_mismatch_emits_warning_event_and_warning_item() -> assert!(warning.message.contains(REQUESTED_MODEL)); assert!(warning.message.contains(SERVER_MODEL)); - let warning_item = wait_for_event(&test.codex, |event| { - matches!( - event, - EventMsg::RawResponseItem(raw) - if matches!( - &raw.item, - ResponseItem::Message { content, .. } - if content.iter().any(|item| matches!( - item, - ContentItem::InputText { text } if text.starts_with("Warning: ") - )) - ) - ) - }) - .await; - let EventMsg::RawResponseItem(raw) = warning_item else { - panic!("expected raw response item event"); - }; - let ResponseItem::Message { role, content, .. } = raw.item else { - panic!("expected warning to be recorded as a message item"); - }; - assert_eq!(role, "user"); - let warning_text = content.iter().find_map(|item| match item { - ContentItem::InputText { text } => Some(text.as_str()), - _ => None, - }); - let warning_text = warning_text.expect("warning message should include input_text content"); - assert!(warning_text.contains(REQUESTED_MODEL)); - assert!(warning_text.contains(SERVER_MODEL)); - let _ = wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_)) })