diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 80d9a7425e..1bc05fed01 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2916,7 +2916,6 @@ dependencies = [ "codex-shell-command", "codex-skills", "codex-utils-absolute-path", - "codex-utils-output-truncation", "codex-utils-path-uri", "codex-utils-plugins", "dirs", diff --git a/codex-rs/core-skills/Cargo.toml b/codex-rs/core-skills/Cargo.toml index f86ecdf75b..d175fceec9 100644 --- a/codex-rs/core-skills/Cargo.toml +++ b/codex-rs/core-skills/Cargo.toml @@ -25,7 +25,6 @@ codex-protocol = { workspace = true } codex-shell-command = { workspace = true } codex-skills = { workspace = true } codex-utils-absolute-path = { workspace = true } -codex-utils-output-truncation = { workspace = true } codex-utils-path-uri = { workspace = true } codex-utils-plugins = { workspace = true } dirs = { workspace = true } diff --git a/codex-rs/core-skills/src/lib.rs b/codex-rs/core-skills/src/lib.rs index dcfc6568a5..ce7211b9fb 100644 --- a/codex-rs/core-skills/src/lib.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -5,7 +5,6 @@ pub mod loader; mod mention_counts; pub mod model; pub mod remote; -pub mod render; mod root_loader; pub mod service; mod skill_instructions; @@ -20,15 +19,6 @@ pub use model::SkillLoadOutcome; pub use model::SkillMetadata; pub use model::SkillPolicy; pub use model::filter_skill_load_outcome_for_product; -pub use render::AvailableSkills; -pub use render::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS; -pub use render::SKILLS_HOW_TO_USE_WITH_ALIASES; -pub use render::SKILLS_INTRO_WITH_ABSOLUTE_PATHS; -pub use render::SkillMetadataBudget; -pub use render::SkillRenderReport; -pub use render::build_available_skills; -pub use render::default_skill_metadata_budget; -pub use render::render_available_skills_body; pub use root_loader::PluginSkillSnapshots; pub use service::SkillsLoadInput; pub use service::SkillsService; diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs deleted file mode 100644 index 73a37e502b..0000000000 --- a/codex-rs/core-skills/src/render.rs +++ /dev/null @@ -1,1575 +0,0 @@ -use std::borrow::Cow; -use std::collections::HashMap; -use std::collections::HashSet; -use std::path::Component; -use std::path::Path; - -use crate::model::SkillLoadOutcome; -use crate::model::SkillMetadata; -use codex_otel::SessionTelemetry; -use codex_otel::THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC; -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::SkillScope; -use codex_utils_absolute_path::AbsolutePathBuf; -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."; -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."; -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."; -const SKILLS_INTRO_WITH_ALIASES: &str = "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 a short path that can be expanded into an absolute path using the skill roots table."; -pub const SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS: &str = r###"- Discovery: The list above is the skills available in this session (name + description + source locator). `file` entries live on the host filesystem, `environment resource` and `orchestrator resource` entries must be accessed through `skills.list` and `skills.read`, and `custom resource` entries use their provider's access mechanism. -- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned. -- Missing/blocked: If a named skill isn't in the list or its source can't be read, say so briefly and continue with the best fallback. -- How to use a skill (progressive disclosure): - 1) After deciding to use a skill, the main agent must read its `SKILL.md` completely before taking task actions. For a `file` entry, open the listed path. For an `environment resource`, call `skills.list` with `{"authority":{"kind":"executor"}}`; for an `orchestrator resource`, use `{"authority":{"kind":"orchestrator"}}`. Select the matching package and pass its exact authority, package, and `main_resource` to `skills.read`. Follow `next_cursor`; if a read is paginated, continue until EOF. - 2) When `SKILL.md` references another resource, use the same access mechanism. Resolve relative references beneath an executor skill's returned package and call `skills.read` with the same authority and package. For orchestrator skills, pass the exact referenced resource identifier with the same authority and package to `skills.read`; do not treat `skill://` identifiers as filesystem paths. - 3) If `SKILL.md` points to extra folders such as `references/`, use its routing instructions to identify the resources required for the task. The main agent must read each required instruction or reference file itself before acting on it. Do not delegate reading, summarizing, or interpreting skill instructions to a subagent. Subagents may still perform task work when the selected skill allows it. - 4) For filesystem-backed skills, prefer running or patching provided scripts instead of retyping large code blocks. For environment and orchestrator skills, use `skills.read` and the available tools; do not invent a local path. - 5) Reuse provided assets or templates through the same source access mechanism instead of recreating them. -- Coordination and sequencing: - - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them. - - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why. -- Context hygiene: - - Progressive disclosure applies to selecting relevant files, not partially reading a selected instruction file. Do not load unrelated references, scripts, or assets. - - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked. - - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice. -- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue."###; -pub const SKILLS_HOW_TO_USE_WITH_ALIASES: &str = r###"- Discovery: The list above is the skills available in this session (name + description + short path). Skill bodies live on disk at the listed paths after expanding the matching alias from `### Skill roots`. -- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned. -- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback. -- How to use a skill (progressive disclosure): - 1) After deciding to use a skill, the main agent must expand the listed short `path` with the matching alias from `### Skill roots`, then open and read its `SKILL.md` completely before taking task actions. If a read is truncated or paginated, continue until EOF. - 2) When `SKILL.md` references relative paths (e.g., `scripts/foo.py`), resolve them relative to the directory containing that expanded `SKILL.md` first, and only consider other paths if needed. - 3) If `SKILL.md` points to extra folders such as `references/`, use its routing instructions to identify the files required for the task. The main agent must read each required instruction or reference file itself before acting on it. Do not delegate reading, summarizing, or interpreting skill instructions to a subagent. Subagents may still perform task work when the selected skill allows it. - 4) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks. - 5) If `assets/` or templates exist, reuse them instead of recreating from scratch. -- Coordination and sequencing: - - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them. - - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why. -- Context hygiene: - - Progressive disclosure applies to selecting relevant files, not partially reading a selected instruction file. Do not load unrelated references, scripts, or assets. - - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked. - - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice. -- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue."###; - -pub fn render_available_skills_body(skill_root_lines: &[String], skill_lines: &[String]) -> String { - let mut lines: Vec = Vec::new(); - lines.push("## Skills".to_string()); - if skill_root_lines.is_empty() { - lines.push(SKILLS_INTRO_WITH_ABSOLUTE_PATHS.to_string()); - } else { - lines.push(SKILLS_INTRO_WITH_ALIASES.to_string()); - lines.push("### Skill roots".to_string()); - lines.extend(skill_root_lines.iter().cloned()); - } - lines.push("### Available skills".to_string()); - lines.extend(skill_lines.iter().cloned()); - - format!("\n{}\n", lines.join("\n")) -} - -#[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(_) => approx_token_count(text), - Self::Characters(_) => text.chars().count(), - } - } - - fn cost_from_counts(self, chars: usize, bytes: usize) -> usize { - match self { - Self::Tokens(_) => approx_token_count_from_bytes(bytes), - Self::Characters(_) => chars, - } - } -} - -fn approx_token_count_from_bytes(bytes: usize) -> usize { - bytes.saturating_add(APPROX_BYTES_PER_TOKEN.saturating_sub(1)) / APPROX_BYTES_PER_TOKEN -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct SkillRenderReport { - pub total_count: usize, - pub included_count: usize, - pub omitted_count: usize, - pub truncated_description_chars: usize, - pub truncated_description_count: usize, -} - -#[derive(Clone, Copy)] -pub enum SkillRenderSideEffects<'a> { - None, - ThreadStart { - session_telemetry: &'a SessionTelemetry, - }, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct AvailableSkills { - pub skill_root_lines: Vec, - pub skill_lines: Vec, - pub report: SkillRenderReport, - pub warning_message: Option, -} - -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 build_available_skills( - outcome: &SkillLoadOutcome, - budget: SkillMetadataBudget, - side_effects: SkillRenderSideEffects<'_>, -) -> Option { - let skills = outcome.allowed_skills_for_implicit_invocation(); - if skills.is_empty() { - record_skill_render_side_effects( - side_effects, - /*total_count*/ 0, - /*included_count*/ 0, - /*omitted_count*/ 0, - /*truncated_description_chars*/ 0, - ); - return None; - } - - let absolute_lines = ordered_absolute_skill_lines(&skills); - let absolute = build_available_skills_from_lines( - absolute_lines, - skills.len(), - budget, - SkillPathAliases::default(), - )?; - - let selected = - if absolute.report.omitted_count == 0 && absolute.report.truncated_description_chars == 0 { - absolute - } else if let Some(aliased) = build_aliased_available_skills(outcome, &skills, budget) { - if aliased_render_is_better(&aliased, &absolute, budget) { - aliased - } else { - absolute - } - } else { - absolute - }; - - record_available_skills_side_effects(&selected, budget, side_effects); - Some(selected) -} - -fn build_available_skills_from_lines( - skill_lines: Vec>, - total_count: usize, - budget: SkillMetadataBudget, - path_aliases: SkillPathAliases, -) -> Option { - if total_count == 0 { - return None; - } - - let (skill_lines, report) = render_skill_lines_from_lines(skill_lines, total_count, budget); - let warning_message = if report.omitted_count > 0 { - let skill_word = if report.omitted_count == 1 { - "skill" - } else { - "skills" - }; - let verb = if report.omitted_count == 1 { - "was" - } else { - "were" - }; - Some(format!( - "{} {} additional {} {} not included in the model-visible skills list.", - budget_warning_prefix(budget, SKILL_DESCRIPTIONS_REMOVED_WARNING_PREFIX), - report.omitted_count, - skill_word, - verb - )) - } else if report.average_truncated_description_chars() - > SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS - { - Some( - match budget { - SkillMetadataBudget::Tokens(_) => SKILL_DESCRIPTION_TRUNCATED_WARNING_WITH_PERCENT, - SkillMetadataBudget::Characters(_) => SKILL_DESCRIPTION_TRUNCATED_WARNING, - } - .to_string(), - ) - } else { - None - }; - let available = AvailableSkills { - skill_root_lines: path_aliases.skill_root_lines, - skill_lines, - report, - warning_message, - }; - Some(available) -} - -fn record_available_skills_side_effects( - available: &AvailableSkills, - budget: SkillMetadataBudget, - side_effects: SkillRenderSideEffects<'_>, -) { - record_skill_render_side_effects( - side_effects, - available.report.total_count, - available.report.included_count, - available.report.omitted_count, - available.report.truncated_description_chars, - ); - if available.report.omitted_count > 0 || available.report.truncated_description_chars > 0 { - tracing::info!( - budget_limit = budget.limit(), - total_skills = available.report.total_count, - included_skills = available.report.included_count, - omitted_skills = available.report.omitted_count, - truncated_description_chars_per_skill = - available.report.average_truncated_description_chars(), - truncated_skill_descriptions = available.report.truncated_description_count, - "truncated skill metadata to fit skills context budget" - ); - } -} - -fn budget_warning_prefix(budget: SkillMetadataBudget, prefix: &str) -> String { - match budget { - SkillMetadataBudget::Tokens(_) => prefix.replacen( - "Exceeded skills context budget.", - "Exceeded skills context budget of 2%.", - 1, - ), - SkillMetadataBudget::Characters(_) => prefix.to_string(), - } -} - -fn record_skill_render_side_effects( - side_effects: SkillRenderSideEffects<'_>, - total_count: usize, - included_count: usize, - omitted_count: usize, - truncated_description_chars: usize, -) { - match side_effects { - SkillRenderSideEffects::None => {} - SkillRenderSideEffects::ThreadStart { session_telemetry } => { - 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 omitted_count > 0 { 1 } else { 0 }, - &[], - ); - session_telemetry.histogram( - THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC, - i64::try_from(truncated_description_chars).unwrap_or(i64::MAX), - &[], - ); - } - } -} - -fn render_skill_lines_from_lines( - skill_lines: Vec>, - total_count: usize, - budget: SkillMetadataBudget, -) -> (Vec, SkillRenderReport) { - let full_cost = skill_lines.iter().fold(0usize, |used, line| { - used.saturating_add(line.full_cost(budget)) - }); - if full_cost <= budget.limit() { - let included = skill_lines - .iter() - .map(SkillLine::render_full) - .collect::>(); - - return ( - included, - skill_render_report( - total_count, - /*included_count*/ skill_lines.len(), - /*omitted_count*/ 0, - /*truncated_description_chars*/ 0, - /*truncated_description_count*/ 0, - ), - ); - } - - let minimum_cost = skill_lines.iter().fold(0usize, |used, line| { - used.saturating_add(line.minimum_cost(budget)) - }); - if minimum_cost <= budget.limit() { - let rendered = render_lines_with_description_budget( - budget, - &skill_lines, - budget.limit().saturating_sub(minimum_cost), - ); - let (truncated_description_chars, truncated_description_count) = - sum_description_truncation(&rendered); - let included = rendered - .into_iter() - .map(|rendered| rendered.line) - .collect::>(); - - return ( - included, - skill_render_report( - total_count, - /*included_count*/ skill_lines.len(), - /*omitted_count*/ 0, - truncated_description_chars, - truncated_description_count, - ), - ); - } - - render_minimum_skill_lines_until_budget(budget, skill_lines, total_count) -} - -fn render_minimum_skill_lines_until_budget( - budget: SkillMetadataBudget, - skill_lines: Vec>, - total_count: usize, -) -> (Vec, SkillRenderReport) { - let mut included = Vec::new(); - let mut used = 0usize; - let mut omitted_count = 0usize; - let mut truncated_description_chars = 0usize; - let mut truncated_description_count = 0usize; - for line in skill_lines { - let line_cost = line.minimum_cost(budget); - let description_char_count = line.description_char_count(); - if used.saturating_add(line_cost) <= budget.limit() { - used = used.saturating_add(line_cost); - included.push(line.render_minimum()); - } else { - omitted_count = omitted_count.saturating_add(1); - } - - truncated_description_chars = - truncated_description_chars.saturating_add(description_char_count); - if description_char_count > 0 { - truncated_description_count = truncated_description_count.saturating_add(1); - } - } - - let report = skill_render_report( - total_count, - included.len(), - omitted_count, - truncated_description_chars, - truncated_description_count, - ); - - (included, report) -} - -fn skill_render_report( - total_count: usize, - included_count: usize, - omitted_count: usize, - truncated_description_chars: usize, - truncated_description_count: usize, -) -> SkillRenderReport { - SkillRenderReport { - total_count, - included_count, - omitted_count, - truncated_description_chars, - truncated_description_count, - } -} - -impl SkillRenderReport { - fn average_truncated_description_chars(&self) -> usize { - if self.total_count == 0 || self.truncated_description_chars == 0 { - return 0; - } - - self.truncated_description_chars - .saturating_add(self.total_count.saturating_sub(1)) - / self.total_count - } -} - -struct SkillLine<'a> { - name: &'a str, - description: Cow<'a, str>, - path: String, -} - -struct RenderedSkillLine { - line: String, - truncated_chars: usize, -} - -struct DescriptionBudgetLine<'a> { - line: &'a SkillLine<'a>, - description_char_count: usize, - extra_costs: Vec, -} - -fn sum_description_truncation(rendered: &[RenderedSkillLine]) -> (usize, usize) { - rendered - .iter() - .fold((0usize, 0usize), |(chars, count), line| { - if line.truncated_chars == 0 { - (chars, count) - } else { - ( - chars.saturating_add(line.truncated_chars), - count.saturating_add(1), - ) - } - }) -} - -impl<'a> SkillLine<'a> { - fn new(skill: &'a SkillMetadata) -> Self { - Self::with_path( - skill, - skill.path_to_skills_md.to_string_lossy().replace('\\', "/"), - ) - } - - fn with_path(skill: &'a SkillMetadata, path: String) -> Self { - let description = truncate_default_context_skill_description(skill.description.as_str()); - Self { - name: skill.name.as_str(), - description, - path, - } - } - - fn full_cost(&self, budget: SkillMetadataBudget) -> usize { - line_cost(budget, &self.render_full()) - } - - fn minimum_cost(&self, budget: SkillMetadataBudget) -> usize { - line_cost(budget, &self.render_minimum()) - } - - fn description_char_count(&self) -> usize { - self.description.chars().count() - } - - fn render_full(&self) -> String { - self.render_with_description(self.description.as_ref()) - } - - fn render_minimum(&self) -> String { - self.render_with_description("") - } - - fn rendered_description_prefix_len(&self, description_chars: usize) -> usize { - self.description - .char_indices() - .nth(description_chars) - .map_or(self.description.len(), |(idx, _)| idx) - } - - fn render_with_description_chars(&self, description_chars: usize) -> String { - if description_chars == 0 { - format!("- {}: (file: {})", self.name, self.path) - } else { - let end = self.rendered_description_prefix_len(description_chars); - let description = &self.description.as_ref()[..end]; - format!("- {}: {} (file: {})", self.name, description, self.path) - } - } - - fn render_with_description(&self, description: &str) -> String { - if description.is_empty() { - format!("- {}: (file: {})", self.name, self.path) - } else { - format!("- {}: {} (file: {})", self.name, description, self.path) - } - } -} - -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(); - let minimum_chars = minimum_line.chars().count().saturating_add(1); - let minimum_bytes = minimum_line.len().saturating_add(1); - let minimum_cost = budget.cost_from_counts(minimum_chars, minimum_bytes); - - let description_char_count = line.description_char_count(); - let mut extra_costs = Vec::with_capacity(description_char_count.saturating_add(1)); - extra_costs.push(0); - - let mut prefix_chars = 0usize; - let mut prefix_bytes = 0usize; - for ch in line.description.chars() { - prefix_chars = prefix_chars.saturating_add(1); - prefix_bytes = prefix_bytes.saturating_add(ch.len_utf8()); - let rendered_chars = minimum_chars.saturating_add(prefix_chars).saturating_add(1); - let rendered_bytes = minimum_bytes.saturating_add(prefix_bytes).saturating_add(1); - let cost = budget - .cost_from_counts(rendered_chars, rendered_bytes) - .saturating_sub(minimum_cost); - extra_costs.push(cost); - } - - Self { - line, - description_char_count, - extra_costs, - } - } -} - -fn line_cost(budget: SkillMetadataBudget, line: &str) -> usize { - budget.cost(&format!("{line}\n")) -} - -fn lines_cost(budget: SkillMetadataBudget, lines: &[String]) -> usize { - lines.iter().fold(0usize, |used, line| { - used.saturating_add(line_cost(budget, line)) - }) -} - -fn render_lines_with_description_budget( - budget: SkillMetadataBudget, - skill_lines: &[SkillLine<'_>], - limit: usize, -) -> Vec { - let budget_lines = skill_lines - .iter() - .map(|line| DescriptionBudgetLine::new(line, budget)) - .collect::>(); - let mut char_allocations = vec![0usize; budget_lines.len()]; - let mut current_extra_costs = vec![0usize; budget_lines.len()]; - let mut remaining = limit; - - // Distribute description space one character at a time across skills. - // Short descriptions naturally drop out, so their unused share can go to - // longer descriptions instead of being stranded in a fixed per-skill quota. - loop { - let mut changed = false; - for (index, line) in budget_lines.iter().enumerate() { - if char_allocations[index] >= line.description_char_count { - continue; - } - - let current_cost = current_extra_costs[index]; - let next_chars = char_allocations[index].saturating_add(1); - let next_cost = line.extra_costs[next_chars]; - let delta = next_cost.saturating_sub(current_cost); - if delta <= remaining { - char_allocations[index] = next_chars; - current_extra_costs[index] = next_cost; - remaining = remaining.saturating_sub(delta); - changed = true; - } - } - - if !changed { - break; - } - } - - budget_lines - .iter() - .zip(char_allocations) - .map(|(line, description_chars)| { - let truncated_chars = line - .description_char_count - .saturating_sub(description_chars); - RenderedSkillLine { - line: line.line.render_with_description_chars(description_chars), - truncated_chars, - } - }) - .collect() -} - -fn build_aliased_available_skills( - outcome: &SkillLoadOutcome, - skills: &[SkillMetadata], - budget: SkillMetadataBudget, -) -> Option { - let plan = build_alias_plan(outcome, skills, budget)?; - if plan.table_cost >= budget.limit() { - return None; - } - - let adjusted_limit = budget.limit().saturating_sub(plan.table_cost); - let adjusted_budget = match budget { - SkillMetadataBudget::Tokens(_) => SkillMetadataBudget::Tokens(adjusted_limit), - SkillMetadataBudget::Characters(_) => SkillMetadataBudget::Characters(adjusted_limit), - }; - let ordered_skills = ordered_skills_for_budget(skills); - let skill_lines = ordered_skills - .into_iter() - .map(|skill| SkillLine::with_path(skill, render_skill_path_with_aliases(skill, &plan))) - .collect::>(); - build_available_skills_from_lines(skill_lines, skills.len(), adjusted_budget, plan.aliases) -} - -#[derive(Debug, Clone, Default, PartialEq, Eq)] -struct SkillPathAliases { - skill_root_lines: Vec, -} - -struct AliasPlan { - aliases: SkillPathAliases, - root_aliases: HashMap, - alias_root_by_path: HashMap, - table_cost: usize, -} - -fn build_alias_plan( - outcome: &SkillLoadOutcome, - skills: &[SkillMetadata], - budget: SkillMetadataBudget, -) -> Option { - let skill_paths = skills - .iter() - .map(|skill| skill.path_to_skills_md.clone()) - .collect::>(); - let skill_root_by_path = outcome - .skill_root_by_path - .iter() - .filter(|(path, _)| skill_paths.contains(*path)) - .map(|(path, root)| (path.clone(), root.clone())) - .collect::>(); - let used_roots = outcome - .skill_roots - .iter() - .filter(|root| { - skill_root_by_path - .values() - .any(|skill_root| skill_root == *root) - }) - .cloned() - .collect::>(); - if used_roots.is_empty() { - return None; - } - - let plugin_version_skill_counts = - plugin_version_skill_counts_for_skill_roots(skill_root_by_path.values()); - let alias_root_by_skill_root = used_roots - .iter() - .map(|root| { - ( - root.clone(), - alias_root_for_skill_root(root, &plugin_version_skill_counts), - ) - }) - .collect::>(); - let alias_roots = ordered_alias_roots(&used_roots, &alias_root_by_skill_root)?; - let root_aliases = alias_roots - .iter() - .enumerate() - .map(|(index, alias_root)| (alias_root.clone(), format!("r{index}"))) - .collect::>(); - let alias_root_by_path = skill_root_by_path - .iter() - .filter_map(|(path, skill_root)| { - alias_root_by_skill_root - .get(skill_root) - .map(|alias_root| (path.clone(), alias_root.clone())) - }) - .collect::>(); - let skill_root_lines = build_skill_root_lines(&alias_roots); - let table_cost = aliased_metadata_overhead_cost(budget, &skill_root_lines); - - Some(AliasPlan { - aliases: SkillPathAliases { skill_root_lines }, - root_aliases, - alias_root_by_path, - table_cost, - }) -} - -fn ordered_alias_roots( - used_roots: &[AbsolutePathBuf], - alias_root_by_skill_root: &HashMap, -) -> Option> { - let mut seen = HashSet::new(); - let mut alias_roots = Vec::new(); - for root in used_roots { - let alias_root = alias_root_by_skill_root.get(root)?.clone(); - if seen.insert(alias_root.clone()) { - alias_roots.push(alias_root); - } - } - Some(alias_roots) -} - -fn alias_root_for_skill_root( - root: &AbsolutePathBuf, - plugin_version_skill_counts: &HashMap, -) -> AbsolutePathBuf { - let Some(plugin_version_base) = plugin_version_base(root.as_path()) else { - return root.clone(); - }; - let skill_count = plugin_version_skill_counts - .get(&plugin_version_base) - .copied() - .unwrap_or_default(); - if skill_count > 1 { - root.clone() - } else { - plugin_marketplace_base(root.as_path()).unwrap_or_else(|| root.clone()) - } -} - -fn plugin_version_skill_counts_for_skill_roots<'a>( - skill_roots: impl Iterator, -) -> HashMap { - let mut counts = HashMap::new(); - for root in skill_roots { - if let Some(plugin_version_base) = plugin_version_base(root.as_path()) { - let count = counts.entry(plugin_version_base).or_insert(0usize); - *count = count.saturating_add(1); - } - } - counts -} - -fn aliased_metadata_overhead_cost( - budget: SkillMetadataBudget, - skill_root_lines: &[String], -) -> usize { - let empty_skill_lines: &[String] = &[]; - let absolute_body = render_available_skills_body(&[], empty_skill_lines); - let aliased_body = render_available_skills_body(skill_root_lines, empty_skill_lines); - budget - .cost(&aliased_body) - .saturating_sub(budget.cost(&absolute_body)) -} - -fn build_skill_root_lines(roots: &[AbsolutePathBuf]) -> Vec { - roots - .iter() - .enumerate() - .map(|(index, root)| { - let root_str = root.to_string_lossy().replace('\\', "/"); - format!("- `r{index}` = `{root_str}`") - }) - .collect() -} - -fn plugin_marketplace_base(path: &Path) -> Option { - let mut candidate = path; - while let Some(parent) = candidate.parent() { - if parent.file_name()?.to_str()? == "cache" - && parent.parent()?.file_name()?.to_str()? == "plugins" - { - return AbsolutePathBuf::from_absolute_path(candidate).ok(); - } - candidate = parent; - } - None -} - -fn plugin_version_base(path: &Path) -> Option { - let marketplace_base = plugin_marketplace_base(path)?; - let mut relative_components = path - .strip_prefix(marketplace_base.as_path()) - .ok()? - .components(); - let plugin = match relative_components.next()? { - Component::Normal(plugin) => plugin, - _ => return None, - }; - let version = match relative_components.next()? { - Component::Normal(version) => version, - _ => return None, - }; - AbsolutePathBuf::from_absolute_path(marketplace_base.join(plugin).join(version)).ok() -} - -fn render_skill_path_with_aliases(skill: &SkillMetadata, plan: &AliasPlan) -> String { - outcome_relative_skill_path(skill, plan) - .unwrap_or_else(|| skill.path_to_skills_md.to_string_lossy().replace('\\', "/")) -} - -fn outcome_relative_skill_path(skill: &SkillMetadata, plan: &AliasPlan) -> Option { - let alias_root = plan.alias_root_by_path.get(&skill.path_to_skills_md)?; - let alias = plan.root_aliases.get(alias_root)?; - let relative_path = skill - .path_to_skills_md - .as_path() - .strip_prefix(alias_root.as_path()) - .ok()?; - let relative_path = relative_path.to_string_lossy().replace('\\', "/"); - Some(format!("{alias}/{relative_path}")) -} - -fn aliased_render_is_better( - aliased: &AvailableSkills, - absolute: &AvailableSkills, - budget: SkillMetadataBudget, -) -> bool { - if aliased.report.included_count != absolute.report.included_count { - return aliased.report.included_count > absolute.report.included_count; - } - if aliased.report.truncated_description_chars != absolute.report.truncated_description_chars { - return aliased.report.truncated_description_chars - < absolute.report.truncated_description_chars; - } - available_skills_cost(budget, aliased) < available_skills_cost(budget, absolute) -} - -fn available_skills_cost(budget: SkillMetadataBudget, available: &AvailableSkills) -> usize { - let metadata_cost = if available.skill_root_lines.is_empty() { - 0 - } else { - aliased_metadata_overhead_cost(budget, &available.skill_root_lines) - }; - metadata_cost.saturating_add(lines_cost(budget, &available.skill_lines)) -} - -fn ordered_absolute_skill_lines(skills: &[SkillMetadata]) -> Vec> { - ordered_skills_for_budget(skills) - .into_iter() - .map(SkillLine::new) - .collect() -} - -fn ordered_skills_for_budget(skills: &[SkillMetadata]) -> Vec<&SkillMetadata> { - let mut ordered = skills.iter().collect::>(); - 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, - } -} - -#[cfg(test)] -mod tests { - use super::*; - use std::collections::HashMap; - use std::sync::Arc; - - 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, - plugin_id: None, - remote_plugin_id: None, - } - } - - fn make_skill_with_description( - name: &str, - scope: SkillScope, - description: &str, - ) -> SkillMetadata { - let mut skill = make_skill(name, scope); - skill.description = description.to_string(); - skill - } - - fn expected_skill_line(skill: &SkillMetadata, description: &str) -> String { - SkillLine::new(skill).render_with_description(description) - } - - fn normalized_path(path: &AbsolutePathBuf) -> String { - path.to_string_lossy().replace('\\', "/") - } - - fn outcome_with_roots( - skills: Vec, - roots: Vec, - ) -> SkillLoadOutcome { - let skill_root_by_path = skills - .iter() - .filter_map(|skill| { - roots - .iter() - .find(|root| { - skill - .path_to_skills_md - .as_path() - .starts_with(root.as_path()) - }) - .map(|root| (skill.path_to_skills_md.clone(), root.clone())) - }) - .collect::>(); - SkillLoadOutcome { - skills, - skill_roots: roots, - skill_root_by_path: Arc::new(skill_root_by_path), - ..Default::default() - } - } - - fn build_available_skills_from_metadata( - skills: &[SkillMetadata], - budget: SkillMetadataBudget, - ) -> Option { - build_available_skills_from_lines( - ordered_absolute_skill_lines(skills), - skills.len(), - budget, - SkillPathAliases::default(), - ) - } - - #[test] - fn skill_usage_instructions_require_complete_main_agent_reads() { - for instructions in [ - SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS, - SKILLS_HOW_TO_USE_WITH_ALIASES, - ] { - assert!(instructions.contains("read its `SKILL.md` completely")); - assert!(instructions.contains("continue until EOF")); - assert!(instructions.contains( - "The main agent must read each required instruction or reference file itself" - )); - assert!(instructions.contains( - "Do not delegate reading, summarizing, or interpreting skill instructions" - )); - assert!(instructions.contains( - "Subagents may still perform task work when the selected skill allows it" - )); - assert!(instructions.contains( - "Progressive disclosure applies to selecting relevant files, not partially reading a selected instruction file" - )); - assert!(!instructions.contains("Read only enough to follow the workflow")); - } - } - - #[test] - fn default_budget_uses_two_percent_of_full_context_window() { - assert_eq!( - default_skill_metadata_budget(Some(200_000)), - SkillMetadataBudget::Tokens(4_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(/*context_window*/ 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 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_DEFAULT_CONTEXT_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), - 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"); - let beta = make_skill_with_description("beta-skill", SkillScope::Repo, "uvwxyz"); - let minimum_cost = SkillLine::new(&alpha) - .minimum_cost(SkillMetadataBudget::Characters(usize::MAX)) - + SkillLine::new(&beta).minimum_cost(SkillMetadataBudget::Characters(usize::MAX)); - let budget = SkillMetadataBudget::Characters(minimum_cost + 6); - - let rendered = build_available_skills_from_metadata(&[beta.clone(), alpha.clone()], budget) - .expect("skills should render"); - - assert_eq!(rendered.report.included_count, 2); - assert_eq!(rendered.report.omitted_count, 0); - assert_eq!(rendered.report.truncated_description_chars, 8); - assert_eq!(rendered.warning_message, None); - assert_eq!( - rendered.skill_lines, - vec![ - expected_skill_line(&alpha, "ab"), - expected_skill_line(&beta, "uv"), - ] - ); - } - - #[test] - fn budgeted_rendering_does_not_warn_when_average_description_truncation_is_within_threshold() { - let alpha = make_skill_with_description("alpha-skill", SkillScope::Repo, "abcdefghij"); - let beta = make_skill_with_description("beta-skill", SkillScope::Repo, "uvwxyzabcd"); - let minimum_cost = SkillLine::new(&alpha) - .minimum_cost(SkillMetadataBudget::Characters(usize::MAX)) - + SkillLine::new(&beta).minimum_cost(SkillMetadataBudget::Characters(usize::MAX)); - let budget = SkillMetadataBudget::Characters(minimum_cost + 6); - - let rendered = build_available_skills_from_metadata(&[alpha, beta], budget) - .expect("skills should render"); - - assert_eq!(rendered.report.included_count, 2); - assert_eq!(rendered.report.omitted_count, 0); - assert_eq!(rendered.report.truncated_description_chars, 16); - assert_eq!(rendered.report.truncated_description_count, 2); - assert_eq!(rendered.warning_message, None); - } - - #[test] - fn budgeted_rendering_warns_when_average_description_truncation_exceeds_threshold() { - let long_description = "a".repeat(250); - let long_skill = - make_skill_with_description("long-skill", SkillScope::Repo, &long_description); - let empty_skill = make_skill_with_description("empty-skill", SkillScope::Repo, ""); - let minimum_cost = SkillLine::new(&long_skill) - .minimum_cost(SkillMetadataBudget::Characters(usize::MAX)) - + SkillLine::new(&empty_skill) - .minimum_cost(SkillMetadataBudget::Characters(usize::MAX)); - let budget = SkillMetadataBudget::Characters(minimum_cost + 49); - - let rendered = build_available_skills_from_metadata(&[long_skill, empty_skill], budget) - .expect("skills should render"); - - assert_eq!(rendered.report.total_count, 2); - assert_eq!(rendered.report.included_count, 2); - assert_eq!(rendered.report.omitted_count, 0); - assert_eq!(rendered.report.truncated_description_chars, 202); - assert_eq!(rendered.report.truncated_description_count, 1); - assert_eq!( - rendered.warning_message, - Some( - "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." - .to_string() - ) - ); - } - - #[test] - fn budgeted_rendering_token_budget_truncation_warning_mentions_two_percent() { - let long_description = "a".repeat(1000); - let long_skill = - make_skill_with_description("long-skill", SkillScope::Repo, &long_description); - let minimum_cost = - SkillLine::new(&long_skill).minimum_cost(SkillMetadataBudget::Tokens(usize::MAX)); - let budget = SkillMetadataBudget::Tokens(minimum_cost + 1); - - let rendered = build_available_skills_from_metadata(&[long_skill], budget) - .expect("skills should render"); - - assert_eq!( - rendered.warning_message, - Some(SKILL_DESCRIPTION_TRUNCATED_WARNING_WITH_PERCENT.to_string()) - ); - } - - #[test] - fn budgeted_rendering_redistributes_unused_description_budget() { - let short = make_skill_with_description("short-skill", SkillScope::Repo, "x"); - let long = make_skill_with_description("long-skill", SkillScope::Repo, "abcdefghi"); - let minimum_cost = SkillLine::new(&short) - .minimum_cost(SkillMetadataBudget::Characters(usize::MAX)) - + SkillLine::new(&long).minimum_cost(SkillMetadataBudget::Characters(usize::MAX)); - let budget = SkillMetadataBudget::Characters(minimum_cost + 11); - - let rendered = build_available_skills_from_metadata(&[short.clone(), long.clone()], budget) - .expect("skills should render"); - - assert_eq!(rendered.report.included_count, 2); - assert_eq!(rendered.report.omitted_count, 0); - assert_eq!(rendered.warning_message, None); - assert_eq!( - rendered.skill_lines, - vec![ - expected_skill_line(&long, "abcdefgh"), - expected_skill_line(&short, "x"), - ] - ); - } - - #[test] - fn budgeted_rendering_preserves_prompt_priority_when_minimum_lines_exceed_budget() { - 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", SkillLine::new(&system).render_minimum())); - let admin_cost = SkillMetadataBudget::Characters(usize::MAX) - .cost(&format!("{}\n", SkillLine::new(&admin).render_minimum())); - let budget = SkillMetadataBudget::Characters(system_cost + admin_cost); - - let rendered = build_available_skills_from_metadata(&[system, user, repo, admin], budget) - .expect("skills should render"); - - assert_eq!(rendered.report.included_count, 2); - assert_eq!(rendered.report.omitted_count, 2); - assert_eq!( - rendered.warning_message, - Some( - "Exceeded skills context budget. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list." - .to_string() - ) - ); - let rendered_text = rendered.skill_lines.join("\n"); - assert!(rendered_text.contains("- system-skill:")); - assert!(rendered_text.contains("- admin-skill:")); - assert!(!rendered_text.contains("desc")); - assert!(!rendered_text.contains("- repo-skill:")); - 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", SkillLine::new(&repo).render_full())); - let budget = SkillMetadataBudget::Characters(repo_cost); - - let rendered = build_available_skills_from_metadata(&[oversized, repo], budget) - .expect("skills render"); - - assert_eq!(rendered.report.included_count, 1); - assert_eq!(rendered.report.omitted_count, 1); - assert_eq!( - rendered.warning_message, - Some( - "Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list." - .to_string() - ) - ); - let rendered_text = rendered.skill_lines.join("\n"); - assert!(!rendered_text.contains("- oversized-system-skill:")); - assert!(rendered_text.contains("- repo-skill:")); - } - - #[test] - fn outcome_rendering_omits_aliases_when_absolute_plan_has_no_budget_pressure() { - let root = test_path_buf("/tmp/skills").abs(); - let alpha_path = root.join("alpha/SKILL.md"); - let beta_path = root.join("beta/SKILL.md"); - let outcome = outcome_with_roots( - vec![ - skill_with_path("alpha-skill", &alpha_path), - skill_with_path("beta-skill", &beta_path), - ], - vec![root], - ); - - let rendered = build_available_skills( - &outcome, - SkillMetadataBudget::Characters(usize::MAX), - SkillRenderSideEffects::None, - ) - .expect("skills should render"); - - assert!(rendered.skill_root_lines.is_empty()); - assert_eq!(rendered.report.included_count, 2); - } - - #[test] - fn outcome_rendering_uses_aliases_when_they_allow_more_skills_to_fit() { - let root = test_path_buf( - "/Users/xl/.codex/plugins/cache/openai-curated/example/hash1234567890/skills-with-a-very-long-shared-prefix", - ) - .abs(); - let skills = (0..12) - .map(|index| { - let name = format!("shared-root-skill-{index}"); - skill_with_path(&name, &root.join(format!("skill-{index}/SKILL.md"))) - }) - .collect::>(); - let outcome = outcome_with_roots(skills.clone(), vec![root]); - let absolute_minimum = skills.iter().fold(0usize, |cost, skill| { - cost.saturating_add( - SkillLine::new(skill).minimum_cost(SkillMetadataBudget::Characters(usize::MAX)), - ) - }); - let plan = build_alias_plan( - &outcome, - &skills, - SkillMetadataBudget::Characters(usize::MAX), - ) - .expect("alias plan should build"); - let alias_minimum = skills.iter().fold(plan.table_cost, |cost, skill| { - cost.saturating_add( - SkillLine::with_path(skill, render_skill_path_with_aliases(skill, &plan)) - .minimum_cost(SkillMetadataBudget::Characters(usize::MAX)), - ) - }); - assert!( - alias_minimum < absolute_minimum, - "test fixture should make aliases cheaper" - ); - - let rendered = build_available_skills( - &outcome, - SkillMetadataBudget::Characters(alias_minimum), - SkillRenderSideEffects::None, - ) - .expect("skills should render"); - - assert_eq!(rendered.report.included_count, skills.len()); - assert_eq!(rendered.report.omitted_count, 0); - assert_eq!( - rendered.skill_root_lines, - vec![format!( - "- `r0` = `{}`", - normalized_path( - &test_path_buf( - "/Users/xl/.codex/plugins/cache/openai-curated/example/hash1234567890/skills-with-a-very-long-shared-prefix" - ) - .abs() - ) - )] - ); - let rendered_text = rendered.skill_lines.join("\n"); - assert!(rendered_text.contains("r0/skill-0/SKILL.md")); - assert!(rendered_text.contains("r0/skill-11/SKILL.md")); - } - - #[test] - fn outcome_rendering_uses_marketplace_root_for_single_skill_plugin_versions() { - let github_root = - test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated/github/hash123/skills") - .abs(); - let marketplace_root = test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated").abs(); - let github = skill_with_path("github:gh-fix-ci", &github_root.join("gh-fix-ci/SKILL.md")); - let outcome = outcome_with_roots(vec![github.clone()], vec![github_root.clone()]); - let plan = build_alias_plan( - &outcome, - &[github], - SkillMetadataBudget::Characters(usize::MAX), - ) - .expect("alias plan should build"); - - assert_eq!( - plan.aliases.skill_root_lines, - vec![format!("- `r0` = `{}`", normalized_path(&marketplace_root))] - ); - assert_eq!( - render_skill_path_with_aliases( - &skill_with_path("github:gh-fix-ci", &github_root.join("gh-fix-ci/SKILL.md")), - &plan - ), - "r0/github/hash123/skills/gh-fix-ci/SKILL.md" - ); - } - - #[test] - fn outcome_rendering_uses_skill_root_for_multiple_skills_in_one_plugin_version() { - let github_root = - test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated/github/hash123/skills") - .abs(); - let fix_ci = skill_with_path("github:gh-fix-ci", &github_root.join("gh-fix-ci/SKILL.md")); - let yeet = skill_with_path("github:yeet", &github_root.join("yeet/SKILL.md")); - let outcome = outcome_with_roots( - vec![fix_ci.clone(), yeet.clone()], - vec![github_root.clone()], - ); - let plan = build_alias_plan( - &outcome, - &[fix_ci, yeet], - SkillMetadataBudget::Characters(usize::MAX), - ) - .expect("alias plan should build"); - - assert_eq!( - plan.aliases.skill_root_lines, - vec![format!("- `r0` = `{}`", normalized_path(&github_root))] - ); - assert_eq!( - render_skill_path_with_aliases( - &skill_with_path("github:gh-fix-ci", &github_root.join("gh-fix-ci/SKILL.md")), - &plan - ), - "r0/gh-fix-ci/SKILL.md" - ); - assert_eq!( - render_skill_path_with_aliases( - &skill_with_path("github:yeet", &github_root.join("yeet/SKILL.md")), - &plan - ), - "r0/yeet/SKILL.md" - ); - } - - #[test] - fn outcome_rendering_counts_plugin_version_skills_before_budget_omission() { - let root = test_path_buf( - "/Users/xl/.codex/plugins/cache/openai-curated/example/hash1234567890/skills-with-a-very-long-shared-prefix", - ) - .abs(); - let alpha = skill_with_path("alpha-skill", &root.join("alpha/SKILL.md")); - let beta = skill_with_path("beta-skill", &root.join("beta/SKILL.md")); - let outcome = outcome_with_roots(vec![alpha.clone(), beta.clone()], vec![root.clone()]); - let plan = build_alias_plan( - &outcome, - &[alpha.clone(), beta.clone()], - SkillMetadataBudget::Characters(usize::MAX), - ) - .expect("alias plan should build"); - let alpha_cost = SkillMetadataBudget::Characters(usize::MAX).cost(&format!( - "{}\n", - SkillLine::with_path(&alpha, render_skill_path_with_aliases(&alpha, &plan)) - .render_minimum() - )); - let rendered = build_aliased_available_skills( - &outcome, - &[alpha, beta], - SkillMetadataBudget::Characters(plan.table_cost + alpha_cost), - ) - .expect("skills should render"); - - assert_eq!(rendered.report.included_count, 1); - assert_eq!( - rendered.skill_root_lines, - vec![format!("- `r0` = `{}`", normalized_path(&root))] - ); - assert_eq!( - rendered.skill_lines, - vec!["- alpha-skill: (file: r0/alpha/SKILL.md)"] - ); - } - - #[test] - fn outcome_rendering_uses_each_skill_root_for_multiple_roots_in_one_plugin_version() { - let skills_root = - test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated/github/hash123/skills") - .abs(); - let extra_root = test_path_buf( - "/Users/xl/.codex/plugins/cache/openai-curated/github/hash123/extra-skills", - ) - .abs(); - let fix_ci = skill_with_path("github:gh-fix-ci", &skills_root.join("gh-fix-ci/SKILL.md")); - let yeet = skill_with_path("github:yeet", &extra_root.join("yeet/SKILL.md")); - let outcome = outcome_with_roots( - vec![fix_ci.clone(), yeet.clone()], - vec![skills_root.clone(), extra_root.clone()], - ); - let plan = build_alias_plan( - &outcome, - &[fix_ci, yeet], - SkillMetadataBudget::Characters(usize::MAX), - ) - .expect("alias plan should build"); - - assert_eq!( - plan.aliases.skill_root_lines, - vec![ - format!("- `r0` = `{}`", normalized_path(&skills_root)), - format!("- `r1` = `{}`", normalized_path(&extra_root)), - ] - ); - assert_eq!( - render_skill_path_with_aliases( - &skill_with_path("github:gh-fix-ci", &skills_root.join("gh-fix-ci/SKILL.md")), - &plan - ), - "r0/gh-fix-ci/SKILL.md" - ); - assert_eq!( - render_skill_path_with_aliases( - &skill_with_path("github:yeet", &extra_root.join("yeet/SKILL.md")), - &plan - ), - "r1/yeet/SKILL.md" - ); - } - - #[test] - fn outcome_rendering_extracts_plugin_marketplace_root_for_multiple_plugins() { - let github_root = - test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated/github/hash123/skills") - .abs(); - let slack_root = - test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated/slack/hash456/skills") - .abs(); - let marketplace_root = test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated").abs(); - let github = skill_with_path("github:gh-fix-ci", &github_root.join("gh-fix-ci/SKILL.md")); - let slack = skill_with_path( - "slack:daily-digest", - &slack_root.join("daily-digest/SKILL.md"), - ); - let outcome = outcome_with_roots( - vec![github.clone(), slack.clone()], - vec![github_root.clone(), slack_root.clone()], - ); - let plan = build_alias_plan( - &outcome, - &[github, slack], - SkillMetadataBudget::Characters(usize::MAX), - ) - .expect("alias plan should build"); - - assert_eq!( - plan.aliases.skill_root_lines, - vec![format!("- `r0` = `{}`", normalized_path(&marketplace_root))] - ); - assert_eq!( - render_skill_path_with_aliases( - &skill_with_path("github:gh-fix-ci", &github_root.join("gh-fix-ci/SKILL.md")), - &plan - ), - "r0/github/hash123/skills/gh-fix-ci/SKILL.md" - ); - assert_eq!( - render_skill_path_with_aliases( - &skill_with_path( - "slack:daily-digest", - &slack_root.join("daily-digest/SKILL.md") - ), - &plan - ), - "r0/slack/hash456/skills/daily-digest/SKILL.md" - ); - } - - #[test] - fn outcome_rendering_uses_one_marketplace_root_for_multiple_plugin_versions() { - let skills_root = - test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated/github/hash123/skills") - .abs(); - let extra_root = test_path_buf( - "/Users/xl/.codex/plugins/cache/openai-curated/github/hash456/extra-skills", - ) - .abs(); - let marketplace_root = test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated").abs(); - let fix_ci = skill_with_path("github:gh-fix-ci", &skills_root.join("gh-fix-ci/SKILL.md")); - let yeet = skill_with_path("github:yeet", &extra_root.join("yeet/SKILL.md")); - let outcome = outcome_with_roots( - vec![fix_ci.clone(), yeet.clone()], - vec![skills_root.clone(), extra_root.clone()], - ); - let plan = build_alias_plan( - &outcome, - &[fix_ci, yeet], - SkillMetadataBudget::Characters(usize::MAX), - ) - .expect("alias plan should build"); - - assert_eq!( - plan.aliases.skill_root_lines, - vec![format!("- `r0` = `{}`", normalized_path(&marketplace_root))] - ); - assert_eq!( - render_skill_path_with_aliases( - &skill_with_path("github:gh-fix-ci", &skills_root.join("gh-fix-ci/SKILL.md")), - &plan - ), - "r0/github/hash123/skills/gh-fix-ci/SKILL.md" - ); - assert_eq!( - render_skill_path_with_aliases( - &skill_with_path("github:yeet", &extra_root.join("yeet/SKILL.md")), - &plan - ), - "r0/github/hash456/extra-skills/yeet/SKILL.md" - ); - } - - fn skill_with_path(name: &str, path: &AbsolutePathBuf) -> SkillMetadata { - let mut skill = make_skill(name, SkillScope::User); - skill.path_to_skills_md = path.clone(); - skill - } -} diff --git a/codex-rs/core/src/context/available_skills_instructions.rs b/codex-rs/core/src/context/available_skills_instructions.rs deleted file mode 100644 index a96f2195cc..0000000000 --- a/codex-rs/core/src/context/available_skills_instructions.rs +++ /dev/null @@ -1,63 +0,0 @@ -use codex_core_skills::AvailableSkills; -use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS; -use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ALIASES; -use codex_core_skills::render_available_skills_body; -use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; -use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; - -use super::ContextualUserFragment; - -/// Model-context fragment describing the skills available to Codex. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct AvailableSkillsInstructions { - skill_root_lines: Vec, - skill_lines: Vec, -} - -impl AvailableSkillsInstructions { - /// Creates a skills context fragment from pre-rendered catalog lines. - pub fn from_skill_lines(skill_lines: Vec) -> Self { - Self { - skill_root_lines: Vec::new(), - skill_lines, - } - } - - pub fn from_available_skills( - available_skills: &AvailableSkills, - include_skills_usage_instructions: bool, - ) -> Self { - let mut skill_lines = available_skills.skill_lines.clone(); - if include_skills_usage_instructions { - skill_lines.push("### How to use skills".to_string()); - let instructions = if available_skills.skill_root_lines.is_empty() { - SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS - } else { - SKILLS_HOW_TO_USE_WITH_ALIASES - }; - skill_lines.push(instructions.to_string()); - } - Self { - skill_root_lines: available_skills.skill_root_lines.clone(), - skill_lines, - } - } -} - -impl ContextualUserFragment for AvailableSkillsInstructions { - fn role(&self) -> &'static str { - "developer" - } - - fn markers(&self) -> (&'static str, &'static str) { - Self::type_markers() - } - - fn type_markers() -> (&'static str, &'static str) { - (SKILLS_INSTRUCTIONS_OPEN_TAG, SKILLS_INSTRUCTIONS_CLOSE_TAG) - } - - fn body(&self) -> String { - render_available_skills_body(&self.skill_root_lines, &self.skill_lines) - } -} diff --git a/codex-rs/core/src/context/mod.rs b/codex-rs/core/src/context/mod.rs index fc52f93217..d8ef85e3e5 100644 --- a/codex-rs/core/src/context/mod.rs +++ b/codex-rs/core/src/context/mod.rs @@ -3,7 +3,6 @@ mod approved_command_prefix_saved; mod apps_instructions; mod available_plugins_instructions; -mod available_skills_instructions; mod contextual_user_message; mod current_time_reminder; mod environment_context; @@ -38,7 +37,6 @@ pub(crate) mod world_state; pub(crate) use approved_command_prefix_saved::ApprovedCommandPrefixSaved; pub(crate) use apps_instructions::AppsInstructions; pub(crate) use available_plugins_instructions::AvailablePluginsInstructions; -pub use available_skills_instructions::AvailableSkillsInstructions; pub(crate) use codex_context_fragments::AdditionalContextDeveloperFragment; pub(crate) use codex_context_fragments::AdditionalContextUserFragment; pub use codex_context_fragments::ContextualUserFragment; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index e37c17ca5d..93d861169b 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -89,11 +89,9 @@ pub mod skills; pub(crate) use skills::SkillInjections; pub(crate) use skills::SkillMetadata; pub(crate) use skills::SkillsService; -pub(crate) use skills::build_available_skills; pub(crate) use skills::build_skill_injections; pub(crate) use skills::build_skill_name_counts; 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::maybe_emit_implicit_skill_invocation; pub(crate) use skills::skills_load_input_from_config; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 5b73965148..cb25b5706b 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -18,20 +18,17 @@ use crate::agent_communication::AgentCommunicationContext; use crate::agent_communication::AgentCommunicationKind; use crate::attestation::AttestationProvider; use crate::audio_preparation::prepare_response_items as prepare_audio_response_items; -use crate::build_available_skills; use crate::compact; use crate::compact::CompactedHistoryMetadata; use crate::config::ManagedFeatures; use crate::config::resolve_tool_suggest_config_from_layer_stack; use crate::context::ApprovedCommandPrefixSaved; -use crate::context::AvailableSkillsInstructions; use crate::context::ContextualUserFragment; use crate::context::ModelSwitchInstructions; use crate::context::NetworkRuleSaved; use crate::context::RecommendedPluginsInstructions; use crate::context::world_state::WorldState; use crate::current_time::TimeProvider; -use crate::default_skill_metadata_budget; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::exec_policy::BANNED_PREFIX_SUGGESTIONS; use crate::exec_policy::ExecPolicyManager; @@ -42,7 +39,6 @@ use crate::realtime_conversation::RealtimeConversationManager; use crate::session::step_context::StepContext; use crate::session::turn_context::TurnEnvironment; use crate::session_prefix::format_inter_agent_completion_message; -use crate::skills::SkillRenderSideEffects; use crate::skills_load_input_from_config; use crate::turn_metadata::TurnMetadataState; use crate::turn_timing::now_unix_timestamp_ms; @@ -55,7 +51,6 @@ use codex_analytics::SubAgentThreadStartedInput; use codex_analytics::TurnCodexErrorFact; use codex_async_utils::OrCancelExt; use codex_connectors::connector_runtime_context_key; -use codex_core_skills::injection::HostSkillsCatalogInWorldState; use codex_exec_server::Environment; use codex_exec_server::EnvironmentManager; use codex_execpolicy::prefix_rule_migration; @@ -304,8 +299,6 @@ pub(crate) struct PreviousTurnSettings { pub(crate) realtime_active: Option, } -#[cfg(test)] -use crate::SkillMetadata; use crate::SkillsService; use crate::exec_policy::ExecPolicyUpdateError; use crate::guardian::GuardianReviewOptions; @@ -315,8 +308,6 @@ use crate::network_policy_decision::execpolicy_network_rule_amendment; use crate::rollout::map_session_init_error; use crate::session_startup_prewarm::SessionStartupPrewarmHandle; use crate::shell; -#[cfg(test)] -use crate::skills::SkillLoadOutcome; use crate::state::AutoCompactWindowIds; use crate::state::AutoCompactWindowSnapshot; use crate::state::PendingRequestPermissions; @@ -3410,37 +3401,6 @@ impl Session { { developer_sections.push(developer_instructions.to_string()); } - if turn_context.config.include_skill_instructions - && turn_context - .extension_data - .get::() - .is_none() - { - let available_skills = build_available_skills( - turn_context.turn_skills.snapshot.outcome(), - default_skill_metadata_budget(turn_context.model_info.context_window), - SkillRenderSideEffects::ThreadStart { - session_telemetry: &self.services.session_telemetry, - }, - ); - if let Some(available_skills) = available_skills { - let warning_message = available_skills.warning_message.clone(); - let skills_instructions = AvailableSkillsInstructions::from_available_skills( - &available_skills, - turn_context.model_info.include_skills_usage_instructions, - ); - if let Some(warning_message) = warning_message { - self.send_event_raw(Event { - id: String::new(), - msg: EventMsg::Warning(WarningEvent { - message: warning_message, - }), - }) - .await; - } - developer_sections.push(skills_instructions.render()); - } - } let loaded_plugins = self .services .plugins_manager diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index c4109088ce..83e037dd4c 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -14,8 +14,6 @@ use crate::function_tool::FunctionCallError; use crate::session::step_context::StepContext; use crate::shell::default_user_shell; use crate::shell_snapshot::ShellSnapshot; -use crate::skills::SkillRenderSideEffects; -use crate::skills::render::SkillMetadataBudget; use crate::test_support::models_manager_with_provider; use crate::tools::format_exec_output_str; use crate::tools::registry::CoreToolRuntime; @@ -33,7 +31,6 @@ use codex_config::loader::project_trust_key; use codex_config::types::McpServerConfig; use codex_config::types::McpServerTransportConfig; use codex_config::types::ToolSuggestDisabledTool; -use codex_core_skills::HostSkillsSnapshot; use core_test_support::test_codex::local_selections; use codex_features::Feature; @@ -109,10 +106,6 @@ use codex_execpolicy::Policy; use codex_network_proxy::NetworkProxyConfig; use codex_otel::MetricsClient; use codex_otel::MetricsConfig; -use codex_otel::THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC; -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; @@ -143,7 +136,6 @@ use codex_protocol::protocol::ResumedHistory; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::SessionMeta; use codex_protocol::protocol::SessionMetaLine; -use codex_protocol::protocol::SkillScope; use codex_protocol::protocol::Submission; use codex_protocol::protocol::ThreadRolledBackEvent; use codex_protocol::protocol::ThreadSettingsOverrides; @@ -318,28 +310,6 @@ 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.4", - "gpt-5.4", - /*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() { @@ -351,21 +321,6 @@ fn find_metric<'a>(resource_metrics: &'a ResourceMetrics, name: &str) -> &'a Met 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 single_histogram_attributes( resource_metrics: &ResourceMetrics, name: &str, @@ -9168,220 +9123,6 @@ async fn build_initial_context_omits_multi_agent_v2_usage_hints_when_hint_is_emp ); } -#[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: test_path_buf("/tmp/admin-skill/SKILL.md").abs(), - scope: SkillScope::Admin, - plugin_id: None, - remote_plugin_id: None, - }, - SkillMetadata { - name: "repo-skill".to_string(), - description: "desc".to_string(), - short_description: None, - interface: None, - dependencies: None, - policy: None, - path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), - scope: SkillScope::Repo, - plugin_id: None, - remote_plugin_id: None, - }, - ]; - turn_context.model_info.context_window = Some(100); - turn_context.turn_skills = TurnSkillsContext::new(HostSkillsSnapshot::new(Arc::new(outcome))); - let turn_context = Arc::new(turn_context); - - let initial_context = build_initial_context(&session, &turn_context).await; - let developer_texts = developer_input_texts(&initial_context); - - assert!( - developer_texts - .iter() - .all(|text| !text.contains("Exceeded skills context budget")), - "expected skill budget warning to stay out of the 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:?}" - ); -} - -#[test] -fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() { - let session_telemetry = test_session_telemetry_without_metadata(); - let mut outcome = SkillLoadOutcome::default(); - outcome.skills = vec![SkillMetadata { - name: "repo-skill".to_string(), - description: "desc".to_string(), - short_description: None, - interface: None, - dependencies: None, - policy: None, - path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), - scope: SkillScope::Repo, - plugin_id: None, - remote_plugin_id: None, - }]; - let rendered = build_available_skills( - &outcome, - SkillMetadataBudget::Characters(1), - SkillRenderSideEffects::ThreadStart { - session_telemetry: &session_telemetry, - }, - ) - .expect("skills should render"); - - assert_eq!( - rendered.warning_message, - Some( - "Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list." - .to_string() - ) - ); - let snapshot = session_telemetry - .snapshot_metrics() - .expect("runtime metrics snapshot"); - assert_eq!( - histogram_sum(&snapshot, THREAD_SKILLS_ENABLED_TOTAL_METRIC), - 1 - ); - assert_eq!(histogram_sum(&snapshot, THREAD_SKILLS_KEPT_TOTAL_METRIC), 0); - assert_eq!(histogram_sum(&snapshot, THREAD_SKILLS_TRUNCATED_METRIC), 1); - assert_eq!( - histogram_sum(&snapshot, THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC), - 4 - ); -} - -#[test] -fn emit_thread_start_skill_metrics_records_description_truncated_chars_without_omitted_skills() { - let session_telemetry = test_session_telemetry_without_metadata(); - let alpha = SkillMetadata { - name: "alpha-skill".to_string(), - description: "abcdef".to_string(), - short_description: None, - interface: None, - dependencies: None, - policy: None, - path_to_skills_md: test_path_buf("/tmp/alpha-skill/SKILL.md").abs(), - scope: SkillScope::Repo, - plugin_id: None, - remote_plugin_id: None, - }; - let beta = SkillMetadata { - name: "beta-skill".to_string(), - description: "uvwxyz".to_string(), - short_description: None, - interface: None, - dependencies: None, - policy: None, - path_to_skills_md: test_path_buf("/tmp/beta-skill/SKILL.md").abs(), - scope: SkillScope::Repo, - plugin_id: None, - remote_plugin_id: None, - }; - let minimum_skill_line_cost = |skill: &SkillMetadata| { - let path = skill.path_to_skills_md.to_string_lossy().replace('\\', "/"); - format!("- {}: (file: {})\n", skill.name, path) - .chars() - .count() - }; - let minimum_budget = minimum_skill_line_cost(&alpha) + minimum_skill_line_cost(&beta); - let mut outcome = SkillLoadOutcome::default(); - outcome.skills = vec![alpha, beta]; - - let rendered = build_available_skills( - &outcome, - SkillMetadataBudget::Characters(minimum_budget + 6), - SkillRenderSideEffects::ThreadStart { - session_telemetry: &session_telemetry, - }, - ) - .expect("skills should render"); - - assert_eq!(rendered.report.omitted_count, 0); - assert_eq!(rendered.report.truncated_description_chars, 8); - let snapshot = session_telemetry - .snapshot_metrics() - .expect("runtime metrics snapshot"); - assert_eq!(histogram_sum(&snapshot, THREAD_SKILLS_TRUNCATED_METRIC), 0); - assert_eq!( - histogram_sum(&snapshot, THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC), - 8 - ); -} - -#[tokio::test] -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 thread settings 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: test_path_buf("/tmp/admin-skill/SKILL.md").abs(), - scope: SkillScope::Admin, - plugin_id: None, - remote_plugin_id: None, - }, - SkillMetadata { - name: "repo-skill".to_string(), - description: "desc".to_string(), - short_description: None, - interface: None, - dependencies: None, - policy: None, - path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), - scope: SkillScope::Repo, - plugin_id: None, - remote_plugin_id: None, - }, - ]; - turn_context.model_info.context_window = Some(100); - turn_context.turn_skills = TurnSkillsContext::new(HostSkillsSnapshot::new(Arc::new(outcome))); - let turn_context = Arc::new(turn_context); - - let _ = build_initial_context(&session, &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 == "Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list." - )); - - let _ = build_initial_context(&session, &turn_context).await; - 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 == "Exceeded skills context budget of 2%. All skill descriptions were removed and 2 additional skills were not included in the model-visible skills list." - )); -} - #[tokio::test] async fn build_initial_context_restates_realtime_start_when_reference_context_is_missing() { let (session, mut turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index 8c68ed5e42..684d4c9294 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -13,13 +13,10 @@ use codex_utils_plugins::PluginSkillRoot; pub use codex_core_skills::SkillError; pub use codex_core_skills::SkillLoadOutcome; -pub use codex_core_skills::SkillRenderReport; pub use codex_core_skills::SkillsLoadInput; pub use codex_core_skills::SkillsService; -pub use codex_core_skills::build_available_skills; pub use codex_core_skills::build_skill_name_counts; 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; @@ -29,8 +26,6 @@ pub use codex_core_skills::injection::collect_explicit_skill_mentions; pub use codex_core_skills::loader; 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::service; pub use codex_core_skills::system; pub use codex_skills::SkillMetadata; diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 895f45b732..345ca5ff4d 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1,4 +1,3 @@ -use codex_config::ConfigLayerStack; use codex_config::types::AuthCredentialsStoreMode; use codex_core::ModelClient; use codex_core::NewThread; @@ -54,7 +53,6 @@ use codex_protocol::protocol::SessionMeta; use codex_protocol::protocol::SessionMetaLine; use codex_protocol::protocol::SessionSource; use codex_protocol::user_input::UserInput; -use core_test_support::PathBufExt; use core_test_support::TestCodexResponsesRequestKind; use core_test_support::apps_test_server::AppsTestServer; use core_test_support::load_default_config_for_test; @@ -79,7 +77,6 @@ use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::local_selections; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; -use dunce::canonicalize as normalize_path; use futures::StreamExt; use pretty_assertions::assert_eq; use serde_json::json; @@ -87,7 +84,6 @@ use std::io::Write; use std::num::NonZeroU64; use std::sync::Arc; use tempfile::TempDir; -use toml::toml; use uuid::Uuid; use wiremock::Mock; use wiremock::MockServer; @@ -2204,172 +2200,6 @@ async fn omits_environment_context_when_configured_off() { ); } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn skills_append_to_developer_message() { - skip_if_no_network!(); - let server = MockServer::start().await; - - let resp_mock = mount_sse_once( - &server, - sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), - ) - .await; - - let codex_home = Arc::new(TempDir::new().unwrap()); - let skill_dir = codex_home.path().join("skills/demo"); - std::fs::create_dir_all(&skill_dir).expect("create skill dir"); - std::fs::write( - skill_dir.join("SKILL.md"), - "---\nname: demo\ndescription: build charts\n---\n\n# body\n", - ) - .expect("write skill"); - - let codex_home_path = codex_home.path().to_path_buf(); - let mut builder = test_codex() - .with_home(codex_home.clone()) - .with_auth(CodexAuth::from_api_key("Test API Key")) - .with_config(move |config| { - config.cwd = codex_home_path.abs(); - }); - let codex = builder - .build(&server) - .await - .expect("create new conversation") - .codex; - - codex - .submit(Op::UserInput { - items: vec![UserInput::Text { - text: "hello".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - responsesapi_client_metadata: None, - additional_context: Default::default(), - thread_settings: Default::default(), - }) - .await - .unwrap(); - - wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - - let request = resp_mock.single_request(); - let developer_messages = request.message_input_texts("developer"); - let developer_text = developer_messages.join("\n\n"); - assert!( - developer_text.contains("## Skills"), - "expected skills section present: {developer_messages:?}" - ); - assert!( - developer_text.contains("demo: build charts"), - "expected skill summary: {developer_messages:?}" - ); - assert!( - developer_text.find("demo: build charts") - < developer_text.find(""), - "host skills should precede permissions: {developer_messages:?}" - ); - let expected_path = normalize_path(skill_dir.join("SKILL.md")).unwrap(); - let expected_path_str = expected_path.to_string_lossy().replace('\\', "/"); - assert!( - developer_text.contains(&expected_path_str), - "expected path {expected_path_str} in developer message: {developer_messages:?}" - ); - let _codex_home_guard = codex_home; -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn skills_use_aliases_in_developer_message_under_budget_pressure() { - skip_if_no_network!(); - let server = MockServer::start().await; - - let resp_mock = mount_sse_once( - &server, - sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), - ) - .await; - - let codex_home_parent = TempDir::new().unwrap(); - let long_home_parent = codex_home_parent - .path() - .join("codex-home-with-long-shared-prefix-for-skill-alias-budget-test"); - std::fs::create_dir_all(&long_home_parent).expect("create long home parent"); - let codex_home = Arc::new(TempDir::new_in(long_home_parent).unwrap()); - let skill_root = codex_home.path().join("skills"); - for index in 0..12 { - let skill_dir = skill_root.join(format!("s{index:02}")); - std::fs::create_dir_all(&skill_dir).expect("create skill dir"); - std::fs::write( - skill_dir.join("SKILL.md"), - format!("---\nname: s{index:02}\ndescription: d\n---\n\n# body\n"), - ) - .expect("write skill"); - } - - let codex_home_path = codex_home.path().to_path_buf(); - let mut builder = test_codex() - .with_home(codex_home.clone()) - .with_auth(CodexAuth::from_api_key("Test API Key")) - .with_config(move |config| { - config.cwd = codex_home_path.abs(); - let user_config_path = codex_home_path.join("config.toml").abs(); - config.config_layer_stack = ConfigLayerStack::default() - .with_user_config( - &user_config_path, - toml! { skills = { bundled = { enabled = false } } }.into(), - ) - .expect("skills user config should be valid"); - config.model_context_window = Some(12_000); - }); - let codex = builder - .build(&server) - .await - .expect("create new conversation") - .codex; - - codex - .submit(Op::UserInput { - items: vec![UserInput::Text { - text: "hello".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - responsesapi_client_metadata: None, - additional_context: Default::default(), - thread_settings: Default::default(), - }) - .await - .unwrap(); - - wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - - let request = resp_mock.single_request(); - let developer_messages = request.message_input_texts("developer"); - let developer_text = developer_messages.join("\n\n"); - let expected_root = normalize_path(skill_root).unwrap(); - let expected_root_str = expected_root.to_string_lossy().replace('\\', "/"); - assert!( - developer_text.contains("### Skill roots"), - "expected aliased skills root section: {developer_messages:?}" - ); - assert!( - developer_text.contains(&format!("- `r0` = `{expected_root_str}`")), - "expected root alias for {expected_root_str}: {developer_messages:?}" - ); - assert!( - developer_text.contains("- s00: d (file: r0/s00/SKILL.md)"), - "expected skill path to use root alias: {developer_messages:?}" - ); - assert!( - developer_text.contains( - "expand the listed short `path` with the matching alias from `### Skill roots`" - ), - "expected alias-specific skill instructions: {developer_messages:?}" - ); - let _codex_home_guard = codex_home; - let _codex_home_parent_guard = codex_home_parent; -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn includes_configured_max_effort_in_request() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/core/tests/suite/model_visible_layout.rs b/codex-rs/core/tests/suite/model_visible_layout.rs index 44b7c9b098..4d63c7ec11 100644 --- a/codex-rs/core/tests/suite/model_visible_layout.rs +++ b/codex-rs/core/tests/suite/model_visible_layout.rs @@ -4,7 +4,10 @@ use std::sync::Arc; use anyhow::Result; use codex_config::types::Personality; +use codex_core::config::Config; use codex_exec_server::LOCAL_ENVIRONMENT_ID; +use codex_extension_api::ExtensionRegistry; +use codex_extension_api::ExtensionRegistryBuilder; use codex_features::Feature; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; @@ -13,6 +16,8 @@ use codex_protocol::protocol::Op; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_protocol::protocol::TurnEnvironmentSelections; use codex_protocol::user_input::UserInput; +use codex_skills_extension::SkillsExtensionConfig; +use codex_skills_extension::install; use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::context_snapshot; @@ -34,6 +39,17 @@ use serde_json::json; const PRETURN_CONTEXT_DIFF_CWD: &str = "PRETURN_CONTEXT_DIFF_CWD"; +fn skills_extensions() -> Arc> { + let mut extensions = ExtensionRegistryBuilder::::new(); + install(&mut extensions, |config: &Config| SkillsExtensionConfig { + include_instructions: config.include_skill_instructions, + bundled_skills_enabled: config.bundled_skills_enabled(), + orchestrator_skills_enabled: config.orchestrator_skills_enabled, + shadow_selection_enabled: config.features.enabled(Feature::SkillSearch), + }); + Arc::new(extensions.build()) +} + fn context_snapshot_options() -> ContextSnapshotOptions { ContextSnapshotOptions::default() .render_mode(ContextSnapshotRenderMode::KindWithTextPrefix { max_chars: 96 }) @@ -162,13 +178,16 @@ async fn snapshot_model_visible_layout_turn_overrides() -> Result<()> { ) .await; - let mut builder = test_codex().with_model("gpt-5.4").with_config(|config| { - config - .features - .enable(Feature::Personality) - .expect("test config should allow feature update"); - config.personality = Some(Personality::Pragmatic); - }); + let mut builder = test_codex() + .with_extensions(skills_extensions()) + .with_model("gpt-5.4") + .with_config(|config| { + config + .features + .enable(Feature::Personality) + .expect("test config should allow feature update"); + config.personality = Some(Personality::Pragmatic); + }); let test = builder.build(&server).await?; let preturn_context_diff_cwd = test.cwd_path().join(PRETURN_CONTEXT_DIFF_CWD); fs::create_dir_all(&preturn_context_diff_cwd)?; @@ -282,7 +301,9 @@ async fn snapshot_model_visible_layout_cwd_change_refreshes_agents() -> Result<( ) .await; - let mut builder = test_codex().with_model("gpt-5.4"); + let mut builder = test_codex() + .with_extensions(skills_extensions()) + .with_model("gpt-5.4"); let test = builder.build(&server).await?; let cwd_one = test.cwd_path().join("agents_one"); let cwd_two = test.cwd_path().join("agents_two"); @@ -396,9 +417,11 @@ async fn snapshot_model_visible_layout_resume_with_personality_change() -> Resul skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let mut initial_builder = test_codex().with_config(|config| { - config.model = Some("gpt-5.2".to_string()); - }); + let mut initial_builder = test_codex() + .with_extensions(skills_extensions()) + .with_config(|config| { + config.model = Some("gpt-5.2".to_string()); + }); let initial = initial_builder.build(&server).await?; let codex = Arc::clone(&initial.codex); let home = initial.home.clone(); @@ -442,14 +465,16 @@ async fn snapshot_model_visible_layout_resume_with_personality_change() -> Resul ) .await; - let mut resume_builder = test_codex().with_config(|config| { - config.model = Some("gpt-5.4".to_string()); - config - .features - .enable(Feature::Personality) - .expect("test config should allow feature update"); - config.personality = Some(Personality::Pragmatic); - }); + let mut resume_builder = test_codex() + .with_extensions(skills_extensions()) + .with_config(|config| { + config.model = Some("gpt-5.4".to_string()); + config + .features + .enable(Feature::Personality) + .expect("test config should allow feature update"); + config.personality = Some(Personality::Pragmatic); + }); let resumed = resume_builder.resume(&server, home, rollout_path).await?; let resume_override_cwd = resumed.cwd_path().join(PRETURN_CONTEXT_DIFF_CWD); fs::create_dir_all(&resume_override_cwd)?; @@ -511,9 +536,11 @@ async fn snapshot_model_visible_layout_resume_override_matches_rollout_model() - skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let mut initial_builder = test_codex().with_config(|config| { - config.model = Some("gpt-5.2".to_string()); - }); + let mut initial_builder = test_codex() + .with_extensions(skills_extensions()) + .with_config(|config| { + config.model = Some("gpt-5.2".to_string()); + }); let initial = initial_builder.build(&server).await?; let codex = Arc::clone(&initial.codex); let home = initial.home.clone(); @@ -557,9 +584,11 @@ async fn snapshot_model_visible_layout_resume_override_matches_rollout_model() - ) .await; - let mut resume_builder = test_codex().with_config(|config| { - config.model = Some("gpt-5.4".to_string()); - }); + let mut resume_builder = test_codex() + .with_extensions(skills_extensions()) + .with_config(|config| { + config.model = Some("gpt-5.4".to_string()); + }); let resumed = resume_builder.resume(&server, home, rollout_path).await?; let resume_override_cwd = resumed.cwd_path().join(PRETURN_CONTEXT_DIFF_CWD); fs::create_dir_all(&resume_override_cwd)?; diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index 10d035a5a9..4eeff249f0 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -6,7 +6,10 @@ use std::time::Duration; use std::time::Instant; use anyhow::Result; +use codex_core::config::Config; use codex_core_plugins::store::PluginStore; +use codex_extension_api::ExtensionRegistry; +use codex_extension_api::ExtensionRegistryBuilder; use codex_features::Feature; use codex_login::CodexAuth; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; @@ -19,6 +22,8 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::user_input::UserInput; +use codex_skills_extension::SkillsExtensionConfig; +use codex_skills_extension::install; use core_test_support::apps_test_server::AppsTestServer; use core_test_support::apps_test_server::SEARCH_CALENDAR_CREATE_TOOL; use core_test_support::responses::ResponseMock; @@ -59,6 +64,17 @@ const PLUGIN_APP_SEARCH_CALL_ID: &str = "plugin-app-search"; const PLUGIN_MCP_SEARCH_CALL_ID: &str = "plugin-mcp-search"; const REMOTE_PLUGIN_CONFIG_NAME: &str = "sample@openai-curated-remote"; +fn skills_extensions() -> Arc> { + let mut extensions = ExtensionRegistryBuilder::::new(); + install(&mut extensions, |config: &Config| SkillsExtensionConfig { + include_instructions: config.include_skill_instructions, + bundled_skills_enabled: config.bundled_skills_enabled(), + orchestrator_skills_enabled: config.orchestrator_skills_enabled, + shadow_selection_enabled: config.features.enabled(Feature::SkillSearch), + }); + Arc::new(extensions.build()) +} + fn sample_plugin_root(home: &TempDir) -> std::path::PathBuf { home.path().join("plugins/cache/test/sample/local") } @@ -408,12 +424,18 @@ async fn capability_sections_render_in_developer_message_in_order() -> Result<() let codex_home = Arc::new(TempDir::new()?); write_plugin_skill_plugin(codex_home.as_ref()); write_plugin_app_plugin(codex_home.as_ref()); - let test_codex = build_apps_enabled_plugin_test_codex( - &server, - Arc::clone(&codex_home), - apps_server.chatgpt_base_url, - ) - .await?; + let mut builder = test_codex() + .with_home(Arc::clone(&codex_home)) + .with_extensions(skills_extensions()) + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(move |config| { + config + .features + .enable(Feature::Apps) + .expect("test config should allow feature update"); + config.chatgpt_base_url = apps_server.chatgpt_base_url; + }); + let test_codex = builder.build(&server).await?; let codex = Arc::clone(&test_codex.codex); codex @@ -645,6 +667,7 @@ enabled = true let mut builder = test_codex() .with_home(Arc::clone(&codex_home)) + .with_extensions(skills_extensions()) .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()); let test_codex = builder.build_with_auto_env(&server).await?; let plugins_manager = test_codex.thread_manager.plugins_manager(); diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__token_budget__token_budget_new_context_window_tool_full_context.snap b/codex-rs/core/tests/suite/snapshots/all__suite__token_budget__token_budget_new_context_window_tool_full_context.snap index 16e7ddac77..648353a1a4 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__token_budget__token_budget_new_context_window_tool_full_context.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__token_budget__token_budget_new_context_window_tool_full_context.snap @@ -6,8 +6,8 @@ Scenario: New context window tool installs fresh full context before the next fo ## Final Follow-Up Request 00:message/developer[3]: - [01] - [02] \nThread id: \nFirst context window id: \nCurrent context window id: \nPrevious context window id: \n + [01] \nThread id: \nFirst context window id: \nCurrent context window id: \nPrevious context window id: \n + [02] [03] 01:message/user:> 02:function_call/update_plan diff --git a/codex-rs/core/tests/suite/token_budget.rs b/codex-rs/core/tests/suite/token_budget.rs index 62f7a699da..0af305f38a 100644 --- a/codex-rs/core/tests/suite/token_budget.rs +++ b/codex-rs/core/tests/suite/token_budget.rs @@ -2,7 +2,9 @@ use anyhow::Result; use codex_config::config_toml::ConfigLockfileToml; use codex_config::types::McpServerConfig; use codex_config::types::McpServerTransportConfig; +use codex_core::config::Config; use codex_core::config::TokenBudgetConfig; +use codex_extension_api::ExtensionRegistryBuilder; use codex_features::Feature; use codex_features::FeatureToml; use codex_features::TokenBudgetConfigToml; @@ -22,6 +24,8 @@ use codex_protocol::protocol::ItemStartedEvent; use codex_protocol::protocol::Op; use codex_protocol::protocol::ThreadSettingsOverrides; use codex_protocol::user_input::UserInput; +use codex_skills_extension::SkillsExtensionConfig; +use codex_skills_extension::install; use core_test_support::PathBufExt; use core_test_support::assert_regex_match; use core_test_support::context_snapshot; @@ -1483,7 +1487,15 @@ async fn new_context_tool_skips_auto_compact_fallback() -> Result<()> { ], ) .await; + let mut extensions = ExtensionRegistryBuilder::::new(); + install(&mut extensions, |config: &Config| SkillsExtensionConfig { + include_instructions: config.include_skill_instructions, + bundled_skills_enabled: config.bundled_skills_enabled(), + orchestrator_skills_enabled: config.orchestrator_skills_enabled, + shadow_selection_enabled: config.features.enabled(Feature::SkillSearch), + }); let test = test_codex() + .with_extensions(Arc::new(extensions.build())) .with_config(|config| { config.model_context_window = Some(10_000); config.token_budget = Some(TokenBudgetConfig { diff --git a/codex-rs/ext/skills/src/catalog_prompt.rs b/codex-rs/ext/skills/src/catalog_prompt.rs new file mode 100644 index 0000000000..7085f0b046 --- /dev/null +++ b/codex-rs/ext/skills/src/catalog_prompt.rs @@ -0,0 +1,55 @@ +pub(crate) 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."; +const SKILLS_INTRO_WITH_ALIASES: &str = "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 a short path that can be expanded into an absolute path using the skill roots table."; +pub(crate) const SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS: &str = r###"- Discovery: The list above is the skills available in this session (name + description + source locator). `file` entries live on the host filesystem, `environment resource` and `orchestrator resource` entries must be accessed through `skills.list` and `skills.read`, and `custom resource` entries use their provider's access mechanism. +- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned. +- Missing/blocked: If a named skill isn't in the list or its source can't be read, say so briefly and continue with the best fallback. +- How to use a skill (progressive disclosure): + 1) After deciding to use a skill, the main agent must read its `SKILL.md` completely before taking task actions. For a `file` entry, open the listed path. For an `environment resource`, call `skills.list` with `{"authority":{"kind":"executor"}}`; for an `orchestrator resource`, use `{"authority":{"kind":"orchestrator"}}`. Select the matching package and pass its exact authority, package, and `main_resource` to `skills.read`. Follow `next_cursor`; if a read is paginated, continue until EOF. + 2) When `SKILL.md` references another resource, use the same access mechanism. Resolve relative references beneath an executor skill's returned package and call `skills.read` with the same authority and package. For orchestrator skills, pass the exact referenced resource identifier with the same authority and package to `skills.read`; do not treat `skill://` identifiers as filesystem paths. + 3) If `SKILL.md` points to extra folders such as `references/`, use its routing instructions to identify the resources required for the task. The main agent must read each required instruction or reference file itself before acting on it. Do not delegate reading, summarizing, or interpreting skill instructions to a subagent. Subagents may still perform task work when the selected skill allows it. + 4) For filesystem-backed skills, prefer running or patching provided scripts instead of retyping large code blocks. For environment and orchestrator skills, use `skills.read` and the available tools; do not invent a local path. + 5) Reuse provided assets or templates through the same source access mechanism instead of recreating them. +- Coordination and sequencing: + - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them. + - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why. +- Context hygiene: + - Progressive disclosure applies to selecting relevant files, not partially reading a selected instruction file. Do not load unrelated references, scripts, or assets. + - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked. + - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice. +- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue."###; +pub(crate) const SKILLS_HOW_TO_USE_WITH_ALIASES: &str = r###"- Discovery: The list above is the skills available in this session (name + description + short path). Skill bodies live on disk at the listed paths after expanding the matching alias from `### Skill roots`. +- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned. +- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback. +- How to use a skill (progressive disclosure): + 1) After deciding to use a skill, the main agent must expand the listed short `path` with the matching alias from `### Skill roots`, then open and read its `SKILL.md` completely before taking task actions. If a read is truncated or paginated, continue until EOF. + 2) When `SKILL.md` references relative paths (e.g., `scripts/foo.py`), resolve them relative to the directory containing that expanded `SKILL.md` first, and only consider other paths if needed. + 3) If `SKILL.md` points to extra folders such as `references/`, use its routing instructions to identify the files required for the task. The main agent must read each required instruction or reference file itself before acting on it. Do not delegate reading, summarizing, or interpreting skill instructions to a subagent. Subagents may still perform task work when the selected skill allows it. + 4) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks. + 5) If `assets/` or templates exist, reuse them instead of recreating from scratch. +- Coordination and sequencing: + - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them. + - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why. +- Context hygiene: + - Progressive disclosure applies to selecting relevant files, not partially reading a selected instruction file. Do not load unrelated references, scripts, or assets. + - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked. + - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice. +- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue."###; + +pub(crate) fn render_available_skills_body( + skill_root_lines: &[String], + skill_lines: &[String], +) -> String { + let mut lines: Vec = Vec::new(); + lines.push("## Skills".to_string()); + if skill_root_lines.is_empty() { + lines.push(SKILLS_INTRO_WITH_ABSOLUTE_PATHS.to_string()); + } else { + lines.push(SKILLS_INTRO_WITH_ALIASES.to_string()); + lines.push("### Skill roots".to_string()); + lines.extend(skill_root_lines.iter().cloned()); + } + lines.push("### Available skills".to_string()); + lines.extend(skill_lines.iter().cloned()); + + format!("\n{}\n", lines.join("\n")) +} diff --git a/codex-rs/ext/skills/src/fragments.rs b/codex-rs/ext/skills/src/fragments.rs index c26d18535b..6251a427f2 100644 --- a/codex-rs/ext/skills/src/fragments.rs +++ b/codex-rs/ext/skills/src/fragments.rs @@ -1,10 +1,11 @@ -use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS; -use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ALIASES; -use codex_core_skills::render_available_skills_body; use codex_extension_api::ContextualUserFragment; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; +use crate::catalog_prompt::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS; +use crate::catalog_prompt::SKILLS_HOW_TO_USE_WITH_ALIASES; +use crate::catalog_prompt::render_available_skills_body; + #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) struct AvailableSkillsInstructions { skill_root_lines: Vec, diff --git a/codex-rs/ext/skills/src/lib.rs b/codex-rs/ext/skills/src/lib.rs index 65955c8d85..3893fd03ed 100644 --- a/codex-rs/ext/skills/src/lib.rs +++ b/codex-rs/ext/skills/src/lib.rs @@ -1,4 +1,5 @@ pub mod catalog; +mod catalog_prompt; mod config; mod dynamic_skill_selector; mod extension; diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index c41f3966e6..7b22230b49 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -5,7 +5,6 @@ use std::path::Component; use std::path::Path; use std::path::PathBuf; -use codex_core_skills::render_available_skills_body; use codex_protocol::protocol::SkillScope; use codex_utils_string::approx_token_count; use codex_utils_string::take_bytes_at_char_boundary; @@ -13,6 +12,7 @@ use codex_utils_string::take_bytes_at_char_boundary; use crate::catalog::SkillCatalog; use crate::catalog::SkillCatalogEntry; use crate::catalog::SkillSourceKind; +use crate::catalog_prompt::render_available_skills_body; use crate::fragments::AvailableSkillsInstructions; const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000; diff --git a/codex-rs/ext/skills/src/render_tests.rs b/codex-rs/ext/skills/src/render_tests.rs index 16c51fcf00..69f0e88843 100644 --- a/codex-rs/ext/skills/src/render_tests.rs +++ b/codex-rs/ext/skills/src/render_tests.rs @@ -8,7 +8,6 @@ use crate::provider::SkillProvider; use codex_core_skills::HostSkillsSnapshot; use codex_core_skills::loader::SkillRoot; use codex_core_skills::loader::load_skills_from_roots; -use codex_core_skills::render_available_skills_body; use codex_exec_server::LOCAL_FS; use codex_extension_api::ContextualUserFragment; use codex_protocol::protocol::SkillScope; @@ -19,6 +18,8 @@ use std::time::SystemTime; use std::time::UNIX_EPOCH; use tokio::sync::Semaphore; +use crate::catalog_prompt::render_available_skills_body; + fn entry(name: &str, description: &str, short_description: Option<&str>) -> SkillCatalogEntry { entry_with_path( name, diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index e2c35896d3..2253983176 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -5,7 +5,6 @@ use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; use codex_core_skills::HostSkillsSnapshot; -use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS; use codex_core_skills::SkillLoadOutcome; use codex_core_skills::injection::InjectedHostSkillPrompts; use codex_core_skills::loader::MAX_CONCURRENT_ROOT_SCANS; @@ -74,6 +73,7 @@ use tokio::sync::Semaphore; type TestResult = Result<(), Box>; static NEXT_CODEX_HOME_ID: AtomicUsize = AtomicUsize::new(0); +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."; const DEMO_SKILL_CONTENTS: &str = "---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n\nUse the demo skill.\n";