diff --git a/codex-rs/app-server/tests/suite/v2/executor_skills.rs b/codex-rs/app-server/tests/suite/v2/executor_skills.rs index e65bd4a008..7b6c421d52 100644 --- a/codex-rs/app-server/tests/suite/v2/executor_skills.rs +++ b/codex-rs/app-server/tests/suite/v2/executor_skills.rs @@ -276,6 +276,11 @@ stream_max_retries = 0 ) }; let package = locator(&skill_dir); + let main_package = if scenario == ExecutorSkillScenario::VisibleWithBudgetWarning { + "e0/skills/deploy".to_string() + } else { + package.clone() + }; let main_resource = locator(&skill_dir.join("SKILL.md")?); let reference_resource = locator(&reference_dir.join("details.md")?); let tool_response = |call_id: &str, tool: &str, arguments: serde_json::Value| { @@ -296,7 +301,7 @@ stream_max_retries = 0 "main", "read", json!({ - "package": package.clone(), + "package": main_package, "resource": main_resource.clone(), }), ), @@ -437,6 +442,14 @@ stream_max_retries = 0 let requests = response_mock.requests(); let request = &requests[0]; + if scenario == ExecutorSkillScenario::VisibleWithBudgetWarning { + assert!( + request + .message_input_texts("developer") + .iter() + .any(|text| text.contains("executor package: e0/skills/deploy")) + ); + } assert!( request .message_input_texts("developer") diff --git a/codex-rs/core/tests/suite/skills_extension.rs b/codex-rs/core/tests/suite/skills_extension.rs index 731fdaf705..7b9367ec4d 100644 --- a/codex-rs/core/tests/suite/skills_extension.rs +++ b/codex-rs/core/tests/suite/skills_extension.rs @@ -1182,8 +1182,8 @@ async fn production_turn_aliases_discovered_singleton_orchestrator_root() -> Res "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}" + developer_text.contains("- Root aliases: Pass short package locators directly"), + "model request should explain how to read aliased packages: {developer_text}" ); Ok(()) @@ -1257,10 +1257,9 @@ async fn production_turn_aliases_executor_skill_roots() -> Result<()> { "Sol should omit optional skill usage instructions: {executor_catalog}" ); assert!( - !executor_catalog.contains("- Root aliases: Expand short locators"), + !executor_catalog.contains("- Root aliases: Pass short package locators directly"), "Sol should not include optional resource-alias instructions: {executor_catalog}" ); - Ok(()) } diff --git a/codex-rs/ext/skills/src/catalog_prompt.rs b/codex-rs/ext/skills/src/catalog_prompt.rs index 3bd3c2ce40..cfd3eec158 100644 --- a/codex-rs/ext/skills/src/catalog_prompt.rs +++ b/codex-rs/ext/skills/src/catalog_prompt.rs @@ -3,12 +3,12 @@ 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, `executor package` locators are owned by their execution environment, `orchestrator package` locators are opaque package identifiers, 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://` identifiers."; +const RESOURCE_ALIAS_INSTRUCTIONS: &str = "- Root aliases: Pass short package locators directly to `skills.read`; it resolves their matching alias from `### Skill roots`."; 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, `executor package` and `orchestrator package` entries are accessed directly through `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 `executor package` or `orchestrator package`, expand its root alias when present and call `skills.read` with the complete locator as `package`; omit `resource` to read `SKILL.md` directly without calling `skills.list`. If a read is paginated, follow `next_cursor` until EOF. + 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 `executor package` or `orchestrator package`, pass the listed locator directly to `skills.read` as `package`; root aliases are resolved automatically. Omit `resource` to read `SKILL.md` directly without calling `skills.list`. If a read is paginated, follow `next_cursor` until EOF. 2) When `SKILL.md` references another resource, use the same access mechanism. For executor and orchestrator skills, pass the complete package-contained resource identifier with the same 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 executor and orchestrator skills, use `skills.read` and the available tools; do not invent a local path. @@ -94,7 +94,7 @@ pub(crate) fn render_available_skills_body( line.contains("(executor package: ") || line.contains("(orchestrator package: ") }) { lines.push( - "Read a skill package directly with `skills.read({\"package\":\"\"})` to read its `SKILL.md`, expanding any root alias. To read another file from that skill, use the same `package` and pass the file's complete `skill://` identifier as `resource`. If the package is not provided, use `skills.list` to find it." + "Read a skill package directly with `skills.read({\"package\":\"\"})` to read its `SKILL.md`; root aliases are resolved automatically. To read another file from that skill, use the same `package` and pass the file's complete `skill://` identifier as `resource`. If the package is not provided, use `skills.list` to find it." .to_string(), ); } diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index 89190a806f..4bed6c2987 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -1046,7 +1046,7 @@ fn build_aliased_catalog( )) } -fn build_alias_plan(entries: &[&SkillCatalogEntry]) -> Option { +pub(crate) fn build_alias_plan(entries: &[&SkillCatalogEntry]) -> Option { let source = &entries.first()?.authority.kind; if entries.iter().any(|entry| &entry.authority.kind != source) { return None; diff --git a/codex-rs/ext/skills/src/render_tests.rs b/codex-rs/ext/skills/src/render_tests.rs index dffddb2c0b..93de8e65b0 100644 --- a/codex-rs/ext/skills/src/render_tests.rs +++ b/codex-rs/ext/skills/src/render_tests.rs @@ -246,7 +246,7 @@ fn host_only_prompts_preserve_existing_behavior_with_and_without_aliases() { - 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 `executor package` or `orchestrator package`, expand its root alias when present and call `skills.read` with the complete locator as `package`; omit `resource` to read `SKILL.md` directly without calling `skills.list`. If a read is paginated, follow `next_cursor` until EOF. + 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 `executor package` or `orchestrator package`, pass the listed locator directly to `skills.read` as `package`; root aliases are resolved automatically. Omit `resource` to read `SKILL.md` directly without calling `skills.list`. If a read is paginated, follow `next_cursor` until EOF. 2) When `SKILL.md` references another resource, use the same access mechanism. For executor and orchestrator skills, pass the complete package-contained resource identifier with the same 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 executor and orchestrator skills, use `skills.read` and the available tools; do not invent a local path. diff --git a/codex-rs/ext/skills/src/tools/read.rs b/codex-rs/ext/skills/src/tools/read.rs index 29f64f0a82..00a3d8d459 100644 --- a/codex-rs/ext/skills/src/tools/read.rs +++ b/codex-rs/ext/skills/src/tools/read.rs @@ -11,6 +11,7 @@ use serde::Serialize; use crate::catalog::SkillResourceId; use crate::provider::SkillReadRequest; +use crate::render::build_alias_plan; use super::MAX_HANDLE_BYTES; use super::SkillToolAuthority; @@ -55,7 +56,7 @@ impl ToolExecutor for ReadTool { fn spec(&self) -> ToolSpec { skill_function_tool::( TOOL_NAME, - "Read one page from a skill. Pass its provided package, expanding any root alias. Omit resource to read SKILL.md; to read another file, use the same package and pass the file's complete skill:// identifier as resource. If the package is not provided, use skills.list to find it. Pass next_cursor back as cursor to continue.", + "Read one page from a skill. Pass its provided package directly; root aliases are resolved automatically. Omit resource to read SKILL.md; to read another file, use the same package and pass the file's complete skill:// identifier as resource. If the package is not provided, use skills.list to find it. Pass next_cursor back as cursor to continue.", ) } @@ -73,9 +74,20 @@ impl ToolExecutor for ReadTool { super::SkillToolAuthoritySelector::Executor, ] { let catalog = self.context.catalog(&call.turn_id, selector).await; + let alias_plan = build_alias_plan( + &catalog + .entries + .iter() + .filter(|entry| entry.is_model_visible()) + .collect::>(), + ); if let Some(entry) = catalog.entries.into_iter().find(|entry| { entry.enabled - && entry.id.0 == args.package + && (entry.id.0 == args.package + || alias_plan + .as_ref() + .and_then(|plan| plan.shorten(&entry.id.0)) + .is_some_and(|alias| alias == args.package)) && SkillToolAuthority::from_authority(&entry.authority) .is_some_and(|authority| authority.selector() == selector) }) {