From bd9a28a839d3dc4cf1facdf66cd02bb5732189e3 Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Wed, 22 Jul 2026 11:49:14 +0000 Subject: [PATCH] Drop skill descriptions before omitting catalog entries (#34738) ## What changed When even the minimum skill catalog exceeds its metadata budget, render each included entry without its description. This allows more skill names and locators to fit before the remaining entries are omitted. ## Testing Add coverage for extreme budget pressure and verify both the extension and production-turn catalogs omit descriptions while staying within budget. GitOrigin-RevId: 72cec10e9587beabbd3d8c6d53b931deb76824e6 --- codex-rs/core/tests/suite/skills_extension.rs | 3 + codex-rs/ext/skills/src/render.rs | 4 +- codex-rs/ext/skills/tests/skills_extension.rs | 62 +++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/tests/suite/skills_extension.rs b/codex-rs/core/tests/suite/skills_extension.rs index 2560d9414b..c4cd98e66b 100644 --- a/codex-rs/core/tests/suite/skills_extension.rs +++ b/codex-rs/core/tests/suite/skills_extension.rs @@ -141,6 +141,9 @@ async fn production_turn_scales_extension_catalog_from_resolved_model_window() - .count(); assert!(catalog_text.contains("additional skills omitted")); + assert!(!catalog_text.contains( + "A description long enough to keep the catalog under sustained budget pressure." + )); assert!(metadata_cost <= expected_budget); included_counts.push(included_count); } diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index 04fa77d9bc..440385d800 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -219,8 +219,8 @@ fn render_skill_lines( let mut used = 0usize; let mut omitted = 0usize; for line in skill_lines { - let rendered = line.render_full(); - let next_used = used.saturating_add(metadata_line_cost(budget, &rendered)); + let rendered = line.render_minimum(); + let next_used = used.saturating_add(line.minimum_cost(budget)); if next_used <= budget.limit() { used = next_used; included.push(rendered); diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index 75e6036dfa..7e95a87c4b 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -477,6 +477,68 @@ async fn moderate_budget_pressure_keeps_every_catalog_entry() -> TestResult { Ok(()) } +#[tokio::test] +async fn extreme_budget_pressure_removes_descriptions_before_omitting_entries() -> TestResult { + let entries = (0..200) + .map(|index| { + let package_id = format!("orchestrator/skill-{index:03}"); + let mut entry = test_entry( + SkillSourceKind::Orchestrator, + "codex_apps", + &package_id, + &format!("skill://{package_id}/SKILL.md"), + ); + entry.description = format!("description-{index:03}"); + entry + }) + .collect(); + let providers = + SkillProviders::new().with_orchestrator_provider(Arc::new(StaticSkillProvider { + catalog: SkillCatalog { + entries, + warnings: Vec::new(), + }, + read_requests: Arc::new(Mutex::new(Vec::new())), + list_calls: None, + fail_first_list: false, + })); + let mut builder = ExtensionRegistryBuilder::new(); + install_with_providers(&mut builder, providers, skills_extension_config); + let registry = builder.build(); + let session_store = ExtensionData::new("session"); + let thread_store = ExtensionData::new("thread"); + let session_source = SessionSource::Cli; + let config = default_config(); + registry.thread_lifecycle_contributors()[0] + .on_thread_start(ThreadStartInput { + config: &config, + session_source: &session_source, + persistent_thread_state_available: true, + environments: &[], + session_store: &session_store, + thread_store: &thread_store, + }) + .await; + + let fragments = registry.context_contributors()[0] + .contribute_thread_context(&session_store, &thread_store, &ExtensionData::new("step")) + .await; + assert_eq!(1, fragments.len()); + let rendered = fragments[0].text(); + let included_count = rendered + .lines() + .filter(|line| line.starts_with("- skill-")) + .count(); + assert!(included_count > 0); + assert!(included_count < 200); + assert!(rendered.contains("- skill-000: (orchestrator resource:")); + assert!(!rendered.contains("- skill-199:")); + assert!(!rendered.contains("description-")); + assert!(rendered.contains("additional skills omitted from this bounded skills list")); + + Ok(()) +} + #[tokio::test] async fn skills_list_truncates_catalog_descriptions_in_tool_output() -> TestResult { let description = "x".repeat(1_025);