Read executor skill packages directly (#38167)

## Why

Executor skills required a `skills.list` lookup before their contents could be
read, even when the catalog already identified the skill.

## What changed

- Render executor skill catalog entries as package locators, including shortened
  locators when skill roots are aliased.
- Tell the model to pass those packages directly to `skills.read`, while keeping
  `skills.list` as the fallback when no package is available.
- Use the shorter package locators when applying the skills context budget.

## Testing

Update catalog, aliasing, budget-pressure, and selected-skill coverage, including
an executor package ID containing a literal backslash.

GitOrigin-RevId: 2ace5e527dc4f29651a8f3214475ccc90d52c73f
This commit is contained in:
rphilizaire-openai
2026-08-12 10:34:25 +00:00
committed by copyberry
parent 3fe19fcd81
commit 69ae78291d
7 changed files with 70 additions and 50 deletions
@@ -608,7 +608,7 @@ fn assert_selected_skill_catalog_available(request: &ResponsesRequest) {
let catalog_fragment = latest_selected_skill_update(request)
.expect("selected skill catalog update should be model-visible");
assert!(catalog_fragment.contains(SKILL_DESCRIPTION));
assert!(catalog_fragment.contains("environment resource:"));
assert!(catalog_fragment.contains("executor package:"));
}
fn latest_selected_skill_update(request: &ResponsesRequest) -> Option<String> {
+17 -8
View File
@@ -1239,15 +1239,19 @@ async fn production_turn_aliases_executor_skill_roots() -> Result<()> {
"model request should include the executor skill root: {developer_text}"
);
assert!(
developer_text.contains("environment resource: e0/search/SKILL.md"),
developer_text.contains("executor package: e0/search"),
"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"))
.find(|fragment| fragment.contains("executor package: e0/search"))
.expect("model request should include an executor skills catalog");
assert!(
executor_catalog.contains("Read a skill package directly with `skills.read"),
"Sol should receive direct-read instructions for executor skills: {executor_catalog}"
);
assert!(
!executor_catalog.contains("### How to use skills"),
"Sol should omit optional skill usage instructions: {executor_catalog}"
@@ -1469,7 +1473,7 @@ async fn production_turn_aliases_combined_skill_catalogs_under_shared_budget() -
);
}
for (source, alias, prefix, suffix) in [
("environment resource", "e0", "executor", "/SKILL.md"),
("executor package", "e0", "executor", ""),
("orchestrator package", "o0", "orchestrator", ""),
("file", "r0", "host", "/SKILL.md"),
] {
@@ -2215,9 +2219,11 @@ async fn production_turn_keeps_orchestrator_world_state_incremental_across_turns
occurrences, 1,
"request {index} should contain the orchestrator catalog exactly once: {developer_texts:?}"
);
assert!(developer_texts.iter().any(|text| {
text.contains("Read an orchestrator package directly with `skills.read")
}));
assert!(
developer_texts
.iter()
.any(|text| text.contains("Read a skill package directly with `skills.read"))
);
}
Ok(())
@@ -2366,7 +2372,10 @@ async fn production_turn_omits_host_skills_before_executor_skills_under_extreme_
let host_lines = skill_lines(catalog_text(&developer_texts, "host"), "host");
let executor_lines = skill_lines(catalog_text(&developer_texts, "exec"), "exec");
assert_eq!(skill_names(&host_lines), vec!["host-beta"]);
assert_eq!(
skill_names(&host_lines),
vec!["host-alpha", "host-beta", "host-delta"]
);
assert_eq!(
skill_names(&executor_lines),
EXECUTOR_CATALOG
@@ -2375,7 +2384,7 @@ async fn production_turn_omits_host_skills_before_executor_skills_under_extreme_
.collect::<Vec<_>>()
);
assert!(warning_messages.contains(
&"Exceeded skills context budget. All skill descriptions were removed and 3 additional skills were not included in the model-visible skills list."
&"Exceeded skills context budget. All skill descriptions were removed and 1 additional skill was not included in the model-visible skills list."
.to_string()
));
+9 -10
View File
@@ -1,17 +1,17 @@
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 package` locators are opaque package identifiers, and `custom resource` locators use their provider's access mechanism.";
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 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` entries are accessed through `skills.list` and `skills.read`, `orchestrator package` entries are accessed directly through `skills.read`, and `custom resource` entries use their provider's access mechanism.
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 `environment resource`, call `skills.list` with `{"authority":{"kind":"executor"}}`, select the matching package, and pass its exact package and `main_resource` to `skills.read`. For an `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`. 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 package. For orchestrator skills, pass the exact referenced resource identifier with the same package to `skills.read`; do not treat `skill://` identifiers as filesystem paths.
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.
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 environment and orchestrator skills, use `skills.read` and the available tools; do not invent a local path.
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.
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.
@@ -90,12 +90,11 @@ pub(crate) fn render_available_skills_body(
lines.push("### Skill roots".to_string());
lines.extend(skill_root_lines.iter().cloned());
}
if skill_lines
.iter()
.any(|line| line.contains("(orchestrator package: "))
{
if skill_lines.iter().any(|line| {
line.contains("(executor package: ") || line.contains("(orchestrator package: ")
}) {
lines.push(
"Read an orchestrator package directly with `skills.read({\"package\":\"<package>\"})` without calling `skills.list`; expand its root alias first when present."
"Read a skill package directly with `skills.read({\"package\":\"<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."
.to_string(),
);
}
+6 -12
View File
@@ -182,10 +182,8 @@ struct SkillLine<'a> {
impl<'a> SkillLine<'a> {
fn new(entry: &'a SkillCatalogEntry, policy: SkillCatalogRenderPolicy) -> Self {
let locator = match &entry.authority.kind {
SkillSourceKind::Orchestrator => entry.id.0.as_str(),
SkillSourceKind::Host | SkillSourceKind::Executor | SkillSourceKind::Custom(_) => {
entry.rendered_path()
}
SkillSourceKind::Executor | SkillSourceKind::Orchestrator => entry.id.0.as_str(),
SkillSourceKind::Host | SkillSourceKind::Custom(_) => entry.rendered_path(),
};
Self::with_locator(entry, policy, locator.to_string())
}
@@ -202,7 +200,7 @@ impl<'a> SkillLine<'a> {
locator,
locator_kind: match &entry.authority.kind {
SkillSourceKind::Host => "file",
SkillSourceKind::Executor => "environment resource",
SkillSourceKind::Executor => "executor package",
SkillSourceKind::Orchestrator => "orchestrator package",
SkillSourceKind::Custom(_) => "custom resource",
},
@@ -1078,17 +1076,13 @@ fn build_alias_plan(entries: &[&SkillCatalogEntry]) -> Option<AliasPlan> {
fn render_skill_locator_with_aliases(entry: &SkillCatalogEntry, plan: &AliasPlan) -> String {
let locator = match &entry.authority.kind {
SkillSourceKind::Orchestrator => entry.id.0.as_str(),
SkillSourceKind::Host | SkillSourceKind::Executor | SkillSourceKind::Custom(_) => {
entry.rendered_path()
}
SkillSourceKind::Executor | SkillSourceKind::Orchestrator => entry.id.0.as_str(),
SkillSourceKind::Host | SkillSourceKind::Custom(_) => entry.rendered_path(),
};
if entry.alias_root().is_none() {
return locator.to_string();
}
let normalized_locator = locator.replace('\\', "/");
plan.shorten(&normalized_locator)
.unwrap_or_else(|| locator.to_string())
plan.shorten(locator).unwrap_or_else(|| locator.to_string())
}
fn aliased_metadata_overhead_cost(
+31 -13
View File
@@ -236,20 +236,20 @@ fn host_only_prompts_preserve_existing_behavior_with_and_without_aliases() {
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 package` locators are opaque package identifiers, and `custom resource` locators use their provider's access mechanism.
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.
### 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` entries are accessed through `skills.list` and `skills.read`, `orchestrator package` entries are accessed directly through `skills.read`, and `custom resource` entries use their provider's access mechanism.
- 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 `environment resource`, call `skills.list` with `{"authority":{"kind":"executor"}}`, select the matching package, and pass its exact package and `main_resource` to `skills.read`. For an `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`. 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 package. For orchestrator skills, pass the exact referenced resource identifier with the same package to `skills.read`; do not treat `skill://` identifiers as filesystem paths.
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.
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 environment and orchestrator skills, use `skills.read` and the available tools; do not invent a local path.
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.
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.
@@ -387,6 +387,23 @@ fn path_aliases_retain_every_skill_under_budget_pressure() {
assert!(!body.contains("additional skills omitted"));
}
#[test]
fn executor_aliases_preserve_literal_backslashes_in_package_ids() {
let root = r"skill://executor/workspace/foo\bar/skills";
let package = format!("{root}/demo");
let entry = SkillCatalogEntry::new(
SkillPackageId(package.clone()),
SkillAuthority::new(SkillSourceKind::Executor, "env-1"),
"demo",
"Description.",
SkillResourceId::new(format!("{package}/SKILL.md")),
)
.with_alias_root(root);
let plan = build_alias_plan(&[&entry]).expect("alias plan should build");
assert_eq!(render_skill_locator_with_aliases(&entry, &plan), "e0/demo");
}
#[tokio::test]
async fn host_alias_roots_follow_core_discovery_order() -> Result<(), Box<dyn std::error::Error>> {
let unique = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos();
@@ -507,7 +524,7 @@ fn mixed_catalogs_keep_absolute_authority_aware_rendering_under_budget_pressure(
assert!(!body.contains("### Skill roots"));
assert!(body.contains(&format!("(file: {root}/skill-0/SKILL.md)")));
assert!(body.contains("(environment resource: skill://executor/demo/SKILL.md)"));
assert!(body.contains("(executor package: executor-skill)"));
assert!(body.contains("For a `file` entry, open the listed path."));
assert!(!body.contains("additional skills omitted"));
}
@@ -522,7 +539,7 @@ fn mixed_catalog_reserves_executor_omission_marker_by_omitting_host_first() {
};
let executor_entry = |name: &str, resource: &str| {
SkillCatalogEntry::new(
SkillPackageId(name.to_string()),
SkillPackageId(resource.to_string()),
SkillAuthority::new(SkillSourceKind::Executor, "env-1"),
name,
"",
@@ -574,7 +591,7 @@ fn mixed_catalog_reserves_executor_omission_marker_by_omitting_host_first() {
assert_eq!(
executor.skill_lines,
vec![
"- e1: (environment resource: skill://executor/one)".to_string(),
"- e1: (executor package: skill://executor/one)".to_string(),
"- 1 additional skill omitted from this bounded skills list.".to_string(),
]
);
@@ -595,14 +612,15 @@ fn mixed_catalogs_alias_all_skill_sources_under_budget_pressure() {
entries: (0..3)
.map(|index| {
let name = format!("{prefix}-{index}");
let package = format!("{root}/{name}");
let locator = match source {
SkillSourceKind::Orchestrator => format!("{root}/{name}"),
SkillSourceKind::Orchestrator => package.clone(),
SkillSourceKind::Host
| SkillSourceKind::Executor
| SkillSourceKind::Custom(_) => format!("{root}/{name}/SKILL.md"),
| SkillSourceKind::Custom(_) => format!("{package}/SKILL.md"),
};
SkillCatalogEntry::new(
SkillPackageId(locator.clone()),
SkillPackageId(package),
SkillAuthority::new(source.clone(), authority),
name,
"Description.",
@@ -657,7 +675,7 @@ fn mixed_catalogs_alias_all_skill_sources_under_budget_pressure() {
executor
.skill_lines
.iter()
.any(|line| line.contains("(environment resource: e0/executor-0/SKILL.md)"))
.any(|line| line.contains("(executor package: e0/executor-0)"))
);
assert!(
orchestrator
@@ -696,7 +714,7 @@ fn mixed_catalog_prefers_executor_inclusion_over_total_aliased_inclusion() {
};
let executor_entry = |name: &str, resource: &str| {
SkillCatalogEntry::new(
SkillPackageId(name.to_string()),
SkillPackageId(resource.to_string()),
SkillAuthority::new(SkillSourceKind::Executor, "env-1"),
name,
"",
+1 -1
View File
@@ -55,7 +55,7 @@ impl ToolExecutor<ToolCall> for ReadTool {
fn spec(&self) -> ToolSpec {
skill_function_tool::<ReadArgs, ReadResponse>(
TOOL_NAME,
"Read one page from a skill resource. Pass the exact orchestrator package shown in the skill catalog, or the executor package from skills.list or resource_access metadata. The package determines its owning authority. Omit resource to read the main SKILL.md, or pass the exact identifier of another resource beneath that package. Pass next_cursor back as cursor to continue.",
"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.",
)
}
@@ -77,7 +77,7 @@ use pretty_assertions::assert_eq;
type TestResult = Result<(), Box<dyn std::error::Error>>;
static NEXT_CODEX_HOME_ID: AtomicUsize = AtomicUsize::new(0);
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 package` locators are opaque package identifiers, and `custom resource` locators use their provider's access mechanism.";
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, `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 DEMO_SKILL_CONTENTS: &str =
"---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n\nUse the demo skill.\n";
@@ -547,7 +547,7 @@ async fn executor_orchestrator_and_host_share_catalog_world_state_flow() -> Test
for (section_id, expected_line) in [
(
"skills",
"- executor-skill: Fix lint errors. (environment resource: skill://executor/executor-skill/SKILL.md)",
"- executor-skill: Fix lint errors. (executor package: executor/executor-skill)",
),
(
"orchestrator_skills",
@@ -922,7 +922,7 @@ async fn selected_executor_catalog_follows_step_availability_and_reuses_its_cach
assert!(
available_fragment
.body()
.contains("(environment resource: skill://executor/lint-fix/SKILL.md)")
.contains("(executor package: executor/lint-fix)")
);
let fragments = registry.turn_input_contributors()[0]
@@ -1198,7 +1198,7 @@ async fn moderate_budget_pressure_keeps_every_catalog_entry() -> TestResult {
let package_id = format!("{source}/{name}");
let line_prefix = format!("- {name}: ");
let line_suffix = if source == "executor" {
format!(" (environment resource: skill://{package_id}/SKILL.md)")
format!(" (executor package: {package_id})")
} else {
format!(" (orchestrator package: {package_id})")
};
@@ -1319,7 +1319,7 @@ async fn extreme_budget_pressure_removes_descriptions_before_omitting_entries()
assert!(
executor
.body()
.contains("- executor-skill-039: (environment resource:")
.contains("- executor-skill-039: (executor package:")
);
assert!(
orchestrator