Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions codex-rs/ext/skills/src/catalog.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use codex_protocol::protocol::SkillScope;
use codex_skills::SkillDependencies;
use codex_utils_path_uri::PathUri;
use std::sync::Arc;
Expand Down Expand Up @@ -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,
Expand All @@ -160,6 +162,7 @@ impl SkillCatalogEntry {
short_description: None,
main_prompt,
display_path: None,
prompt_scope: None,
dependencies: None,
enabled: true,
prompt_visible: true,
Expand All @@ -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
Expand All @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions codex-rs/ext/skills/src/provider/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -127,3 +128,7 @@ fn catalog_entry_from_skill(skill: &SkillMetadata, enabled: bool) -> SkillCatalo

entry
}

#[cfg(test)]
#[path = "host_tests.rs"]
mod tests;
54 changes: 54 additions & 0 deletions codex-rs/ext/skills/src/provider/host_tests.rs
Original file line number Diff line number Diff line change
@@ -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(())
}
28 changes: 27 additions & 1 deletion codex-rs/ext/skills/src/render.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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);
Expand Down
102 changes: 99 additions & 3 deletions codex-rs/ext/skills/src/render_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(),
],
)
);
Expand Down
Loading