From 6dd56b62a6e17394d508ee630d4f57c2fec6316e Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Thu, 18 Jun 2026 17:09:33 -0700 Subject: [PATCH 1/8] Truncate overlong skill descriptions --- codex-rs/core-skills/src/loader.rs | 12 +++++- codex-rs/core-skills/src/loader_tests.rs | 31 ++++++++------ .../ext/skills/src/provider/orchestrator.rs | 14 +++++-- codex-rs/external-agent-migration/src/lib.rs | 41 ++++++++++++++++--- 4 files changed, 77 insertions(+), 21 deletions(-) diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index 64861c45ebd0..48022d58b3e1 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -698,12 +698,22 @@ async fn parse_skill_file( .as_deref() .map(sanitize_single_line) .unwrap_or_default(); + let description = description + .chars() + .take(MAX_DESCRIPTION_LEN) + .collect::(); let short_description = parsed .metadata .short_description .as_deref() .map(sanitize_single_line) - .filter(|value| !value.is_empty()); + .filter(|value| !value.is_empty()) + .map(|value| { + value + .chars() + .take(MAX_SHORT_DESCRIPTION_LEN) + .collect::() + }); let LoadedSkillMetadata { interface, dependencies, diff --git a/codex-rs/core-skills/src/loader_tests.rs b/codex-rs/core-skills/src/loader_tests.rs index 3a6ae9b1c4e9..a3a917eba78c 100644 --- a/codex-rs/core-skills/src/loader_tests.rs +++ b/codex-rs/core-skills/src/loader_tests.rs @@ -1548,7 +1548,7 @@ async fn preserves_block_scalar_body_while_repairing_other_fields() { } #[tokio::test] -async fn enforces_short_description_length_limits() { +async fn truncates_overlong_short_descriptions() { let codex_home = tempfile::tempdir().expect("tempdir"); let skill_dir = codex_home.path().join("skills/demo"); fs::create_dir_all(&skill_dir).unwrap(); @@ -1560,15 +1560,16 @@ async fn enforces_short_description_length_limits() { let cfg = make_config(&codex_home).await; let outcome = load_skills_for_test(&cfg).await; - assert_eq!(outcome.skills.len(), 0); - assert_eq!(outcome.errors.len(), 1); assert!( - outcome.errors[0] - .message - .contains("invalid metadata.short-description"), - "expected length error, got: {:?}", + outcome.errors.is_empty(), + "unexpected errors: {:?}", outcome.errors ); + assert_eq!(outcome.skills.len(), 1); + assert_eq!( + outcome.skills[0].short_description, + Some("x".repeat(MAX_SHORT_DESCRIPTION_LEN)) + ); } #[tokio::test] @@ -1600,7 +1601,7 @@ async fn skips_hidden_and_invalid() { } #[tokio::test] -async fn enforces_length_limits() { +async fn truncates_overlong_descriptions() { let codex_home = tempfile::tempdir().expect("tempdir"); let max_desc = "\u{1F4A1}".repeat(MAX_DESCRIPTION_LEN); write_skill(&codex_home, "max-len", "max-len", &max_desc); @@ -1617,12 +1618,18 @@ async fn enforces_length_limits() { let too_long_desc = "\u{1F4A1}".repeat(MAX_DESCRIPTION_LEN + 1); write_skill(&codex_home, "too-long", "too-long", &too_long_desc); let outcome = load_skills_for_test(&cfg).await; - assert_eq!(outcome.skills.len(), 1); - assert_eq!(outcome.errors.len(), 1); assert!( - outcome.errors[0].message.contains("invalid description"), - "expected length error" + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors ); + assert_eq!(outcome.skills.len(), 2); + let too_long_skill = outcome + .skills + .iter() + .find(|skill| skill.name == "too-long") + .expect("too-long skill"); + assert_eq!(too_long_skill.description, max_desc); } #[tokio::test] diff --git a/codex-rs/ext/skills/src/provider/orchestrator.rs b/codex-rs/ext/skills/src/provider/orchestrator.rs index 045b67de62c7..f1f59d8e6e17 100644 --- a/codex-rs/ext/skills/src/provider/orchestrator.rs +++ b/codex-rs/ext/skills/src/provider/orchestrator.rs @@ -308,12 +308,20 @@ fn normalized_label(value: &str, max_chars: usize) -> Option { } fn normalized_description(value: &str) -> Option { - normalized_single_line(value, MAX_SKILL_DESCRIPTION_CHARS).map(|value| { + let value = value.split_whitespace().collect::>().join(" "); + if value.chars().any(char::is_control) { + return None; + } + + Some( value + .chars() + .take(MAX_SKILL_DESCRIPTION_CHARS) + .collect::() .replace('&', "&") .replace('<', "<") - .replace('>', ">") - }) + .replace('>', ">"), + ) } fn normalized_single_line(value: &str, max_chars: usize) -> Option { diff --git a/codex-rs/external-agent-migration/src/lib.rs b/codex-rs/external-agent-migration/src/lib.rs index 13a6bc63a3f9..5a2161b45eb7 100644 --- a/codex-rs/external-agent-migration/src/lib.rs +++ b/codex-rs/external-agent-migration/src/lib.rs @@ -1130,14 +1130,11 @@ fn command_skill_name_if_supported( return None; } let source_name = command_source_name(source_commands, source_file); - let description = command_skill_description(document, &source_name)?; + command_skill_description(document, &source_name)?; let name = command_skill_name(source_commands, source_file); if name.chars().count() > MAX_SKILL_NAME_LEN { return None; } - if description.chars().count() > MAX_SKILL_DESCRIPTION_LEN { - return None; - } if has_unsupported_command_template_features(&document.body) { return None; } @@ -1166,6 +1163,10 @@ fn command_source_name(source_commands: &Path, source_file: &Path) -> String { fn render_command_skill(body: &str, name: &str, description: &str, source_name: &str) -> String { let body = rewrite_external_agent_terms(body.trim()); + let description = rewrite_external_agent_terms(description) + .chars() + .take(MAX_SKILL_DESCRIPTION_LEN) + .collect::(); let template_body = if body.is_empty() { "No command template body was found.".to_string() } else { @@ -1174,7 +1175,7 @@ fn render_command_skill(body: &str, name: &str, description: &str, source_name: format!( "---\nname: {}\ndescription: {}\n---\n\n# {name}\n\nUse this skill when the user asks to run the migrated source command `{source_name}`.\n\n## Command Template\n\n{template_body}\n", yaml_string(name), - yaml_string(&rewrite_external_agent_terms(description)), + yaml_string(&description), ) } @@ -1722,6 +1723,36 @@ command = "enabled-server" assert!(command_skill_name_if_supported(&root, &file, &document).is_none()); } + #[test] + fn commands_with_overlong_descriptions_are_truncated() { + let root = source_path("commands"); + let file = source_path("commands/review.md"); + let description = "x".repeat(MAX_SKILL_DESCRIPTION_LEN + 1); + let document = + parse_document_content(&format!("---\ndescription: {description}\n---\nReview\n")); + + assert_eq!( + command_skill_name_if_supported(&root, &file, &document), + Some("source-command-review".to_string()) + ); + + let rendered = render_command_skill( + &document.body, + "source-command-review", + &description, + "review", + ); + let rendered_document = parse_document_content(&rendered); + let expected_description = "x".repeat(MAX_SKILL_DESCRIPTION_LEN); + assert_eq!( + rendered_document + .frontmatter + .get("description") + .and_then(FrontmatterValue::as_scalar), + Some(expected_description.as_str()) + ); + } + #[test] fn commands_with_provider_runtime_expansion_are_skipped() { let root = source_path("commands"); From 6de8ae0988e53a044bb079efc557cf706cb3810c Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Thu, 18 Jun 2026 17:30:53 -0700 Subject: [PATCH 2/8] Preserve full skill descriptions outside context --- codex-rs/core-skills/src/loader.rs | 21 ++------ codex-rs/core-skills/src/loader_tests.rs | 11 ++--- codex-rs/core-skills/src/render.rs | 27 +++++++++- .../ext/skills/src/provider/orchestrator.rs | 4 -- codex-rs/ext/skills/src/render.rs | 5 ++ codex-rs/ext/skills/tests/skills_extension.rs | 49 +++++++++++++++++++ codex-rs/external-agent-migration/src/lib.rs | 10 ++-- 7 files changed, 90 insertions(+), 37 deletions(-) diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index 48022d58b3e1..1c0560d09378 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -698,22 +698,12 @@ async fn parse_skill_file( .as_deref() .map(sanitize_single_line) .unwrap_or_default(); - let description = description - .chars() - .take(MAX_DESCRIPTION_LEN) - .collect::(); let short_description = parsed .metadata .short_description .as_deref() .map(sanitize_single_line) - .filter(|value| !value.is_empty()) - .map(|value| { - value - .chars() - .take(MAX_SHORT_DESCRIPTION_LEN) - .collect::() - }); + .filter(|value| !value.is_empty()); let LoadedSkillMetadata { interface, dependencies, @@ -722,13 +712,8 @@ async fn parse_skill_file( validate_len(&base_name, MAX_NAME_LEN, "name")?; validate_len(&name, MAX_QUALIFIED_NAME_LEN, "qualified name")?; - validate_len(&description, MAX_DESCRIPTION_LEN, "description")?; - if let Some(short_description) = short_description.as_deref() { - validate_len( - short_description, - MAX_SHORT_DESCRIPTION_LEN, - "metadata.short-description", - )?; + if description.is_empty() { + return Err(SkillParseError::MissingField("description")); } let resolved_path = canonicalize_for_skill_identity(fs, path).await; diff --git a/codex-rs/core-skills/src/loader_tests.rs b/codex-rs/core-skills/src/loader_tests.rs index a3a917eba78c..710c877e68f4 100644 --- a/codex-rs/core-skills/src/loader_tests.rs +++ b/codex-rs/core-skills/src/loader_tests.rs @@ -1548,7 +1548,7 @@ async fn preserves_block_scalar_body_while_repairing_other_fields() { } #[tokio::test] -async fn truncates_overlong_short_descriptions() { +async fn preserves_overlong_short_descriptions() { let codex_home = tempfile::tempdir().expect("tempdir"); let skill_dir = codex_home.path().join("skills/demo"); fs::create_dir_all(&skill_dir).unwrap(); @@ -1566,10 +1566,7 @@ async fn truncates_overlong_short_descriptions() { outcome.errors ); assert_eq!(outcome.skills.len(), 1); - assert_eq!( - outcome.skills[0].short_description, - Some("x".repeat(MAX_SHORT_DESCRIPTION_LEN)) - ); + assert_eq!(outcome.skills[0].short_description, Some(too_long)); } #[tokio::test] @@ -1601,7 +1598,7 @@ async fn skips_hidden_and_invalid() { } #[tokio::test] -async fn truncates_overlong_descriptions() { +async fn preserves_overlong_descriptions() { let codex_home = tempfile::tempdir().expect("tempdir"); let max_desc = "\u{1F4A1}".repeat(MAX_DESCRIPTION_LEN); write_skill(&codex_home, "max-len", "max-len", &max_desc); @@ -1629,7 +1626,7 @@ async fn truncates_overlong_descriptions() { .iter() .find(|skill| skill.name == "too-long") .expect("too-long skill"); - assert_eq!(too_long_skill.description, max_desc); + assert_eq!(too_long_skill.description, too_long_desc); } #[tokio::test] diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 1477a299b6a9..9b3d24da9816 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -20,6 +20,7 @@ const SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS: usize = 100; const APPROX_BYTES_PER_TOKEN: usize = 4; pub const SKILL_DESCRIPTION_TRUNCATED_WARNING: &str = "Skill descriptions were shortened to fit the skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest."; pub const SKILL_DESCRIPTION_TRUNCATED_WARNING_WITH_PERCENT: &str = "Skill descriptions were shortened to fit the 2% skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest."; +const MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS: usize = 1024; pub const SKILL_DESCRIPTIONS_REMOVED_WARNING_PREFIX: &str = "Exceeded skills context budget. All skill descriptions were removed and"; pub const SKILLS_INTRO_WITH_ABSOLUTE_PATHS: &str = "A skill is a set of instructions provided through a `SKILL.md` source. Below is the list of skills that can be used. Each entry includes a name, description, and source locator. `file` locators are on the host filesystem, `environment resource` locators are owned by an execution environment, `orchestrator resource` locators are opaque non-filesystem resources, and `custom resource` locators use their provider's access mechanism."; @@ -485,9 +486,14 @@ impl<'a> SkillLine<'a> { } fn with_path(skill: &'a SkillMetadata, path: String) -> Self { + let description = skill.description.as_str(); + let description = description + .char_indices() + .nth(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS) + .map_or(description, |(index, _)| &description[..index]); Self { name: skill.name.as_str(), - description: skill.description.as_str(), + description, path, } } @@ -1030,6 +1036,25 @@ mod tests { ); } + #[test] + fn rendering_caps_model_visible_descriptions_without_mutating_metadata() { + let description = "\u{1F4A1}".repeat(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + 1); + let skill = make_skill_with_description("long-skill", SkillScope::Repo, &description); + let expected_description = "\u{1F4A1}".repeat(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS); + + let rendered = build_available_skills_from_metadata( + std::slice::from_ref(&skill), + SkillMetadataBudget::Characters(usize::MAX), + ) + .expect("skill should render"); + + assert_eq!(skill.description, description); + assert_eq!( + rendered.skill_lines, + vec![expected_skill_line(&skill, &expected_description)] + ); + } + #[test] fn budgeted_rendering_truncates_descriptions_equally_before_omitting_skills() { let alpha = make_skill_with_description("alpha-skill", SkillScope::Repo, "abcdef"); diff --git a/codex-rs/ext/skills/src/provider/orchestrator.rs b/codex-rs/ext/skills/src/provider/orchestrator.rs index f1f59d8e6e17..8d97bf32a1da 100644 --- a/codex-rs/ext/skills/src/provider/orchestrator.rs +++ b/codex-rs/ext/skills/src/provider/orchestrator.rs @@ -28,7 +28,6 @@ const MAX_RESOURCE_PAGES: usize = 10; const MAX_ORCHESTRATOR_SKILLS: usize = 100; const MAX_SKILL_NAME_CHARS: usize = 64; const MAX_QUALIFIED_SKILL_NAME_CHARS: usize = 128; -const MAX_SKILL_DESCRIPTION_CHARS: usize = 1_024; const MAX_SKILL_PACKAGE_URI_CHARS: usize = 1_024; const MAX_SKILL_RESOURCE_URI_CHARS: usize = 2_048; const MAX_SKILL_RESOURCE_CONTENT_BYTES: usize = 1024 * 1024; @@ -315,9 +314,6 @@ fn normalized_description(value: &str) -> Option { Some( value - .chars() - .take(MAX_SKILL_DESCRIPTION_CHARS) - .collect::() .replace('&', "&") .replace('<', "<") .replace('>', ">"), diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index d913e4161cfd..405c7c79dff9 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -7,6 +7,7 @@ use crate::fragments::AvailableSkillsInstructions; const MAX_AVAILABLE_SKILLS_BYTES: usize = 8_000; const MAX_MAIN_PROMPT_BYTES: usize = 8_000; +const MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS: usize = 1_024; pub(crate) const MAX_SKILL_NAME_BYTES: usize = 256; pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024; @@ -26,6 +27,10 @@ pub(crate) fn available_skills_fragment( .short_description .as_deref() .unwrap_or(entry.description.as_str()); + let description = description + .char_indices() + .nth(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS) + .map_or(description, |(index, _)| &description[..index]); let line = render_skill_line(entry, description); let next_bytes = total_bytes.saturating_add(line.len()); if next_bytes > MAX_AVAILABLE_SKILLS_BYTES { diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index 16d9bdd1a391..031573c68981 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -255,6 +255,55 @@ async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_in Ok(()) } +#[tokio::test] +async fn model_context_truncates_catalog_descriptions() -> TestResult { + let description = "x".repeat(1_025); + let mut entry = test_entry( + SkillSourceKind::Orchestrator, + "codex_apps", + "orchestrator/long-description", + "skill://orchestrator/long-description/SKILL.md", + ); + entry.description = description.clone(); + let providers = + SkillProviders::new().with_orchestrator_provider(Arc::new(StaticSkillProvider { + catalog: SkillCatalog { + entries: vec![entry], + 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) + .await; + assert_eq!(1, fragments.len()); + let rendered = fragments[0].text(); + assert!(rendered.contains(&"x".repeat(1_024))); + assert!(!rendered.contains(&description)); + + Ok(()) +} + #[tokio::test] async fn orchestrator_catalog_snapshot_caches_failure() -> TestResult { let list_calls = Arc::new(AtomicUsize::new(0)); diff --git a/codex-rs/external-agent-migration/src/lib.rs b/codex-rs/external-agent-migration/src/lib.rs index 5a2161b45eb7..7edb973772f0 100644 --- a/codex-rs/external-agent-migration/src/lib.rs +++ b/codex-rs/external-agent-migration/src/lib.rs @@ -1163,10 +1163,7 @@ fn command_source_name(source_commands: &Path, source_file: &Path) -> String { fn render_command_skill(body: &str, name: &str, description: &str, source_name: &str) -> String { let body = rewrite_external_agent_terms(body.trim()); - let description = rewrite_external_agent_terms(description) - .chars() - .take(MAX_SKILL_DESCRIPTION_LEN) - .collect::(); + let description = rewrite_external_agent_terms(description); let template_body = if body.is_empty() { "No command template body was found.".to_string() } else { @@ -1724,7 +1721,7 @@ command = "enabled-server" } #[test] - fn commands_with_overlong_descriptions_are_truncated() { + fn commands_with_overlong_descriptions_are_preserved() { let root = source_path("commands"); let file = source_path("commands/review.md"); let description = "x".repeat(MAX_SKILL_DESCRIPTION_LEN + 1); @@ -1743,13 +1740,12 @@ command = "enabled-server" "review", ); let rendered_document = parse_document_content(&rendered); - let expected_description = "x".repeat(MAX_SKILL_DESCRIPTION_LEN); assert_eq!( rendered_document .frontmatter .get("description") .and_then(FrontmatterValue::as_scalar), - Some(expected_description.as_str()) + Some(description.as_str()) ); } From 4c0d915e57a03974ee0bf740fab2b39de9c8841c Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Thu, 18 Jun 2026 18:57:03 -0700 Subject: [PATCH 3/8] Mark truncated skill descriptions --- codex-rs/core-skills/src/render.rs | 30 +++++++++++++++---- codex-rs/ext/skills/src/render.rs | 22 ++++++++++++-- codex-rs/ext/skills/tests/skills_extension.rs | 3 +- 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 9b3d24da9816..1124e89b47b9 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::collections::HashMap; use std::collections::HashSet; use std::path::Component; @@ -21,6 +22,7 @@ const APPROX_BYTES_PER_TOKEN: usize = 4; pub const SKILL_DESCRIPTION_TRUNCATED_WARNING: &str = "Skill descriptions were shortened to fit the skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest."; pub const SKILL_DESCRIPTION_TRUNCATED_WARNING_WITH_PERCENT: &str = "Skill descriptions were shortened to fit the 2% skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest."; const MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS: usize = 1024; +const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "..."; pub const SKILL_DESCRIPTIONS_REMOVED_WARNING_PREFIX: &str = "Exceeded skills context budget. All skill descriptions were removed and"; pub const SKILLS_INTRO_WITH_ABSOLUTE_PATHS: &str = "A skill is a set of instructions provided through a `SKILL.md` source. Below is the list of skills that can be used. Each entry includes a name, description, and source locator. `file` locators are on the host filesystem, `environment resource` locators are owned by an execution environment, `orchestrator resource` locators are opaque non-filesystem resources, and `custom resource` locators use their provider's access mechanism."; @@ -447,7 +449,7 @@ impl SkillRenderReport { struct SkillLine<'a> { name: &'a str, - description: &'a str, + description: Cow<'a, str>, path: String, } @@ -487,10 +489,23 @@ impl<'a> SkillLine<'a> { fn with_path(skill: &'a SkillMetadata, path: String) -> Self { let description = skill.description.as_str(); - let description = description + let description = if description .char_indices() .nth(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS) - .map_or(description, |(index, _)| &description[..index]); + .is_some() + { + let prefix_chars = MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + .saturating_sub(TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count()); + let prefix_end = description + .char_indices() + .nth(prefix_chars) + .map_or(description.len(), |(index, _)| index); + let mut truncated = description[..prefix_end].to_string(); + truncated.push_str(TRUNCATED_SKILL_DESCRIPTION_SUFFIX); + Cow::Owned(truncated) + } else { + Cow::Borrowed(description) + }; Self { name: skill.name.as_str(), description, @@ -511,7 +526,7 @@ impl<'a> SkillLine<'a> { } fn render_full(&self) -> String { - self.render_with_description(self.description) + self.render_with_description(self.description.as_ref()) } fn render_minimum(&self) -> String { @@ -530,7 +545,7 @@ impl<'a> SkillLine<'a> { format!("- {}: (file: {})", self.name, self.path) } else { let end = self.rendered_description_prefix_len(description_chars); - let description = &self.description[..end]; + let description = &self.description.as_ref()[..end]; format!("- {}: {} (file: {})", self.name, description, self.path) } } @@ -1040,7 +1055,10 @@ mod tests { fn rendering_caps_model_visible_descriptions_without_mutating_metadata() { let description = "\u{1F4A1}".repeat(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + 1); let skill = make_skill_with_description("long-skill", SkillScope::Repo, &description); - let expected_description = "\u{1F4A1}".repeat(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS); + let expected_description = "\u{1F4A1}".repeat( + MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + - TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count(), + ) + TRUNCATED_SKILL_DESCRIPTION_SUFFIX; let rendered = build_available_skills_from_metadata( std::slice::from_ref(&skill), diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index 405c7c79dff9..901288a64944 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use codex_utils_string::take_bytes_at_char_boundary; use crate::catalog::SkillCatalog; @@ -8,6 +10,7 @@ use crate::fragments::AvailableSkillsInstructions; const MAX_AVAILABLE_SKILLS_BYTES: usize = 8_000; const MAX_MAIN_PROMPT_BYTES: usize = 8_000; const MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS: usize = 1_024; +const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "..."; pub(crate) const MAX_SKILL_NAME_BYTES: usize = 256; pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024; @@ -27,11 +30,24 @@ pub(crate) fn available_skills_fragment( .short_description .as_deref() .unwrap_or(entry.description.as_str()); - let description = description + let description = if description .char_indices() .nth(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS) - .map_or(description, |(index, _)| &description[..index]); - let line = render_skill_line(entry, description); + .is_some() + { + let prefix_chars = MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + .saturating_sub(TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count()); + let prefix_end = description + .char_indices() + .nth(prefix_chars) + .map_or(description.len(), |(index, _)| index); + let mut truncated = description[..prefix_end].to_string(); + truncated.push_str(TRUNCATED_SKILL_DESCRIPTION_SUFFIX); + Cow::Owned(truncated) + } else { + Cow::Borrowed(description) + }; + let line = render_skill_line(entry, description.as_ref()); let next_bytes = total_bytes.saturating_add(line.len()); if next_bytes > MAX_AVAILABLE_SKILLS_BYTES { omitted = omitted.saturating_add(1); diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index 031573c68981..e01ae7c5750e 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -298,7 +298,8 @@ async fn model_context_truncates_catalog_descriptions() -> TestResult { .await; assert_eq!(1, fragments.len()); let rendered = fragments[0].text(); - assert!(rendered.contains(&"x".repeat(1_024))); + assert!(rendered.contains(&("x".repeat(1_021) + "..."))); + assert!(!rendered.contains(&"x".repeat(1_024))); assert!(!rendered.contains(&description)); Ok(()) From ec572aa759e62404fe7e7821fb6a0c90ee022953 Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Thu, 18 Jun 2026 19:19:05 -0700 Subject: [PATCH 4/8] codex: fix CI failure on PR #29006 --- codex-rs/external-agent-migration/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codex-rs/external-agent-migration/src/lib.rs b/codex-rs/external-agent-migration/src/lib.rs index 7edb973772f0..69b1b1380314 100644 --- a/codex-rs/external-agent-migration/src/lib.rs +++ b/codex-rs/external-agent-migration/src/lib.rs @@ -18,7 +18,6 @@ const EXTERNAL_AGENT_HOOKS_SUBDIR: &str = "hooks"; const EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR: &str = "hooks"; const COMMAND_SKILL_PREFIX: &str = "source-command"; const MAX_SKILL_NAME_LEN: usize = 64; -const MAX_SKILL_DESCRIPTION_LEN: usize = 1024; #[derive(Debug)] struct ParsedDocument { @@ -1386,6 +1385,8 @@ mod tests { use super::*; use pretty_assertions::assert_eq; + const MAX_SKILL_DESCRIPTION_LEN: usize = 1024; + fn source_path(relative_path: &str) -> PathBuf { Path::new("/repo") .join(external_agent_config_dir()) From 428dd6c66e0778bf54c57b90364a8f43775321dd Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Fri, 19 Jun 2026 10:19:54 -0700 Subject: [PATCH 5/8] codex: bound model-visible skill descriptions --- codex-rs/core-skills/src/lib.rs | 1 + codex-rs/core-skills/src/model_visible.rs | 89 ++++++++++++++++++ .../core-skills/src/model_visible_tests.rs | 39 ++++++++ codex-rs/core-skills/src/render.rs | 24 +---- .../core-skills/src/skill_instructions.rs | 6 +- codex-rs/core/tests/suite/skills.rs | 91 +++++++++++++++++++ codex-rs/ext/skills/src/render.rs | 38 ++++---- codex-rs/ext/skills/src/tools/list.rs | 3 +- codex-rs/ext/skills/tests/skills_extension.rs | 77 ++++++++++++++++ 9 files changed, 329 insertions(+), 39 deletions(-) create mode 100644 codex-rs/core-skills/src/model_visible.rs create mode 100644 codex-rs/core-skills/src/model_visible_tests.rs diff --git a/codex-rs/core-skills/src/lib.rs b/codex-rs/core-skills/src/lib.rs index 31e5a7c531b0..5636976ac4c1 100644 --- a/codex-rs/core-skills/src/lib.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -4,6 +4,7 @@ pub(crate) mod invocation_utils; pub mod loader; mod mention_counts; pub mod model; +mod model_visible; pub mod remote; pub mod render; pub mod service; diff --git a/codex-rs/core-skills/src/model_visible.rs b/codex-rs/core-skills/src/model_visible.rs new file mode 100644 index 000000000000..d23287ccdc68 --- /dev/null +++ b/codex-rs/core-skills/src/model_visible.rs @@ -0,0 +1,89 @@ +use std::borrow::Cow; + +use serde_yaml::Mapping; +use serde_yaml::Value; + +pub(crate) const MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS: usize = 1_024; +pub(crate) const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "..."; + +pub(crate) fn truncate_skill_description(description: &str) -> Cow<'_, str> { + if description + .char_indices() + .nth(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS) + .is_none() + { + return Cow::Borrowed(description); + } + + let prefix_chars = MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + .saturating_sub(TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count()); + let prefix_end = description + .char_indices() + .nth(prefix_chars) + .map_or(description.len(), |(index, _)| index); + let mut truncated = description[..prefix_end].to_string(); + truncated.push_str(TRUNCATED_SKILL_DESCRIPTION_SUFFIX); + Cow::Owned(truncated) +} + +pub(crate) fn render_model_visible_skill_contents(contents: &str) -> Cow<'_, str> { + let Some(rest) = contents + .strip_prefix("---\n") + .or_else(|| contents.strip_prefix("---\r\n")) + else { + return Cow::Borrowed(contents); + }; + let Some((frontmatter_end, body_start)) = [ + "\r\n---\r\n", + "\r\n---\n", + "\n---\r\n", + "\n---\n", + "\r\n---", + "\n---", + ] + .into_iter() + .filter_map(|delimiter| rest.find(delimiter).map(|end| (end, end + delimiter.len()))) + .min_by_key(|(end, _body_start)| *end) else { + return Cow::Borrowed(contents); + }; + + let raw_frontmatter = &rest[..frontmatter_end]; + let Ok(mut frontmatter) = serde_yaml::from_str::(raw_frontmatter) else { + return Cow::Borrowed(contents); + }; + let mut changed = false; + let description_key = Value::String("description".to_string()); + if let Some(Value::String(description)) = frontmatter.get_mut(&description_key) + && let Cow::Owned(truncated) = truncate_skill_description(description) + { + *description = truncated; + changed = true; + } + let metadata_key = Value::String("metadata".to_string()); + let short_description_key = Value::String("short-description".to_string()); + if let Some(Value::Mapping(metadata)) = frontmatter.get_mut(&metadata_key) + && let Some(Value::String(short_description)) = metadata.get_mut(&short_description_key) + && let Cow::Owned(truncated) = truncate_skill_description(short_description) + { + *short_description = truncated; + changed = true; + } + if !changed { + return Cow::Borrowed(contents); + } + + let Ok(frontmatter) = serde_yaml::to_string(&frontmatter) else { + return Cow::Borrowed(contents); + }; + let body = &rest[body_start..]; + let mut rendered = String::with_capacity(contents.len()); + rendered.push_str("---\n"); + rendered.push_str(frontmatter.trim_end()); + rendered.push_str("\n---\n"); + rendered.push_str(body); + Cow::Owned(rendered) +} + +#[cfg(test)] +#[path = "model_visible_tests.rs"] +mod tests; diff --git a/codex-rs/core-skills/src/model_visible_tests.rs b/codex-rs/core-skills/src/model_visible_tests.rs new file mode 100644 index 000000000000..2d4eb6ea50da --- /dev/null +++ b/codex-rs/core-skills/src/model_visible_tests.rs @@ -0,0 +1,39 @@ +use super::MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS; +use super::TRUNCATED_SKILL_DESCRIPTION_SUFFIX; +use super::render_model_visible_skill_contents; + +#[test] +fn skill_contents_cap_only_model_visible_descriptions() { + let description = "💡".repeat(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + 1); + let short_description = "s".repeat(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + 1); + let contents = format!( + "---\nname: demo\ndescription: {description}\nmetadata:\n short-description: {short_description}\n---\n\n# Demo\n\nUse the full body.\n" + ); + let expected_description = "💡".repeat( + MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + - TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count(), + ) + TRUNCATED_SKILL_DESCRIPTION_SUFFIX; + let expected_short_description = "s".repeat( + MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + - TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count(), + ) + TRUNCATED_SKILL_DESCRIPTION_SUFFIX; + + let rendered = render_model_visible_skill_contents(&contents); + + assert!(rendered.contains(&format!("description: {expected_description}"))); + assert!(rendered.contains(&format!("short-description: {expected_short_description}"))); + assert!(!rendered.contains(&description)); + assert!(!rendered.contains(&short_description)); + assert!(rendered.contains("Use the full body.")); + assert!(contents.contains(&description)); +} + +#[test] +fn skill_contents_without_overlong_description_are_borrowed() { + let contents = "---\nname: demo\ndescription: short\n---\n\n# Demo\n"; + + let rendered = render_model_visible_skill_contents(contents); + + assert!(matches!(rendered, std::borrow::Cow::Borrowed(_))); + assert_eq!(rendered, contents); +} diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 1124e89b47b9..5c069b9c69ee 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -6,6 +6,7 @@ use std::path::Path; use crate::model::SkillLoadOutcome; use crate::model::SkillMetadata; +use crate::model_visible::truncate_skill_description; use codex_otel::SessionTelemetry; use codex_otel::THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC; use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC; @@ -21,8 +22,6 @@ const SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS: usize = 100; const APPROX_BYTES_PER_TOKEN: usize = 4; pub const SKILL_DESCRIPTION_TRUNCATED_WARNING: &str = "Skill descriptions were shortened to fit the skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest."; pub const SKILL_DESCRIPTION_TRUNCATED_WARNING_WITH_PERCENT: &str = "Skill descriptions were shortened to fit the 2% skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest."; -const MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS: usize = 1024; -const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "..."; pub const SKILL_DESCRIPTIONS_REMOVED_WARNING_PREFIX: &str = "Exceeded skills context budget. All skill descriptions were removed and"; pub const SKILLS_INTRO_WITH_ABSOLUTE_PATHS: &str = "A skill is a set of instructions provided through a `SKILL.md` source. Below is the list of skills that can be used. Each entry includes a name, description, and source locator. `file` locators are on the host filesystem, `environment resource` locators are owned by an execution environment, `orchestrator resource` locators are opaque non-filesystem resources, and `custom resource` locators use their provider's access mechanism."; @@ -488,24 +487,7 @@ impl<'a> SkillLine<'a> { } fn with_path(skill: &'a SkillMetadata, path: String) -> Self { - let description = skill.description.as_str(); - let description = if description - .char_indices() - .nth(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS) - .is_some() - { - let prefix_chars = MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS - .saturating_sub(TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count()); - let prefix_end = description - .char_indices() - .nth(prefix_chars) - .map_or(description.len(), |(index, _)| index); - let mut truncated = description[..prefix_end].to_string(); - truncated.push_str(TRUNCATED_SKILL_DESCRIPTION_SUFFIX); - Cow::Owned(truncated) - } else { - Cow::Borrowed(description) - }; + let description = truncate_skill_description(skill.description.as_str()); Self { name: skill.name.as_str(), description, @@ -926,6 +908,8 @@ fn prompt_scope_rank(scope: SkillScope) -> u8 { #[cfg(test)] mod tests { use super::*; + use crate::model_visible::MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS; + use crate::model_visible::TRUNCATED_SKILL_DESCRIPTION_SUFFIX; use std::collections::HashMap; use std::sync::Arc; diff --git a/codex-rs/core-skills/src/skill_instructions.rs b/codex-rs/core-skills/src/skill_instructions.rs index b2a002d9025b..f5fd36203e29 100644 --- a/codex-rs/core-skills/src/skill_instructions.rs +++ b/codex-rs/core-skills/src/skill_instructions.rs @@ -1,6 +1,7 @@ use codex_context_fragments::ContextualUserFragment; use crate::injection::SkillInjection; +use crate::model_visible::render_model_visible_skill_contents; #[derive(Debug, Clone, PartialEq)] pub struct SkillInstructions { @@ -33,9 +34,12 @@ impl ContextualUserFragment for SkillInstructions { } fn body(&self) -> String { + let contents = render_model_visible_skill_contents(&self.contents); format!( "\n{}\n{}\n{}\n", - self.name, self.path, self.contents + self.name, + self.path, + contents.as_ref() ) } } diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index 2de9bf26cb93..1cf74bec7e84 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -133,3 +133,94 @@ async fn user_turn_includes_skill_instructions() -> Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn user_turn_truncates_skill_description_only_in_model_context() -> Result<()> { + // TODO(anp): Remove after skill-path helpers use target-native paths. + skip_if_wine_exec!(Ok(()), "requires native cross-OS skill paths"); + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let description = "x".repeat(1_025); + let setup_description = description.clone(); + let skill_body = "skill body"; + let mut builder = test_codex().with_workspace_setup(move |cwd, fs| async move { + write_repo_skill(cwd, fs, "demo", &setup_description, skill_body).await + }); + let test = builder.build_with_remote_env(&server).await?; + + let skill_path = test + .config + .cwd + .join(".agents/skills/demo/SKILL.md") + .canonicalize() + .unwrap_or_else(|_| test.config.cwd.join(".agents/skills/demo/SKILL.md")) + .to_path_buf(); + + let mock = mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-1"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-1"), + ]), + ) + .await; + + let session_model = test.session_configured.model.clone(); + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path()); + test.codex + .submit(Op::UserInput { + items: vec![ + UserInput::Text { + text: "please use $demo".to_string(), + text_elements: Vec::new(), + }, + UserInput::Skill { + name: "demo".to_string(), + path: skill_path.clone(), + }, + ], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: codex_protocol::protocol::ThreadSettingsOverrides { + environments: Some(local_selections(test.config.cwd.clone())), + approval_policy: Some(AskForApproval::Never), + sandbox_policy: Some(sandbox_policy), + permission_profile, + collaboration_mode: Some(codex_protocol::config_types::CollaborationMode { + mode: codex_protocol::config_types::ModeKind::Default, + settings: codex_protocol::config_types::Settings { + model: session_model, + reasoning_effort: None, + developer_instructions: None, + }, + }), + ..Default::default() + }, + }) + .await?; + + core_test_support::wait_for_event(test.codex.as_ref(), |event| { + matches!(event, codex_protocol::protocol::EventMsg::TurnComplete(_)) + }) + .await; + + let expected_description = "x".repeat(1_021) + "..."; + let request = mock.single_request(); + let user_texts = request.message_input_texts("user"); + let injected_skill = user_texts + .iter() + .find(|text| text.contains("\ndemo")) + .ok_or_else(|| anyhow::anyhow!("expected injected skill in {user_texts:?}"))?; + assert!(injected_skill.contains(&format!("description: {expected_description}"))); + assert!(!injected_skill.contains(&format!("description: {description}"))); + assert!(injected_skill.contains(skill_body)); + + let source = std::fs::read_to_string(&skill_path)?; + assert!(source.contains(&format!("description: {description}"))); + + Ok(()) +} diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index 901288a64944..2307e2d49e6f 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -30,23 +30,7 @@ pub(crate) fn available_skills_fragment( .short_description .as_deref() .unwrap_or(entry.description.as_str()); - let description = if description - .char_indices() - .nth(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS) - .is_some() - { - let prefix_chars = MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS - .saturating_sub(TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count()); - let prefix_end = description - .char_indices() - .nth(prefix_chars) - .map_or(description.len(), |(index, _)| index); - let mut truncated = description[..prefix_end].to_string(); - truncated.push_str(TRUNCATED_SKILL_DESCRIPTION_SUFFIX); - Cow::Owned(truncated) - } else { - Cow::Borrowed(description) - }; + let description = truncate_model_visible_skill_description(description); let line = render_skill_line(entry, description.as_ref()); let next_bytes = total_bytes.saturating_add(line.len()); if next_bytes > MAX_AVAILABLE_SKILLS_BYTES { @@ -70,6 +54,26 @@ pub(crate) fn available_skills_fragment( Some(AvailableSkillsInstructions::from_skill_lines(skill_lines)) } +pub(crate) fn truncate_model_visible_skill_description(description: &str) -> Cow<'_, str> { + if description + .char_indices() + .nth(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS) + .is_none() + { + return Cow::Borrowed(description); + } + + let prefix_chars = MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + .saturating_sub(TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count()); + let prefix_end = description + .char_indices() + .nth(prefix_chars) + .map_or(description.len(), |(index, _)| index); + let mut truncated = description[..prefix_end].to_string(); + truncated.push_str(TRUNCATED_SKILL_DESCRIPTION_SUFFIX); + Cow::Owned(truncated) +} + fn render_skill_line(entry: &SkillCatalogEntry, description: &str) -> String { let locator_kind = match &entry.authority.kind { SkillSourceKind::Host => "file", diff --git a/codex-rs/ext/skills/src/tools/list.rs b/codex-rs/ext/skills/src/tools/list.rs index 31cb68003d69..49d0488fa82f 100644 --- a/codex-rs/ext/skills/src/tools/list.rs +++ b/codex-rs/ext/skills/src/tools/list.rs @@ -8,6 +8,7 @@ use serde::Deserialize; use serde::Serialize; use crate::catalog::SkillCatalogEntry; +use crate::render::truncate_model_visible_skill_description; use crate::render::truncate_utf8_to_bytes; use super::MAX_HANDLE_BYTES; @@ -95,7 +96,7 @@ fn listed_skill(entry: SkillCatalogEntry) -> Option { authority, package: entry.id.0, name: entry.name, - description: entry.description, + description: truncate_model_visible_skill_description(&entry.description).into_owned(), main_resource: entry.main_prompt.as_str().to_string(), }) } diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index e01ae7c5750e..4512f6961ce6 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -10,10 +10,14 @@ use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS; use codex_core_skills::SkillLoadOutcome; use codex_core_skills::SkillMetadata; use codex_core_skills::injection::InjectedHostSkillPrompts; +use codex_extension_api::ConversationHistory; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionEventSink; use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::NoopTurnItemEmitter; use codex_extension_api::ThreadStartInput; +use codex_extension_api::ToolCall; +use codex_extension_api::ToolPayload; use codex_extension_api::TurnInputContext; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; @@ -23,6 +27,7 @@ use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SkillScope; +use codex_protocol::protocol::TruncationPolicy; use codex_protocol::user_input::UserInput; use codex_skills_extension::SkillProviders; use codex_skills_extension::SkillsExtensionConfig; @@ -305,6 +310,78 @@ async fn model_context_truncates_catalog_descriptions() -> TestResult { Ok(()) } +#[tokio::test] +async fn skills_list_truncates_catalog_descriptions_in_tool_output() -> TestResult { + let description = "x".repeat(1_025); + let mut entry = test_entry( + SkillSourceKind::Orchestrator, + "codex_apps", + "orchestrator/long-description", + "skill://orchestrator/long-description/SKILL.md", + ); + entry.description = description.clone(); + let providers = + SkillProviders::new().with_orchestrator_provider(Arc::new(StaticSkillProvider { + catalog: SkillCatalog { + entries: vec![entry], + 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 tools = registry.tool_contributors()[0].tools(&session_store, &thread_store); + let list_tool = tools + .iter() + .find(|tool| tool.tool_name().name == "list") + .ok_or("skills.list tool should be registered")?; + let payload = ToolPayload::Function { + arguments: serde_json::json!({"authority": {"kind": "orchestrator"}}).to_string(), + }; + let output = list_tool + .handle(ToolCall { + turn_id: "turn-1".to_string(), + call_id: "call-1".to_string(), + tool_name: list_tool.tool_name(), + model: "gpt-test".to_string(), + truncation_policy: TruncationPolicy::Bytes(1_024), + conversation_history: ConversationHistory::default(), + turn_item_emitter: Arc::new(NoopTurnItemEmitter), + environments: Vec::new(), + payload: payload.clone(), + }) + .await?; + let response = output + .post_tool_use_response("call-1", &payload) + .ok_or("skills.list should expose structured output")?; + let rendered_description = response["skills"][0]["description"] + .as_str() + .ok_or("skills.list response should include a description")?; + + assert_eq!(rendered_description, "x".repeat(1_021) + "..."); + assert_ne!(rendered_description, description); + + Ok(()) +} + #[tokio::test] async fn orchestrator_catalog_snapshot_caches_failure() -> TestResult { let list_calls = Arc::new(AtomicUsize::new(0)); From 6e4ffa3907897aec4450ca4802b423c7e9262f1c Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Fri, 19 Jun 2026 10:54:34 -0700 Subject: [PATCH 6/8] codex: remove redundant skill injection test --- codex-rs/core/tests/suite/skills.rs | 91 ----------------------------- 1 file changed, 91 deletions(-) diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index 1cf74bec7e84..2de9bf26cb93 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -133,94 +133,3 @@ async fn user_turn_includes_skill_instructions() -> Result<()> { Ok(()) } - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn user_turn_truncates_skill_description_only_in_model_context() -> Result<()> { - // TODO(anp): Remove after skill-path helpers use target-native paths. - skip_if_wine_exec!(Ok(()), "requires native cross-OS skill paths"); - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let description = "x".repeat(1_025); - let setup_description = description.clone(); - let skill_body = "skill body"; - let mut builder = test_codex().with_workspace_setup(move |cwd, fs| async move { - write_repo_skill(cwd, fs, "demo", &setup_description, skill_body).await - }); - let test = builder.build_with_remote_env(&server).await?; - - let skill_path = test - .config - .cwd - .join(".agents/skills/demo/SKILL.md") - .canonicalize() - .unwrap_or_else(|_| test.config.cwd.join(".agents/skills/demo/SKILL.md")) - .to_path_buf(); - - let mock = mount_sse_once( - &server, - sse(vec![ - ev_response_created("resp-1"), - ev_assistant_message("msg-1", "done"), - ev_completed("resp-1"), - ]), - ) - .await; - - let session_model = test.session_configured.model.clone(); - let (sandbox_policy, permission_profile) = - turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path()); - test.codex - .submit(Op::UserInput { - items: vec![ - UserInput::Text { - text: "please use $demo".to_string(), - text_elements: Vec::new(), - }, - UserInput::Skill { - name: "demo".to_string(), - path: skill_path.clone(), - }, - ], - final_output_json_schema: None, - responsesapi_client_metadata: None, - additional_context: Default::default(), - thread_settings: codex_protocol::protocol::ThreadSettingsOverrides { - environments: Some(local_selections(test.config.cwd.clone())), - approval_policy: Some(AskForApproval::Never), - sandbox_policy: Some(sandbox_policy), - permission_profile, - collaboration_mode: Some(codex_protocol::config_types::CollaborationMode { - mode: codex_protocol::config_types::ModeKind::Default, - settings: codex_protocol::config_types::Settings { - model: session_model, - reasoning_effort: None, - developer_instructions: None, - }, - }), - ..Default::default() - }, - }) - .await?; - - core_test_support::wait_for_event(test.codex.as_ref(), |event| { - matches!(event, codex_protocol::protocol::EventMsg::TurnComplete(_)) - }) - .await; - - let expected_description = "x".repeat(1_021) + "..."; - let request = mock.single_request(); - let user_texts = request.message_input_texts("user"); - let injected_skill = user_texts - .iter() - .find(|text| text.contains("\ndemo")) - .ok_or_else(|| anyhow::anyhow!("expected injected skill in {user_texts:?}"))?; - assert!(injected_skill.contains(&format!("description: {expected_description}"))); - assert!(!injected_skill.contains(&format!("description: {description}"))); - assert!(injected_skill.contains(skill_body)); - - let source = std::fs::read_to_string(&skill_path)?; - assert!(source.contains(&format!("description: {description}"))); - - Ok(()) -} From c82343eff93841a91f589265b86fc374427b33f3 Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Fri, 19 Jun 2026 11:29:43 -0700 Subject: [PATCH 7/8] codex: scope skill description caps to default context --- codex-rs/core-skills/src/lib.rs | 1 - codex-rs/core-skills/src/model_visible.rs | 89 ------------------- .../core-skills/src/model_visible_tests.rs | 39 -------- codex-rs/core-skills/src/render.rs | 33 +++++-- .../core-skills/src/skill_instructions.rs | 6 +- codex-rs/ext/skills/src/render.rs | 10 +-- codex-rs/ext/skills/src/tools/list.rs | 3 +- codex-rs/ext/skills/tests/skills_extension.rs | 79 +--------------- codex-rs/external-agent-migration/src/lib.rs | 3 +- 9 files changed, 35 insertions(+), 228 deletions(-) delete mode 100644 codex-rs/core-skills/src/model_visible.rs delete mode 100644 codex-rs/core-skills/src/model_visible_tests.rs diff --git a/codex-rs/core-skills/src/lib.rs b/codex-rs/core-skills/src/lib.rs index 5636976ac4c1..31e5a7c531b0 100644 --- a/codex-rs/core-skills/src/lib.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -4,7 +4,6 @@ pub(crate) mod invocation_utils; pub mod loader; mod mention_counts; pub mod model; -mod model_visible; pub mod remote; pub mod render; pub mod service; diff --git a/codex-rs/core-skills/src/model_visible.rs b/codex-rs/core-skills/src/model_visible.rs deleted file mode 100644 index d23287ccdc68..000000000000 --- a/codex-rs/core-skills/src/model_visible.rs +++ /dev/null @@ -1,89 +0,0 @@ -use std::borrow::Cow; - -use serde_yaml::Mapping; -use serde_yaml::Value; - -pub(crate) const MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS: usize = 1_024; -pub(crate) const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "..."; - -pub(crate) fn truncate_skill_description(description: &str) -> Cow<'_, str> { - if description - .char_indices() - .nth(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS) - .is_none() - { - return Cow::Borrowed(description); - } - - let prefix_chars = MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS - .saturating_sub(TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count()); - let prefix_end = description - .char_indices() - .nth(prefix_chars) - .map_or(description.len(), |(index, _)| index); - let mut truncated = description[..prefix_end].to_string(); - truncated.push_str(TRUNCATED_SKILL_DESCRIPTION_SUFFIX); - Cow::Owned(truncated) -} - -pub(crate) fn render_model_visible_skill_contents(contents: &str) -> Cow<'_, str> { - let Some(rest) = contents - .strip_prefix("---\n") - .or_else(|| contents.strip_prefix("---\r\n")) - else { - return Cow::Borrowed(contents); - }; - let Some((frontmatter_end, body_start)) = [ - "\r\n---\r\n", - "\r\n---\n", - "\n---\r\n", - "\n---\n", - "\r\n---", - "\n---", - ] - .into_iter() - .filter_map(|delimiter| rest.find(delimiter).map(|end| (end, end + delimiter.len()))) - .min_by_key(|(end, _body_start)| *end) else { - return Cow::Borrowed(contents); - }; - - let raw_frontmatter = &rest[..frontmatter_end]; - let Ok(mut frontmatter) = serde_yaml::from_str::(raw_frontmatter) else { - return Cow::Borrowed(contents); - }; - let mut changed = false; - let description_key = Value::String("description".to_string()); - if let Some(Value::String(description)) = frontmatter.get_mut(&description_key) - && let Cow::Owned(truncated) = truncate_skill_description(description) - { - *description = truncated; - changed = true; - } - let metadata_key = Value::String("metadata".to_string()); - let short_description_key = Value::String("short-description".to_string()); - if let Some(Value::Mapping(metadata)) = frontmatter.get_mut(&metadata_key) - && let Some(Value::String(short_description)) = metadata.get_mut(&short_description_key) - && let Cow::Owned(truncated) = truncate_skill_description(short_description) - { - *short_description = truncated; - changed = true; - } - if !changed { - return Cow::Borrowed(contents); - } - - let Ok(frontmatter) = serde_yaml::to_string(&frontmatter) else { - return Cow::Borrowed(contents); - }; - let body = &rest[body_start..]; - let mut rendered = String::with_capacity(contents.len()); - rendered.push_str("---\n"); - rendered.push_str(frontmatter.trim_end()); - rendered.push_str("\n---\n"); - rendered.push_str(body); - Cow::Owned(rendered) -} - -#[cfg(test)] -#[path = "model_visible_tests.rs"] -mod tests; diff --git a/codex-rs/core-skills/src/model_visible_tests.rs b/codex-rs/core-skills/src/model_visible_tests.rs deleted file mode 100644 index 2d4eb6ea50da..000000000000 --- a/codex-rs/core-skills/src/model_visible_tests.rs +++ /dev/null @@ -1,39 +0,0 @@ -use super::MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS; -use super::TRUNCATED_SKILL_DESCRIPTION_SUFFIX; -use super::render_model_visible_skill_contents; - -#[test] -fn skill_contents_cap_only_model_visible_descriptions() { - let description = "💡".repeat(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + 1); - let short_description = "s".repeat(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + 1); - let contents = format!( - "---\nname: demo\ndescription: {description}\nmetadata:\n short-description: {short_description}\n---\n\n# Demo\n\nUse the full body.\n" - ); - let expected_description = "💡".repeat( - MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS - - TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count(), - ) + TRUNCATED_SKILL_DESCRIPTION_SUFFIX; - let expected_short_description = "s".repeat( - MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS - - TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count(), - ) + TRUNCATED_SKILL_DESCRIPTION_SUFFIX; - - let rendered = render_model_visible_skill_contents(&contents); - - assert!(rendered.contains(&format!("description: {expected_description}"))); - assert!(rendered.contains(&format!("short-description: {expected_short_description}"))); - assert!(!rendered.contains(&description)); - assert!(!rendered.contains(&short_description)); - assert!(rendered.contains("Use the full body.")); - assert!(contents.contains(&description)); -} - -#[test] -fn skill_contents_without_overlong_description_are_borrowed() { - let contents = "---\nname: demo\ndescription: short\n---\n\n# Demo\n"; - - let rendered = render_model_visible_skill_contents(contents); - - assert!(matches!(rendered, std::borrow::Cow::Borrowed(_))); - assert_eq!(rendered, contents); -} diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 5c069b9c69ee..89ea1675d0ee 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -6,7 +6,6 @@ use std::path::Path; use crate::model::SkillLoadOutcome; use crate::model::SkillMetadata; -use crate::model_visible::truncate_skill_description; use codex_otel::SessionTelemetry; use codex_otel::THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC; use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC; @@ -18,6 +17,8 @@ use codex_utils_output_truncation::approx_token_count; const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000; const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2; +const MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS: usize = 1_024; +const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "..."; const SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS: usize = 100; const APPROX_BYTES_PER_TOKEN: usize = 4; pub const SKILL_DESCRIPTION_TRUNCATED_WARNING: &str = "Skill descriptions were shortened to fit the skills context budget. Codex can still see every skill, but some descriptions are shorter. Disable unused skills or plugins to leave more room for the rest."; @@ -487,7 +488,7 @@ impl<'a> SkillLine<'a> { } fn with_path(skill: &'a SkillMetadata, path: String) -> Self { - let description = truncate_skill_description(skill.description.as_str()); + let description = truncate_default_context_skill_description(skill.description.as_str()); Self { name: skill.name.as_str(), description, @@ -541,6 +542,26 @@ impl<'a> SkillLine<'a> { } } +fn truncate_default_context_skill_description(description: &str) -> Cow<'_, str> { + if description + .char_indices() + .nth(MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS) + .is_none() + { + return Cow::Borrowed(description); + } + + let prefix_chars = MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS + .saturating_sub(TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count()); + let prefix_end = description + .char_indices() + .nth(prefix_chars) + .map_or(description.len(), |(index, _)| index); + let mut truncated = description[..prefix_end].to_string(); + truncated.push_str(TRUNCATED_SKILL_DESCRIPTION_SUFFIX); + Cow::Owned(truncated) +} + impl<'a> DescriptionBudgetLine<'a> { fn new(line: &'a SkillLine<'a>, budget: SkillMetadataBudget) -> Self { let minimum_line = line.render_minimum(); @@ -908,8 +929,6 @@ fn prompt_scope_rank(scope: SkillScope) -> u8 { #[cfg(test)] mod tests { use super::*; - use crate::model_visible::MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS; - use crate::model_visible::TRUNCATED_SKILL_DESCRIPTION_SUFFIX; use std::collections::HashMap; use std::sync::Arc; @@ -1036,11 +1055,11 @@ mod tests { } #[test] - fn rendering_caps_model_visible_descriptions_without_mutating_metadata() { - let description = "\u{1F4A1}".repeat(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + 1); + fn default_context_caps_descriptions_without_mutating_metadata() { + let description = "\u{1F4A1}".repeat(MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS + 1); let skill = make_skill_with_description("long-skill", SkillScope::Repo, &description); let expected_description = "\u{1F4A1}".repeat( - MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS - TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count(), ) + TRUNCATED_SKILL_DESCRIPTION_SUFFIX; diff --git a/codex-rs/core-skills/src/skill_instructions.rs b/codex-rs/core-skills/src/skill_instructions.rs index f5fd36203e29..b2a002d9025b 100644 --- a/codex-rs/core-skills/src/skill_instructions.rs +++ b/codex-rs/core-skills/src/skill_instructions.rs @@ -1,7 +1,6 @@ use codex_context_fragments::ContextualUserFragment; use crate::injection::SkillInjection; -use crate::model_visible::render_model_visible_skill_contents; #[derive(Debug, Clone, PartialEq)] pub struct SkillInstructions { @@ -34,12 +33,9 @@ impl ContextualUserFragment for SkillInstructions { } fn body(&self) -> String { - let contents = render_model_visible_skill_contents(&self.contents); format!( "\n{}\n{}\n{}\n", - self.name, - self.path, - contents.as_ref() + self.name, self.path, self.contents ) } } diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index 2307e2d49e6f..19f39e69fd8c 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -9,7 +9,7 @@ use crate::fragments::AvailableSkillsInstructions; const MAX_AVAILABLE_SKILLS_BYTES: usize = 8_000; const MAX_MAIN_PROMPT_BYTES: usize = 8_000; -const MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS: usize = 1_024; +const MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS: usize = 1_024; const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "..."; pub(crate) const MAX_SKILL_NAME_BYTES: usize = 256; pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024; @@ -30,7 +30,7 @@ pub(crate) fn available_skills_fragment( .short_description .as_deref() .unwrap_or(entry.description.as_str()); - let description = truncate_model_visible_skill_description(description); + let description = truncate_default_context_skill_description(description); let line = render_skill_line(entry, description.as_ref()); let next_bytes = total_bytes.saturating_add(line.len()); if next_bytes > MAX_AVAILABLE_SKILLS_BYTES { @@ -54,16 +54,16 @@ pub(crate) fn available_skills_fragment( Some(AvailableSkillsInstructions::from_skill_lines(skill_lines)) } -pub(crate) fn truncate_model_visible_skill_description(description: &str) -> Cow<'_, str> { +fn truncate_default_context_skill_description(description: &str) -> Cow<'_, str> { if description .char_indices() - .nth(MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS) + .nth(MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS) .is_none() { return Cow::Borrowed(description); } - let prefix_chars = MAX_MODEL_VISIBLE_SKILL_DESCRIPTION_CHARS + let prefix_chars = MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS .saturating_sub(TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count()); let prefix_end = description .char_indices() diff --git a/codex-rs/ext/skills/src/tools/list.rs b/codex-rs/ext/skills/src/tools/list.rs index 49d0488fa82f..31cb68003d69 100644 --- a/codex-rs/ext/skills/src/tools/list.rs +++ b/codex-rs/ext/skills/src/tools/list.rs @@ -8,7 +8,6 @@ use serde::Deserialize; use serde::Serialize; use crate::catalog::SkillCatalogEntry; -use crate::render::truncate_model_visible_skill_description; use crate::render::truncate_utf8_to_bytes; use super::MAX_HANDLE_BYTES; @@ -96,7 +95,7 @@ fn listed_skill(entry: SkillCatalogEntry) -> Option { authority, package: entry.id.0, name: entry.name, - description: truncate_model_visible_skill_description(&entry.description).into_owned(), + description: entry.description, main_resource: entry.main_prompt.as_str().to_string(), }) } diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index 4512f6961ce6..f5cfb336d6e8 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -10,14 +10,10 @@ use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS; use codex_core_skills::SkillLoadOutcome; use codex_core_skills::SkillMetadata; use codex_core_skills::injection::InjectedHostSkillPrompts; -use codex_extension_api::ConversationHistory; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionEventSink; use codex_extension_api::ExtensionRegistryBuilder; -use codex_extension_api::NoopTurnItemEmitter; use codex_extension_api::ThreadStartInput; -use codex_extension_api::ToolCall; -use codex_extension_api::ToolPayload; use codex_extension_api::TurnInputContext; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; @@ -27,7 +23,6 @@ use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SkillScope; -use codex_protocol::protocol::TruncationPolicy; use codex_protocol::user_input::UserInput; use codex_skills_extension::SkillProviders; use codex_skills_extension::SkillsExtensionConfig; @@ -261,7 +256,7 @@ async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_in } #[tokio::test] -async fn model_context_truncates_catalog_descriptions() -> TestResult { +async fn default_context_truncates_catalog_descriptions() -> TestResult { let description = "x".repeat(1_025); let mut entry = test_entry( SkillSourceKind::Orchestrator, @@ -310,78 +305,6 @@ async fn model_context_truncates_catalog_descriptions() -> TestResult { Ok(()) } -#[tokio::test] -async fn skills_list_truncates_catalog_descriptions_in_tool_output() -> TestResult { - let description = "x".repeat(1_025); - let mut entry = test_entry( - SkillSourceKind::Orchestrator, - "codex_apps", - "orchestrator/long-description", - "skill://orchestrator/long-description/SKILL.md", - ); - entry.description = description.clone(); - let providers = - SkillProviders::new().with_orchestrator_provider(Arc::new(StaticSkillProvider { - catalog: SkillCatalog { - entries: vec![entry], - 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 tools = registry.tool_contributors()[0].tools(&session_store, &thread_store); - let list_tool = tools - .iter() - .find(|tool| tool.tool_name().name == "list") - .ok_or("skills.list tool should be registered")?; - let payload = ToolPayload::Function { - arguments: serde_json::json!({"authority": {"kind": "orchestrator"}}).to_string(), - }; - let output = list_tool - .handle(ToolCall { - turn_id: "turn-1".to_string(), - call_id: "call-1".to_string(), - tool_name: list_tool.tool_name(), - model: "gpt-test".to_string(), - truncation_policy: TruncationPolicy::Bytes(1_024), - conversation_history: ConversationHistory::default(), - turn_item_emitter: Arc::new(NoopTurnItemEmitter), - environments: Vec::new(), - payload: payload.clone(), - }) - .await?; - let response = output - .post_tool_use_response("call-1", &payload) - .ok_or("skills.list should expose structured output")?; - let rendered_description = response["skills"][0]["description"] - .as_str() - .ok_or("skills.list response should include a description")?; - - assert_eq!(rendered_description, "x".repeat(1_021) + "..."); - assert_ne!(rendered_description, description); - - Ok(()) -} - #[tokio::test] async fn orchestrator_catalog_snapshot_caches_failure() -> TestResult { let list_calls = Arc::new(AtomicUsize::new(0)); diff --git a/codex-rs/external-agent-migration/src/lib.rs b/codex-rs/external-agent-migration/src/lib.rs index 69b1b1380314..7e5f626d8d01 100644 --- a/codex-rs/external-agent-migration/src/lib.rs +++ b/codex-rs/external-agent-migration/src/lib.rs @@ -1162,7 +1162,6 @@ fn command_source_name(source_commands: &Path, source_file: &Path) -> String { fn render_command_skill(body: &str, name: &str, description: &str, source_name: &str) -> String { let body = rewrite_external_agent_terms(body.trim()); - let description = rewrite_external_agent_terms(description); let template_body = if body.is_empty() { "No command template body was found.".to_string() } else { @@ -1171,7 +1170,7 @@ fn render_command_skill(body: &str, name: &str, description: &str, source_name: format!( "---\nname: {}\ndescription: {}\n---\n\n# {name}\n\nUse this skill when the user asks to run the migrated source command `{source_name}`.\n\n## Command Template\n\n{template_body}\n", yaml_string(name), - yaml_string(&description), + yaml_string(&rewrite_external_agent_terms(description)), ) } From 36423db11cfc3ab0f03b35df905846c88c8c61d6 Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Fri, 19 Jun 2026 12:04:21 -0700 Subject: [PATCH 8/8] codex: bound skills list descriptions --- codex-rs/ext/skills/src/render.rs | 10 +-- codex-rs/ext/skills/src/tools/list.rs | 3 +- codex-rs/ext/skills/tests/skills_extension.rs | 77 +++++++++++++++++++ 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index 19f39e69fd8c..4d0d0d70b524 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -9,7 +9,7 @@ use crate::fragments::AvailableSkillsInstructions; const MAX_AVAILABLE_SKILLS_BYTES: usize = 8_000; const MAX_MAIN_PROMPT_BYTES: usize = 8_000; -const MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS: usize = 1_024; +const MAX_CATALOG_SKILL_DESCRIPTION_CHARS: usize = 1_024; const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "..."; pub(crate) const MAX_SKILL_NAME_BYTES: usize = 256; pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024; @@ -30,7 +30,7 @@ pub(crate) fn available_skills_fragment( .short_description .as_deref() .unwrap_or(entry.description.as_str()); - let description = truncate_default_context_skill_description(description); + let description = truncate_catalog_skill_description(description); let line = render_skill_line(entry, description.as_ref()); let next_bytes = total_bytes.saturating_add(line.len()); if next_bytes > MAX_AVAILABLE_SKILLS_BYTES { @@ -54,16 +54,16 @@ pub(crate) fn available_skills_fragment( Some(AvailableSkillsInstructions::from_skill_lines(skill_lines)) } -fn truncate_default_context_skill_description(description: &str) -> Cow<'_, str> { +pub(crate) fn truncate_catalog_skill_description(description: &str) -> Cow<'_, str> { if description .char_indices() - .nth(MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS) + .nth(MAX_CATALOG_SKILL_DESCRIPTION_CHARS) .is_none() { return Cow::Borrowed(description); } - let prefix_chars = MAX_DEFAULT_CONTEXT_SKILL_DESCRIPTION_CHARS + let prefix_chars = MAX_CATALOG_SKILL_DESCRIPTION_CHARS .saturating_sub(TRUNCATED_SKILL_DESCRIPTION_SUFFIX.chars().count()); let prefix_end = description .char_indices() diff --git a/codex-rs/ext/skills/src/tools/list.rs b/codex-rs/ext/skills/src/tools/list.rs index 31cb68003d69..e556f9783f06 100644 --- a/codex-rs/ext/skills/src/tools/list.rs +++ b/codex-rs/ext/skills/src/tools/list.rs @@ -8,6 +8,7 @@ use serde::Deserialize; use serde::Serialize; use crate::catalog::SkillCatalogEntry; +use crate::render::truncate_catalog_skill_description; use crate::render::truncate_utf8_to_bytes; use super::MAX_HANDLE_BYTES; @@ -95,7 +96,7 @@ fn listed_skill(entry: SkillCatalogEntry) -> Option { authority, package: entry.id.0, name: entry.name, - description: entry.description, + description: truncate_catalog_skill_description(&entry.description).into_owned(), main_resource: entry.main_prompt.as_str().to_string(), }) } diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index f5cfb336d6e8..22657ec26f62 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -10,10 +10,14 @@ use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS; use codex_core_skills::SkillLoadOutcome; use codex_core_skills::SkillMetadata; use codex_core_skills::injection::InjectedHostSkillPrompts; +use codex_extension_api::ConversationHistory; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionEventSink; use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::NoopTurnItemEmitter; use codex_extension_api::ThreadStartInput; +use codex_extension_api::ToolCall; +use codex_extension_api::ToolPayload; use codex_extension_api::TurnInputContext; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; @@ -23,6 +27,7 @@ use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SkillScope; +use codex_protocol::protocol::TruncationPolicy; use codex_protocol::user_input::UserInput; use codex_skills_extension::SkillProviders; use codex_skills_extension::SkillsExtensionConfig; @@ -305,6 +310,78 @@ async fn default_context_truncates_catalog_descriptions() -> TestResult { Ok(()) } +#[tokio::test] +async fn skills_list_truncates_catalog_descriptions_in_tool_output() -> TestResult { + let description = "x".repeat(1_025); + let mut entry = test_entry( + SkillSourceKind::Orchestrator, + "codex_apps", + "orchestrator/long-description", + "skill://orchestrator/long-description/SKILL.md", + ); + entry.description = description.clone(); + let providers = + SkillProviders::new().with_orchestrator_provider(Arc::new(StaticSkillProvider { + catalog: SkillCatalog { + entries: vec![entry], + 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 tools = registry.tool_contributors()[0].tools(&session_store, &thread_store); + let list_tool = tools + .iter() + .find(|tool| tool.tool_name().name == "list") + .ok_or("skills.list tool should be registered")?; + let payload = ToolPayload::Function { + arguments: serde_json::json!({"authority": {"kind": "orchestrator"}}).to_string(), + }; + let output = list_tool + .handle(ToolCall { + turn_id: "turn-1".to_string(), + call_id: "call-1".to_string(), + tool_name: list_tool.tool_name(), + model: "gpt-test".to_string(), + truncation_policy: TruncationPolicy::Bytes(1_024), + conversation_history: ConversationHistory::default(), + turn_item_emitter: Arc::new(NoopTurnItemEmitter), + environments: Vec::new(), + payload: payload.clone(), + }) + .await?; + let response = output + .post_tool_use_response("call-1", &payload) + .ok_or("skills.list should expose structured output")?; + let rendered_description = response["skills"][0]["description"] + .as_str() + .ok_or("skills.list response should include a description")?; + + assert_eq!(rendered_description, "x".repeat(1_021) + "..."); + assert_ne!(rendered_description, description); + + Ok(()) +} + #[tokio::test] async fn orchestrator_catalog_snapshot_caches_failure() -> TestResult { let list_calls = Arc::new(AtomicUsize::new(0));