From 47f5026d98871629597b4da0583789ef1f938c87 Mon Sep 17 00:00:00 2001 From: Matthew Zeng Date: Thu, 16 Apr 2026 14:52:38 -0700 Subject: [PATCH 1/7] Add skill metadata budget handling --- codex-rs/core-skills/src/lib.rs | 5 + codex-rs/core-skills/src/render.rs | 319 ++++++++++++++++++++++++++++- codex-rs/core/src/codex.rs | 48 ++++- codex-rs/core/src/codex_tests.rs | 47 +++++ codex-rs/core/src/lib.rs | 4 +- codex-rs/core/src/skills.rs | 3 + 6 files changed, 408 insertions(+), 18 deletions(-) diff --git a/codex-rs/core-skills/src/lib.rs b/codex-rs/core-skills/src/lib.rs index b967e4dddc94..80ba32d8005f 100644 --- a/codex-rs/core-skills/src/lib.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -22,4 +22,9 @@ pub use model::SkillLoadOutcome; pub use model::SkillMetadata; pub use model::SkillPolicy; pub use model::filter_skill_load_outcome_for_product; +pub use render::RenderedSkillsSection; +pub use render::SkillMetadataBudget; +pub use render::SkillRenderReport; +pub use render::default_skill_metadata_budget; pub use render::render_skills_section; +pub use render::render_skills_section_with_budget; diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 9089a05436e8..fa7775bc8b86 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -1,22 +1,144 @@ use crate::model::SkillMetadata; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; +use codex_protocol::protocol::SkillScope; + +pub const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000; +pub const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 1; +const DEFAULT_OMITTED_SKILL_SAMPLE_LIMIT: usize = 5; +const APPROX_BYTES_PER_TOKEN: usize = 4; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SkillMetadataBudget { + Tokens(usize), + Characters(usize), +} + +impl SkillMetadataBudget { + fn limit(self) -> usize { + match self { + Self::Tokens(limit) | Self::Characters(limit) => limit, + } + } + + fn cost(self, text: &str) -> usize { + match self { + Self::Tokens(_) => { + text.len() + .saturating_add(APPROX_BYTES_PER_TOKEN.saturating_sub(1)) + / APPROX_BYTES_PER_TOKEN + } + Self::Characters(_) => text.chars().count(), + } + } + + fn describe(self) -> String { + match self { + Self::Tokens(limit) => format!("{limit} tokens"), + Self::Characters(limit) => format!("{limit} characters"), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct OmittedSkillSummary { + pub name: String, + pub scope: SkillScope, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SkillRenderReport { + pub budget: SkillMetadataBudget, + pub total_count: usize, + pub included_count: usize, + pub omitted_count: usize, + pub omitted_samples: Vec, +} + +impl SkillRenderReport { + pub fn warning_message(&self) -> Option { + if self.omitted_count == 0 { + return None; + } + + let omitted = if self.omitted_samples.is_empty() { + "some skills".to_string() + } else { + let mut names = self + .omitted_samples + .iter() + .map(|skill| format!("{} ({})", skill.name, scope_label(skill.scope))) + .collect::>(); + let hidden_count = self.omitted_count.saturating_sub(names.len()); + if hidden_count > 0 { + names.push(format!("{hidden_count} more")); + } + names.join(", ") + }; + + Some(format!( + "Skills list trimmed to fit the metadata budget: showing {included} of {total} enabled skills ({budget}). Omitted skills include {omitted}. Explicitly mention a skill by name or path if needed, or disable unused skills to reduce the list.", + included = self.included_count, + total = self.total_count, + budget = self.budget.describe(), + )) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RenderedSkillsSection { + pub text: String, + pub report: SkillRenderReport, +} + +pub fn default_skill_metadata_budget(context_window: Option) -> SkillMetadataBudget { + context_window + .and_then(|window| usize::try_from(window).ok()) + .filter(|window| *window > 0) + .map(|window| { + SkillMetadataBudget::Tokens( + window + .saturating_mul(SKILL_METADATA_CONTEXT_WINDOW_PERCENT) + .saturating_div(100) + .max(1), + ) + }) + .unwrap_or(SkillMetadataBudget::Characters( + DEFAULT_SKILL_METADATA_CHAR_BUDGET, + )) +} pub fn render_skills_section(skills: &[SkillMetadata]) -> Option { + render_skills_section_inner(skills, None).map(|rendered| rendered.text) +} + +pub fn render_skills_section_with_budget( + skills: &[SkillMetadata], + budget: SkillMetadataBudget, +) -> Option { + render_skills_section_inner(skills, Some(budget)) +} + +fn render_skills_section_inner( + skills: &[SkillMetadata], + budget: Option, +) -> Option { if skills.is_empty() { return None; } + let (skill_lines, report) = render_skill_lines(skills, budget); let mut lines: Vec = Vec::new(); lines.push("## Skills".to_string()); lines.push("A skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and file path so you can open the source for full instructions when using a specific skill.".to_string()); lines.push("### Available skills".to_string()); - - for skill in skills { - let path_str = skill.path_to_skills_md.to_string_lossy().replace('\\', "/"); - let name = skill.name.as_str(); - let description = skill.description.as_str(); - lines.push(format!("- {name}: {description} (file: {path_str})")); + if skill_lines.is_empty() { + lines.push("No skill metadata entries fit within the configured budget.".to_string()); + } else { + lines.extend(skill_lines); + } + if let Some(message) = report.warning_message() { + lines.push(message); } lines.push("### How to use skills".to_string()); @@ -42,7 +164,186 @@ pub fn render_skills_section(skills: &[SkillMetadata]) -> Option { ); let body = lines.join("\n"); - Some(format!( - "{SKILLS_INSTRUCTIONS_OPEN_TAG}\n{body}\n{SKILLS_INSTRUCTIONS_CLOSE_TAG}" - )) + Some(RenderedSkillsSection { + text: format!("{SKILLS_INSTRUCTIONS_OPEN_TAG}\n{body}\n{SKILLS_INSTRUCTIONS_CLOSE_TAG}"), + report, + }) +} + +fn render_skill_lines( + skills: &[SkillMetadata], + budget: Option, +) -> (Vec, SkillRenderReport) { + let ordered_skills = ordered_skills_for_budget(skills, budget.is_some()); + let Some(budget) = budget else { + return ( + ordered_skills + .iter() + .map(|skill| render_skill_line(skill)) + .collect(), + SkillRenderReport { + budget: SkillMetadataBudget::Characters(usize::MAX), + total_count: skills.len(), + included_count: skills.len(), + omitted_count: 0, + omitted_samples: Vec::new(), + }, + ); + }; + + let mut included = Vec::new(); + let mut omitted_samples = Vec::new(); + let mut used = 0usize; + let mut omitted_count = 0usize; + let mut budget_reached = false; + + for skill in ordered_skills { + let line = render_skill_line(skill); + let line_cost = budget.cost(&format!("{line}\n")); + if !budget_reached && used.saturating_add(line_cost) <= budget.limit() { + used = used.saturating_add(line_cost); + included.push(line); + continue; + } + + budget_reached = true; + omitted_count = omitted_count.saturating_add(1); + if omitted_samples.len() < DEFAULT_OMITTED_SKILL_SAMPLE_LIMIT { + omitted_samples.push(OmittedSkillSummary { + name: skill.name.clone(), + scope: skill.scope, + }); + } + } + + let report = SkillRenderReport { + budget, + total_count: skills.len(), + included_count: included.len(), + omitted_count, + omitted_samples, + }; + + (included, report) +} + +fn ordered_skills_for_budget( + skills: &[SkillMetadata], + prioritize_for_budget: bool, +) -> Vec<&SkillMetadata> { + let mut ordered = skills.iter().collect::>(); + if prioritize_for_budget { + ordered.sort_by(|a, b| { + prompt_scope_rank(a.scope) + .cmp(&prompt_scope_rank(b.scope)) + .then_with(|| a.name.cmp(&b.name)) + .then_with(|| a.path_to_skills_md.cmp(&b.path_to_skills_md)) + }); + } + ordered +} + +fn prompt_scope_rank(scope: SkillScope) -> u8 { + match scope { + SkillScope::System => 0, + SkillScope::Admin => 1, + SkillScope::Repo => 2, + SkillScope::User => 3, + } +} + +fn render_skill_line(skill: &SkillMetadata) -> String { + let path_str = skill.path_to_skills_md.to_string_lossy().replace('\\', "/"); + let name = skill.name.as_str(); + let description = skill.description.as_str(); + format!("- {name}: {description} (file: {path_str})") +} + +fn scope_label(scope: SkillScope) -> &'static str { + match scope { + SkillScope::Admin => "admin", + SkillScope::Repo => "repo", + SkillScope::User => "user", + SkillScope::System => "system", + } +} + +#[cfg(test)] +mod tests { + use super::*; + use codex_utils_absolute_path::test_support::PathBufExt; + use codex_utils_absolute_path::test_support::test_path_buf; + use pretty_assertions::assert_eq; + + fn make_skill(name: &str, scope: SkillScope) -> SkillMetadata { + SkillMetadata { + name: name.to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: test_path_buf(&format!("/tmp/{name}/SKILL.md")).abs(), + scope, + } + } + + #[test] + fn default_budget_uses_one_percent_of_full_context_window() { + assert_eq!( + default_skill_metadata_budget(Some(200_000)), + SkillMetadataBudget::Tokens(2_000) + ); + assert_eq!( + default_skill_metadata_budget(Some(99)), + SkillMetadataBudget::Tokens(1) + ); + } + + #[test] + fn default_budget_falls_back_to_characters_without_context_window() { + assert_eq!( + default_skill_metadata_budget(None), + SkillMetadataBudget::Characters(DEFAULT_SKILL_METADATA_CHAR_BUDGET) + ); + assert_eq!( + default_skill_metadata_budget(Some(-1)), + SkillMetadataBudget::Characters(DEFAULT_SKILL_METADATA_CHAR_BUDGET) + ); + } + + #[test] + fn budgeted_rendering_preserves_prompt_priority() { + let system = make_skill("system-skill", SkillScope::System); + let user = make_skill("user-skill", SkillScope::User); + let repo = make_skill("repo-skill", SkillScope::Repo); + let admin = make_skill("admin-skill", SkillScope::Admin); + let system_cost = SkillMetadataBudget::Characters(usize::MAX) + .cost(&format!("{}\n", render_skill_line(&system))); + let admin_cost = SkillMetadataBudget::Characters(usize::MAX) + .cost(&format!("{}\n", render_skill_line(&admin))); + let budget = SkillMetadataBudget::Characters(system_cost + admin_cost); + + let rendered = render_skills_section_with_budget(&[system, user, repo, admin], budget) + .expect("skills should render"); + + assert_eq!(rendered.report.included_count, 2); + assert_eq!(rendered.report.omitted_count, 2); + assert!(rendered.text.contains("- system-skill:")); + assert!(rendered.text.contains("- admin-skill:")); + assert!(!rendered.text.contains("- repo-skill:")); + assert!(!rendered.text.contains("- user-skill:")); + } + + #[test] + fn unbudgeted_rendering_preserves_input_order() { + let user = make_skill("user-skill", SkillScope::User); + let admin = make_skill("admin-skill", SkillScope::Admin); + + let rendered = render_skills_section(&[user, admin]).expect("skills should render"); + + let user_index = rendered.find("- user-skill:").expect("user skill"); + let admin_index = rendered.find("- admin-skill:").expect("admin skill"); + assert!(user_index < admin_index); + } } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 50a4c25abd1c..8d1cf8128d2c 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -7,6 +7,7 @@ use std::sync::atomic::AtomicU64; use std::time::SystemTime; use std::time::UNIX_EPOCH; +use crate::SkillRenderReport; use crate::agent::AgentControl; use crate::agent::AgentStatus; use crate::agent::Mailbox; @@ -23,6 +24,7 @@ use crate::compact::should_use_remote_compact_task; use crate::compact_remote::run_inline_remote_auto_compact_task; use crate::config::ManagedFeatures; use crate::connectors; +use crate::default_skill_metadata_budget; use crate::exec_policy::ExecPolicyManager; use crate::installation_id::resolve_installation_id; use crate::mcp_tool_exposure::build_mcp_tool_exposure; @@ -33,7 +35,7 @@ use crate::realtime_conversation::handle_audio as handle_realtime_conversation_a use crate::realtime_conversation::handle_close as handle_realtime_conversation_close; use crate::realtime_conversation::handle_start as handle_realtime_conversation_start; use crate::realtime_conversation::handle_text as handle_realtime_conversation_text; -use crate::render_skills_section; +use crate::render_skills_section_with_budget; use crate::rollout::find_thread_name_by_id; use crate::session_prefix::format_subagent_notification_message; use crate::skills_load_input_from_config; @@ -1349,6 +1351,11 @@ pub(crate) struct AppServerClientMetadata { pub(crate) client_version: Option, } +struct InitialContextBuild { + items: Vec, + skills_report: Option, +} + impl Session { pub(crate) async fn app_server_client_metadata(&self) -> AppServerClientMetadata { let state = self.state.lock().await; @@ -3820,8 +3827,18 @@ impl Session { &self, turn_context: &TurnContext, ) -> Vec { + self.build_initial_context_with_report(turn_context) + .await + .items + } + + async fn build_initial_context_with_report( + &self, + turn_context: &TurnContext, + ) -> InitialContextBuild { let mut developer_sections = Vec::::with_capacity(8); let mut contextual_user_sections = Vec::::with_capacity(2); + let mut skills_report = None; let shell = self.user_shell(); let ( reference_context_item, @@ -3931,8 +3948,12 @@ impl Session { .turn_skills .outcome .allowed_skills_for_implicit_invocation(); - if let Some(skills_section) = render_skills_section(&implicit_skills) { - developer_sections.push(skills_section); + if let Some(rendered_skills) = render_skills_section_with_budget( + &implicit_skills, + default_skill_metadata_budget(turn_context.model_info.context_window), + ) { + developer_sections.push(rendered_skills.text); + skills_report = Some(rendered_skills.report); } let loaded_plugins = self .services @@ -3995,7 +4016,10 @@ impl Session { { items.push(guardian_developer_message); } - items + InitialContextBuild { + items, + skills_report, + } } pub(crate) async fn persist_rollout_items(&self, items: &[RolloutItem]) { @@ -4042,18 +4066,26 @@ impl Session { state.reference_context_item() }; let should_inject_full_context = reference_context_item.is_none(); - let context_items = if should_inject_full_context { - self.build_initial_context(turn_context).await + let (context_items, skills_report) = if should_inject_full_context { + let initial_context = self.build_initial_context_with_report(turn_context).await; + (initial_context.items, initial_context.skills_report) } else { // Steady-state path: append only context diffs to minimize token overhead. - self.build_settings_update_items(reference_context_item.as_ref(), turn_context) - .await + ( + self.build_settings_update_items(reference_context_item.as_ref(), turn_context) + .await, + None, + ) }; let turn_context_item = turn_context.to_turn_context_item(); if !context_items.is_empty() { self.record_conversation_items(turn_context, &context_items) .await; } + if let Some(message) = skills_report.and_then(|report| report.warning_message()) { + self.send_event(turn_context, EventMsg::Warning(WarningEvent { message })) + .await; + } // Persist one `TurnContextItem` per real user turn so resume/lazy replay can recover the // latest durable baseline even when this turn emitted no model-visible context diffs. self.persist_rollout_items(&[RolloutItem::TurnContext(turn_context_item.clone())]) diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 6f4276b07748..914800208858 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -85,6 +85,7 @@ use codex_protocol::protocol::RealtimeVoice; use codex_protocol::protocol::RealtimeVoicesList; use codex_protocol::protocol::ResumedHistory; use codex_protocol::protocol::RolloutItem; +use codex_protocol::protocol::SkillScope; use codex_protocol::protocol::Submission; use codex_protocol::protocol::ThreadRolledBackEvent; use codex_protocol::protocol::TokenCountEvent; @@ -4586,6 +4587,52 @@ async fn build_initial_context_omits_default_image_save_location_without_image_h ); } +#[tokio::test] +async fn build_initial_context_trims_skill_metadata_from_context_window_budget() { + let (session, mut turn_context) = make_session_and_context().await; + let mut outcome = SkillLoadOutcome::default(); + outcome.skills = vec![ + SkillMetadata { + name: "admin-skill".to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: PathBuf::from("/tmp/admin-skill/SKILL.md").abs(), + scope: SkillScope::Admin, + }, + SkillMetadata { + name: "repo-skill".to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: PathBuf::from("/tmp/repo-skill/SKILL.md").abs(), + scope: SkillScope::Repo, + }, + ]; + turn_context.model_info.context_window = Some(100); + turn_context.turn_skills = TurnSkillsContext::new(Arc::new(outcome)); + + let initial_context = session.build_initial_context(&turn_context).await; + let developer_texts = developer_input_texts(&initial_context); + + assert!( + developer_texts + .iter() + .any(|text| text.contains("Skills list trimmed to fit the metadata budget")), + "expected skill budget warning in initial context, got {developer_texts:?}" + ); + assert!( + developer_texts + .iter() + .all(|text| !text.contains("- admin-skill:") && !text.contains("- repo-skill:")), + "expected no skill metadata entries to fit the tiny budget, got {developer_texts:?}" + ); +} + #[tokio::test] async fn handle_output_item_done_records_image_save_history_message() { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 50777ad157a2..eb57a923a36f 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -93,16 +93,18 @@ pub(crate) use skills::SkillError; pub(crate) use skills::SkillInjections; pub(crate) use skills::SkillLoadOutcome; pub(crate) use skills::SkillMetadata; +pub(crate) use skills::SkillRenderReport; pub(crate) use skills::SkillsLoadInput; pub(crate) use skills::SkillsManager; pub(crate) use skills::build_skill_injections; pub(crate) use skills::build_skill_name_counts; pub(crate) use skills::collect_env_var_dependencies; pub(crate) use skills::collect_explicit_skill_mentions; +pub(crate) use skills::default_skill_metadata_budget; pub(crate) use skills::injection; pub(crate) use skills::manager; pub(crate) use skills::maybe_emit_implicit_skill_invocation; -pub(crate) use skills::render_skills_section; +pub(crate) use skills::render_skills_section_with_budget; pub(crate) use skills::resolve_skill_dependencies_for_turn; pub(crate) use skills::skills_load_input_from_config; mod skills_watcher; diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index ed2fa20ab2e1..564ae0c2ccf2 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -21,11 +21,13 @@ pub use codex_core_skills::SkillError; pub use codex_core_skills::SkillLoadOutcome; pub use codex_core_skills::SkillMetadata; pub use codex_core_skills::SkillPolicy; +pub use codex_core_skills::SkillRenderReport; pub use codex_core_skills::SkillsLoadInput; pub use codex_core_skills::SkillsManager; pub use codex_core_skills::build_skill_name_counts; pub use codex_core_skills::collect_env_var_dependencies; pub use codex_core_skills::config_rules; +pub use codex_core_skills::default_skill_metadata_budget; pub use codex_core_skills::detect_implicit_skill_invocation_for_command; pub use codex_core_skills::filter_skill_load_outcome_for_product; pub use codex_core_skills::injection; @@ -38,6 +40,7 @@ pub use codex_core_skills::model; pub use codex_core_skills::remote; pub use codex_core_skills::render; pub use codex_core_skills::render_skills_section; +pub use codex_core_skills::render_skills_section_with_budget; pub use codex_core_skills::system; pub(crate) fn skills_load_input_from_config( From 82e5ae9611cdb7e9c1fa5c34c563582d4b607a31 Mon Sep 17 00:00:00 2001 From: Matthew Zeng Date: Thu, 16 Apr 2026 16:35:04 -0700 Subject: [PATCH 2/7] Warn on trimmed skills at thread start --- codex-rs/Cargo.lock | 1 + codex-rs/app-server/README.md | 2 + .../app-server/src/codex_message_processor.rs | 25 +++++++- codex-rs/core-skills/Cargo.toml | 1 + codex-rs/core-skills/src/render.rs | 30 +++++++--- codex-rs/core/src/codex.rs | 58 +++++++------------ codex-rs/core/src/lib.rs | 1 - codex-rs/core/src/thread_manager.rs | 9 ++- codex-rs/core/src/thread_manager_tests.rs | 50 ++++++++++++++++ codex-rs/mcp-server/src/codex_tool_runner.rs | 1 + 10 files changed, 128 insertions(+), 50 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index a499a511ca9f..103153d9a355 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2058,6 +2058,7 @@ dependencies = [ "codex-protocol", "codex-skills", "codex-utils-absolute-path", + "codex-utils-output-truncation", "codex-utils-plugins", "dirs", "dunce", diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 5cfe052b508e..8d7089ca8399 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -937,6 +937,8 @@ Event notifications are the server-initiated event stream for thread lifecycles, Thread realtime uses a separate thread-scoped notification surface. `thread/realtime/*` notifications are ephemeral transport events, not `ThreadItem`s, and are not returned by `thread/read`, `thread/resume`, or `thread/fork`. +Recoverable startup warnings use the existing `configWarning` notification: `{ summary, details?, path?, range? }`. App-server may emit it during initialization and immediately after a thread starts for session setup warnings such as skill metadata being trimmed to fit its budget. + ### Notification opt-out Clients can suppress specific notifications per connection by sending exact method names in `initialize.params.capabilities.optOutNotificationMethods`. diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 410f22ece8b1..4e349575fa1e 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -46,6 +46,7 @@ use codex_app_server_protocol::CommandExecParams; use codex_app_server_protocol::CommandExecResizeParams; use codex_app_server_protocol::CommandExecTerminateParams; use codex_app_server_protocol::CommandExecWriteParams; +use codex_app_server_protocol::ConfigWarningNotification; use codex_app_server_protocol::ConversationGitInfo; use codex_app_server_protocol::ConversationSummary; use codex_app_server_protocol::DynamicToolSpec as ApiDynamicToolSpec; @@ -2552,7 +2553,7 @@ impl CodexMessageProcessor { thread_id, thread, session_configured, - .. + thread_start_warnings, } = new_conv; if let Err(error) = Self::set_app_server_client_info( thread.as_ref(), @@ -2645,6 +2646,7 @@ impl CodexMessageProcessor { ); } + let response_connection_id = request_id.connection_id; listener_task_context .outgoing .send_response(request_id, response) @@ -2663,6 +2665,26 @@ impl CodexMessageProcessor { otel.name = "app_server.thread_start.notify_started", )) .await; + + let warning_connection_ids = [response_connection_id]; + for warning in thread_start_warnings { + listener_task_context + .outgoing + .send_server_notification_to_connections( + &warning_connection_ids, + ServerNotification::ConfigWarning(ConfigWarningNotification { + summary: warning, + details: None, + path: None, + range: None, + }), + ) + .instrument(tracing::info_span!( + "app_server.thread_start.notify_config_warning", + otel.name = "app_server.thread_start.notify_config_warning", + )) + .await; + } } Err(err) => { let error = JSONRPCErrorError { @@ -4105,6 +4127,7 @@ impl CodexMessageProcessor { thread_id, thread, session_configured, + .. }) => { let SessionConfiguredEvent { rollout_path, .. } = session_configured; let Some(rollout_path) = rollout_path else { diff --git a/codex-rs/core-skills/Cargo.toml b/codex-rs/core-skills/Cargo.toml index bc73909f929e..09278c8d0e42 100644 --- a/codex-rs/core-skills/Cargo.toml +++ b/codex-rs/core-skills/Cargo.toml @@ -24,6 +24,7 @@ codex-otel = { workspace = true } codex-protocol = { workspace = true } codex-skills = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-output-truncation = { workspace = true } codex-utils-plugins = { workspace = true } dirs = { workspace = true } dunce = { workspace = true } diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index fa7775bc8b86..ecf8f9fbac48 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -2,11 +2,11 @@ use crate::model::SkillMetadata; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; use codex_protocol::protocol::SkillScope; +use codex_utils_output_truncation::approx_token_count; pub const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000; pub const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 1; const DEFAULT_OMITTED_SKILL_SAMPLE_LIMIT: usize = 5; -const APPROX_BYTES_PER_TOKEN: usize = 4; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SkillMetadataBudget { @@ -23,11 +23,7 @@ impl SkillMetadataBudget { fn cost(self, text: &str) -> usize { match self { - Self::Tokens(_) => { - text.len() - .saturating_add(APPROX_BYTES_PER_TOKEN.saturating_sub(1)) - / APPROX_BYTES_PER_TOKEN - } + Self::Tokens(_) => approx_token_count(text), Self::Characters(_) => text.chars().count(), } } @@ -195,18 +191,16 @@ fn render_skill_lines( let mut omitted_samples = Vec::new(); let mut used = 0usize; let mut omitted_count = 0usize; - let mut budget_reached = false; for skill in ordered_skills { let line = render_skill_line(skill); let line_cost = budget.cost(&format!("{line}\n")); - if !budget_reached && used.saturating_add(line_cost) <= budget.limit() { + if used.saturating_add(line_cost) <= budget.limit() { used = used.saturating_add(line_cost); included.push(line); continue; } - budget_reached = true; omitted_count = omitted_count.saturating_add(1); if omitted_samples.len() < DEFAULT_OMITTED_SKILL_SAMPLE_LIMIT { omitted_samples.push(OmittedSkillSummary { @@ -335,6 +329,24 @@ mod tests { assert!(!rendered.text.contains("- user-skill:")); } + #[test] + fn budgeted_rendering_keeps_scanning_after_oversized_entry() { + let mut oversized = make_skill("oversized-system-skill", SkillScope::System); + oversized.description = "desc ".repeat(100); + let repo = make_skill("repo-skill", SkillScope::Repo); + let repo_cost = SkillMetadataBudget::Characters(usize::MAX) + .cost(&format!("{}\n", render_skill_line(&repo))); + let budget = SkillMetadataBudget::Characters(repo_cost); + + let rendered = + render_skills_section_with_budget(&[oversized, repo], budget).expect("skills render"); + + assert_eq!(rendered.report.included_count, 1); + assert_eq!(rendered.report.omitted_count, 1); + assert!(!rendered.text.contains("- oversized-system-skill:")); + assert!(rendered.text.contains("- repo-skill:")); + } + #[test] fn unbudgeted_rendering_preserves_input_order() { let user = make_skill("user-skill", SkillScope::User); diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 8d1cf8128d2c..d2fa602acdeb 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -7,7 +7,6 @@ use std::sync::atomic::AtomicU64; use std::time::SystemTime; use std::time::UNIX_EPOCH; -use crate::SkillRenderReport; use crate::agent::AgentControl; use crate::agent::AgentStatus; use crate::agent::Mailbox; @@ -418,12 +417,12 @@ pub struct Codex { pub(crate) type SessionLoopTermination = Shared>; -/// Wrapper returned by [`Codex::spawn`] containing the spawned [`Codex`], -/// the submission id for the initial `ConfigureSession` request and the -/// unique session id. +/// Wrapper returned by [`Codex::spawn`] containing the spawned [`Codex`], the +/// unique session id, and any warnings known at thread start. pub struct CodexSpawnOk { pub codex: Codex, pub thread_id: ThreadId, + pub thread_start_warnings: Vec, } pub(crate) struct CodexSpawnArgs { @@ -601,6 +600,14 @@ impl Codex { let model_info = models_manager .get_model_info(model.as_str(), &config.to_models_manager_config()) .await; + let implicit_skills = loaded_skills.allowed_skills_for_implicit_invocation(); + let thread_start_warnings = render_skills_section_with_budget( + &implicit_skills, + default_skill_metadata_budget(model_info.context_window), + ) + .and_then(|rendered| rendered.report.warning_message()) + .into_iter() + .collect(); let base_instructions = config .base_instructions .clone() @@ -718,7 +725,11 @@ impl Codex { session_loop_termination: session_loop_termination_from_handle(session_loop_handle), }; - Ok(CodexSpawnOk { codex, thread_id }) + Ok(CodexSpawnOk { + codex, + thread_id, + thread_start_warnings, + }) } /// Submit the `op` wrapped in a `Submission` with a unique ID. @@ -1351,11 +1362,6 @@ pub(crate) struct AppServerClientMetadata { pub(crate) client_version: Option, } -struct InitialContextBuild { - items: Vec, - skills_report: Option, -} - impl Session { pub(crate) async fn app_server_client_metadata(&self) -> AppServerClientMetadata { let state = self.state.lock().await; @@ -3827,18 +3833,8 @@ impl Session { &self, turn_context: &TurnContext, ) -> Vec { - self.build_initial_context_with_report(turn_context) - .await - .items - } - - async fn build_initial_context_with_report( - &self, - turn_context: &TurnContext, - ) -> InitialContextBuild { let mut developer_sections = Vec::::with_capacity(8); let mut contextual_user_sections = Vec::::with_capacity(2); - let mut skills_report = None; let shell = self.user_shell(); let ( reference_context_item, @@ -3953,7 +3949,6 @@ impl Session { default_skill_metadata_budget(turn_context.model_info.context_window), ) { developer_sections.push(rendered_skills.text); - skills_report = Some(rendered_skills.report); } let loaded_plugins = self .services @@ -4016,10 +4011,7 @@ impl Session { { items.push(guardian_developer_message); } - InitialContextBuild { - items, - skills_report, - } + items } pub(crate) async fn persist_rollout_items(&self, items: &[RolloutItem]) { @@ -4066,26 +4058,18 @@ impl Session { state.reference_context_item() }; let should_inject_full_context = reference_context_item.is_none(); - let (context_items, skills_report) = if should_inject_full_context { - let initial_context = self.build_initial_context_with_report(turn_context).await; - (initial_context.items, initial_context.skills_report) + let context_items = if should_inject_full_context { + self.build_initial_context(turn_context).await } else { // Steady-state path: append only context diffs to minimize token overhead. - ( - self.build_settings_update_items(reference_context_item.as_ref(), turn_context) - .await, - None, - ) + self.build_settings_update_items(reference_context_item.as_ref(), turn_context) + .await }; let turn_context_item = turn_context.to_turn_context_item(); if !context_items.is_empty() { self.record_conversation_items(turn_context, &context_items) .await; } - if let Some(message) = skills_report.and_then(|report| report.warning_message()) { - self.send_event(turn_context, EventMsg::Warning(WarningEvent { message })) - .await; - } // Persist one `TurnContextItem` per real user turn so resume/lazy replay can recover the // latest durable baseline even when this turn emitted no model-visible context diffs. self.persist_rollout_items(&[RolloutItem::TurnContext(turn_context_item.clone())]) diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index eb57a923a36f..a093b819c857 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -93,7 +93,6 @@ pub(crate) use skills::SkillError; pub(crate) use skills::SkillInjections; pub(crate) use skills::SkillLoadOutcome; pub(crate) use skills::SkillMetadata; -pub(crate) use skills::SkillRenderReport; pub(crate) use skills::SkillsLoadInput; pub(crate) use skills::SkillsManager; pub(crate) use skills::build_skill_injections; diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index b8ad75b49e1e..62546b25c798 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -137,6 +137,7 @@ pub struct NewThread { pub thread_id: ThreadId, pub thread: Arc, pub session_configured: SessionConfiguredEvent, + pub thread_start_warnings: Vec, } // TODO(ccunningham): Add an explicit non-interrupting live-turn snapshot once @@ -926,7 +927,9 @@ impl ThreadManagerState { Some(_) | None => crate::file_watcher::WatchRegistration::default(), }; let CodexSpawnOk { - codex, thread_id, .. + codex, + thread_id, + thread_start_warnings, } = Codex::spawn(CodexSpawnArgs { config, auth_manager, @@ -949,7 +952,7 @@ impl ThreadManagerState { analytics_events_client: self.analytics_events_client.clone(), }) .await?; - self.finalize_thread_spawn(codex, thread_id, watch_registration) + self.finalize_thread_spawn(codex, thread_id, watch_registration, thread_start_warnings) .await } @@ -958,6 +961,7 @@ impl ThreadManagerState { codex: Codex, thread_id: ThreadId, watch_registration: crate::file_watcher::WatchRegistration, + thread_start_warnings: Vec, ) -> CodexResult { let event = codex.next_event().await?; let session_configured = match event { @@ -982,6 +986,7 @@ impl ThreadManagerState { thread_id, thread, session_configured, + thread_start_warnings, }) } diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 06f89fc72824..186c993d9e60 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -1,5 +1,7 @@ use super::*; use crate::codex::make_session_and_context; +use crate::config::ConfigBuilder; +use crate::config::ConfigOverrides; use crate::config::test_config; use crate::rollout::RolloutRecorder; use crate::tasks::interrupted_turn_history_marker; @@ -43,6 +45,14 @@ fn assistant_msg(text: &str) -> ResponseItem { } } +fn write_skill(codex_home: &std::path::Path, name: &str) { + let skill_dir = codex_home.join("skills").join(name); + std::fs::create_dir_all(&skill_dir).expect("create skill dir"); + let description = "desc ".repeat(200); + let content = format!("---\nname: {name}\ndescription: {description}\n---\n\n# Body\n"); + std::fs::write(skill_dir.join("SKILL.md"), content).expect("write skill"); +} + #[test] fn truncates_before_requested_user_message() { let items = [ @@ -234,6 +244,46 @@ async fn ignores_session_prefix_messages_when_truncating() { ); } +#[tokio::test] +async fn start_thread_reports_skill_metadata_budget_warning() { + let temp_dir = tempdir().expect("tempdir"); + let codex_home = temp_dir.path().join("codex-home"); + let cwd = codex_home.clone(); + std::fs::create_dir_all(&codex_home).expect("create codex home"); + for i in 0..120 { + write_skill(&codex_home, &format!("demo-{i:03}")); + } + let mut config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.clone()) + .harness_overrides(ConfigOverrides { + cwd: Some(cwd), + ..Default::default() + }) + .build() + .await + .expect("load test config"); + config.model_context_window = Some(100); + + let manager = ThreadManager::with_models_provider_and_home_for_tests( + CodexAuth::from_api_key("dummy"), + config.model_provider.clone(), + config.codex_home.to_path_buf(), + Arc::new(codex_exec_server::EnvironmentManager::new( + /*exec_server_url*/ None, + )), + ); + + let new_thread = manager.start_thread(config).await.expect("start thread"); + + assert_eq!(new_thread.thread_start_warnings.len(), 1); + let warning = &new_thread.thread_start_warnings[0]; + assert!(warning.contains("Skills list trimmed to fit the metadata budget")); + assert!(warning.contains("enabled skills")); + let _ = manager + .shutdown_all_threads_bounded(Duration::from_secs(10)) + .await; +} + #[tokio::test] async fn shutdown_all_threads_bounded_submits_shutdown_to_every_thread() { let temp_dir = tempdir().expect("tempdir"); diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index eee2347202b3..d9aa4b37b26d 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -68,6 +68,7 @@ pub async fn run_codex_tool_session( thread_id, thread, session_configured, + .. } = match thread_manager.start_thread(config).await { Ok(res) => res, Err(e) => { From ce11cd4e1ab70c539dee75b138851e484bbdabab Mon Sep 17 00:00:00 2001 From: Xin Lin Date: Thu, 16 Apr 2026 23:16:10 -0700 Subject: [PATCH 3/7] Add warning + telemetry. --- .../schema/json/ServerNotification.json | 32 ++++ .../codex_app_server_protocol.schemas.json | 34 ++++ .../codex_app_server_protocol.v2.schemas.json | 34 ++++ .../schema/json/v2/WarningNotification.json | 14 ++ .../schema/typescript/ServerNotification.ts | 3 +- .../typescript/v2/WarningNotification.ts | 9 ++ .../schema/typescript/v2/index.ts | 1 + .../src/protocol/common.rs | 1 + .../app-server-protocol/src/protocol/v2.rs | 8 + codex-rs/app-server/README.md | 4 +- .../app-server/src/bespoke_event_handling.rs | 16 +- .../app-server/src/codex_message_processor.rs | 24 +-- codex-rs/core-skills/src/lib.rs | 1 - codex-rs/core-skills/src/render.rs | 145 ++---------------- codex-rs/core/src/codex.rs | 78 +++++++--- codex-rs/core/src/codex_tests.rs | 133 ++++++++++++++++ codex-rs/core/src/lib.rs | 2 +- codex-rs/core/src/skills.rs | 1 - codex-rs/core/src/thread_manager.rs | 7 +- codex-rs/core/src/thread_manager_tests.rs | 50 ------ codex-rs/otel/src/metrics/names.rs | 3 + codex-rs/tui/src/app/app_server_adapter.rs | 1 + codex-rs/tui/src/chatwidget.rs | 25 ++- codex-rs/tui/src/chatwidget/tests.rs | 2 + .../tui/src/chatwidget/tests/app_server.rs | 49 ++++++ 25 files changed, 440 insertions(+), 237 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/json/v2/WarningNotification.json create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/WarningNotification.ts diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index d02a28e370d7..c80c955b2c3d 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -3966,6 +3966,18 @@ } ] }, + "WarningNotification": { + "properties": { + "message": { + "description": "Concise warning message for the user.", + "type": "string" + } + }, + "required": [ + "message" + ], + "type": "object" + }, "WebSearchAction": { "oneOf": [ { @@ -4886,6 +4898,26 @@ "title": "Model/reroutedNotification", "type": "object" }, + { + "properties": { + "method": { + "enum": [ + "warning" + ], + "title": "WarningNotificationMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/WarningNotification" + } + }, + "required": [ + "method", + "params" + ], + "title": "WarningNotification", + "type": "object" + }, { "properties": { "method": { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index e87fa179281c..459b26d75653 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -4259,6 +4259,26 @@ "title": "Model/reroutedNotification", "type": "object" }, + { + "properties": { + "method": { + "enum": [ + "warning" + ], + "title": "WarningNotificationMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/v2/WarningNotification" + } + }, + "required": [ + "method", + "params" + ], + "title": "WarningNotification", + "type": "object" + }, { "properties": { "method": { @@ -15572,6 +15592,20 @@ ], "type": "string" }, + "WarningNotification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "message": { + "description": "Concise warning message for the user.", + "type": "string" + } + }, + "required": [ + "message" + ], + "title": "WarningNotification", + "type": "object" + }, "WebSearchAction": { "oneOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 8254ad0127e6..69058784e8ca 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -9624,6 +9624,26 @@ "title": "Model/reroutedNotification", "type": "object" }, + { + "properties": { + "method": { + "enum": [ + "warning" + ], + "title": "WarningNotificationMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/WarningNotification" + } + }, + "required": [ + "method", + "params" + ], + "title": "WarningNotification", + "type": "object" + }, { "properties": { "method": { @@ -13416,6 +13436,20 @@ ], "type": "string" }, + "WarningNotification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "message": { + "description": "Concise warning message for the user.", + "type": "string" + } + }, + "required": [ + "message" + ], + "title": "WarningNotification", + "type": "object" + }, "WebSearchAction": { "oneOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/v2/WarningNotification.json b/codex-rs/app-server-protocol/schema/json/v2/WarningNotification.json new file mode 100644 index 000000000000..a54f232e0ae5 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/json/v2/WarningNotification.json @@ -0,0 +1,14 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "message": { + "description": "Concise warning message for the user.", + "type": "string" + } + }, + "required": [ + "message" + ], + "title": "WarningNotification", + "type": "object" +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts b/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts index 1db7027febf8..99f42a170373 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts @@ -53,10 +53,11 @@ import type { TurnCompletedNotification } from "./v2/TurnCompletedNotification"; import type { TurnDiffUpdatedNotification } from "./v2/TurnDiffUpdatedNotification"; import type { TurnPlanUpdatedNotification } from "./v2/TurnPlanUpdatedNotification"; import type { TurnStartedNotification } from "./v2/TurnStartedNotification"; +import type { WarningNotification } from "./v2/WarningNotification"; import type { WindowsSandboxSetupCompletedNotification } from "./v2/WindowsSandboxSetupCompletedNotification"; import type { WindowsWorldWritableWarningNotification } from "./v2/WindowsWorldWritableWarningNotification"; /** * Notification sent from the server to the client. */ -export type ServerNotification = { "method": "error", "params": ErrorNotification } | { "method": "thread/started", "params": ThreadStartedNotification } | { "method": "thread/status/changed", "params": ThreadStatusChangedNotification } | { "method": "thread/archived", "params": ThreadArchivedNotification } | { "method": "thread/unarchived", "params": ThreadUnarchivedNotification } | { "method": "thread/closed", "params": ThreadClosedNotification } | { "method": "skills/changed", "params": SkillsChangedNotification } | { "method": "thread/name/updated", "params": ThreadNameUpdatedNotification } | { "method": "thread/tokenUsage/updated", "params": ThreadTokenUsageUpdatedNotification } | { "method": "turn/started", "params": TurnStartedNotification } | { "method": "hook/started", "params": HookStartedNotification } | { "method": "turn/completed", "params": TurnCompletedNotification } | { "method": "hook/completed", "params": HookCompletedNotification } | { "method": "turn/diff/updated", "params": TurnDiffUpdatedNotification } | { "method": "turn/plan/updated", "params": TurnPlanUpdatedNotification } | { "method": "item/started", "params": ItemStartedNotification } | { "method": "item/autoApprovalReview/started", "params": ItemGuardianApprovalReviewStartedNotification } | { "method": "item/autoApprovalReview/completed", "params": ItemGuardianApprovalReviewCompletedNotification } | { "method": "item/completed", "params": ItemCompletedNotification } | { "method": "rawResponseItem/completed", "params": RawResponseItemCompletedNotification } | { "method": "item/agentMessage/delta", "params": AgentMessageDeltaNotification } | { "method": "item/plan/delta", "params": PlanDeltaNotification } | { "method": "command/exec/outputDelta", "params": CommandExecOutputDeltaNotification } | { "method": "item/commandExecution/outputDelta", "params": CommandExecutionOutputDeltaNotification } | { "method": "item/commandExecution/terminalInteraction", "params": TerminalInteractionNotification } | { "method": "item/fileChange/outputDelta", "params": FileChangeOutputDeltaNotification } | { "method": "serverRequest/resolved", "params": ServerRequestResolvedNotification } | { "method": "item/mcpToolCall/progress", "params": McpToolCallProgressNotification } | { "method": "mcpServer/oauthLogin/completed", "params": McpServerOauthLoginCompletedNotification } | { "method": "mcpServer/startupStatus/updated", "params": McpServerStatusUpdatedNotification } | { "method": "account/updated", "params": AccountUpdatedNotification } | { "method": "account/rateLimits/updated", "params": AccountRateLimitsUpdatedNotification } | { "method": "app/list/updated", "params": AppListUpdatedNotification } | { "method": "fs/changed", "params": FsChangedNotification } | { "method": "item/reasoning/summaryTextDelta", "params": ReasoningSummaryTextDeltaNotification } | { "method": "item/reasoning/summaryPartAdded", "params": ReasoningSummaryPartAddedNotification } | { "method": "item/reasoning/textDelta", "params": ReasoningTextDeltaNotification } | { "method": "thread/compacted", "params": ContextCompactedNotification } | { "method": "model/rerouted", "params": ModelReroutedNotification } | { "method": "deprecationNotice", "params": DeprecationNoticeNotification } | { "method": "configWarning", "params": ConfigWarningNotification } | { "method": "fuzzyFileSearch/sessionUpdated", "params": FuzzyFileSearchSessionUpdatedNotification } | { "method": "fuzzyFileSearch/sessionCompleted", "params": FuzzyFileSearchSessionCompletedNotification } | { "method": "thread/realtime/started", "params": ThreadRealtimeStartedNotification } | { "method": "thread/realtime/itemAdded", "params": ThreadRealtimeItemAddedNotification } | { "method": "thread/realtime/transcript/delta", "params": ThreadRealtimeTranscriptDeltaNotification } | { "method": "thread/realtime/transcript/done", "params": ThreadRealtimeTranscriptDoneNotification } | { "method": "thread/realtime/outputAudio/delta", "params": ThreadRealtimeOutputAudioDeltaNotification } | { "method": "thread/realtime/sdp", "params": ThreadRealtimeSdpNotification } | { "method": "thread/realtime/error", "params": ThreadRealtimeErrorNotification } | { "method": "thread/realtime/closed", "params": ThreadRealtimeClosedNotification } | { "method": "windows/worldWritableWarning", "params": WindowsWorldWritableWarningNotification } | { "method": "windowsSandbox/setupCompleted", "params": WindowsSandboxSetupCompletedNotification } | { "method": "account/login/completed", "params": AccountLoginCompletedNotification }; +export type ServerNotification = { "method": "error", "params": ErrorNotification } | { "method": "thread/started", "params": ThreadStartedNotification } | { "method": "thread/status/changed", "params": ThreadStatusChangedNotification } | { "method": "thread/archived", "params": ThreadArchivedNotification } | { "method": "thread/unarchived", "params": ThreadUnarchivedNotification } | { "method": "thread/closed", "params": ThreadClosedNotification } | { "method": "skills/changed", "params": SkillsChangedNotification } | { "method": "thread/name/updated", "params": ThreadNameUpdatedNotification } | { "method": "thread/tokenUsage/updated", "params": ThreadTokenUsageUpdatedNotification } | { "method": "turn/started", "params": TurnStartedNotification } | { "method": "hook/started", "params": HookStartedNotification } | { "method": "turn/completed", "params": TurnCompletedNotification } | { "method": "hook/completed", "params": HookCompletedNotification } | { "method": "turn/diff/updated", "params": TurnDiffUpdatedNotification } | { "method": "turn/plan/updated", "params": TurnPlanUpdatedNotification } | { "method": "item/started", "params": ItemStartedNotification } | { "method": "item/autoApprovalReview/started", "params": ItemGuardianApprovalReviewStartedNotification } | { "method": "item/autoApprovalReview/completed", "params": ItemGuardianApprovalReviewCompletedNotification } | { "method": "item/completed", "params": ItemCompletedNotification } | { "method": "rawResponseItem/completed", "params": RawResponseItemCompletedNotification } | { "method": "item/agentMessage/delta", "params": AgentMessageDeltaNotification } | { "method": "item/plan/delta", "params": PlanDeltaNotification } | { "method": "command/exec/outputDelta", "params": CommandExecOutputDeltaNotification } | { "method": "item/commandExecution/outputDelta", "params": CommandExecutionOutputDeltaNotification } | { "method": "item/commandExecution/terminalInteraction", "params": TerminalInteractionNotification } | { "method": "item/fileChange/outputDelta", "params": FileChangeOutputDeltaNotification } | { "method": "serverRequest/resolved", "params": ServerRequestResolvedNotification } | { "method": "item/mcpToolCall/progress", "params": McpToolCallProgressNotification } | { "method": "mcpServer/oauthLogin/completed", "params": McpServerOauthLoginCompletedNotification } | { "method": "mcpServer/startupStatus/updated", "params": McpServerStatusUpdatedNotification } | { "method": "account/updated", "params": AccountUpdatedNotification } | { "method": "account/rateLimits/updated", "params": AccountRateLimitsUpdatedNotification } | { "method": "app/list/updated", "params": AppListUpdatedNotification } | { "method": "fs/changed", "params": FsChangedNotification } | { "method": "item/reasoning/summaryTextDelta", "params": ReasoningSummaryTextDeltaNotification } | { "method": "item/reasoning/summaryPartAdded", "params": ReasoningSummaryPartAddedNotification } | { "method": "item/reasoning/textDelta", "params": ReasoningTextDeltaNotification } | { "method": "thread/compacted", "params": ContextCompactedNotification } | { "method": "model/rerouted", "params": ModelReroutedNotification } | { "method": "warning", "params": WarningNotification } | { "method": "deprecationNotice", "params": DeprecationNoticeNotification } | { "method": "configWarning", "params": ConfigWarningNotification } | { "method": "fuzzyFileSearch/sessionUpdated", "params": FuzzyFileSearchSessionUpdatedNotification } | { "method": "fuzzyFileSearch/sessionCompleted", "params": FuzzyFileSearchSessionCompletedNotification } | { "method": "thread/realtime/started", "params": ThreadRealtimeStartedNotification } | { "method": "thread/realtime/itemAdded", "params": ThreadRealtimeItemAddedNotification } | { "method": "thread/realtime/transcript/delta", "params": ThreadRealtimeTranscriptDeltaNotification } | { "method": "thread/realtime/transcript/done", "params": ThreadRealtimeTranscriptDoneNotification } | { "method": "thread/realtime/outputAudio/delta", "params": ThreadRealtimeOutputAudioDeltaNotification } | { "method": "thread/realtime/sdp", "params": ThreadRealtimeSdpNotification } | { "method": "thread/realtime/error", "params": ThreadRealtimeErrorNotification } | { "method": "thread/realtime/closed", "params": ThreadRealtimeClosedNotification } | { "method": "windows/worldWritableWarning", "params": WindowsWorldWritableWarningNotification } | { "method": "windowsSandbox/setupCompleted", "params": WindowsSandboxSetupCompletedNotification } | { "method": "account/login/completed", "params": AccountLoginCompletedNotification }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/WarningNotification.ts b/codex-rs/app-server-protocol/schema/typescript/v2/WarningNotification.ts new file mode 100644 index 000000000000..013f6ba9bf82 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/WarningNotification.ts @@ -0,0 +1,9 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type WarningNotification = { +/** + * Concise warning message for the user. + */ +message: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index 69cec75e77bf..3ba2f22412dd 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -357,6 +357,7 @@ export type { TurnStatus } from "./TurnStatus"; export type { TurnSteerParams } from "./TurnSteerParams"; export type { TurnSteerResponse } from "./TurnSteerResponse"; export type { UserInput } from "./UserInput"; +export type { WarningNotification } from "./WarningNotification"; export type { WebSearchAction } from "./WebSearchAction"; export type { WindowsSandboxSetupCompletedNotification } from "./WindowsSandboxSetupCompletedNotification"; export type { WindowsSandboxSetupMode } from "./WindowsSandboxSetupMode"; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 7f555e068868..4fbaa093a273 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1024,6 +1024,7 @@ server_notification_definitions! { /// Deprecated: Use `ContextCompaction` item type instead. ContextCompacted => "thread/compacted" (v2::ContextCompactedNotification), ModelRerouted => "model/rerouted" (v2::ModelReroutedNotification), + Warning => "warning" (v2::WarningNotification), DeprecationNotice => "deprecationNotice" (v2::DeprecationNoticeNotification), ConfigWarning => "configWarning" (v2::ConfigWarningNotification), FuzzyFileSearchSessionUpdated => "fuzzyFileSearch/sessionUpdated" (FuzzyFileSearchSessionUpdatedNotification), diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 5173549326d3..323911934ce0 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -6564,6 +6564,14 @@ pub struct DeprecationNoticeNotification { pub details: Option, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct WarningNotification { + /// Concise warning message for the user. + pub message: String, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index deff3e1aef2e..017a4f1f1d63 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -937,7 +937,9 @@ Event notifications are the server-initiated event stream for thread lifecycles, Thread realtime uses a separate thread-scoped notification surface. `thread/realtime/*` notifications are ephemeral transport events, not `ThreadItem`s, and are not returned by `thread/read`, `thread/resume`, or `thread/fork`. -Recoverable startup warnings use the existing `configWarning` notification: `{ summary, details?, path?, range? }`. App-server may emit it during initialization and immediately after a thread starts for session setup warnings such as skill metadata being trimmed to fit its budget. +Recoverable configuration and initialization warnings use the existing `configWarning` notification: `{ summary, details?, path?, range? }`. App-server may emit it during initialization for config parsing and related setup diagnostics. + +Generic runtime warnings use the `warning` notification: `{ message }`. App-server emits this for non-fatal warnings from the core event stream, including cases where not all enabled skills are included in the model-visible skills list for a session. ### Notification opt-out diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 5d19bde70c71..a143b4471e0d 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -101,6 +101,7 @@ use codex_app_server_protocol::TurnPlanStep; use codex_app_server_protocol::TurnPlanUpdatedNotification; use codex_app_server_protocol::TurnStartedNotification; use codex_app_server_protocol::TurnStatus; +use codex_app_server_protocol::WarningNotification; use codex_app_server_protocol::build_command_execution_end_item; use codex_app_server_protocol::build_file_change_approval_request_item; use codex_app_server_protocol::build_file_change_begin_item; @@ -268,7 +269,20 @@ pub(crate) async fn apply_bespoke_event_handling( .await; } } - EventMsg::Warning(_warning_event) => {} + EventMsg::Warning(warning_event) => { + if let ApiVersion::V2 = api_version { + let notification = WarningNotification { + message: warning_event.message, + }; + if let Some(analytics_events_client) = analytics_events_client.as_ref() { + analytics_events_client + .track_notification(ServerNotification::Warning(notification.clone())); + } + outgoing + .send_server_notification(ServerNotification::Warning(notification)) + .await; + } + } EventMsg::GuardianAssessment(assessment) => { if let ApiVersion::V2 = api_version { let pending_command_execution = match build_item_from_guardian_event( diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index d84c6b193a4e..733803189c2c 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -46,7 +46,6 @@ use codex_app_server_protocol::CommandExecParams; use codex_app_server_protocol::CommandExecResizeParams; use codex_app_server_protocol::CommandExecTerminateParams; use codex_app_server_protocol::CommandExecWriteParams; -use codex_app_server_protocol::ConfigWarningNotification; use codex_app_server_protocol::ConversationGitInfo; use codex_app_server_protocol::ConversationSummary; use codex_app_server_protocol::DynamicToolSpec as ApiDynamicToolSpec; @@ -2558,7 +2557,7 @@ impl CodexMessageProcessor { thread_id, thread, session_configured, - thread_start_warnings, + .. } = new_conv; if let Err(error) = Self::set_app_server_client_info( thread.as_ref(), @@ -2651,7 +2650,6 @@ impl CodexMessageProcessor { ); } - let response_connection_id = request_id.connection_id; listener_task_context .outgoing .send_response(request_id, response) @@ -2670,26 +2668,6 @@ impl CodexMessageProcessor { otel.name = "app_server.thread_start.notify_started", )) .await; - - let warning_connection_ids = [response_connection_id]; - for warning in thread_start_warnings { - listener_task_context - .outgoing - .send_server_notification_to_connections( - &warning_connection_ids, - ServerNotification::ConfigWarning(ConfigWarningNotification { - summary: warning, - details: None, - path: None, - range: None, - }), - ) - .instrument(tracing::info_span!( - "app_server.thread_start.notify_config_warning", - otel.name = "app_server.thread_start.notify_config_warning", - )) - .await; - } } Err(err) => { let error = JSONRPCErrorError { diff --git a/codex-rs/core-skills/src/lib.rs b/codex-rs/core-skills/src/lib.rs index 80ba32d8005f..6d3ac7a4fe57 100644 --- a/codex-rs/core-skills/src/lib.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -27,4 +27,3 @@ pub use render::SkillMetadataBudget; pub use render::SkillRenderReport; pub use render::default_skill_metadata_budget; pub use render::render_skills_section; -pub use render::render_skills_section_with_budget; diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index ecf8f9fbac48..fa455cc8682b 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -5,8 +5,7 @@ use codex_protocol::protocol::SkillScope; use codex_utils_output_truncation::approx_token_count; pub const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000; -pub const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 1; -const DEFAULT_OMITTED_SKILL_SAMPLE_LIMIT: usize = 5; +pub const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SkillMetadataBudget { @@ -27,58 +26,13 @@ impl SkillMetadataBudget { Self::Characters(_) => text.chars().count(), } } - - fn describe(self) -> String { - match self { - Self::Tokens(limit) => format!("{limit} tokens"), - Self::Characters(limit) => format!("{limit} characters"), - } - } -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct OmittedSkillSummary { - pub name: String, - pub scope: SkillScope, } #[derive(Debug, Clone, PartialEq, Eq)] pub struct SkillRenderReport { - pub budget: SkillMetadataBudget, pub total_count: usize, pub included_count: usize, pub omitted_count: usize, - pub omitted_samples: Vec, -} - -impl SkillRenderReport { - pub fn warning_message(&self) -> Option { - if self.omitted_count == 0 { - return None; - } - - let omitted = if self.omitted_samples.is_empty() { - "some skills".to_string() - } else { - let mut names = self - .omitted_samples - .iter() - .map(|skill| format!("{} ({})", skill.name, scope_label(skill.scope))) - .collect::>(); - let hidden_count = self.omitted_count.saturating_sub(names.len()); - if hidden_count > 0 { - names.push(format!("{hidden_count} more")); - } - names.join(", ") - }; - - Some(format!( - "Skills list trimmed to fit the metadata budget: showing {included} of {total} enabled skills ({budget}). Omitted skills include {omitted}. Explicitly mention a skill by name or path if needed, or disable unused skills to reduce the list.", - included = self.included_count, - total = self.total_count, - budget = self.budget.describe(), - )) - } } #[derive(Debug, Clone, PartialEq, Eq)] @@ -104,20 +58,9 @@ pub fn default_skill_metadata_budget(context_window: Option) -> SkillMetada )) } -pub fn render_skills_section(skills: &[SkillMetadata]) -> Option { - render_skills_section_inner(skills, None).map(|rendered| rendered.text) -} - -pub fn render_skills_section_with_budget( +pub fn render_skills_section( skills: &[SkillMetadata], budget: SkillMetadataBudget, -) -> Option { - render_skills_section_inner(skills, Some(budget)) -} - -fn render_skills_section_inner( - skills: &[SkillMetadata], - budget: Option, ) -> Option { if skills.is_empty() { return None; @@ -128,14 +71,9 @@ fn render_skills_section_inner( lines.push("## Skills".to_string()); lines.push("A skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and file path so you can open the source for full instructions when using a specific skill.".to_string()); lines.push("### Available skills".to_string()); - if skill_lines.is_empty() { - lines.push("No skill metadata entries fit within the configured budget.".to_string()); - } else { + if !skill_lines.is_empty() { lines.extend(skill_lines); } - if let Some(message) = report.warning_message() { - lines.push(message); - } lines.push("### How to use skills".to_string()); lines.push( @@ -168,27 +106,11 @@ fn render_skills_section_inner( fn render_skill_lines( skills: &[SkillMetadata], - budget: Option, + budget: SkillMetadataBudget, ) -> (Vec, SkillRenderReport) { - let ordered_skills = ordered_skills_for_budget(skills, budget.is_some()); - let Some(budget) = budget else { - return ( - ordered_skills - .iter() - .map(|skill| render_skill_line(skill)) - .collect(), - SkillRenderReport { - budget: SkillMetadataBudget::Characters(usize::MAX), - total_count: skills.len(), - included_count: skills.len(), - omitted_count: 0, - omitted_samples: Vec::new(), - }, - ); - }; + let ordered_skills = ordered_skills_for_budget(skills); let mut included = Vec::new(); - let mut omitted_samples = Vec::new(); let mut used = 0usize; let mut omitted_count = 0usize; @@ -202,38 +124,25 @@ fn render_skill_lines( } omitted_count = omitted_count.saturating_add(1); - if omitted_samples.len() < DEFAULT_OMITTED_SKILL_SAMPLE_LIMIT { - omitted_samples.push(OmittedSkillSummary { - name: skill.name.clone(), - scope: skill.scope, - }); - } } let report = SkillRenderReport { - budget, total_count: skills.len(), included_count: included.len(), omitted_count, - omitted_samples, }; (included, report) } -fn ordered_skills_for_budget( - skills: &[SkillMetadata], - prioritize_for_budget: bool, -) -> Vec<&SkillMetadata> { +fn ordered_skills_for_budget(skills: &[SkillMetadata]) -> Vec<&SkillMetadata> { let mut ordered = skills.iter().collect::>(); - if prioritize_for_budget { - ordered.sort_by(|a, b| { - prompt_scope_rank(a.scope) - .cmp(&prompt_scope_rank(b.scope)) - .then_with(|| a.name.cmp(&b.name)) - .then_with(|| a.path_to_skills_md.cmp(&b.path_to_skills_md)) - }); - } + ordered.sort_by(|a, b| { + prompt_scope_rank(a.scope) + .cmp(&prompt_scope_rank(b.scope)) + .then_with(|| a.name.cmp(&b.name)) + .then_with(|| a.path_to_skills_md.cmp(&b.path_to_skills_md)) + }); ordered } @@ -253,15 +162,6 @@ fn render_skill_line(skill: &SkillMetadata) -> String { format!("- {name}: {description} (file: {path_str})") } -fn scope_label(scope: SkillScope) -> &'static str { - match scope { - SkillScope::Admin => "admin", - SkillScope::Repo => "repo", - SkillScope::User => "user", - SkillScope::System => "system", - } -} - #[cfg(test)] mod tests { use super::*; @@ -283,10 +183,10 @@ mod tests { } #[test] - fn default_budget_uses_one_percent_of_full_context_window() { + fn default_budget_uses_two_percent_of_full_context_window() { assert_eq!( default_skill_metadata_budget(Some(200_000)), - SkillMetadataBudget::Tokens(2_000) + SkillMetadataBudget::Tokens(4_000) ); assert_eq!( default_skill_metadata_budget(Some(99)), @@ -318,7 +218,7 @@ mod tests { .cost(&format!("{}\n", render_skill_line(&admin))); let budget = SkillMetadataBudget::Characters(system_cost + admin_cost); - let rendered = render_skills_section_with_budget(&[system, user, repo, admin], budget) + let rendered = render_skills_section(&[system, user, repo, admin], budget) .expect("skills should render"); assert_eq!(rendered.report.included_count, 2); @@ -338,24 +238,11 @@ mod tests { .cost(&format!("{}\n", render_skill_line(&repo))); let budget = SkillMetadataBudget::Characters(repo_cost); - let rendered = - render_skills_section_with_budget(&[oversized, repo], budget).expect("skills render"); + let rendered = render_skills_section(&[oversized, repo], budget).expect("skills render"); assert_eq!(rendered.report.included_count, 1); assert_eq!(rendered.report.omitted_count, 1); assert!(!rendered.text.contains("- oversized-system-skill:")); assert!(rendered.text.contains("- repo-skill:")); } - - #[test] - fn unbudgeted_rendering_preserves_input_order() { - let user = make_skill("user-skill", SkillScope::User); - let admin = make_skill("admin-skill", SkillScope::Admin); - - let rendered = render_skills_section(&[user, admin]).expect("skills should render"); - - let user_index = rendered.find("- user-skill:").expect("user skill"); - let admin_index = rendered.find("- admin-skill:").expect("admin skill"); - assert!(user_index < admin_index); - } } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index beb95e0bb358..70b9165cbfe9 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -3,7 +3,9 @@ use std::collections::HashSet; use std::fmt::Debug; use std::path::PathBuf; use std::sync::Arc; +use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicU64; +use std::sync::atomic::Ordering; use std::time::SystemTime; use std::time::UNIX_EPOCH; @@ -26,7 +28,7 @@ use crate::installation_id::resolve_installation_id; use crate::parse_turn_item; use crate::path_utils::normalize_for_native_workdir; use crate::realtime_conversation::RealtimeConversationManager; -use crate::render_skills_section_with_budget; +use crate::render_skills_section; use crate::rollout::find_thread_name_by_id; use crate::session_prefix::format_subagent_notification_message; use crate::skills_load_input_from_config; @@ -285,6 +287,9 @@ use codex_git_utils::get_git_repo_root; use codex_mcp::compute_auth_statuses; use codex_mcp::with_codex_apps_mcp; use codex_otel::SessionTelemetry; +use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_KEPT_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_TRUNCATED_METRIC; use codex_otel::THREAD_STARTED_METRIC; use codex_otel::TelemetryAuthMode; use codex_protocol::config_types::CollaborationMode; @@ -362,12 +367,13 @@ pub struct Codex { pub(crate) type SessionLoopTermination = Shared>; -/// Wrapper returned by [`Codex::spawn`] containing the spawned [`Codex`], the -/// unique session id, and any warnings known at thread start. +pub(crate) const THREAD_START_SKILLS_TRIMMED_WARNING_MESSAGE: &str = "Some enabled skills were not included in the model-visible skills list for this session. Mention a skill by name or path if you need it."; + +/// Wrapper returned by [`Codex::spawn`] containing the spawned [`Codex`] and +/// the unique session id. pub struct CodexSpawnOk { pub codex: Codex, pub thread_id: ThreadId, - pub thread_start_warnings: Vec, } pub(crate) struct CodexSpawnArgs { @@ -396,6 +402,21 @@ pub(crate) const INITIAL_SUBMIT_ID: &str = ""; pub(crate) const SUBMISSION_CHANNEL_CAPACITY: usize = 512; const CYBER_VERIFY_URL: &str = "https://chatgpt.com/cyber"; const CYBER_SAFETY_URL: &str = "https://developers.openai.com/codex/concepts/cyber-safety"; + +fn emit_thread_start_skill_metrics( + session_telemetry: &SessionTelemetry, + skill_counts: Option<(usize, usize, bool)>, +) { + let (enabled_total, kept_total, truncated) = skill_counts.unwrap_or((0, 0, false)); + let enabled_total = i64::try_from(enabled_total).unwrap_or(i64::MAX); + let kept_total = i64::try_from(kept_total).unwrap_or(i64::MAX); + let truncated = if truncated { 1 } else { 0 }; + + session_telemetry.histogram(THREAD_SKILLS_ENABLED_TOTAL_METRIC, enabled_total, &[]); + session_telemetry.histogram(THREAD_SKILLS_KEPT_TOTAL_METRIC, kept_total, &[]); + session_telemetry.histogram(THREAD_SKILLS_TRUNCATED_METRIC, truncated, &[]); +} + impl Codex { /// Spawn a new [`Codex`] and initialize the session. pub(crate) async fn spawn(args: CodexSpawnArgs) -> CodexResult { @@ -545,14 +566,6 @@ impl Codex { let model_info = models_manager .get_model_info(model.as_str(), &config.to_models_manager_config()) .await; - let implicit_skills = loaded_skills.allowed_skills_for_implicit_invocation(); - let thread_start_warnings = render_skills_section_with_budget( - &implicit_skills, - default_skill_metadata_budget(model_info.context_window), - ) - .and_then(|rendered| rendered.report.warning_message()) - .into_iter() - .collect(); let base_instructions = config .base_instructions .clone() @@ -670,11 +683,7 @@ impl Codex { session_loop_termination: session_loop_termination_from_handle(session_loop_handle), }; - Ok(CodexSpawnOk { - codex, - thread_id, - thread_start_warnings, - }) + Ok(CodexSpawnOk { codex, thread_id }) } /// Submit the `op` wrapped in a `Submission` with a unique ID. @@ -823,6 +832,7 @@ pub(crate) struct Session { pub(crate) guardian_review_session: GuardianReviewSessionManager, pub(crate) services: SessionServices, js_repl: Arc, + thread_start_skill_reported: AtomicBool, next_internal_sub_id: AtomicU64, } @@ -2247,6 +2257,7 @@ impl Session { guardian_review_session: GuardianReviewSessionManager::default(), services, js_repl, + thread_start_skill_reported: AtomicBool::new(false), next_internal_sub_id: AtomicU64::new(0), }); if let Some(network_policy_decider_session) = network_policy_decider_session { @@ -3983,10 +3994,39 @@ impl Session { .turn_skills .outcome .allowed_skills_for_implicit_invocation(); - if let Some(rendered_skills) = render_skills_section_with_budget( + let rendered_skills = render_skills_section( &implicit_skills, default_skill_metadata_budget(turn_context.model_info.context_window), - ) { + ); + if !self + .thread_start_skill_reported + .swap(true, Ordering::Relaxed) + { + let skill_counts = rendered_skills + .as_ref() + .map(|rendered| { + ( + rendered.report.total_count, + rendered.report.included_count, + rendered.report.omitted_count > 0, + ) + }) + .or_else(|| implicit_skills.is_empty().then_some((0, 0, false))); + emit_thread_start_skill_metrics(&self.services.session_telemetry, skill_counts); + if rendered_skills + .as_ref() + .is_some_and(|rendered| rendered.report.omitted_count > 0) + { + self.send_event_raw(Event { + id: String::new(), + msg: EventMsg::Warning(WarningEvent { + message: THREAD_START_SKILLS_TRIMMED_WARNING_MESSAGE.to_string(), + }), + }) + .await; + } + } + if let Some(rendered_skills) = rendered_skills { developer_sections.push(rendered_skills.text); } let loaded_plugins = self diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 83cf35002183..6f7d0e1cdb8a 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -60,6 +60,11 @@ use codex_execpolicy::Decision; use codex_execpolicy::NetworkRuleProtocol; use codex_execpolicy::Policy; use codex_network_proxy::NetworkProxyConfig; +use codex_otel::MetricsClient; +use codex_otel::MetricsConfig; +use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_KEPT_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_TRUNCATED_METRIC; use codex_otel::TelemetryAuthMode; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; @@ -110,6 +115,11 @@ use core_test_support::tracing::install_test_tracing; use core_test_support::wait_for_event; use opentelemetry::trace::TraceContextExt; use opentelemetry::trace::TraceId; +use opentelemetry_sdk::metrics::InMemoryMetricExporter; +use opentelemetry_sdk::metrics::data::AggregatedMetrics; +use opentelemetry_sdk::metrics::data::Metric; +use opentelemetry_sdk::metrics::data::MetricData; +use opentelemetry_sdk::metrics::data::ResourceMetrics; use std::path::Path; use std::time::Duration; use tokio::time::sleep; @@ -158,6 +168,54 @@ fn assistant_message(text: &str) -> ResponseItem { } } +fn test_session_telemetry_without_metadata() -> SessionTelemetry { + let exporter = InMemoryMetricExporter::default(); + let metrics = MetricsClient::new( + MetricsConfig::in_memory("test", "codex-core", env!("CARGO_PKG_VERSION"), exporter) + .with_runtime_reader(), + ) + .expect("in-memory metrics client"); + SessionTelemetry::new( + ThreadId::new(), + "gpt-5.1", + "gpt-5.1", + /*account_id*/ None, + /*account_email*/ None, + /*auth_mode*/ None, + "test_originator".to_string(), + /*log_user_prompts*/ false, + "tty".to_string(), + SessionSource::Cli, + ) + .with_metrics_without_metadata_tags(metrics) +} + +fn find_metric<'a>(resource_metrics: &'a ResourceMetrics, name: &str) -> &'a Metric { + for scope_metrics in resource_metrics.scope_metrics() { + for metric in scope_metrics.metrics() { + if metric.name() == name { + return metric; + } + } + } + panic!("metric {name} missing"); +} + +fn histogram_sum(resource_metrics: &ResourceMetrics, name: &str) -> u64 { + let metric = find_metric(resource_metrics, name); + match metric.data() { + AggregatedMetrics::F64(data) => match data { + MetricData::Histogram(histogram) => { + let points: Vec<_> = histogram.data_points().collect(); + assert_eq!(points.len(), 1); + points[0].sum().round() as u64 + } + _ => panic!("unexpected histogram aggregation"), + }, + _ => panic!("unexpected metric data type"), + } +} + fn skill_message(text: &str) -> ResponseItem { ResponseItem::Message { id: None, @@ -4626,6 +4684,81 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget() ); } +#[test] +fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() { + let session_telemetry = test_session_telemetry_without_metadata(); + + emit_thread_start_skill_metrics(&session_telemetry, Some((30, 12, true))); + + let snapshot = session_telemetry + .snapshot_metrics() + .expect("runtime metrics snapshot"); + assert_eq!( + histogram_sum(&snapshot, THREAD_SKILLS_ENABLED_TOTAL_METRIC), + 30 + ); + assert_eq!( + histogram_sum(&snapshot, THREAD_SKILLS_KEPT_TOTAL_METRIC), + 12 + ); + assert_eq!(histogram_sum(&snapshot, THREAD_SKILLS_TRUNCATED_METRIC), 1); +} + +#[tokio::test] +async fn build_initial_context_emits_thread_start_skill_warning_once() { + let (session, mut turn_context, rx) = make_session_and_context_with_rx().await; + let outcome = SkillLoadOutcome { + explicit_invocation_only: vec![], + allowlisted: vec![], + denylisted: vec![], + activated: vec![ + SkillMetadata { + name: "admin-skill".to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: PathBuf::from("/tmp/admin-skill/SKILL.md").abs(), + scope: SkillScope::Admin, + }, + SkillMetadata { + name: "repo-skill".to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: PathBuf::from("/tmp/repo-skill/SKILL.md").abs(), + scope: SkillScope::Repo, + }, + ], + unactivated: vec![], + invalid: vec![], + }; + turn_context.model_info.context_window = Some(100); + turn_context.turn_skills = TurnSkillsContext::new(Arc::new(outcome)); + + let _ = session.build_initial_context(&turn_context).await; + let warning_event = timeout(Duration::from_secs(1), rx.recv()) + .await + .expect("warning event should arrive") + .expect("warning event should be readable"); + assert!(matches!( + warning_event.msg, + EventMsg::Warning(WarningEvent { message }) + if message == THREAD_START_SKILLS_TRIMMED_WARNING_MESSAGE + )); + + let _ = session.build_initial_context(&turn_context).await; + assert!( + timeout(Duration::from_millis(100), rx.recv()) + .await + .is_err(), + "skill warning should only be emitted once" + ); +} + #[tokio::test] async fn handle_output_item_done_records_image_save_history_message() { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index a093b819c857..61ee0e648ca3 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -103,7 +103,7 @@ pub(crate) use skills::default_skill_metadata_budget; pub(crate) use skills::injection; pub(crate) use skills::manager; pub(crate) use skills::maybe_emit_implicit_skill_invocation; -pub(crate) use skills::render_skills_section_with_budget; +pub(crate) use skills::render_skills_section; pub(crate) use skills::resolve_skill_dependencies_for_turn; pub(crate) use skills::skills_load_input_from_config; mod skills_watcher; diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index 564ae0c2ccf2..320912b0fd8d 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -40,7 +40,6 @@ pub use codex_core_skills::model; pub use codex_core_skills::remote; pub use codex_core_skills::render; pub use codex_core_skills::render_skills_section; -pub use codex_core_skills::render_skills_section_with_budget; pub use codex_core_skills::system; pub(crate) fn skills_load_input_from_config( diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 62546b25c798..0701ec108694 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -137,7 +137,6 @@ pub struct NewThread { pub thread_id: ThreadId, pub thread: Arc, pub session_configured: SessionConfiguredEvent, - pub thread_start_warnings: Vec, } // TODO(ccunningham): Add an explicit non-interrupting live-turn snapshot once @@ -929,7 +928,7 @@ impl ThreadManagerState { let CodexSpawnOk { codex, thread_id, - thread_start_warnings, + .. } = Codex::spawn(CodexSpawnArgs { config, auth_manager, @@ -952,7 +951,7 @@ impl ThreadManagerState { analytics_events_client: self.analytics_events_client.clone(), }) .await?; - self.finalize_thread_spawn(codex, thread_id, watch_registration, thread_start_warnings) + self.finalize_thread_spawn(codex, thread_id, watch_registration) .await } @@ -961,7 +960,6 @@ impl ThreadManagerState { codex: Codex, thread_id: ThreadId, watch_registration: crate::file_watcher::WatchRegistration, - thread_start_warnings: Vec, ) -> CodexResult { let event = codex.next_event().await?; let session_configured = match event { @@ -986,7 +984,6 @@ impl ThreadManagerState { thread_id, thread, session_configured, - thread_start_warnings, }) } diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 186c993d9e60..06f89fc72824 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -1,7 +1,5 @@ use super::*; use crate::codex::make_session_and_context; -use crate::config::ConfigBuilder; -use crate::config::ConfigOverrides; use crate::config::test_config; use crate::rollout::RolloutRecorder; use crate::tasks::interrupted_turn_history_marker; @@ -45,14 +43,6 @@ fn assistant_msg(text: &str) -> ResponseItem { } } -fn write_skill(codex_home: &std::path::Path, name: &str) { - let skill_dir = codex_home.join("skills").join(name); - std::fs::create_dir_all(&skill_dir).expect("create skill dir"); - let description = "desc ".repeat(200); - let content = format!("---\nname: {name}\ndescription: {description}\n---\n\n# Body\n"); - std::fs::write(skill_dir.join("SKILL.md"), content).expect("write skill"); -} - #[test] fn truncates_before_requested_user_message() { let items = [ @@ -244,46 +234,6 @@ async fn ignores_session_prefix_messages_when_truncating() { ); } -#[tokio::test] -async fn start_thread_reports_skill_metadata_budget_warning() { - let temp_dir = tempdir().expect("tempdir"); - let codex_home = temp_dir.path().join("codex-home"); - let cwd = codex_home.clone(); - std::fs::create_dir_all(&codex_home).expect("create codex home"); - for i in 0..120 { - write_skill(&codex_home, &format!("demo-{i:03}")); - } - let mut config = ConfigBuilder::without_managed_config_for_tests() - .codex_home(codex_home.clone()) - .harness_overrides(ConfigOverrides { - cwd: Some(cwd), - ..Default::default() - }) - .build() - .await - .expect("load test config"); - config.model_context_window = Some(100); - - let manager = ThreadManager::with_models_provider_and_home_for_tests( - CodexAuth::from_api_key("dummy"), - config.model_provider.clone(), - config.codex_home.to_path_buf(), - Arc::new(codex_exec_server::EnvironmentManager::new( - /*exec_server_url*/ None, - )), - ); - - let new_thread = manager.start_thread(config).await.expect("start thread"); - - assert_eq!(new_thread.thread_start_warnings.len(), 1); - let warning = &new_thread.thread_start_warnings[0]; - assert!(warning.contains("Skills list trimmed to fit the metadata budget")); - assert!(warning.contains("enabled skills")); - let _ = manager - .shutdown_all_threads_bounded(Duration::from_secs(10)) - .await; -} - #[tokio::test] async fn shutdown_all_threads_bounded_submits_shutdown_to_every_thread() { let temp_dir = tempdir().expect("tempdir"); diff --git a/codex-rs/otel/src/metrics/names.rs b/codex-rs/otel/src/metrics/names.rs index fdc03fbaab1d..9dc718d2d7fd 100644 --- a/codex-rs/otel/src/metrics/names.rs +++ b/codex-rs/otel/src/metrics/names.rs @@ -37,3 +37,6 @@ pub const STARTUP_PREWARM_DURATION_METRIC: &str = "codex.startup_prewarm.duratio pub const STARTUP_PREWARM_AGE_AT_FIRST_TURN_METRIC: &str = "codex.startup_prewarm.age_at_first_turn_ms"; pub const THREAD_STARTED_METRIC: &str = "codex.thread.started"; +pub const THREAD_SKILLS_ENABLED_TOTAL_METRIC: &str = "codex.thread.skills.enabled_total"; +pub const THREAD_SKILLS_KEPT_TOTAL_METRIC: &str = "codex.thread.skills.kept_total"; +pub const THREAD_SKILLS_TRUNCATED_METRIC: &str = "codex.thread.skills.truncated"; diff --git a/codex-rs/tui/src/app/app_server_adapter.rs b/codex-rs/tui/src/app/app_server_adapter.rs index f2490ca6a2f7..2d317d7139a5 100644 --- a/codex-rs/tui/src/app/app_server_adapter.rs +++ b/codex-rs/tui/src/app/app_server_adapter.rs @@ -413,6 +413,7 @@ fn server_notification_thread_target( | ServerNotification::AccountUpdated(_) | ServerNotification::AccountRateLimitsUpdated(_) | ServerNotification::AppListUpdated(_) + | ServerNotification::Warning(_) | ServerNotification::DeprecationNotice(_) | ServerNotification::ConfigWarning(_) | ServerNotification::FuzzyFileSearchSessionUpdated(_) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 7cfcd56e17af..beeabb038ddc 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -6224,18 +6224,33 @@ impl ChatWidget { self.refresh_skills_for_current_cwd(/*force_reload*/ true); } ServerNotification::ModelRerouted(_) => {} + ServerNotification::Warning(notification) => self.on_warning(notification.message), ServerNotification::DeprecationNotice(notification) => { self.on_deprecation_notice(DeprecationNoticeEvent { summary: notification.summary, details: notification.details, }) } - ServerNotification::ConfigWarning(notification) => self.on_warning( - notification + ServerNotification::ConfigWarning(notification) => { + let summary = notification.summary.trim(); + let details = notification .details - .map(|details| format!("{}: {details}", notification.summary)) - .unwrap_or(notification.summary), - ), + .as_deref() + .map(str::trim) + .filter(|details| !details.is_empty()); + let mut message = if summary.is_empty() { + "Session warning.".to_string() + } else { + format!("Session warning: {summary}") + }; + if let Some(details) = details + && !summary.eq(details) + { + message.push(' '); + message.push_str(details); + } + self.on_warning(message) + } ServerNotification::McpServerStatusUpdated(notification) => { self.on_mcp_server_status_updated(notification) } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index e92a809e8021..6067f3a768f0 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -46,6 +46,7 @@ pub(super) use codex_app_server_protocol::CommandAction as AppServerCommandActio pub(super) use codex_app_server_protocol::CommandExecutionRequestApprovalParams as AppServerCommandExecutionRequestApprovalParams; pub(super) use codex_app_server_protocol::CommandExecutionSource as AppServerCommandExecutionSource; pub(super) use codex_app_server_protocol::CommandExecutionStatus as AppServerCommandExecutionStatus; +pub(super) use codex_app_server_protocol::ConfigWarningNotification; pub(super) use codex_app_server_protocol::ErrorNotification; pub(super) use codex_app_server_protocol::FileUpdateChange; pub(super) use codex_app_server_protocol::GuardianApprovalReview; @@ -94,6 +95,7 @@ pub(super) use codex_app_server_protocol::TurnError as AppServerTurnError; pub(super) use codex_app_server_protocol::TurnStartedNotification; pub(super) use codex_app_server_protocol::TurnStatus as AppServerTurnStatus; pub(super) use codex_app_server_protocol::UserInput as AppServerUserInput; +pub(super) use codex_app_server_protocol::WarningNotification; pub(super) use codex_config::types::ApprovalsReviewer; pub(super) use codex_config::types::Notifications; #[cfg(target_os = "windows")] diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index acfd358a2500..5bdf3ebe1d5b 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -184,6 +184,55 @@ async fn live_app_server_turn_started_sets_feedback_turn_id() { ); } +#[tokio::test] +async fn live_app_server_warning_notification_renders_message() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + chat.handle_server_notification( + ServerNotification::Warning(WarningNotification { + message: "Some enabled skills were not included in the model-visible skills list for this session. Mention a skill by name or path if you need it.".to_string(), + }), + /*replay_kind*/ None, + ); + + let cells = drain_insert_history(&mut rx); + assert_eq!(cells.len(), 1, "expected one warning history cell"); + let rendered = lines_to_single_string(&cells[0]); + assert!( + rendered.contains( + "Some enabled skills were not included in the model-visible skills list for this session." + ), + "expected warning notification message, got {rendered}" + ); + assert!( + rendered.contains("Mention a skill by name or path if you need it."), + "expected warning guidance, got {rendered}" + ); +} + +#[tokio::test] +async fn live_app_server_config_warning_prefixes_summary() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + chat.handle_server_notification( + ServerNotification::ConfigWarning(ConfigWarningNotification { + summary: "Invalid configuration; using defaults.".to_string(), + details: None, + path: None, + range: None, + }), + /*replay_kind*/ None, + ); + + let cells = drain_insert_history(&mut rx); + assert_eq!(cells.len(), 1, "expected one warning history cell"); + let rendered = lines_to_single_string(&cells[0]); + assert!( + rendered.contains("Session warning: Invalid configuration; using defaults."), + "expected prefixed config warning, got {rendered}" + ); +} + #[tokio::test] async fn live_app_server_file_change_item_started_preserves_changes() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; From f0ad9fc9ddea18fbefe7afd23e75c87d2043f6bf Mon Sep 17 00:00:00 2001 From: Xin Lin Date: Fri, 17 Apr 2026 00:13:10 -0700 Subject: [PATCH 4/7] Refactor emit_thread_start_skill_metrics. --- codex-rs/core-skills/src/render.rs | 75 ++++++++++++++++++++++- codex-rs/core/src/codex.rs | 46 +++------------ codex-rs/core/src/codex_tests.rs | 92 +++++++++++++++++------------ codex-rs/core/src/skills.rs | 1 + codex-rs/core/src/thread_manager.rs | 4 +- 5 files changed, 135 insertions(+), 83 deletions(-) diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index fa455cc8682b..48b034e739e3 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -1,8 +1,14 @@ use crate::model::SkillMetadata; +use codex_otel::SessionTelemetry; +use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_KEPT_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_TRUNCATED_METRIC; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; use codex_protocol::protocol::SkillScope; use codex_utils_output_truncation::approx_token_count; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; pub const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000; pub const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2; @@ -35,10 +41,20 @@ pub struct SkillRenderReport { pub omitted_count: usize, } +#[derive(Clone, Copy)] +pub enum SkillRenderSideEffects<'a> { + None, + ThreadStart { + session_telemetry: &'a SessionTelemetry, + reported: &'a AtomicBool, + }, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct RenderedSkillsSection { pub text: String, pub report: SkillRenderReport, + pub emit_warning: bool, } pub fn default_skill_metadata_budget(context_window: Option) -> SkillMetadataBudget { @@ -61,12 +77,20 @@ pub fn default_skill_metadata_budget(context_window: Option) -> SkillMetada pub fn render_skills_section( skills: &[SkillMetadata], budget: SkillMetadataBudget, + side_effects: SkillRenderSideEffects<'_>, ) -> Option { if skills.is_empty() { + let _ = record_skill_render_side_effects(side_effects, 0, 0, false); return None; } let (skill_lines, report) = render_skill_lines(skills, budget); + let emit_warning = record_skill_render_side_effects( + side_effects, + report.total_count, + report.included_count, + report.omitted_count > 0, + ); let mut lines: Vec = Vec::new(); lines.push("## Skills".to_string()); lines.push("A skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and file path so you can open the source for full instructions when using a specific skill.".to_string()); @@ -101,9 +125,46 @@ pub fn render_skills_section( Some(RenderedSkillsSection { text: format!("{SKILLS_INSTRUCTIONS_OPEN_TAG}\n{body}\n{SKILLS_INSTRUCTIONS_CLOSE_TAG}"), report, + emit_warning, }) } +fn record_skill_render_side_effects( + side_effects: SkillRenderSideEffects<'_>, + total_count: usize, + included_count: usize, + truncated: bool, +) -> bool { + match side_effects { + SkillRenderSideEffects::None => false, + SkillRenderSideEffects::ThreadStart { + session_telemetry, + reported, + } => { + if reported.swap(true, Ordering::Relaxed) { + return false; + } + + session_telemetry.histogram( + THREAD_SKILLS_ENABLED_TOTAL_METRIC, + i64::try_from(total_count).unwrap_or(i64::MAX), + &[], + ); + session_telemetry.histogram( + THREAD_SKILLS_KEPT_TOTAL_METRIC, + i64::try_from(included_count).unwrap_or(i64::MAX), + &[], + ); + session_telemetry.histogram( + THREAD_SKILLS_TRUNCATED_METRIC, + if truncated { 1 } else { 0 }, + &[], + ); + truncated + } + } +} + fn render_skill_lines( skills: &[SkillMetadata], budget: SkillMetadataBudget, @@ -218,11 +279,16 @@ mod tests { .cost(&format!("{}\n", render_skill_line(&admin))); let budget = SkillMetadataBudget::Characters(system_cost + admin_cost); - let rendered = render_skills_section(&[system, user, repo, admin], budget) - .expect("skills should render"); + let rendered = render_skills_section( + &[system, user, repo, admin], + budget, + SkillRenderSideEffects::None, + ) + .expect("skills should render"); assert_eq!(rendered.report.included_count, 2); assert_eq!(rendered.report.omitted_count, 2); + assert!(!rendered.emit_warning); assert!(rendered.text.contains("- system-skill:")); assert!(rendered.text.contains("- admin-skill:")); assert!(!rendered.text.contains("- repo-skill:")); @@ -238,10 +304,13 @@ mod tests { .cost(&format!("{}\n", render_skill_line(&repo))); let budget = SkillMetadataBudget::Characters(repo_cost); - let rendered = render_skills_section(&[oversized, repo], budget).expect("skills render"); + let rendered = + render_skills_section(&[oversized, repo], budget, SkillRenderSideEffects::None) + .expect("skills render"); assert_eq!(rendered.report.included_count, 1); assert_eq!(rendered.report.omitted_count, 1); + assert!(!rendered.emit_warning); assert!(!rendered.text.contains("- oversized-system-skill:")); assert!(rendered.text.contains("- repo-skill:")); } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 70b9165cbfe9..23492a47ba5f 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -5,7 +5,6 @@ use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicU64; -use std::sync::atomic::Ordering; use std::time::SystemTime; use std::time::UNIX_EPOCH; @@ -31,6 +30,7 @@ use crate::realtime_conversation::RealtimeConversationManager; use crate::render_skills_section; use crate::rollout::find_thread_name_by_id; use crate::session_prefix::format_subagent_notification_message; +use crate::skills::SkillRenderSideEffects; use crate::skills_load_input_from_config; use crate::turn_metadata::TurnMetadataState; use async_channel::Receiver; @@ -287,9 +287,6 @@ use codex_git_utils::get_git_repo_root; use codex_mcp::compute_auth_statuses; use codex_mcp::with_codex_apps_mcp; use codex_otel::SessionTelemetry; -use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC; -use codex_otel::THREAD_SKILLS_KEPT_TOTAL_METRIC; -use codex_otel::THREAD_SKILLS_TRUNCATED_METRIC; use codex_otel::THREAD_STARTED_METRIC; use codex_otel::TelemetryAuthMode; use codex_protocol::config_types::CollaborationMode; @@ -403,20 +400,6 @@ pub(crate) const SUBMISSION_CHANNEL_CAPACITY: usize = 512; const CYBER_VERIFY_URL: &str = "https://chatgpt.com/cyber"; const CYBER_SAFETY_URL: &str = "https://developers.openai.com/codex/concepts/cyber-safety"; -fn emit_thread_start_skill_metrics( - session_telemetry: &SessionTelemetry, - skill_counts: Option<(usize, usize, bool)>, -) { - let (enabled_total, kept_total, truncated) = skill_counts.unwrap_or((0, 0, false)); - let enabled_total = i64::try_from(enabled_total).unwrap_or(i64::MAX); - let kept_total = i64::try_from(kept_total).unwrap_or(i64::MAX); - let truncated = if truncated { 1 } else { 0 }; - - session_telemetry.histogram(THREAD_SKILLS_ENABLED_TOTAL_METRIC, enabled_total, &[]); - session_telemetry.histogram(THREAD_SKILLS_KEPT_TOTAL_METRIC, kept_total, &[]); - session_telemetry.histogram(THREAD_SKILLS_TRUNCATED_METRIC, truncated, &[]); -} - impl Codex { /// Spawn a new [`Codex`] and initialize the session. pub(crate) async fn spawn(args: CodexSpawnArgs) -> CodexResult { @@ -3997,26 +3980,13 @@ impl Session { let rendered_skills = render_skills_section( &implicit_skills, default_skill_metadata_budget(turn_context.model_info.context_window), + SkillRenderSideEffects::ThreadStart { + session_telemetry: &self.services.session_telemetry, + reported: &self.thread_start_skill_reported, + }, ); - if !self - .thread_start_skill_reported - .swap(true, Ordering::Relaxed) - { - let skill_counts = rendered_skills - .as_ref() - .map(|rendered| { - ( - rendered.report.total_count, - rendered.report.included_count, - rendered.report.omitted_count > 0, - ) - }) - .or_else(|| implicit_skills.is_empty().then_some((0, 0, false))); - emit_thread_start_skill_metrics(&self.services.session_telemetry, skill_counts); - if rendered_skills - .as_ref() - .is_some_and(|rendered| rendered.report.omitted_count > 0) - { + if let Some(rendered_skills) = rendered_skills { + if rendered_skills.emit_warning { self.send_event_raw(Event { id: String::new(), msg: EventMsg::Warning(WarningEvent { @@ -4025,8 +3995,6 @@ impl Session { }) .await; } - } - if let Some(rendered_skills) = rendered_skills { developer_sections.push(rendered_skills.text); } let loaded_plugins = self diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 6f7d0e1cdb8a..522f08f47999 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -13,6 +13,8 @@ use crate::function_tool::FunctionCallError; use crate::mcp_tool_exposure::DIRECT_MCP_TOOL_EXPOSURE_THRESHOLD; use crate::mcp_tool_exposure::build_mcp_tool_exposure; use crate::shell::default_user_shell; +use crate::skills::SkillRenderSideEffects; +use crate::skills::render::SkillMetadataBudget; use crate::tools::format_exec_output_str; use codex_features::Features; @@ -134,6 +136,7 @@ use serde::Deserialize; use serde_json::json; use std::path::PathBuf; use std::sync::Arc; +use std::sync::atomic::AtomicBool; use std::time::Duration as StdDuration; #[path = "codex_tests_guardian.rs"] @@ -3261,6 +3264,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { guardian_review_session: crate::guardian::GuardianReviewSessionManager::default(), services, js_repl, + thread_start_skill_reported: AtomicBool::new(false), next_internal_sub_id: AtomicU64::new(0), }; @@ -4224,6 +4228,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( guardian_review_session: crate::guardian::GuardianReviewSessionManager::default(), services, js_repl, + thread_start_skill_reported: AtomicBool::new(false), next_internal_sub_id: AtomicU64::new(0), }); @@ -4673,8 +4678,8 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget() assert!( developer_texts .iter() - .any(|text| text.contains("Skills list trimmed to fit the metadata budget")), - "expected skill budget warning in initial context, got {developer_texts:?}" + .all(|text| !text.contains(THREAD_START_SKILLS_TRIMMED_WARNING_MESSAGE)), + "expected skill budget warning to stay out of the initial context, got {developer_texts:?}" ); assert!( developer_texts @@ -4687,55 +4692,66 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget() #[test] fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() { let session_telemetry = test_session_telemetry_without_metadata(); + let reported = AtomicBool::new(false); - emit_thread_start_skill_metrics(&session_telemetry, Some((30, 12, true))); + let rendered = render_skills_section( + &[SkillMetadata { + name: "repo-skill".to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: PathBuf::from("/tmp/repo-skill/SKILL.md").abs(), + scope: SkillScope::Repo, + }], + SkillMetadataBudget::Characters(1), + SkillRenderSideEffects::ThreadStart { + session_telemetry: &session_telemetry, + reported: &reported, + }, + ) + .expect("skills should render"); + assert!(rendered.emit_warning); let snapshot = session_telemetry .snapshot_metrics() .expect("runtime metrics snapshot"); assert_eq!( histogram_sum(&snapshot, THREAD_SKILLS_ENABLED_TOTAL_METRIC), - 30 - ); - assert_eq!( - histogram_sum(&snapshot, THREAD_SKILLS_KEPT_TOTAL_METRIC), - 12 + 1 ); + assert_eq!(histogram_sum(&snapshot, THREAD_SKILLS_KEPT_TOTAL_METRIC), 0); assert_eq!(histogram_sum(&snapshot, THREAD_SKILLS_TRUNCATED_METRIC), 1); } #[tokio::test] async fn build_initial_context_emits_thread_start_skill_warning_once() { - let (session, mut turn_context, rx) = make_session_and_context_with_rx().await; - let outcome = SkillLoadOutcome { - explicit_invocation_only: vec![], - allowlisted: vec![], - denylisted: vec![], - activated: vec![ - SkillMetadata { - name: "admin-skill".to_string(), - description: "desc".to_string(), - short_description: None, - interface: None, - dependencies: None, - policy: None, - path_to_skills_md: PathBuf::from("/tmp/admin-skill/SKILL.md").abs(), - scope: SkillScope::Admin, - }, - SkillMetadata { - name: "repo-skill".to_string(), - description: "desc".to_string(), - short_description: None, - interface: None, - dependencies: None, - policy: None, - path_to_skills_md: PathBuf::from("/tmp/repo-skill/SKILL.md").abs(), - scope: SkillScope::Repo, - }, - ], - unactivated: vec![], - invalid: vec![], - }; + let (session, turn_context, rx) = make_session_and_context_with_rx().await; + let mut turn_context = Arc::into_inner(turn_context).expect("sole turn context owner"); + let mut outcome = SkillLoadOutcome::default(); + outcome.skills = vec![ + SkillMetadata { + name: "admin-skill".to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: PathBuf::from("/tmp/admin-skill/SKILL.md").abs(), + scope: SkillScope::Admin, + }, + SkillMetadata { + name: "repo-skill".to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: PathBuf::from("/tmp/repo-skill/SKILL.md").abs(), + scope: SkillScope::Repo, + }, + ]; turn_context.model_info.context_window = Some(100); turn_context.turn_skills = TurnSkillsContext::new(Arc::new(outcome)); diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index 320912b0fd8d..5b6577376cb5 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -39,6 +39,7 @@ pub use codex_core_skills::manager; pub use codex_core_skills::model; pub use codex_core_skills::remote; pub use codex_core_skills::render; +pub use codex_core_skills::render::SkillRenderSideEffects; pub use codex_core_skills::render_skills_section; pub use codex_core_skills::system; diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 0701ec108694..b8ad75b49e1e 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -926,9 +926,7 @@ impl ThreadManagerState { Some(_) | None => crate::file_watcher::WatchRegistration::default(), }; let CodexSpawnOk { - codex, - thread_id, - .. + codex, thread_id, .. } = Codex::spawn(CodexSpawnArgs { config, auth_manager, From 99e1fc8b9ef1f29e003d5015469a4789670a0e4c Mon Sep 17 00:00:00 2001 From: Xin Lin Date: Fri, 17 Apr 2026 10:26:46 -0700 Subject: [PATCH 5/7] Address comments. --- .../schema/json/ServerNotification.json | 7 ++ .../codex_app_server_protocol.schemas.json | 7 ++ .../codex_app_server_protocol.v2.schemas.json | 7 ++ .../schema/json/v2/WarningNotification.json | 7 ++ .../typescript/v2/WarningNotification.ts | 4 + .../app-server-protocol/src/protocol/v2.rs | 2 + .../app-server/src/bespoke_event_handling.rs | 1 + .../app-server/tests/suite/v2/turn_start.rs | 100 ++++++++++++++++++ codex-rs/core-skills/src/render.rs | 16 +-- codex-rs/core/src/codex.rs | 1 - codex-rs/core/src/codex/session.rs | 3 - codex-rs/core/src/codex_tests.rs | 33 +++--- codex-rs/tui/src/app/app_server_adapter.rs | 16 ++- codex-rs/tui/src/chatwidget.rs | 2 +- .../tui/src/chatwidget/tests/app_server.rs | 1 + 15 files changed, 171 insertions(+), 36 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index c80c955b2c3d..d80c2d594dc4 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -3971,6 +3971,13 @@ "message": { "description": "Concise warning message for the user.", "type": "string" + }, + "threadId": { + "description": "Optional thread target when the warning applies to a specific thread.", + "type": [ + "string", + "null" + ] } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 459b26d75653..acc1b7d747ea 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -15598,6 +15598,13 @@ "message": { "description": "Concise warning message for the user.", "type": "string" + }, + "threadId": { + "description": "Optional thread target when the warning applies to a specific thread.", + "type": [ + "string", + "null" + ] } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 69058784e8ca..5eff0104d51c 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -13442,6 +13442,13 @@ "message": { "description": "Concise warning message for the user.", "type": "string" + }, + "threadId": { + "description": "Optional thread target when the warning applies to a specific thread.", + "type": [ + "string", + "null" + ] } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/WarningNotification.json b/codex-rs/app-server-protocol/schema/json/v2/WarningNotification.json index a54f232e0ae5..460486896df4 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/WarningNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/WarningNotification.json @@ -4,6 +4,13 @@ "message": { "description": "Concise warning message for the user.", "type": "string" + }, + "threadId": { + "description": "Optional thread target when the warning applies to a specific thread.", + "type": [ + "string", + "null" + ] } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/WarningNotification.ts b/codex-rs/app-server-protocol/schema/typescript/v2/WarningNotification.ts index 013f6ba9bf82..bd3433be41ba 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/WarningNotification.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/WarningNotification.ts @@ -3,6 +3,10 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. export type WarningNotification = { +/** + * Optional thread target when the warning applies to a specific thread. + */ +threadId: string | null, /** * Concise warning message for the user. */ diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 323911934ce0..d54ad32fe8dd 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -6568,6 +6568,8 @@ pub struct DeprecationNoticeNotification { #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct WarningNotification { + /// Optional thread target when the warning applies to a specific thread. + pub thread_id: Option, /// Concise warning message for the user. pub message: String, } diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index a143b4471e0d..01fedec67622 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -272,6 +272,7 @@ pub(crate) async fn apply_bespoke_event_handling( EventMsg::Warning(warning_event) => { if let ApiVersion::V2 = api_version { let notification = WarningNotification { + thread_id: Some(conversation_id.to_string()), message: warning_event.message, }; if let Some(analytics_events_client) = analytics_events_client.as_ref() { diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index e8682d7325b2..7c136380392f 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -11,6 +11,7 @@ use app_test_support::create_shell_command_sse_response; use app_test_support::format_with_current_shell_display; use app_test_support::to_response; use app_test_support::write_mock_responses_config_toml_with_chatgpt_base_url; +use app_test_support::write_models_cache; use codex_app_server::INPUT_TOO_LARGE_ERROR_CODE; use codex_app_server::INVALID_PARAMS_ERROR_CODE; use codex_app_server_protocol::ByteRange; @@ -45,6 +46,7 @@ use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::TurnStartedNotification; use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput as V2UserInput; +use codex_app_server_protocol::WarningNotification; use codex_config::config_toml::ConfigToml; use codex_core::personality_migration::PERSONALITY_MIGRATION_FILENAME; use codex_features::FEATURES; @@ -244,6 +246,95 @@ async fn turn_start_emits_user_message_item_with_text_elements() -> Result<()> { Ok(()) } +#[tokio::test] +async fn turn_start_emits_thread_scoped_warning_notification_for_trimmed_skills() -> Result<()> { + let responses = vec![create_final_assistant_message_sse_response("Done")?]; + let server = create_mock_responses_server_sequence_unchecked(responses).await; + + let codex_home = TempDir::new()?; + create_config_toml( + codex_home.path(), + &server.uri(), + "never", + &BTreeMap::from([(Feature::Personality, true)]), + )?; + write_models_cache(codex_home.path())?; + let cache_path = codex_home.path().join("models_cache.json"); + let mut cache: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&cache_path)?)?; + let models = cache["models"] + .as_array_mut() + .expect("models_cache.json models should be an array"); + let entry = models + .first_mut() + .expect("models cache should not be empty"); + let model = entry["slug"] + .as_str() + .expect("model slug should be present") + .to_string(); + entry["context_window"] = serde_json::Value::from(100); + std::fs::write(&cache_path, serde_json::to_string_pretty(&cache)?)?; + let config_path = codex_home.path().join("config.toml"); + let config = std::fs::read_to_string(&config_path)?; + std::fs::write( + &config_path, + config.replace("model = \"mock-model\"", &format!("model = \"{model}\"")), + )?; + write_test_skill(codex_home.path(), "alpha-skill")?; + write_test_skill(codex_home.path(), "beta-skill")?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let thread_req = mcp + .send_thread_start_request(ThreadStartParams::default()) + .await?; + let thread_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; + + let turn_req = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![V2UserInput::Text { + text: "Hello".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_req)), + ) + .await??; + + let notification = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("warning"), + ) + .await??; + let params = notification.params.expect("warning params"); + let warning: WarningNotification = + serde_json::from_value(params).expect("deserialize warning notification"); + assert_eq!(warning.thread_id.as_deref(), Some(thread.id.as_str())); + assert_eq!( + warning.message, + "Some enabled skills were not included in the model-visible skills list for this session. Mention a skill by name or path if you need it." + ); + + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + Ok(()) +} + #[tokio::test] async fn thread_start_omits_empty_instruction_overrides_from_model_request() -> Result<()> { let server = responses::start_mock_server().await; @@ -2902,3 +2993,12 @@ stream_max_retries = 0 ), ) } + +fn write_test_skill(codex_home: &Path, name: &str) -> std::io::Result<()> { + let skill_dir = codex_home.join("skills").join(name); + std::fs::create_dir_all(&skill_dir)?; + std::fs::write( + skill_dir.join("SKILL.md"), + format!("---\nname: {name}\ndescription: {name} description\n---\n\n# Body\n"), + ) +} diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 48b034e739e3..f05e3d758abd 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -7,11 +7,9 @@ use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; use codex_protocol::protocol::SkillScope; use codex_utils_output_truncation::approx_token_count; -use std::sync::atomic::AtomicBool; -use std::sync::atomic::Ordering; -pub const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000; -pub const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2; +const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000; +const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SkillMetadataBudget { @@ -46,7 +44,6 @@ pub enum SkillRenderSideEffects<'a> { None, ThreadStart { session_telemetry: &'a SessionTelemetry, - reported: &'a AtomicBool, }, } @@ -137,14 +134,7 @@ fn record_skill_render_side_effects( ) -> bool { match side_effects { SkillRenderSideEffects::None => false, - SkillRenderSideEffects::ThreadStart { - session_telemetry, - reported, - } => { - if reported.swap(true, Ordering::Relaxed) { - return false; - } - + SkillRenderSideEffects::ThreadStart { session_telemetry } => { session_telemetry.histogram( THREAD_SKILLS_ENABLED_TOTAL_METRIC, i64::try_from(total_count).unwrap_or(i64::MAX), diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 0ad95b93096e..b93c7373c307 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2427,7 +2427,6 @@ impl Session { default_skill_metadata_budget(turn_context.model_info.context_window), SkillRenderSideEffects::ThreadStart { session_telemetry: &self.services.session_telemetry, - reported: &self.thread_start_skill_reported, }, ); if let Some(rendered_skills) = rendered_skills { diff --git a/codex-rs/core/src/codex/session.rs b/codex-rs/core/src/codex/session.rs index 1f1df4d8e615..766ac79ec14c 100644 --- a/codex-rs/core/src/codex/session.rs +++ b/codex-rs/core/src/codex/session.rs @@ -1,5 +1,4 @@ use super::*; -use std::sync::atomic::AtomicBool; /// Context for an initialized model agent /// @@ -25,7 +24,6 @@ pub(crate) struct Session { pub(crate) guardian_review_session: GuardianReviewSessionManager, pub(crate) services: SessionServices, pub(super) js_repl: Arc, - pub(super) thread_start_skill_reported: AtomicBool, pub(super) next_internal_sub_id: AtomicU64, } @@ -708,7 +706,6 @@ impl Session { guardian_review_session: GuardianReviewSessionManager::default(), services, js_repl, - thread_start_skill_reported: AtomicBool::new(false), next_internal_sub_id: AtomicU64::new(0), }); if let Some(network_policy_decider_session) = network_policy_decider_session { diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 522f08f47999..741876595ee0 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -113,6 +113,7 @@ use core_test_support::responses::mount_sse_once; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::test_codex::test_codex; +use core_test_support::test_path_buf; use core_test_support::tracing::install_test_tracing; use core_test_support::wait_for_event; use opentelemetry::trace::TraceContextExt; @@ -3264,7 +3265,6 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { guardian_review_session: crate::guardian::GuardianReviewSessionManager::default(), services, js_repl, - thread_start_skill_reported: AtomicBool::new(false), next_internal_sub_id: AtomicU64::new(0), }; @@ -4228,7 +4228,6 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( guardian_review_session: crate::guardian::GuardianReviewSessionManager::default(), services, js_repl, - thread_start_skill_reported: AtomicBool::new(false), next_internal_sub_id: AtomicU64::new(0), }); @@ -4655,7 +4654,7 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget() interface: None, dependencies: None, policy: None, - path_to_skills_md: PathBuf::from("/tmp/admin-skill/SKILL.md").abs(), + path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(), scope: SkillScope::Admin, }, SkillMetadata { @@ -4665,7 +4664,7 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget() interface: None, dependencies: None, policy: None, - path_to_skills_md: PathBuf::from("/tmp/repo-skill/SKILL.md").abs(), + path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), scope: SkillScope::Repo, }, ]; @@ -4692,8 +4691,6 @@ async fn build_initial_context_trims_skill_metadata_from_context_window_budget() #[test] fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() { let session_telemetry = test_session_telemetry_without_metadata(); - let reported = AtomicBool::new(false); - let rendered = render_skills_section( &[SkillMetadata { name: "repo-skill".to_string(), @@ -4702,13 +4699,12 @@ fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() { interface: None, dependencies: None, policy: None, - path_to_skills_md: PathBuf::from("/tmp/repo-skill/SKILL.md").abs(), + path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), scope: SkillScope::Repo, }], SkillMetadataBudget::Characters(1), SkillRenderSideEffects::ThreadStart { session_telemetry: &session_telemetry, - reported: &reported, }, ) .expect("skills should render"); @@ -4726,7 +4722,7 @@ fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() { } #[tokio::test] -async fn build_initial_context_emits_thread_start_skill_warning_once() { +async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_builds() { let (session, turn_context, rx) = make_session_and_context_with_rx().await; let mut turn_context = Arc::into_inner(turn_context).expect("sole turn context owner"); let mut outcome = SkillLoadOutcome::default(); @@ -4738,7 +4734,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_once() { interface: None, dependencies: None, policy: None, - path_to_skills_md: PathBuf::from("/tmp/admin-skill/SKILL.md").abs(), + path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(), scope: SkillScope::Admin, }, SkillMetadata { @@ -4748,7 +4744,7 @@ async fn build_initial_context_emits_thread_start_skill_warning_once() { interface: None, dependencies: None, policy: None, - path_to_skills_md: PathBuf::from("/tmp/repo-skill/SKILL.md").abs(), + path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), scope: SkillScope::Repo, }, ]; @@ -4767,12 +4763,15 @@ async fn build_initial_context_emits_thread_start_skill_warning_once() { )); let _ = session.build_initial_context(&turn_context).await; - assert!( - timeout(Duration::from_millis(100), rx.recv()) - .await - .is_err(), - "skill warning should only be emitted once" - ); + let warning_event = timeout(Duration::from_secs(1), rx.recv()) + .await + .expect("warning event should arrive on repeated build") + .expect("warning event should be readable"); + assert!(matches!( + warning_event.msg, + EventMsg::Warning(WarningEvent { message }) + if message == THREAD_START_SKILLS_TRIMMED_WARNING_MESSAGE + )); } #[tokio::test] diff --git a/codex-rs/tui/src/app/app_server_adapter.rs b/codex-rs/tui/src/app/app_server_adapter.rs index 2d317d7139a5..7e438fe48c26 100644 --- a/codex-rs/tui/src/app/app_server_adapter.rs +++ b/codex-rs/tui/src/app/app_server_adapter.rs @@ -407,13 +407,13 @@ fn server_notification_thread_target( ServerNotification::ThreadRealtimeClosed(notification) => { Some(notification.thread_id.as_str()) } + ServerNotification::Warning(notification) => notification.thread_id.as_deref(), ServerNotification::SkillsChanged(_) | ServerNotification::McpServerStatusUpdated(_) | ServerNotification::McpServerOauthLoginCompleted(_) | ServerNotification::AccountUpdated(_) | ServerNotification::AccountRateLimitsUpdated(_) | ServerNotification::AppListUpdated(_) - | ServerNotification::Warning(_) | ServerNotification::DeprecationNotice(_) | ServerNotification::ConfigWarning(_) | ServerNotification::FuzzyFileSearchSessionUpdated(_) @@ -1056,6 +1056,7 @@ mod tests { use codex_app_server_protocol::TurnCompletedNotification; use codex_app_server_protocol::TurnError; use codex_app_server_protocol::TurnStatus; + use codex_app_server_protocol::WarningNotification; use codex_protocol::ThreadId; use codex_protocol::items::AgentMessageContent; use codex_protocol::items::AgentMessageItem; @@ -1650,4 +1651,17 @@ mod tests { assert_eq!(raw_reasoning.text, "hidden chain"); assert!(matches!(events[3].msg, EventMsg::TurnComplete(_))); } + + #[test] + fn warning_notifications_route_to_threads_when_thread_id_is_present() { + let thread_id = ThreadId::new(); + let notification = ServerNotification::Warning(WarningNotification { + thread_id: Some(thread_id.to_string()), + message: "warning".to_string(), + }); + + let target = server_notification_thread_target(¬ification); + + assert_eq!(target, ServerNotificationThreadTarget::Thread(thread_id)); + } } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index b67c5f33c809..9696c47e69a3 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -6246,7 +6246,7 @@ impl ChatWidget { if let Some(details) = details && !summary.eq(details) { - message.push(' '); + message.push_str(": "); message.push_str(details); } self.on_warning(message) diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index 5bdf3ebe1d5b..556018a4234b 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -190,6 +190,7 @@ async fn live_app_server_warning_notification_renders_message() { chat.handle_server_notification( ServerNotification::Warning(WarningNotification { + thread_id: None, message: "Some enabled skills were not included in the model-visible skills list for this session. Mention a skill by name or path if you need it.".to_string(), }), /*replay_kind*/ None, From a514dd4cd4ad1c5bf20994af718f359af51b9fba Mon Sep 17 00:00:00 2001 From: Xin Lin Date: Fri, 17 Apr 2026 11:25:16 -0700 Subject: [PATCH 6/7] cleanup. --- codex-rs/core-skills/src/render.rs | 9 +++++-- codex-rs/core/src/session/tests.rs | 1 - codex-rs/tui/src/app/app_server_adapter.rs | 2 ++ codex-rs/tui/src/chatwidget.rs | 24 ++++--------------- .../tui/src/chatwidget/tests/app_server.rs | 9 +++---- 5 files changed, 19 insertions(+), 26 deletions(-) diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index f05e3d758abd..ad674fbffa90 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -77,7 +77,12 @@ pub fn render_skills_section( side_effects: SkillRenderSideEffects<'_>, ) -> Option { if skills.is_empty() { - let _ = record_skill_render_side_effects(side_effects, 0, 0, false); + let _ = record_skill_render_side_effects( + side_effects, + /*total_count*/ 0, + /*included_count*/ 0, + /*truncated*/ false, + ); return None; } @@ -248,7 +253,7 @@ mod tests { #[test] fn default_budget_falls_back_to_characters_without_context_window() { assert_eq!( - default_skill_metadata_budget(None), + default_skill_metadata_budget(/*context_window*/ None), SkillMetadataBudget::Characters(DEFAULT_SKILL_METADATA_CHAR_BUDGET) ); assert_eq!( diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index be80407ef26e..30de1a073e58 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -132,7 +132,6 @@ use serde::Deserialize; use serde_json::json; use std::path::PathBuf; use std::sync::Arc; -use std::sync::atomic::AtomicBool; use std::time::Duration as StdDuration; mod guardian_tests; diff --git a/codex-rs/tui/src/app/app_server_adapter.rs b/codex-rs/tui/src/app/app_server_adapter.rs index 4dc09be44f9e..f0abb5326ee3 100644 --- a/codex-rs/tui/src/app/app_server_adapter.rs +++ b/codex-rs/tui/src/app/app_server_adapter.rs @@ -1050,8 +1050,10 @@ fn app_server_codex_error_info_to_core( #[cfg(test)] mod tests { + use super::ServerNotificationThreadTarget; use super::command_execution_started_event; use super::server_notification_thread_events; + use super::server_notification_thread_target; use super::thread_snapshot_events; use super::turn_snapshot_events; use codex_app_server_protocol::AgentMessageDeltaNotification; diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index e1aa6c0cf7b2..918136d64283 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -6201,26 +6201,12 @@ impl ChatWidget { details: notification.details, }) } - ServerNotification::ConfigWarning(notification) => { - let summary = notification.summary.trim(); - let details = notification + ServerNotification::ConfigWarning(notification) => self.on_warning( + notification .details - .as_deref() - .map(str::trim) - .filter(|details| !details.is_empty()); - let mut message = if summary.is_empty() { - "Session warning.".to_string() - } else { - format!("Session warning: {summary}") - }; - if let Some(details) = details - && !summary.eq(details) - { - message.push_str(": "); - message.push_str(details); - } - self.on_warning(message) - } + .map(|details| format!("{}: {details}", notification.summary)) + .unwrap_or(notification.summary), + ), ServerNotification::McpServerStatusUpdated(notification) => { self.on_mcp_server_status_updated(notification) } diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index 556018a4234b..702bd08ef908 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -199,14 +199,15 @@ async fn live_app_server_warning_notification_renders_message() { let cells = drain_insert_history(&mut rx); assert_eq!(cells.len(), 1, "expected one warning history cell"); let rendered = lines_to_single_string(&cells[0]); + let normalized = rendered.split_whitespace().collect::>().join(" "); assert!( - rendered.contains( + normalized.contains( "Some enabled skills were not included in the model-visible skills list for this session." ), "expected warning notification message, got {rendered}" ); assert!( - rendered.contains("Mention a skill by name or path if you need it."), + normalized.contains("Mention a skill by name or path if you need it."), "expected warning guidance, got {rendered}" ); } @@ -229,8 +230,8 @@ async fn live_app_server_config_warning_prefixes_summary() { assert_eq!(cells.len(), 1, "expected one warning history cell"); let rendered = lines_to_single_string(&cells[0]); assert!( - rendered.contains("Session warning: Invalid configuration; using defaults."), - "expected prefixed config warning, got {rendered}" + rendered.contains("Invalid configuration; using defaults."), + "expected config warning summary, got {rendered}" ); } From 0d1de00d5e01b6d1ba135f5792baf238b6526d47 Mon Sep 17 00:00:00 2001 From: Xin Lin Date: Fri, 17 Apr 2026 17:39:32 -0700 Subject: [PATCH 7/7] More cleanup. --- codex-rs/app-server/README.md | 2 +- codex-rs/app-server/tests/suite/v2/turn_start.rs | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 3b33ee8b6539..9f5373378dd3 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -939,7 +939,7 @@ Thread realtime uses a separate thread-scoped notification surface. `thread/real Recoverable configuration and initialization warnings use the existing `configWarning` notification: `{ summary, details?, path?, range? }`. App-server may emit it during initialization for config parsing and related setup diagnostics. -Generic runtime warnings use the `warning` notification: `{ message }`. App-server emits this for non-fatal warnings from the core event stream, including cases where not all enabled skills are included in the model-visible skills list for a session. +Generic runtime warnings use the `warning` notification: `{ threadId?, message }`. App-server emits this for non-fatal warnings from the core event stream, including cases where not all enabled skills are included in the model-visible skills list for a session. ### Notification opt-out diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 7c136380392f..9f491b336835 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -332,6 +332,22 @@ async fn turn_start_emits_thread_scoped_warning_notification_for_trimmed_skills( ) .await??; + let requests = server + .received_requests() + .await + .expect("failed to fetch received requests"); + let request = requests + .last() + .expect("expected at least one model request"); + assert!( + body_contains(request, "## Skills"), + "expected outgoing request to include the skills section" + ); + assert!( + !body_contains(request, "- alpha-skill:") && !body_contains(request, "- beta-skill:"), + "expected trimmed skills to be omitted from the outgoing request body" + ); + Ok(()) }