mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Alias resource-backed skill locators under context pressure (#37489)
## Why Long executor and orchestrator resource identifiers can consume enough of the skills context budget to omit otherwise available skills. ## What changed - Add source-aware root aliases for executor, orchestrator, and host skill catalogs, including catalogs with a single shared root. - Select the best combination of aliased catalogs under the shared metadata budget while accounting for root tables and alias usage instructions. - Explain how to expand shortened `skill://` locators and accept the resulting description-shortening warning in app-server coverage. ## Testing - Add rendering and production-turn coverage for singleton, executor, orchestrator, host, and combined catalog aliases. GitOrigin-RevId: c1bc1f3d371eae9526232674060235272cc28f16
This commit is contained in:
committed by
copyberry
parent
ce22ea9712
commit
ba94150c2a
@@ -410,24 +410,23 @@ stream_max_retries = 0
|
||||
.await?;
|
||||
}
|
||||
if scenario == ExecutorSkillScenario::VisibleWithBudgetWarning {
|
||||
let is_skills_budget_warning = |message: &str| {
|
||||
message.starts_with("Exceeded skills context budget.")
|
||||
|| message.starts_with(
|
||||
"Skill descriptions were shortened to fit the skills context budget.",
|
||||
)
|
||||
};
|
||||
let warning = timeout(READ_TIMEOUT, async {
|
||||
loop {
|
||||
let warning: WarningNotification = app_server.read_notification("warning").await?;
|
||||
if warning
|
||||
.message
|
||||
.starts_with("Exceeded skills context budget.")
|
||||
{
|
||||
if is_skills_budget_warning(&warning.message) {
|
||||
return Ok::<WarningNotification, anyhow::Error>(warning);
|
||||
}
|
||||
}
|
||||
})
|
||||
.await??;
|
||||
assert_eq!(warning.thread_id, Some(thread_id));
|
||||
assert!(
|
||||
warning
|
||||
.message
|
||||
.starts_with("Exceeded skills context budget.")
|
||||
);
|
||||
assert!(is_skills_budget_warning(&warning.message));
|
||||
}
|
||||
timeout(
|
||||
READ_TIMEOUT,
|
||||
|
||||
@@ -835,6 +835,297 @@ async fn explicit_only_orchestrator_skill_is_hidden_but_can_be_invoked() -> Resu
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_aliases_discovered_singleton_orchestrator_root() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
const SKILL_ROOT: &str = "skill://plugin_connector_1p_2330815c823c8191941e5dc465bb899f";
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let apps_server = AppsTestServer::mount(&server).await?;
|
||||
let response = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||
)
|
||||
.await;
|
||||
|
||||
Mock::given(method("POST"))
|
||||
.and(path_regex("^/api/codex/ps/mcp/?$"))
|
||||
.and(|request: &Request| {
|
||||
serde_json::from_slice::<Value>(&request.body)
|
||||
.is_ok_and(|body| body["method"].as_str() == Some("resources/list"))
|
||||
})
|
||||
.respond_with(|request: &Request| {
|
||||
let body: Value = serde_json::from_slice(&request.body)
|
||||
.expect("MCP resource request should be valid JSON");
|
||||
ResponseTemplate::new(/*status*/ 200).set_body_json(json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": body["id"],
|
||||
"result": {
|
||||
"resources": [{
|
||||
"name": "search",
|
||||
"uri": format!("{SKILL_ROOT}/search"),
|
||||
"description": "Search company knowledge.",
|
||||
"mimeType": "mcp/skill",
|
||||
"_meta": {
|
||||
"plugin_name": "demo",
|
||||
"skill_name": "search",
|
||||
"allow_implicit_invocation": true,
|
||||
},
|
||||
}],
|
||||
},
|
||||
}))
|
||||
})
|
||||
.with_priority(/*p*/ 1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let mut extensions = ExtensionRegistryBuilder::new();
|
||||
install_with_providers(
|
||||
&mut extensions,
|
||||
SkillProviders::new()
|
||||
.with_orchestrator_provider(Arc::new(OrchestratorSkillProvider::new())),
|
||||
|config: &Config| SkillsExtensionConfig {
|
||||
include_instructions: config.include_skill_instructions,
|
||||
bundled_skills_enabled: false,
|
||||
orchestrator_skills_enabled: true,
|
||||
shadow_selection_enabled: false,
|
||||
},
|
||||
);
|
||||
let mut builder = apps_enabled_builder(apps_server.chatgpt_base_url)
|
||||
.with_exec_server_url("none")
|
||||
.with_extensions(Arc::new(extensions.build()))
|
||||
.with_model_info_override("gpt-5.5", |model_info| {
|
||||
model_info.context_window = Some(1_000);
|
||||
model_info.max_context_window = None;
|
||||
})
|
||||
.with_config(|config| {
|
||||
config.include_skill_instructions = true;
|
||||
config.orchestrator_skills_enabled = true;
|
||||
});
|
||||
let test = builder.build_with_auto_env(&server).await?;
|
||||
wait_for_mcp_server(&test.codex, CODEX_APPS_MCP_SERVER_NAME).await?;
|
||||
|
||||
test.submit_turn("Inspect the available skills.").await?;
|
||||
|
||||
let developer_text = response
|
||||
.single_request()
|
||||
.message_input_texts("developer")
|
||||
.join("\n");
|
||||
assert!(
|
||||
developer_text.contains(&format!("- `o0` = `{SKILL_ROOT}`")),
|
||||
"model request should include the discovered orchestrator root: {developer_text}"
|
||||
);
|
||||
assert!(
|
||||
developer_text.lines().any(|line| {
|
||||
line.starts_with("- demo:search:")
|
||||
&& line.ends_with("(orchestrator resource: o0/search)")
|
||||
}),
|
||||
"model request should include the aliased orchestrator skill: {developer_text}"
|
||||
);
|
||||
assert!(
|
||||
developer_text.contains("- Root aliases: Expand short locators"),
|
||||
"model request should explain how to expand resource aliases: {developer_text}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_aliases_executor_skill_roots() -> Result<()> {
|
||||
const SKILL_ROOT: &str =
|
||||
"skill://integration-executor/workspace/plugins/cache/executor-plugin/1.0.0/skills";
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let response = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||
)
|
||||
.await;
|
||||
let catalog = SkillCatalog {
|
||||
entries: ["search", "review", "summarize"]
|
||||
.into_iter()
|
||||
.map(|name| {
|
||||
let resource = format!("{SKILL_ROOT}/{name}/SKILL.md");
|
||||
SkillCatalogEntry::new(
|
||||
SkillPackageId(format!("{SKILL_ROOT}/{name}")),
|
||||
SkillAuthority::new(SkillSourceKind::Executor, "integration-executor"),
|
||||
name,
|
||||
"Inspect executor resources.",
|
||||
SkillResourceId::new(resource.clone()),
|
||||
)
|
||||
.with_display_path(resource)
|
||||
.with_alias_root(SKILL_ROOT)
|
||||
})
|
||||
.collect(),
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
let (extensions, _) = catalog_extensions(catalog, /*include_host_provider*/ false);
|
||||
let mut builder = test_codex()
|
||||
.with_extensions(extensions)
|
||||
.with_model_info_override("gpt-5.6-sol", |model_info| {
|
||||
model_info.context_window = Some(3_000);
|
||||
model_info.max_context_window = None;
|
||||
})
|
||||
.with_config(configure_catalog_test);
|
||||
let test = builder.build_with_auto_env(&server).await?;
|
||||
|
||||
test.submit_turn("Inspect the available skills.").await?;
|
||||
|
||||
let developer_text = response
|
||||
.single_request()
|
||||
.message_input_texts("developer")
|
||||
.join("\n");
|
||||
assert!(
|
||||
developer_text.contains(&format!("- `e0` = `{SKILL_ROOT}`")),
|
||||
"model request should include the executor skill root: {developer_text}"
|
||||
);
|
||||
assert!(
|
||||
developer_text.contains("environment resource: e0/search/SKILL.md"),
|
||||
"model request should include the aliased executor skill: {developer_text}"
|
||||
);
|
||||
let executor_catalog = developer_text
|
||||
.split("<skills_instructions>")
|
||||
.filter_map(|fragment| fragment.split_once("</skills_instructions>"))
|
||||
.map(|(fragment, _)| fragment)
|
||||
.find(|fragment| fragment.contains("environment resource: e0/search/SKILL.md"))
|
||||
.expect("model request should include an executor skills catalog");
|
||||
assert!(
|
||||
!executor_catalog.contains("### How to use skills"),
|
||||
"Sol should omit optional skill usage instructions: {executor_catalog}"
|
||||
);
|
||||
assert!(
|
||||
!executor_catalog.contains("- Root aliases: Expand short locators"),
|
||||
"Sol should not include optional resource-alias instructions: {executor_catalog}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_aliases_combined_skill_catalogs_under_shared_budget() -> Result<()> {
|
||||
const EXECUTOR_ROOT: &str =
|
||||
"skill://integration-executor/workspace/plugins/cache/executor-plugin/1.0.0/skills";
|
||||
const ORCHESTRATOR_ROOT: &str = "skill://plugin_connector_1p_2330815c823c8191941e5dc465bb899f";
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let response = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||
)
|
||||
.await;
|
||||
let codex_home = Arc::new(
|
||||
tempfile::Builder::new()
|
||||
.prefix("codex-integration-shared-skill-catalog-roots-")
|
||||
.tempdir()?,
|
||||
);
|
||||
write_host_skills(
|
||||
codex_home.path(),
|
||||
&[
|
||||
("host-search", "Inspect host resources."),
|
||||
("host-review", "Review host resources."),
|
||||
("host-summarize", "Summarize host resources."),
|
||||
],
|
||||
)?;
|
||||
|
||||
let resource_catalog =
|
||||
|kind: SkillSourceKind, authority: &str, root: &str, prefix: &str| SkillCatalog {
|
||||
entries: ["search", "review", "summarize"]
|
||||
.into_iter()
|
||||
.map(|suffix| {
|
||||
let name = format!("{prefix}-{suffix}");
|
||||
let resource = format!("{root}/{name}/SKILL.md");
|
||||
SkillCatalogEntry::new(
|
||||
SkillPackageId(format!("{root}/{name}")),
|
||||
SkillAuthority::new(kind.clone(), authority),
|
||||
name,
|
||||
"Inspect provider resources.",
|
||||
SkillResourceId::new(resource.clone()),
|
||||
)
|
||||
.with_display_path(resource)
|
||||
.with_alias_root(root)
|
||||
})
|
||||
.collect(),
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
|
||||
let mut extensions = ExtensionRegistryBuilder::new();
|
||||
install_with_providers(
|
||||
&mut extensions,
|
||||
SkillProviders::new()
|
||||
.with_executor_provider(Arc::new(CatalogSkillProvider {
|
||||
catalog: resource_catalog(
|
||||
SkillSourceKind::Executor,
|
||||
"integration-executor",
|
||||
EXECUTOR_ROOT,
|
||||
"executor",
|
||||
),
|
||||
}))
|
||||
.with_orchestrator_provider(Arc::new(CatalogSkillProvider {
|
||||
catalog: resource_catalog(
|
||||
SkillSourceKind::Orchestrator,
|
||||
"integration-orchestrator",
|
||||
ORCHESTRATOR_ROOT,
|
||||
"orchestrator",
|
||||
),
|
||||
}))
|
||||
.with_host_provider(Arc::new(HostSkillProvider::new())),
|
||||
|config: &Config| SkillsExtensionConfig {
|
||||
include_instructions: config.include_skill_instructions,
|
||||
bundled_skills_enabled: false,
|
||||
orchestrator_skills_enabled: true,
|
||||
shadow_selection_enabled: false,
|
||||
},
|
||||
);
|
||||
let mut builder = test_codex()
|
||||
.with_home(Arc::clone(&codex_home))
|
||||
.with_exec_server_url("none")
|
||||
.with_extensions(Arc::new(extensions.build()))
|
||||
.with_model_info_override("gpt-5.5", |model_info| {
|
||||
model_info.context_window = Some(10_000);
|
||||
model_info.max_context_window = None;
|
||||
})
|
||||
.with_config(|config| {
|
||||
configure_catalog_test(config);
|
||||
config.orchestrator_skills_enabled = true;
|
||||
});
|
||||
let test = builder.build_with_auto_env(&server).await?;
|
||||
|
||||
test.submit_turn("Inspect the available skills.").await?;
|
||||
|
||||
let developer_text = response
|
||||
.single_request()
|
||||
.message_input_texts("developer")
|
||||
.join("\n");
|
||||
let host_root = dunce::canonicalize(codex_home.path().join("skills"))?
|
||||
.to_string_lossy()
|
||||
.replace('\\', "/");
|
||||
for (alias, root) in [
|
||||
("e0", EXECUTOR_ROOT),
|
||||
("o0", ORCHESTRATOR_ROOT),
|
||||
("r0", host_root.as_str()),
|
||||
] {
|
||||
assert!(
|
||||
developer_text.contains(&format!("- `{alias}` = `{root}`")),
|
||||
"model request should include the {alias} skill root: {developer_text}"
|
||||
);
|
||||
}
|
||||
for (source, alias, prefix) in [
|
||||
("environment resource", "e0", "executor"),
|
||||
("orchestrator resource", "o0", "orchestrator"),
|
||||
("file", "r0", "host"),
|
||||
] {
|
||||
for name in ["search", "review", "summarize"] {
|
||||
assert!(
|
||||
developer_text.contains(&format!("{source}: {alias}/{prefix}-{name}/SKILL.md")),
|
||||
"model request should retain the aliased {prefix}-{name} skill: {developer_text}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn production_turn_scales_extension_catalog_from_resolved_model_window() -> Result<()> {
|
||||
let skill_count = 800;
|
||||
|
||||
@@ -71,3 +71,15 @@ fn rejects_unknown_roots_and_locators_outside_the_root() {
|
||||
fn returns_none_when_no_roots_are_provided() {
|
||||
assert!(AliasPlan::build("r", &[]).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn aliases_roots_used_by_only_one_skill() {
|
||||
let root = "/skills/singleton";
|
||||
let plan = AliasPlan::build("r", &[root]).expect("singleton root should produce an alias");
|
||||
|
||||
assert_eq!(vec!["- `r0` = `/skills/singleton`"], plan.root_lines());
|
||||
assert_eq!(
|
||||
Some("r0/alpha/SKILL.md".to_string()),
|
||||
plan.shorten("/skills/singleton/alpha/SKILL.md")
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
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.
|
||||
use crate::catalog::SkillSourceKind;
|
||||
|
||||
const SKILLS_INTRO_WITH_SOURCE_LOCATORS: &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_HOST_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.";
|
||||
const SKILLS_INTRO_WITH_RESOURCE_ALIASES: &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. Short locators can be expanded using the skill roots table.";
|
||||
const RESOURCE_ALIAS_INSTRUCTIONS: &str = "- Root aliases: Expand short locators using their matching alias from `### Skill roots` into complete `skill://` resource identifiers.";
|
||||
const SKILLS_HOW_TO_USE_WITH_SOURCE_LOCATORS: &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):
|
||||
@@ -17,7 +21,7 @@ pub(crate) const SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS: &str = r###"- Discovery:
|
||||
- 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`.
|
||||
const SKILLS_HOW_TO_USE_WITH_HOST_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):
|
||||
@@ -35,16 +39,54 @@ pub(crate) const SKILLS_HOW_TO_USE_WITH_ALIASES: &str = r###"- Discovery: The li
|
||||
- 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."###;
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
pub(crate) enum SkillPromptKind {
|
||||
Unaliased,
|
||||
HostAliases,
|
||||
ResourceAliases,
|
||||
}
|
||||
|
||||
impl SkillPromptKind {
|
||||
pub(crate) fn for_aliased_source(source: &SkillSourceKind) -> Self {
|
||||
match source {
|
||||
SkillSourceKind::Host => Self::HostAliases,
|
||||
SkillSourceKind::Executor | SkillSourceKind::Orchestrator => Self::ResourceAliases,
|
||||
SkillSourceKind::Custom(_) => Self::Unaliased,
|
||||
}
|
||||
}
|
||||
|
||||
fn intro(self) -> &'static str {
|
||||
match self {
|
||||
Self::Unaliased => SKILLS_INTRO_WITH_SOURCE_LOCATORS,
|
||||
Self::HostAliases => SKILLS_INTRO_WITH_HOST_ALIASES,
|
||||
Self::ResourceAliases => SKILLS_INTRO_WITH_RESOURCE_ALIASES,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn usage_instructions(self) -> &'static str {
|
||||
match self {
|
||||
Self::Unaliased | Self::ResourceAliases => SKILLS_HOW_TO_USE_WITH_SOURCE_LOCATORS,
|
||||
Self::HostAliases => SKILLS_HOW_TO_USE_WITH_HOST_ALIASES,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn alias_instructions(self) -> Option<&'static str> {
|
||||
match self {
|
||||
Self::ResourceAliases => Some(RESOURCE_ALIAS_INSTRUCTIONS),
|
||||
Self::Unaliased | Self::HostAliases => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn render_available_skills_body(
|
||||
prompt_kind: SkillPromptKind,
|
||||
skill_root_lines: &[String],
|
||||
skill_lines: &[String],
|
||||
) -> String {
|
||||
let mut lines: Vec<String> = 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(prompt_kind.intro().to_string());
|
||||
if !skill_root_lines.is_empty() {
|
||||
lines.push("### Skill roots".to_string());
|
||||
lines.extend(skill_root_lines.iter().cloned());
|
||||
}
|
||||
|
||||
@@ -97,7 +97,7 @@ fn render_catalog(
|
||||
catalog_surface,
|
||||
include_skills_usage_instructions,
|
||||
budget,
|
||||
render_available_skills(catalog, policy, budget),
|
||||
render_available_skills(catalog, policy, budget, include_skills_usage_instructions),
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -2,33 +2,33 @@ 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::SkillPromptKind;
|
||||
use crate::catalog_prompt::render_available_skills_body;
|
||||
use crate::tools::SkillToolAuthority;
|
||||
|
||||
#[derive(Clone, Debug, Eq, PartialEq)]
|
||||
pub(crate) struct AvailableSkillsInstructions {
|
||||
prompt_kind: SkillPromptKind,
|
||||
skill_root_lines: Vec<String>,
|
||||
skill_lines: Vec<String>,
|
||||
}
|
||||
|
||||
impl AvailableSkillsInstructions {
|
||||
pub(crate) fn from_skill_lines(
|
||||
prompt_kind: SkillPromptKind,
|
||||
skill_root_lines: Vec<String>,
|
||||
mut skill_lines: Vec<String>,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> Self {
|
||||
if include_skills_usage_instructions {
|
||||
skill_lines.push("### How to use skills".to_string());
|
||||
let instructions = if 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());
|
||||
if let Some(instructions) = prompt_kind.alias_instructions() {
|
||||
skill_lines.push(instructions.to_string());
|
||||
}
|
||||
skill_lines.push(prompt_kind.usage_instructions().to_string());
|
||||
}
|
||||
Self {
|
||||
prompt_kind,
|
||||
skill_root_lines,
|
||||
skill_lines,
|
||||
}
|
||||
@@ -49,7 +49,7 @@ impl ContextualUserFragment for AvailableSkillsInstructions {
|
||||
}
|
||||
|
||||
fn body(&self) -> String {
|
||||
render_available_skills_body(&self.skill_root_lines, &self.skill_lines)
|
||||
render_available_skills_body(self.prompt_kind, &self.skill_root_lines, &self.skill_lines)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@ use crate::aliases::AliasPlan;
|
||||
use crate::catalog::SkillCatalog;
|
||||
use crate::catalog::SkillCatalogEntry;
|
||||
use crate::catalog::SkillSourceKind;
|
||||
use crate::catalog_prompt::SkillPromptKind;
|
||||
use crate::catalog_prompt::render_available_skills_body;
|
||||
use crate::fragments::AvailableSkillsInstructions;
|
||||
use crate::host_aliases::shared_host_alias_roots;
|
||||
@@ -429,12 +430,14 @@ fn allocate_description_chars(
|
||||
}
|
||||
|
||||
struct RenderedCatalog {
|
||||
prompt_kind: SkillPromptKind,
|
||||
skill_root_lines: Vec<String>,
|
||||
skill_lines: Vec<String>,
|
||||
report: SkillRenderReport,
|
||||
}
|
||||
|
||||
pub(crate) struct AvailableSkillsRender {
|
||||
prompt_kind: SkillPromptKind,
|
||||
skill_root_lines: Vec<String>,
|
||||
skill_lines: Vec<String>,
|
||||
preserve_empty_fragment: bool,
|
||||
@@ -455,6 +458,7 @@ impl AvailableSkillsRender {
|
||||
) -> Option<AvailableSkillsInstructions> {
|
||||
(self.preserve_empty_fragment || !self.skill_lines.is_empty()).then(|| {
|
||||
AvailableSkillsInstructions::from_skill_lines(
|
||||
self.prompt_kind,
|
||||
self.skill_root_lines,
|
||||
self.skill_lines,
|
||||
include_skills_usage_instructions,
|
||||
@@ -472,6 +476,7 @@ pub(crate) fn render_available_skills(
|
||||
catalog: &SkillCatalog,
|
||||
policy: SkillCatalogRenderPolicy,
|
||||
budget: SkillMetadataBudget,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> Option<AvailableSkillsRender> {
|
||||
let mut entries = catalog
|
||||
.entries
|
||||
@@ -490,13 +495,20 @@ pub(crate) fn render_available_skills(
|
||||
.collect(),
|
||||
budget,
|
||||
Vec::new(),
|
||||
SkillPromptKind::Unaliased,
|
||||
policy,
|
||||
);
|
||||
let selected =
|
||||
if absolute.report.omitted_count == 0 && absolute.report.truncated_description_chars == 0 {
|
||||
absolute
|
||||
} else if let Some(aliased) = build_aliased_catalog(&entries, policy, budget)
|
||||
&& aliased_render_is_better(&aliased, &absolute, budget)
|
||||
} else if let Some(aliased) =
|
||||
build_aliased_catalog(&entries, policy, budget, include_skills_usage_instructions)
|
||||
&& aliased_render_is_better(
|
||||
&aliased,
|
||||
&absolute,
|
||||
budget,
|
||||
include_skills_usage_instructions,
|
||||
)
|
||||
{
|
||||
aliased
|
||||
} else {
|
||||
@@ -504,6 +516,7 @@ pub(crate) fn render_available_skills(
|
||||
};
|
||||
|
||||
Some(AvailableSkillsRender {
|
||||
prompt_kind: selected.prompt_kind,
|
||||
skill_root_lines: selected.skill_root_lines,
|
||||
skill_lines: selected.skill_lines,
|
||||
preserve_empty_fragment: policy == SkillCatalogRenderPolicy::CoreCompatible,
|
||||
@@ -516,6 +529,7 @@ pub(crate) fn render_combined_available_skills(
|
||||
orchestrator_catalog: &SkillCatalog,
|
||||
host_catalog: &SkillCatalog,
|
||||
budget: SkillMetadataBudget,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> RenderedSkillCatalogs {
|
||||
let mut executor_entries = executor_catalog
|
||||
.entries
|
||||
@@ -549,49 +563,82 @@ pub(crate) fn render_combined_available_skills(
|
||||
executor_catalog,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
budget,
|
||||
include_skills_usage_instructions,
|
||||
),
|
||||
orchestrator: render_available_skills(
|
||||
orchestrator_catalog,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
budget,
|
||||
include_skills_usage_instructions,
|
||||
),
|
||||
host: render_available_skills(
|
||||
host_catalog,
|
||||
SkillCatalogRenderPolicy::CoreCompatible,
|
||||
budget,
|
||||
include_skills_usage_instructions,
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
let extension_policy = SkillCatalogRenderPolicy::ExtensionCompatible;
|
||||
let host_policy = SkillCatalogRenderPolicy::CoreCompatible;
|
||||
let absolute = render_combined_lines(
|
||||
executor_entries
|
||||
.iter()
|
||||
.map(|entry| SkillLine::new(entry, SkillCatalogRenderPolicy::ExtensionCompatible))
|
||||
.collect(),
|
||||
orchestrator_entries
|
||||
.iter()
|
||||
.map(|entry| SkillLine::new(entry, SkillCatalogRenderPolicy::ExtensionCompatible))
|
||||
.collect(),
|
||||
host_entries
|
||||
.iter()
|
||||
.map(|entry| SkillLine::new(entry, SkillCatalogRenderPolicy::CoreCompatible))
|
||||
.collect(),
|
||||
CatalogLines::unaliased(&executor_entries, extension_policy),
|
||||
CatalogLines::unaliased(&orchestrator_entries, extension_policy),
|
||||
CatalogLines::unaliased(&host_entries, host_policy),
|
||||
budget,
|
||||
Vec::new(),
|
||||
);
|
||||
let selected = if combined_catalog_fully_rendered(&absolute) {
|
||||
absolute
|
||||
} else if let Some(aliased) = build_aliased_combined_catalog(
|
||||
&executor_entries,
|
||||
&orchestrator_entries,
|
||||
&host_entries,
|
||||
budget,
|
||||
) && combined_render_is_better(&aliased, &absolute, budget)
|
||||
{
|
||||
aliased
|
||||
} else {
|
||||
absolute
|
||||
};
|
||||
|
||||
let mut selected = absolute;
|
||||
if !combined_catalog_fully_rendered(&selected) {
|
||||
let host_only_aliases = build_aliased_combined_catalog(
|
||||
CatalogLines::unaliased(&executor_entries, extension_policy),
|
||||
CatalogLines::unaliased(&orchestrator_entries, extension_policy),
|
||||
CatalogLines::aliased(&host_entries, host_policy),
|
||||
budget,
|
||||
include_skills_usage_instructions,
|
||||
);
|
||||
let executor_only_aliases = build_aliased_combined_catalog(
|
||||
CatalogLines::aliased(&executor_entries, extension_policy),
|
||||
CatalogLines::unaliased(&orchestrator_entries, extension_policy),
|
||||
CatalogLines::unaliased(&host_entries, host_policy),
|
||||
budget,
|
||||
include_skills_usage_instructions,
|
||||
);
|
||||
let orchestrator_only_aliases = build_aliased_combined_catalog(
|
||||
CatalogLines::unaliased(&executor_entries, extension_policy),
|
||||
CatalogLines::aliased(&orchestrator_entries, extension_policy),
|
||||
CatalogLines::unaliased(&host_entries, host_policy),
|
||||
budget,
|
||||
include_skills_usage_instructions,
|
||||
);
|
||||
let all_source_aliases = build_aliased_combined_catalog(
|
||||
CatalogLines::aliased(&executor_entries, extension_policy),
|
||||
CatalogLines::aliased(&orchestrator_entries, extension_policy),
|
||||
CatalogLines::aliased(&host_entries, host_policy),
|
||||
budget,
|
||||
include_skills_usage_instructions,
|
||||
);
|
||||
|
||||
for candidate in [
|
||||
host_only_aliases,
|
||||
executor_only_aliases,
|
||||
orchestrator_only_aliases,
|
||||
all_source_aliases,
|
||||
]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
{
|
||||
if combined_render_is_better(
|
||||
&candidate,
|
||||
&selected,
|
||||
budget,
|
||||
include_skills_usage_instructions,
|
||||
) {
|
||||
selected = candidate;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
RenderedSkillCatalogs {
|
||||
executor: Some(selected.executor),
|
||||
@@ -606,18 +653,60 @@ struct CombinedAvailableSkillsRender {
|
||||
host: AvailableSkillsRender,
|
||||
}
|
||||
|
||||
struct CatalogLines<'a> {
|
||||
prompt_kind: SkillPromptKind,
|
||||
skills: Vec<SkillLine<'a>>,
|
||||
root_lines: Vec<String>,
|
||||
}
|
||||
|
||||
impl<'a> CatalogLines<'a> {
|
||||
fn unaliased(entries: &[&'a SkillCatalogEntry], policy: SkillCatalogRenderPolicy) -> Self {
|
||||
Self {
|
||||
prompt_kind: SkillPromptKind::Unaliased,
|
||||
skills: entries
|
||||
.iter()
|
||||
.map(|entry| SkillLine::new(entry, policy))
|
||||
.collect(),
|
||||
root_lines: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn aliased(entries: &[&'a SkillCatalogEntry], policy: SkillCatalogRenderPolicy) -> Self {
|
||||
let Some(plan) = build_alias_plan(entries) else {
|
||||
return Self::unaliased(entries, policy);
|
||||
};
|
||||
|
||||
Self {
|
||||
prompt_kind: entries
|
||||
.first()
|
||||
.map(|entry| SkillPromptKind::for_aliased_source(&entry.authority.kind))
|
||||
.unwrap_or(SkillPromptKind::Unaliased),
|
||||
skills: entries
|
||||
.iter()
|
||||
.map(|entry| {
|
||||
SkillLine::with_locator(
|
||||
entry,
|
||||
policy,
|
||||
render_skill_locator_with_aliases(entry, &plan),
|
||||
)
|
||||
})
|
||||
.collect(),
|
||||
root_lines: plan.root_lines(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn render_combined_lines(
|
||||
executor_lines: Vec<SkillLine<'_>>,
|
||||
orchestrator_lines: Vec<SkillLine<'_>>,
|
||||
host_lines: Vec<SkillLine<'_>>,
|
||||
executor: CatalogLines<'_>,
|
||||
orchestrator: CatalogLines<'_>,
|
||||
host: CatalogLines<'_>,
|
||||
budget: SkillMetadataBudget,
|
||||
host_skill_root_lines: Vec<String>,
|
||||
) -> CombinedAvailableSkillsRender {
|
||||
let executor_end = executor_lines.len();
|
||||
let orchestrator_end = executor_end.saturating_add(orchestrator_lines.len());
|
||||
let mut lines = executor_lines;
|
||||
lines.extend(orchestrator_lines);
|
||||
lines.extend(host_lines);
|
||||
let executor_end = executor.skills.len();
|
||||
let orchestrator_end = executor_end.saturating_add(orchestrator.skills.len());
|
||||
let mut lines = executor.skills;
|
||||
lines.extend(orchestrator.skills);
|
||||
lines.extend(host.skills);
|
||||
let mut allocations = allocate_skill_lines(&lines, budget);
|
||||
let omission_marker = reserve_non_host_omission_marker(
|
||||
&lines,
|
||||
@@ -637,19 +726,22 @@ fn render_combined_lines(
|
||||
executor: render_combined_group(
|
||||
&lines[..executor_end],
|
||||
&allocations[..executor_end],
|
||||
Vec::new(),
|
||||
executor.prompt_kind,
|
||||
executor.root_lines,
|
||||
executor_omission_marker,
|
||||
),
|
||||
orchestrator: render_combined_group(
|
||||
&lines[executor_end..orchestrator_end],
|
||||
&allocations[executor_end..orchestrator_end],
|
||||
Vec::new(),
|
||||
orchestrator.prompt_kind,
|
||||
orchestrator.root_lines,
|
||||
orchestrator_omission_marker,
|
||||
),
|
||||
host: render_combined_group(
|
||||
&lines[orchestrator_end..],
|
||||
&allocations[orchestrator_end..],
|
||||
host_skill_root_lines,
|
||||
host.prompt_kind,
|
||||
host.root_lines,
|
||||
/*omission_marker*/ None,
|
||||
),
|
||||
}
|
||||
@@ -658,6 +750,7 @@ fn render_combined_lines(
|
||||
fn render_combined_group(
|
||||
skill_lines: &[SkillLine<'_>],
|
||||
allocations: &[SkillLineAllocation],
|
||||
prompt_kind: SkillPromptKind,
|
||||
skill_root_lines: Vec<String>,
|
||||
omission_marker: Option<String>,
|
||||
) -> AvailableSkillsRender {
|
||||
@@ -671,6 +764,7 @@ fn render_combined_group(
|
||||
lines.push(RenderedSkillLine { line: marker });
|
||||
}
|
||||
AvailableSkillsRender {
|
||||
prompt_kind,
|
||||
skill_root_lines,
|
||||
skill_lines: lines.into_iter().map(|rendered| rendered.line).collect(),
|
||||
preserve_empty_fragment: false,
|
||||
@@ -685,14 +779,35 @@ fn render_combined_group(
|
||||
}
|
||||
|
||||
fn build_aliased_combined_catalog(
|
||||
executor_entries: &[&SkillCatalogEntry],
|
||||
orchestrator_entries: &[&SkillCatalogEntry],
|
||||
host_entries: &[&SkillCatalogEntry],
|
||||
executor: CatalogLines<'_>,
|
||||
orchestrator: CatalogLines<'_>,
|
||||
host: CatalogLines<'_>,
|
||||
budget: SkillMetadataBudget,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> Option<CombinedAvailableSkillsRender> {
|
||||
let plan = build_alias_plan(host_entries)?;
|
||||
let skill_root_lines = plan.root_lines();
|
||||
let table_cost = aliased_metadata_overhead_cost(budget, &skill_root_lines);
|
||||
if [
|
||||
&executor.root_lines,
|
||||
&orchestrator.root_lines,
|
||||
&host.root_lines,
|
||||
]
|
||||
.into_iter()
|
||||
.all(Vec::is_empty)
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
let table_cost = [&executor, &orchestrator, &host]
|
||||
.into_iter()
|
||||
.filter(|catalog| !catalog.root_lines.is_empty())
|
||||
.map(|catalog| {
|
||||
aliased_metadata_overhead_cost(
|
||||
budget,
|
||||
catalog.prompt_kind,
|
||||
&catalog.root_lines,
|
||||
include_skills_usage_instructions,
|
||||
)
|
||||
})
|
||||
.fold(0usize, usize::saturating_add);
|
||||
if table_cost >= budget.limit() {
|
||||
return None;
|
||||
}
|
||||
@@ -702,30 +817,11 @@ fn build_aliased_combined_catalog(
|
||||
SkillMetadataBudget::Tokens(_) => SkillMetadataBudget::Tokens(adjusted_limit),
|
||||
SkillMetadataBudget::Characters(_) => SkillMetadataBudget::Characters(adjusted_limit),
|
||||
};
|
||||
let executor_lines = executor_entries
|
||||
.iter()
|
||||
.map(|entry| SkillLine::new(entry, SkillCatalogRenderPolicy::ExtensionCompatible))
|
||||
.collect();
|
||||
let orchestrator_lines = orchestrator_entries
|
||||
.iter()
|
||||
.map(|entry| SkillLine::new(entry, SkillCatalogRenderPolicy::ExtensionCompatible))
|
||||
.collect();
|
||||
let host_lines = host_entries
|
||||
.iter()
|
||||
.map(|entry| {
|
||||
SkillLine::with_locator(
|
||||
entry,
|
||||
SkillCatalogRenderPolicy::CoreCompatible,
|
||||
render_skill_locator_with_aliases(entry, &plan),
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
Some(render_combined_lines(
|
||||
executor_lines,
|
||||
orchestrator_lines,
|
||||
host_lines,
|
||||
executor,
|
||||
orchestrator,
|
||||
host,
|
||||
adjusted_budget,
|
||||
skill_root_lines,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -741,6 +837,7 @@ fn combined_render_is_better(
|
||||
candidate: &CombinedAvailableSkillsRender,
|
||||
current: &CombinedAvailableSkillsRender,
|
||||
budget: SkillMetadataBudget,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> bool {
|
||||
let priority = |rendered: &CombinedAvailableSkillsRender| {
|
||||
(
|
||||
@@ -764,19 +861,25 @@ fn combined_render_is_better(
|
||||
return truncated_chars(candidate) < truncated_chars(current);
|
||||
}
|
||||
|
||||
combined_available_skills_cost(budget, candidate)
|
||||
< combined_available_skills_cost(budget, current)
|
||||
combined_available_skills_cost(budget, candidate, include_skills_usage_instructions)
|
||||
< combined_available_skills_cost(budget, current, include_skills_usage_instructions)
|
||||
}
|
||||
|
||||
fn combined_available_skills_cost(
|
||||
budget: SkillMetadataBudget,
|
||||
rendered: &CombinedAvailableSkillsRender,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> usize {
|
||||
[&rendered.executor, &rendered.orchestrator, &rendered.host]
|
||||
.into_iter()
|
||||
.fold(0usize, |used, catalog| {
|
||||
let root_cost = if !catalog.skill_root_lines.is_empty() {
|
||||
aliased_metadata_overhead_cost(budget, &catalog.skill_root_lines)
|
||||
aliased_metadata_overhead_cost(
|
||||
budget,
|
||||
catalog.prompt_kind,
|
||||
&catalog.skill_root_lines,
|
||||
include_skills_usage_instructions,
|
||||
)
|
||||
} else {
|
||||
Default::default()
|
||||
};
|
||||
@@ -848,6 +951,7 @@ fn render_catalog(
|
||||
skill_lines: Vec<SkillLine<'_>>,
|
||||
budget: SkillMetadataBudget,
|
||||
skill_root_lines: Vec<String>,
|
||||
prompt_kind: SkillPromptKind,
|
||||
policy: SkillCatalogRenderPolicy,
|
||||
) -> RenderedCatalog {
|
||||
let total_count = skill_lines.len();
|
||||
@@ -877,6 +981,7 @@ fn render_catalog(
|
||||
}
|
||||
|
||||
RenderedCatalog {
|
||||
prompt_kind,
|
||||
skill_root_lines,
|
||||
skill_lines: rendered_lines
|
||||
.into_iter()
|
||||
@@ -899,7 +1004,7 @@ fn available_skills_fragment(
|
||||
policy: SkillCatalogRenderPolicy,
|
||||
budget: SkillMetadataBudget,
|
||||
) -> Option<AvailableSkillsInstructions> {
|
||||
render_available_skills(catalog, policy, budget)?
|
||||
render_available_skills(catalog, policy, budget, include_skills_usage_instructions)?
|
||||
.into_fragment(include_skills_usage_instructions)
|
||||
}
|
||||
|
||||
@@ -907,18 +1012,18 @@ fn build_aliased_catalog(
|
||||
entries: &[&SkillCatalogEntry],
|
||||
policy: SkillCatalogRenderPolicy,
|
||||
budget: SkillMetadataBudget,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> Option<RenderedCatalog> {
|
||||
// Alias instructions currently describe only host filesystem skills.
|
||||
if entries
|
||||
.iter()
|
||||
.any(|entry| entry.authority.kind != SkillSourceKind::Host)
|
||||
{
|
||||
let catalog = CatalogLines::aliased(entries, policy);
|
||||
if catalog.root_lines.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let plan = build_alias_plan(entries)?;
|
||||
let skill_root_lines = plan.root_lines();
|
||||
let table_cost = aliased_metadata_overhead_cost(budget, &skill_root_lines);
|
||||
let table_cost = aliased_metadata_overhead_cost(
|
||||
budget,
|
||||
catalog.prompt_kind,
|
||||
&catalog.root_lines,
|
||||
include_skills_usage_instructions,
|
||||
);
|
||||
if table_cost >= budget.limit() {
|
||||
return None;
|
||||
}
|
||||
@@ -928,20 +1033,11 @@ fn build_aliased_catalog(
|
||||
SkillMetadataBudget::Tokens(_) => SkillMetadataBudget::Tokens(adjusted_limit),
|
||||
SkillMetadataBudget::Characters(_) => SkillMetadataBudget::Characters(adjusted_limit),
|
||||
};
|
||||
let skill_lines = entries
|
||||
.iter()
|
||||
.map(|entry| {
|
||||
SkillLine::with_locator(
|
||||
entry,
|
||||
policy,
|
||||
render_skill_locator_with_aliases(entry, &plan),
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
Some(render_catalog(
|
||||
skill_lines,
|
||||
catalog.skills,
|
||||
adjusted_budget,
|
||||
skill_root_lines,
|
||||
catalog.root_lines,
|
||||
catalog.prompt_kind,
|
||||
policy,
|
||||
))
|
||||
}
|
||||
@@ -985,13 +1081,25 @@ fn render_skill_locator_with_aliases(entry: &SkillCatalogEntry, plan: &AliasPlan
|
||||
|
||||
fn aliased_metadata_overhead_cost(
|
||||
budget: SkillMetadataBudget,
|
||||
prompt_kind: SkillPromptKind,
|
||||
skill_root_lines: &[String],
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> 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);
|
||||
let absolute_body =
|
||||
render_available_skills_body(SkillPromptKind::Unaliased, &[], empty_skill_lines);
|
||||
let aliased_body =
|
||||
render_available_skills_body(prompt_kind, skill_root_lines, empty_skill_lines);
|
||||
let alias_instruction_cost = if include_skills_usage_instructions {
|
||||
prompt_kind
|
||||
.alias_instructions()
|
||||
.map_or(0, |instructions| metadata_line_cost(budget, instructions))
|
||||
} else {
|
||||
0
|
||||
};
|
||||
budget
|
||||
.cost(&aliased_body)
|
||||
.saturating_add(alias_instruction_cost)
|
||||
.saturating_sub(budget.cost(&absolute_body))
|
||||
}
|
||||
|
||||
@@ -999,6 +1107,7 @@ fn aliased_render_is_better(
|
||||
aliased: &RenderedCatalog,
|
||||
absolute: &RenderedCatalog,
|
||||
budget: SkillMetadataBudget,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> bool {
|
||||
if aliased.report.included_count != absolute.report.included_count {
|
||||
return aliased.report.included_count > absolute.report.included_count;
|
||||
@@ -1007,14 +1116,24 @@ fn aliased_render_is_better(
|
||||
return aliased.report.truncated_description_chars
|
||||
< absolute.report.truncated_description_chars;
|
||||
}
|
||||
rendered_catalog_cost(budget, aliased) < rendered_catalog_cost(budget, absolute)
|
||||
rendered_catalog_cost(budget, aliased, include_skills_usage_instructions)
|
||||
< rendered_catalog_cost(budget, absolute, include_skills_usage_instructions)
|
||||
}
|
||||
|
||||
fn rendered_catalog_cost(budget: SkillMetadataBudget, rendered: &RenderedCatalog) -> usize {
|
||||
fn rendered_catalog_cost(
|
||||
budget: SkillMetadataBudget,
|
||||
rendered: &RenderedCatalog,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> usize {
|
||||
let metadata_cost = if rendered.skill_root_lines.is_empty() {
|
||||
0
|
||||
} else {
|
||||
aliased_metadata_overhead_cost(budget, &rendered.skill_root_lines)
|
||||
aliased_metadata_overhead_cost(
|
||||
budget,
|
||||
rendered.prompt_kind,
|
||||
&rendered.skill_root_lines,
|
||||
include_skills_usage_instructions,
|
||||
)
|
||||
};
|
||||
rendered
|
||||
.skill_lines
|
||||
|
||||
@@ -99,6 +99,7 @@ fn ordering_follows_render_policy() {
|
||||
assert_eq!(
|
||||
render(SkillCatalogRenderPolicy::CoreCompatible),
|
||||
render_available_skills_body(
|
||||
SkillPromptKind::Unaliased,
|
||||
&[],
|
||||
&[
|
||||
"- system-zeta: Description. (file: /skills/system-zeta/SKILL.md)".to_string(),
|
||||
@@ -113,6 +114,7 @@ fn ordering_follows_render_policy() {
|
||||
assert_eq!(
|
||||
render(SkillCatalogRenderPolicy::ExtensionCompatible),
|
||||
render_available_skills_body(
|
||||
SkillPromptKind::Unaliased,
|
||||
&[],
|
||||
&[
|
||||
"- repo-zeta: Description. (file: /skills/repo-zeta/SKILL.md)".to_string(),
|
||||
@@ -158,6 +160,7 @@ fn description_selection_follows_render_policy() {
|
||||
assert_eq!(
|
||||
core.body(),
|
||||
render_available_skills_body(
|
||||
SkillPromptKind::Unaliased,
|
||||
&[],
|
||||
&[
|
||||
"- fallback: fallback description (file: /skills/fallback/SKILL.md)".to_string(),
|
||||
@@ -168,6 +171,7 @@ fn description_selection_follows_render_policy() {
|
||||
assert_eq!(
|
||||
extension.body(),
|
||||
render_available_skills_body(
|
||||
SkillPromptKind::Unaliased,
|
||||
&[],
|
||||
&[
|
||||
"- shortened: short description (file: /skills/shortened/SKILL.md)".to_string(),
|
||||
@@ -193,6 +197,98 @@ fn catalog_budget_uses_context_percentage_or_character_fallback() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn host_only_prompts_preserve_existing_behavior_with_and_without_aliases() {
|
||||
let root = "/Users/test/.codex/plugins/cache/openai-curated/host-plugin/1.0.0/skills-with-a-long-shared-root";
|
||||
let catalog = SkillCatalog {
|
||||
entries: [
|
||||
("alpha", "Alpha skill."),
|
||||
("beta", "Beta skill."),
|
||||
("gamma", "Gamma skill."),
|
||||
]
|
||||
.into_iter()
|
||||
.map(|(name, description)| {
|
||||
entry(name, description, /*short_description*/ None)
|
||||
.with_display_path(format!("{root}/{name}/SKILL.md"))
|
||||
.with_alias_root(root)
|
||||
})
|
||||
.collect(),
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
|
||||
let unaliased = available_skills_fragment(
|
||||
&catalog,
|
||||
/*include_skills_usage_instructions*/ true,
|
||||
SkillCatalogRenderPolicy::CoreCompatible,
|
||||
SkillMetadataBudget::Characters(usize::MAX),
|
||||
)
|
||||
.expect("unaliased host catalog should render");
|
||||
insta::assert_snapshot!(unaliased.body(), @r###"
|
||||
|
||||
## Skills
|
||||
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.
|
||||
### Available skills
|
||||
- alpha: Alpha skill. (file: /Users/test/.codex/plugins/cache/openai-curated/host-plugin/1.0.0/skills-with-a-long-shared-root/alpha/SKILL.md)
|
||||
- beta: Beta skill. (file: /Users/test/.codex/plugins/cache/openai-curated/host-plugin/1.0.0/skills-with-a-long-shared-root/beta/SKILL.md)
|
||||
- gamma: Gamma skill. (file: /Users/test/.codex/plugins/cache/openai-curated/host-plugin/1.0.0/skills-with-a-long-shared-root/gamma/SKILL.md)
|
||||
### How to use skills
|
||||
- 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.
|
||||
"###);
|
||||
|
||||
let aliased = available_skills_fragment(
|
||||
&catalog,
|
||||
/*include_skills_usage_instructions*/ true,
|
||||
SkillCatalogRenderPolicy::CoreCompatible,
|
||||
SkillMetadataBudget::Characters(200),
|
||||
)
|
||||
.expect("aliased host catalog should render");
|
||||
insta::assert_snapshot!(aliased.body(), @r###"
|
||||
|
||||
## Skills
|
||||
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.
|
||||
### Skill roots
|
||||
- `r0` = `/Users/test/.codex/plugins/cache/openai-curated/host-plugin/1.0.0/skills-with-a-long-shared-root`
|
||||
### Available skills
|
||||
- alpha: Alpha skill. (file: r0/alpha/SKILL.md)
|
||||
- beta: Beta skill. (file: r0/beta/SKILL.md)
|
||||
- gamma: Gamma skill. (file: r0/gamma/SKILL.md)
|
||||
### How to use skills
|
||||
- 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.
|
||||
"###);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn path_aliases_are_not_used_without_budget_pressure() {
|
||||
let root = "/Users/test/.codex/plugins/cache/openai-curated/example/hash/skills";
|
||||
@@ -243,7 +339,9 @@ fn path_aliases_retain_every_skill_under_budget_pressure() {
|
||||
let plan = build_alias_plan(&visible_entries).expect("alias plan should build");
|
||||
let table_cost = aliased_metadata_overhead_cost(
|
||||
SkillMetadataBudget::Characters(usize::MAX),
|
||||
SkillPromptKind::HostAliases,
|
||||
&plan.root_lines(),
|
||||
/*include_skills_usage_instructions*/ true,
|
||||
);
|
||||
let alias_minimum = visible_entries.iter().fold(table_cost, |cost, entry| {
|
||||
cost.saturating_add(
|
||||
@@ -290,7 +388,9 @@ async fn host_alias_roots_follow_core_discovery_order() -> Result<(), Box<dyn st
|
||||
let system_root_path = parent.join("system-root");
|
||||
for (root, name) in [
|
||||
(&user_root_path, "user-skill"),
|
||||
(&user_root_path, "another-user-skill"),
|
||||
(&system_root_path, "system-skill"),
|
||||
(&system_root_path, "another-system-skill"),
|
||||
] {
|
||||
let skill_dir = root.join(name);
|
||||
std::fs::create_dir_all(&skill_dir)?;
|
||||
@@ -436,6 +536,7 @@ fn mixed_catalog_reserves_executor_omission_marker_by_omitting_host_first() {
|
||||
&SkillCatalog::default(),
|
||||
&host_catalog,
|
||||
SkillMetadataBudget::Tokens(28),
|
||||
/*include_skills_usage_instructions*/ false,
|
||||
);
|
||||
let host = host.expect("host catalog should render");
|
||||
let executor = executor.expect("executor catalog should render");
|
||||
@@ -473,6 +574,102 @@ fn mixed_catalog_reserves_executor_omission_marker_by_omitting_host_first() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mixed_catalogs_alias_all_skill_sources_under_budget_pressure() {
|
||||
let executor_root =
|
||||
"skill://executor-environment-with-a-long-shared-root/workspaces/project/.agents/skills";
|
||||
let orchestrator_root = "skill://plugin_connector_1p_2330815c823c8191941e5dc465bb899f";
|
||||
let host_root = "/Users/test/.codex/plugins/cache/openai-curated/example/hash1234567890/skills";
|
||||
let catalog =
|
||||
|source: SkillSourceKind, authority: &str, root: &str, prefix: &str| SkillCatalog {
|
||||
entries: (0..3)
|
||||
.map(|index| {
|
||||
let name = format!("{prefix}-{index}");
|
||||
let locator = match source {
|
||||
SkillSourceKind::Orchestrator => format!("{root}/{name}"),
|
||||
SkillSourceKind::Host
|
||||
| SkillSourceKind::Executor
|
||||
| SkillSourceKind::Custom(_) => format!("{root}/{name}/SKILL.md"),
|
||||
};
|
||||
SkillCatalogEntry::new(
|
||||
SkillPackageId(locator.clone()),
|
||||
SkillAuthority::new(source.clone(), authority),
|
||||
name,
|
||||
"Description.",
|
||||
SkillResourceId::new(locator.clone()),
|
||||
)
|
||||
.with_display_path(locator)
|
||||
.with_alias_root(root)
|
||||
})
|
||||
.collect(),
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
let executor_catalog = catalog(
|
||||
SkillSourceKind::Executor,
|
||||
"executor-environment",
|
||||
executor_root,
|
||||
"executor",
|
||||
);
|
||||
let orchestrator_catalog = catalog(
|
||||
SkillSourceKind::Orchestrator,
|
||||
"codex_apps",
|
||||
orchestrator_root,
|
||||
"orchestrator",
|
||||
);
|
||||
let host_catalog = catalog(SkillSourceKind::Host, "host", host_root, "host");
|
||||
|
||||
let rendered = render_combined_available_skills(
|
||||
&executor_catalog,
|
||||
&orchestrator_catalog,
|
||||
&host_catalog,
|
||||
SkillMetadataBudget::Characters(900),
|
||||
/*include_skills_usage_instructions*/ true,
|
||||
);
|
||||
let executor = rendered.executor.expect("executor catalog should render");
|
||||
let orchestrator = rendered
|
||||
.orchestrator
|
||||
.expect("orchestrator catalog should render");
|
||||
let host = rendered.host.expect("host catalog should render");
|
||||
|
||||
assert_eq!(
|
||||
executor.skill_root_lines,
|
||||
vec![format!("- `e0` = `{executor_root}`")]
|
||||
);
|
||||
assert_eq!(
|
||||
orchestrator.skill_root_lines,
|
||||
vec![format!("- `o0` = `{orchestrator_root}`")]
|
||||
);
|
||||
assert_eq!(
|
||||
host.skill_root_lines,
|
||||
vec![format!("- `r0` = `{host_root}`")]
|
||||
);
|
||||
assert!(
|
||||
executor
|
||||
.skill_lines
|
||||
.iter()
|
||||
.any(|line| line.contains("(environment resource: e0/executor-0/SKILL.md)"))
|
||||
);
|
||||
assert!(
|
||||
orchestrator
|
||||
.skill_lines
|
||||
.iter()
|
||||
.any(|line| line.contains("(orchestrator resource: o0/orchestrator-0)"))
|
||||
);
|
||||
assert!(
|
||||
host.skill_lines
|
||||
.iter()
|
||||
.any(|line| line.contains("(file: r0/host-0/SKILL.md)"))
|
||||
);
|
||||
assert_eq!(
|
||||
(
|
||||
executor.report.included_count,
|
||||
orchestrator.report.included_count,
|
||||
host.report.included_count,
|
||||
),
|
||||
(3, 3, 3)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mixed_catalog_prefers_executor_inclusion_over_total_aliased_inclusion() {
|
||||
let root = format!("/{}", "r".repeat(219));
|
||||
@@ -528,6 +725,7 @@ fn mixed_catalog_prefers_executor_inclusion_over_total_aliased_inclusion() {
|
||||
&orchestrator_catalog,
|
||||
&host_catalog,
|
||||
SkillMetadataBudget::Tokens(74),
|
||||
/*include_skills_usage_instructions*/ false,
|
||||
);
|
||||
let host = host.expect("host catalog should render");
|
||||
let executor = executor.expect("executor catalog should render");
|
||||
@@ -724,6 +922,7 @@ fn catalog_report_counts_partial_description_truncation() {
|
||||
&catalog,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
budget,
|
||||
/*include_skills_usage_instructions*/ false,
|
||||
)
|
||||
.expect("catalog should render");
|
||||
assert_eq!(
|
||||
@@ -773,6 +972,7 @@ fn catalog_emits_omission_marker_when_every_minimum_skill_line_exceeds_budget()
|
||||
&catalog,
|
||||
SkillCatalogRenderPolicy::CoreCompatible,
|
||||
SkillMetadataBudget::Tokens(100),
|
||||
/*include_skills_usage_instructions*/ false,
|
||||
)
|
||||
.expect("core-compatible report should render");
|
||||
assert_eq!(core_render.report, expected_report);
|
||||
@@ -785,6 +985,7 @@ fn catalog_emits_omission_marker_when_every_minimum_skill_line_exceeds_budget()
|
||||
&catalog,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
SkillMetadataBudget::Tokens(100),
|
||||
/*include_skills_usage_instructions*/ false,
|
||||
)
|
||||
.expect("catalog should render");
|
||||
assert_eq!(render.report, expected_report);
|
||||
@@ -817,6 +1018,7 @@ fn catalog_preserves_report_when_no_fragment_fits_budget() {
|
||||
&catalog,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
SkillMetadataBudget::Tokens(1),
|
||||
/*include_skills_usage_instructions*/ false,
|
||||
)
|
||||
.expect("catalog should produce a report");
|
||||
assert_eq!(
|
||||
@@ -861,6 +1063,7 @@ fn substantial_description_shortening_emits_warning() {
|
||||
&catalog,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
SkillMetadataBudget::Characters(minimum_cost + 49),
|
||||
/*include_skills_usage_instructions*/ false,
|
||||
)
|
||||
.expect("catalog should render");
|
||||
|
||||
|
||||
@@ -252,6 +252,7 @@ impl<'a> CatalogContext<'a> {
|
||||
&catalogs.orchestrator.catalog,
|
||||
&catalogs.host.catalog,
|
||||
self.metadata_budget,
|
||||
self.include_usage,
|
||||
)
|
||||
} else {
|
||||
RenderedSkillCatalogs::default()
|
||||
|
||||
Reference in New Issue
Block a user