github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-21 14:23:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

Match core skill ordering in extension catalogs (#34746)

Browse Source 
## Why

Core-compatible catalog rendering needs to preserve the core renderer's skill
priority, including when metadata budgets omit lower-priority entries.

## What changed

- Carry each host skill's `SkillScope` into its catalog entry.
- Under `CoreCompatible`, order visible skills by system, admin, repo, and user
  scope, then by name and prompt resource ID.
- Preserve insertion order under `ExtensionCompatible`.

## Testing

Add coverage for propagating host skill scope and for both rendering policies.

GitOrigin-RevId: 9594a177edb2166abf1b5719c51a46fcee000600
This commit is contained in:
felixxia-oai
2026-07-22 12:40:16 +00:00
committed by copyberry
parent f21f98936c
commit 2ffe8cd579
5 changed files with 197 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/ext/skills/src/catalog.rs
+12
View File
@@ -1,3 +1,4 @@
use codex_protocol::protocol::SkillScope;
use codex_skills::SkillDependencies;
use codex_utils_path_uri::PathUri;
use std::sync::Arc;
@@ -139,6 +140,7 @@ pub struct SkillCatalogEntry {
pub short_description: Option<String>,
pub main_prompt: SkillResourceId,
pub display_path: Option<String>,
prompt_scope: Option<SkillScope>,
pub dependencies: Option<SkillDependencies>,
pub enabled: bool,
pub prompt_visible: bool,
@@ -160,6 +162,7 @@ impl SkillCatalogEntry {
short_description: None,
main_prompt,
display_path: None,
prompt_scope: None,
dependencies: None,
enabled: true,
prompt_visible: true,
@@ -176,6 +179,11 @@ impl SkillCatalogEntry {
self
}
pub(crate) fn with_prompt_scope(mut self, prompt_scope: SkillScope) -> Self {
self.prompt_scope = Some(prompt_scope);
self
}
pub fn with_dependencies(mut self, dependencies: Option<SkillDependencies>) -> Self {
self.dependencies = dependencies;
self
@@ -196,6 +204,10 @@ impl SkillCatalogEntry {
.as_deref()
.unwrap_or_else(|| self.main_prompt.as_str())
}
pub(crate) fn prompt_scope(&self) -> Option<SkillScope> {
self.prompt_scope
}
}
/// Merged catalog for one turn.
codex-rs/ext/skills/src/provider/host.rs
+5
View File
@@ -116,6 +116,7 @@ fn catalog_entry_from_skill(skill: &SkillMetadata, enabled: bool) -> SkillCatalo
)
.with_short_description(skill.short_description.clone())
.with_display_path(display_path)
.with_prompt_scope(skill.scope)
.with_dependencies(skill.dependencies.clone());
if !enabled {
@@ -127,3 +128,7 @@ fn catalog_entry_from_skill(skill: &SkillMetadata, enabled: bool) -> SkillCatalo
entry
}
#[cfg(test)]
#[path = "host_tests.rs"]
mod tests;
codex-rs/ext/skills/src/provider/host_tests.rs
+54
View File
@@ -0,0 +1,54 @@
use std::sync::Arc;
use std::time::SystemTime;
use std::time::UNIX_EPOCH;
use codex_core_skills::loader::SkillRoot;
use codex_core_skills::loader::load_skills_from_roots;
use codex_exec_server::LOCAL_FS;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use tokio::sync::Semaphore;
use super::catalog_from_outcome;
#[tokio::test]
async fn host_catalog_entries_carry_their_prompt_scope() -> Result<(), Box<dyn std::error::Error>> {
let unique = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos();
let root = std::env::temp_dir().join(format!(
"codex-skills-extension-host-provider-{}-{unique}",
std::process::id()
));
let skill_path = root.join("demo").join("SKILL.md");
std::fs::create_dir_all(
skill_path
.parent()
.ok_or("skill path should have a parent")?,
)?;
std::fs::write(
&skill_path,
"---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n",
)?;
let root = AbsolutePathBuf::try_from(std::fs::canonicalize(root)?)?;
let outcome = load_skills_from_roots(
[SkillRoot {
path: root.clone(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_namespace: None,
plugin_root: None,
}],
/*plugin_skill_snapshots*/ None,
Arc::new(Semaphore::new(1)),
)
.await;
let catalog = catalog_from_outcome(&outcome);
assert_eq!(catalog.entries.len(), 1);
assert_eq!(catalog.entries[0].prompt_scope(), Some(SkillScope::User));
std::fs::remove_dir_all(root.as_path())?;
Ok(())
}
codex-rs/ext/skills/src/render.rs
+27 -1
View File
@@ -1,5 +1,6 @@
use std::borrow::Cow;
use codex_protocol::protocol::SkillScope;
use codex_utils_string::approx_token_count;
use codex_utils_string::take_bytes_at_char_boundary;
@@ -41,6 +42,27 @@ impl SkillCatalogRenderPolicy {
.unwrap_or(entry.description.as_str()),
}
}
fn order_entries(self, entries: &mut [&SkillCatalogEntry]) {
match self {
Self::CoreCompatible => {
let scope_rank = |entry: &SkillCatalogEntry| match entry.prompt_scope() {
Some(SkillScope::System) => 0,
Some(SkillScope::Admin) => 1,
Some(SkillScope::Repo) => 2,
Some(SkillScope::User) => 3,
None => 4,
};
entries.sort_by(|a, b| {
scope_rank(a)
.cmp(&scope_rank(b))
.then_with(|| a.name.cmp(&b.name))
.then_with(|| a.main_prompt.as_str().cmp(b.main_prompt.as_str()))
});
}
Self::ExtensionCompatible => {}
}
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -287,10 +309,14 @@ pub(crate) fn available_skills_fragment(
policy: SkillCatalogRenderPolicy,
budget: SkillMetadataBudget,
) -> Option<AvailableSkillsInstructions> {
let skill_lines = catalog
let mut entries = catalog
.entries
.iter()
.filter(|entry| entry.enabled && entry.prompt_visible)
.collect::<Vec<_>>();
policy.order_entries(&mut entries);
let skill_lines = entries
.iter()
.map(|entry| SkillLine::new(entry, policy))
.collect();
let (mut skill_lines, mut omitted) = render_skill_lines(skill_lines, budget);
codex-rs/ext/skills/src/render_tests.rs
+99 -3
View File
@@ -4,19 +4,115 @@ use crate::catalog::SkillPackageId;
use crate::catalog::SkillResourceId;
use codex_core_skills::render_available_skills_body;
use codex_extension_api::ContextualUserFragment;
use codex_protocol::protocol::SkillScope;
use pretty_assertions::assert_eq;
fn entry(name: &str, description: &str, short_description: Option<&str>) -> SkillCatalogEntry {
entry_with_path(
name,
description,
short_description,
&format!("/skills/{name}/SKILL.md"),
)
}
fn entry_with_path(
name: &str,
description: &str,
short_description: Option<&str>,
path: &str,
) -> SkillCatalogEntry {
SkillCatalogEntry::new(
SkillPackageId(name.to_string()),
SkillPackageId(path.to_string()),
SkillAuthority::new(SkillSourceKind::Host, "host"),
name,
description,
SkillResourceId::new(format!("/skills/{name}/SKILL.md")),
SkillResourceId::new(path),
)
.with_short_description(short_description.map(str::to_string))
}
#[test]
fn ordering_follows_render_policy() {
let catalog = SkillCatalog {
entries: [
("repo-zeta", SkillScope::Repo, "/skills/repo-zeta/SKILL.md"),
(
"user-alpha",
SkillScope::User,
"/skills/user-alpha/SKILL.md",
),
(
"system-zeta",
SkillScope::System,
"/skills/system-zeta/SKILL.md",
),
(
"admin-alpha",
SkillScope::Admin,
"/skills/admin-alpha/SKILL.md",
),
(
"repo-alpha",
SkillScope::Repo,
"/skills/repo-alpha-z/SKILL.md",
),
(
"repo-alpha",
SkillScope::Repo,
"/skills/repo-alpha-a/SKILL.md",
),
]
.into_iter()
.map(|(name, scope, path)| {
entry_with_path(name, "Description.", /*short_description*/ None, path)
.with_prompt_scope(scope)
})
.collect(),
warnings: Vec::new(),
};
let render = |policy| {
available_skills_fragment(
&catalog,
/*include_skills_usage_instructions*/ false,
policy,
SkillMetadataBudget::Characters(usize::MAX),
)
.expect("catalog should render")
.body()
};
assert_eq!(
render(SkillCatalogRenderPolicy::CoreCompatible),
render_available_skills_body(
&[],
&[
"- system-zeta: Description. (file: /skills/system-zeta/SKILL.md)".to_string(),
"- admin-alpha: Description. (file: /skills/admin-alpha/SKILL.md)".to_string(),
"- repo-alpha: Description. (file: /skills/repo-alpha-a/SKILL.md)".to_string(),
"- repo-alpha: Description. (file: /skills/repo-alpha-z/SKILL.md)".to_string(),
"- repo-zeta: Description. (file: /skills/repo-zeta/SKILL.md)".to_string(),
"- user-alpha: Description. (file: /skills/user-alpha/SKILL.md)".to_string(),
],
)
);
assert_eq!(
render(SkillCatalogRenderPolicy::ExtensionCompatible),
render_available_skills_body(
&[],
&[
"- repo-zeta: Description. (file: /skills/repo-zeta/SKILL.md)".to_string(),
"- user-alpha: Description. (file: /skills/user-alpha/SKILL.md)".to_string(),
"- system-zeta: Description. (file: /skills/system-zeta/SKILL.md)".to_string(),
"- admin-alpha: Description. (file: /skills/admin-alpha/SKILL.md)".to_string(),
"- repo-alpha: Description. (file: /skills/repo-alpha-z/SKILL.md)".to_string(),
"- repo-alpha: Description. (file: /skills/repo-alpha-a/SKILL.md)".to_string(),
],
)
);
}
#[test]
fn description_selection_follows_render_policy() {
let catalog = SkillCatalog {
@@ -51,8 +147,8 @@ fn description_selection_follows_render_policy() {
render_available_skills_body(
&[],
&[
"- shortened: full description (file: /skills/shortened/SKILL.md)".to_string(),
"- fallback: fallback description (file: /skills/fallback/SKILL.md)".to_string(),
"- shortened: full description (file: /skills/shortened/SKILL.md)".to_string(),
],
)
);