mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
skills: render catalog locators by authority (#27591)
## Why Hosted skills introduced by #27388 use opaque `skill://` resource identifiers, but the skills catalog rendered every locator as a `file` and told the model that every skill body lived on disk. That can send the model toward filesystem tools for a resource that must instead be read through its owning authority. The catalog should describe how each source is accessed without changing the underlying discovery or invocation behavior. ## What changed - Render host skills as `file`, executor-owned skills as `environment resource`, orchestrator-owned skills as `orchestrator resource`, and custom-provider skills as `custom resource`. - Update the shared no-alias guidance to describe source locators rather than assuming every skill is stored on the host filesystem. - Direct orchestrator resources through `skills.list` and `skills.read`, and explicitly tell the model not to treat `skill://` identifiers as filesystem paths. - Preserve the existing filesystem and alias behavior for local skills. ## Scope This PR changes only model-visible catalog rendering and guidance. It does not change skill discovery, selection, prompt injection, provider routing, catalog caching or refresh behavior, resource validation, or the `skills.*` tool contract. ## Verification - Extended skills-extension coverage for host-file and executor-resource labels. - Extended the no-executor app-server flow to assert orchestrator-resource wording and non-filesystem guidance.
This commit is contained in:
@@ -2,6 +2,8 @@ use codex_core::context::AvailableSkillsInstructions;
|
||||
use codex_utils_string::take_bytes_at_char_boundary;
|
||||
|
||||
use crate::catalog::SkillCatalog;
|
||||
use crate::catalog::SkillCatalogEntry;
|
||||
use crate::catalog::SkillSourceKind;
|
||||
|
||||
const MAX_AVAILABLE_SKILLS_BYTES: usize = 8_000;
|
||||
const MAX_MAIN_PROMPT_BYTES: usize = 8_000;
|
||||
@@ -24,7 +26,7 @@ pub(crate) fn available_skills_fragment(
|
||||
.short_description
|
||||
.as_deref()
|
||||
.unwrap_or(entry.description.as_str());
|
||||
let line = render_skill_line(entry.name.as_str(), description, entry.rendered_path());
|
||||
let line = render_skill_line(entry, description);
|
||||
let next_bytes = total_bytes.saturating_add(line.len());
|
||||
if next_bytes > MAX_AVAILABLE_SKILLS_BYTES {
|
||||
omitted = omitted.saturating_add(1);
|
||||
@@ -47,11 +49,19 @@ pub(crate) fn available_skills_fragment(
|
||||
Some(AvailableSkillsInstructions::from_skill_lines(skill_lines))
|
||||
}
|
||||
|
||||
fn render_skill_line(name: &str, description: &str, path: &str) -> String {
|
||||
fn render_skill_line(entry: &SkillCatalogEntry, description: &str) -> String {
|
||||
let locator_kind = match &entry.authority.kind {
|
||||
SkillSourceKind::Host => "file",
|
||||
SkillSourceKind::Executor => "environment resource",
|
||||
SkillSourceKind::Orchestrator => "orchestrator resource",
|
||||
SkillSourceKind::Custom(_) => "custom resource",
|
||||
};
|
||||
let name = entry.name.as_str();
|
||||
let path = entry.rendered_path();
|
||||
if description.is_empty() {
|
||||
format!("- {name}: (file: {path})")
|
||||
format!("- {name}: ({locator_kind}: {path})")
|
||||
} else {
|
||||
format!("- {name}: {description} (file: {path})")
|
||||
format!("- {name}: {description} ({locator_kind}: {path})")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -120,6 +120,11 @@ async fn installed_extension_loads_host_skills_from_legacy_roots() -> TestResult
|
||||
assert_eq!("developer", fragments[0].role());
|
||||
assert!(fragments[0].render().contains("demo"));
|
||||
assert!(fragments[0].render().contains(&skill_prompt_path));
|
||||
assert!(
|
||||
fragments[0]
|
||||
.render()
|
||||
.contains(&format!("(file: {skill_prompt_path})"))
|
||||
);
|
||||
assert_eq!("user", fragments[1].role());
|
||||
assert!(fragments[1].render().contains("<name>demo</name>"));
|
||||
assert!(fragments[1].render().contains("# Demo"));
|
||||
@@ -187,6 +192,11 @@ async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_in
|
||||
.starts_with(SKILLS_INSTRUCTIONS_OPEN_TAG)
|
||||
);
|
||||
assert!(prompt_fragments[0].text().contains("lint-fix"));
|
||||
assert!(
|
||||
prompt_fragments[0]
|
||||
.text()
|
||||
.contains("(environment resource: skill://executor/lint-fix/SKILL.md)")
|
||||
);
|
||||
|
||||
let turn_store = ExtensionData::new("turn-1");
|
||||
let fragments = registry.turn_input_contributors()[0]
|
||||
|
||||
Reference in New Issue
Block a user