mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Simplify package-based skill reads (#37808)
## What changed - Make `package` the only required argument to `skills.read`, resolving the owning orchestrator or executor catalog automatically. - Default omitted `resource` values to the package's main `SKILL.md`. - Render orchestrator skills as package locators and instruct the model to read them directly without first calling `skills.list`. - Update skill extension and app-server coverage for direct main-resource reads, referenced resources, aliases, and the revised tool schema. GitOrigin-RevId: 9faf57a8d0935566a15094931eaabaa8f0613e9e
This commit is contained in:
jif
@@ -296,7 +296,6 @@ stream_max_retries = 0
|
||||
"main",
|
||||
"read",
|
||||
json!({
|
||||
"authority": {"kind": "executor", "id": authority_id},
|
||||
"package": package.clone(),
|
||||
"resource": main_resource.clone(),
|
||||
}),
|
||||
@@ -305,8 +304,11 @@ stream_max_retries = 0
|
||||
"reference",
|
||||
"read",
|
||||
json!({
|
||||
"authority": {"kind": "executor", "id": authority_id},
|
||||
"package": package.clone(),
|
||||
"authority": {
|
||||
"kind": "executor",
|
||||
"id": authority_id,
|
||||
},
|
||||
"resource": reference_resource.clone(),
|
||||
}),
|
||||
),
|
||||
@@ -342,7 +344,6 @@ stream_max_retries = 0
|
||||
"approved-reference",
|
||||
"read",
|
||||
json!({
|
||||
"authority": {"kind": "executor", "id": authority_id},
|
||||
"package": package.clone(),
|
||||
"resource": reference_resource.clone(),
|
||||
}),
|
||||
|
||||
@@ -88,6 +88,7 @@ const SKILL_CONTENTS: &str = concat!(
|
||||
const SKILL_REFERENCE_CONTENTS: &str =
|
||||
"# Deploy reference\n\nUse the orchestrator deployment API.\n";
|
||||
const SKILLS_LIST_CALL_ID: &str = "skills-list";
|
||||
const SKILLS_READ_MAIN_CALL_ID: &str = "skills-read-main";
|
||||
const SKILLS_READ_CALL_ID: &str = "skills-read";
|
||||
const SKILLS_READ_AGAIN_CALL_ID: &str = "skills-read-again";
|
||||
|
||||
@@ -153,6 +154,19 @@ async fn orchestrator_skill_can_read_referenced_resource_without_an_executor() -
|
||||
let response_mock = responses::mount_sse_sequence(
|
||||
&responses_server,
|
||||
vec![
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-skills-read-main"),
|
||||
responses::ev_function_call_with_namespace(
|
||||
SKILLS_READ_MAIN_CALL_ID,
|
||||
"skills",
|
||||
"read",
|
||||
&json!({
|
||||
"package": SKILL_RESOURCE_URI,
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
responses::ev_completed("resp-skills-read-main"),
|
||||
]),
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-skills-list"),
|
||||
responses::ev_function_call_with_namespace(
|
||||
@@ -175,10 +189,10 @@ async fn orchestrator_skill_can_read_referenced_resource_without_an_executor() -
|
||||
"skills",
|
||||
"read",
|
||||
&json!({
|
||||
"package": SKILL_RESOURCE_URI,
|
||||
"authority": {
|
||||
"kind": "orchestrator",
|
||||
},
|
||||
"package": SKILL_RESOURCE_URI,
|
||||
"resource": SKILL_REFERENCE_URI,
|
||||
})
|
||||
.to_string(),
|
||||
@@ -192,9 +206,6 @@ async fn orchestrator_skill_can_read_referenced_resource_without_an_executor() -
|
||||
"skills",
|
||||
"read",
|
||||
&json!({
|
||||
"authority": {
|
||||
"kind": "orchestrator",
|
||||
},
|
||||
"package": SKILL_RESOURCE_URI,
|
||||
"resource": SKILL_REFERENCE_URI,
|
||||
})
|
||||
@@ -219,7 +230,7 @@ async fn orchestrator_skill_can_read_referenced_resource_without_an_executor() -
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![UserInput::Text {
|
||||
text: format!("Use ${SKILL_NAME}"),
|
||||
text: "Use the deployment capability.".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -234,16 +245,23 @@ async fn orchestrator_skill_can_read_referenced_resource_without_an_executor() -
|
||||
.await??;
|
||||
|
||||
let requests = response_mock.requests();
|
||||
assert_eq!(requests.len(), 4);
|
||||
assert_eq!(requests.len(), 5);
|
||||
let first_request = &requests[0];
|
||||
assert!(first_request.tool_by_name("skills", "list").is_some());
|
||||
assert!(first_request.tool_by_name("skills", "read").is_some());
|
||||
let read_tool = first_request
|
||||
.tool_by_name("skills", "read")
|
||||
.ok_or_else(|| anyhow::anyhow!("skills.read should be available"))?;
|
||||
assert_eq!(read_tool["parameters"]["required"], json!(["package"]));
|
||||
assert!(
|
||||
read_tool["parameters"]["properties"]
|
||||
.get("authority")
|
||||
.is_none()
|
||||
);
|
||||
assert!(first_request.tool_by_name("skills", "search").is_none());
|
||||
|
||||
let developer_messages = first_request.message_input_texts("developer");
|
||||
let catalog_line = format!(
|
||||
"- {SKILL_NAME}: {SKILL_DESCRIPTION} (orchestrator resource: {SKILL_RESOURCE_URI})"
|
||||
);
|
||||
let catalog_line =
|
||||
format!("- {SKILL_NAME}: {SKILL_DESCRIPTION} (orchestrator package: {SKILL_RESOURCE_URI})");
|
||||
assert_eq!(
|
||||
1,
|
||||
developer_messages
|
||||
@@ -261,17 +279,26 @@ async fn orchestrator_skill_can_read_referenced_resource_without_an_executor() -
|
||||
.iter()
|
||||
.any(|text| text.contains("do not treat `skill://` identifiers as filesystem paths"))
|
||||
);
|
||||
let skill_fragments = first_request
|
||||
.message_input_texts("user")
|
||||
.into_iter()
|
||||
.filter(|text| text.starts_with("<skill>"))
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(1, skill_fragments.len());
|
||||
assert!(skill_fragments[0].contains(&format!("<name>{SKILL_NAME}</name>")));
|
||||
assert!(skill_fragments[0].contains(SKILL_MARKER));
|
||||
assert!(skill_fragments[0].contains(SKILL_REFERENCE_URI));
|
||||
assert!(
|
||||
first_request
|
||||
.message_input_texts("user")
|
||||
.into_iter()
|
||||
.all(|text| !text.starts_with("<skill>"))
|
||||
);
|
||||
|
||||
let list_output = requests[1]
|
||||
let main_read_output = requests[1]
|
||||
.function_call_output_text(SKILLS_READ_MAIN_CALL_ID)
|
||||
.ok_or_else(|| anyhow::anyhow!("skills.read output should be sent to the model"))?;
|
||||
assert_eq!(
|
||||
serde_json::from_str::<serde_json::Value>(&main_read_output)?,
|
||||
json!({
|
||||
"resource": SKILL_MAIN_PROMPT_URI,
|
||||
"contents": SKILL_CONTENTS,
|
||||
"next_cursor": null,
|
||||
})
|
||||
);
|
||||
|
||||
let list_output = requests[2]
|
||||
.function_call_output_text(SKILLS_LIST_CALL_ID)
|
||||
.ok_or_else(|| anyhow::anyhow!("skills.list output should be sent to the model"))?;
|
||||
assert_eq!(
|
||||
@@ -291,7 +318,7 @@ async fn orchestrator_skill_can_read_referenced_resource_without_an_executor() -
|
||||
})
|
||||
);
|
||||
|
||||
let read_output = requests[2]
|
||||
let read_output = requests[3]
|
||||
.function_call_output_text(SKILLS_READ_CALL_ID)
|
||||
.ok_or_else(|| anyhow::anyhow!("skills.read output should be sent to the model"))?;
|
||||
assert_eq!(
|
||||
@@ -302,7 +329,7 @@ async fn orchestrator_skill_can_read_referenced_resource_without_an_executor() -
|
||||
"next_cursor": null,
|
||||
})
|
||||
);
|
||||
let repeated_read_output = requests[3]
|
||||
let repeated_read_output = requests[4]
|
||||
.function_call_output_text(SKILLS_READ_AGAIN_CALL_ID)
|
||||
.ok_or_else(|| {
|
||||
anyhow::anyhow!("repeated skills.read output should be sent to the model")
|
||||
@@ -348,7 +375,16 @@ async fn orchestrator_skill_can_read_referenced_resource_without_an_executor() -
|
||||
.await??;
|
||||
|
||||
let requests = response_mock.requests();
|
||||
assert_eq!(requests.len(), 5);
|
||||
assert_eq!(requests.len(), 6);
|
||||
let skill_fragments = requests[5]
|
||||
.message_input_texts("user")
|
||||
.into_iter()
|
||||
.filter(|text| text.starts_with("<skill>"))
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(1, skill_fragments.len());
|
||||
assert!(skill_fragments[0].contains(&format!("<name>{SKILL_NAME}</name>")));
|
||||
assert!(skill_fragments[0].contains(SKILL_MARKER));
|
||||
assert!(skill_fragments[0].contains(SKILL_REFERENCE_URI));
|
||||
assert_eq!(
|
||||
ResourceAppsMcpCallCounts {
|
||||
list_resources: 6,
|
||||
|
||||
@@ -828,8 +828,10 @@ async fn explicit_only_orchestrator_skill_is_hidden_but_can_be_invoked() -> Resu
|
||||
"skills",
|
||||
"read",
|
||||
&json!({
|
||||
"authority": { "kind": "orchestrator" },
|
||||
"package": SKILL_PACKAGE,
|
||||
"authority": {
|
||||
"kind": "orchestrator",
|
||||
},
|
||||
"resource": REFERENCED_RESOURCE,
|
||||
})
|
||||
.to_string(),
|
||||
@@ -1076,7 +1078,7 @@ async fn production_turn_aliases_discovered_singleton_orchestrator_root() -> Res
|
||||
assert!(
|
||||
developer_text.lines().any(|line| {
|
||||
line.starts_with("- demo:search:")
|
||||
&& line.ends_with("(orchestrator resource: o0/search)")
|
||||
&& line.ends_with("(orchestrator package: o0/search)")
|
||||
}),
|
||||
"model request should include the aliased orchestrator skill: {developer_text}"
|
||||
);
|
||||
@@ -1267,14 +1269,14 @@ async fn production_turn_aliases_combined_skill_catalogs_under_shared_budget() -
|
||||
"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 (source, alias, prefix, suffix) in [
|
||||
("environment resource", "e0", "executor", "/SKILL.md"),
|
||||
("orchestrator package", "o0", "orchestrator", ""),
|
||||
("file", "r0", "host", "/SKILL.md"),
|
||||
] {
|
||||
for name in ["search", "review", "summarize"] {
|
||||
assert!(
|
||||
developer_text.contains(&format!("{source}: {alias}/{prefix}-{name}/SKILL.md")),
|
||||
developer_text.contains(&format!("{source}: {alias}/{prefix}-{name}{suffix}")),
|
||||
"model request should retain the aliased {prefix}-{name} skill: {developer_text}"
|
||||
);
|
||||
}
|
||||
@@ -2021,8 +2023,9 @@ async fn production_turn_keeps_orchestrator_world_state_incremental_across_turns
|
||||
|
||||
let requests = response.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
let expected_line =
|
||||
format!("- {skill_name}: {skill_description} (orchestrator resource: {skill_resource})");
|
||||
let expected_line = format!(
|
||||
"- {skill_name}: {skill_description} (orchestrator package: orchestrator/orchestrator-search)"
|
||||
);
|
||||
for (index, request) in requests.iter().enumerate() {
|
||||
let developer_texts = request.message_input_texts("developer");
|
||||
let occurrences = developer_texts
|
||||
@@ -2033,6 +2036,9 @@ 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")
|
||||
}));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -1,15 +1,15 @@
|
||||
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_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_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.
|
||||
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.
|
||||
- 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.
|
||||
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.
|
||||
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.
|
||||
@@ -90,6 +90,15 @@ 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: "))
|
||||
{
|
||||
lines.push(
|
||||
"Read an orchestrator package directly with `skills.read({\"package\":\"<package>\"})` without calling `skills.list`; expand its root alias first when present."
|
||||
.to_string(),
|
||||
);
|
||||
}
|
||||
lines.push("### Available skills".to_string());
|
||||
lines.extend(skill_lines.iter().cloned());
|
||||
|
||||
|
||||
@@ -181,7 +181,13 @@ struct SkillLine<'a> {
|
||||
|
||||
impl<'a> SkillLine<'a> {
|
||||
fn new(entry: &'a SkillCatalogEntry, policy: SkillCatalogRenderPolicy) -> Self {
|
||||
Self::with_locator(entry, policy, entry.rendered_path().to_string())
|
||||
let locator = match &entry.authority.kind {
|
||||
SkillSourceKind::Orchestrator => entry.id.0.as_str(),
|
||||
SkillSourceKind::Host | SkillSourceKind::Executor | SkillSourceKind::Custom(_) => {
|
||||
entry.rendered_path()
|
||||
}
|
||||
};
|
||||
Self::with_locator(entry, policy, locator.to_string())
|
||||
}
|
||||
|
||||
fn with_locator(
|
||||
@@ -197,7 +203,7 @@ impl<'a> SkillLine<'a> {
|
||||
locator_kind: match &entry.authority.kind {
|
||||
SkillSourceKind::Host => "file",
|
||||
SkillSourceKind::Executor => "environment resource",
|
||||
SkillSourceKind::Orchestrator => "orchestrator resource",
|
||||
SkillSourceKind::Orchestrator => "orchestrator package",
|
||||
SkillSourceKind::Custom(_) => "custom resource",
|
||||
},
|
||||
}
|
||||
@@ -1071,12 +1077,18 @@ 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()
|
||||
}
|
||||
};
|
||||
if entry.alias_root().is_none() {
|
||||
return entry.rendered_path().to_string();
|
||||
return locator.to_string();
|
||||
}
|
||||
let locator = entry.rendered_path().replace('\\', "/");
|
||||
plan.shorten(&locator)
|
||||
.unwrap_or_else(|| entry.rendered_path().to_string())
|
||||
let normalized_locator = locator.replace('\\', "/");
|
||||
plan.shorten(&normalized_locator)
|
||||
.unwrap_or_else(|| locator.to_string())
|
||||
}
|
||||
|
||||
fn aliased_metadata_overhead_cost(
|
||||
|
||||
@@ -236,18 +236,18 @@ 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 resource` locators are opaque non-filesystem resources, 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, `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.
|
||||
### 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.
|
||||
- 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.
|
||||
- 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.
|
||||
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.
|
||||
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.
|
||||
@@ -663,7 +663,7 @@ fn mixed_catalogs_alias_all_skill_sources_under_budget_pressure() {
|
||||
orchestrator
|
||||
.skill_lines
|
||||
.iter()
|
||||
.any(|line| line.contains("(orchestrator resource: o0/orchestrator-0)"))
|
||||
.any(|line| line.contains("(orchestrator package: o0/orchestrator-0)"))
|
||||
);
|
||||
assert!(
|
||||
host.skill_lines
|
||||
|
||||
@@ -71,7 +71,7 @@ impl ToolExecutor<ToolCall> for ListTool {
|
||||
fn spec(&self) -> ToolSpec {
|
||||
skill_function_tool::<ListArgs, ListResponse>(
|
||||
TOOL_NAME,
|
||||
"List skills owned by the requested authority. Returns the exact authority, package, and main_resource values required by skills.read. Pass next_cursor back as cursor to continue.",
|
||||
"List skills owned by the requested authority. Returns each skill's authority, package, and main_resource. Pass the package to skills.read, and pass next_cursor back as cursor to continue.",
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -161,18 +161,6 @@ impl SkillToolAuthority {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn matches(&self, authority: &SkillAuthority) -> bool {
|
||||
match self {
|
||||
Self::Orchestrator => {
|
||||
authority.kind == SkillSourceKind::Orchestrator
|
||||
&& authority.id == CODEX_APPS_MCP_SERVER_NAME
|
||||
}
|
||||
Self::Executor { id } => {
|
||||
authority.kind == SkillSourceKind::Executor && authority.id == *id
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn skill_tool_name(name: &str) -> ToolName {
|
||||
|
||||
@@ -27,11 +27,9 @@ const TOOL_NAME: &str = "read";
|
||||
const MAX_READ_RESPONSE_BYTES: usize = 512 * 1024;
|
||||
|
||||
#[derive(Deserialize, JsonSchema)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
struct ReadArgs {
|
||||
authority: SkillToolAuthority,
|
||||
package: String,
|
||||
resource: String,
|
||||
resource: Option<String>,
|
||||
cursor: Option<String>,
|
||||
}
|
||||
|
||||
@@ -56,39 +54,48 @@ 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 authority and package from skills.list or an explicitly selected skill's resource_access metadata, plus its main_resource or a referenced resource beneath that package. Pass next_cursor back as cursor to continue.",
|
||||
"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.",
|
||||
)
|
||||
}
|
||||
|
||||
fn handle(&self, call: ToolCall) -> ToolExecutorFuture<'_> {
|
||||
Box::pin(async move {
|
||||
let args: ReadArgs = parse_args(&call)?;
|
||||
if let SkillToolAuthority::Executor { id } = &args.authority {
|
||||
validate_handle("authority.id", id, MAX_HANDLE_BYTES)?;
|
||||
}
|
||||
validate_handle("package", &args.package, MAX_HANDLE_BYTES)?;
|
||||
validate_handle("resource", &args.resource, MAX_HANDLE_BYTES)?;
|
||||
if let Some(resource) = args.resource.as_deref() {
|
||||
validate_handle("resource", resource, MAX_HANDLE_BYTES)?;
|
||||
}
|
||||
|
||||
let output_authority = args.authority.selector();
|
||||
let catalog = self.context.catalog(&call.turn_id, output_authority).await;
|
||||
let Some(skill_entry) = catalog.entries.iter().find(|entry| {
|
||||
entry.enabled
|
||||
&& args.authority.matches(&entry.authority)
|
||||
&& entry.id.0 == args.package
|
||||
}) else {
|
||||
let mut selected_skill = None;
|
||||
for selector in [
|
||||
super::SkillToolAuthoritySelector::Orchestrator,
|
||||
super::SkillToolAuthoritySelector::Executor,
|
||||
] {
|
||||
let catalog = self.context.catalog(&call.turn_id, selector).await;
|
||||
if let Some(entry) = catalog.entries.into_iter().find(|entry| {
|
||||
entry.enabled
|
||||
&& entry.id.0 == args.package
|
||||
&& SkillToolAuthority::from_authority(&entry.authority)
|
||||
.is_some_and(|authority| authority.selector() == selector)
|
||||
}) {
|
||||
selected_skill = Some((entry, selector));
|
||||
break;
|
||||
}
|
||||
}
|
||||
let Some((skill_entry, output_authority)) = selected_skill else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"skill package is not available from the requested authority".to_string(),
|
||||
"skill package is not available".to_string(),
|
||||
));
|
||||
};
|
||||
let authority = skill_entry.authority.clone();
|
||||
let package = skill_entry.id.clone();
|
||||
let main_prompt = skill_entry.main_prompt.clone();
|
||||
let requested_resource = if args.resource == main_prompt.as_str() {
|
||||
main_prompt.clone()
|
||||
} else {
|
||||
main_prompt
|
||||
.bind_environment_package_resource(&package, args.resource.clone())
|
||||
.unwrap_or_else(|| SkillResourceId::new(args.resource))
|
||||
let requested_resource = match args.resource {
|
||||
None => main_prompt.clone(),
|
||||
Some(resource) if resource == main_prompt.as_str() => main_prompt.clone(),
|
||||
Some(resource) => main_prompt
|
||||
.bind_environment_package_resource(&package, resource.clone())
|
||||
.unwrap_or_else(|| SkillResourceId::new(resource)),
|
||||
};
|
||||
let resolved_executor_roots = self
|
||||
.context
|
||||
|
||||
@@ -75,7 +75,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 resource` locators are opaque non-filesystem resources, 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, `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 DEMO_SKILL_CONTENTS: &str =
|
||||
"---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n\nUse the demo skill.\n";
|
||||
|
||||
@@ -549,7 +549,7 @@ async fn executor_orchestrator_and_host_share_catalog_world_state_flow() -> Test
|
||||
),
|
||||
(
|
||||
"orchestrator_skills",
|
||||
"- orchestrator-skill: Fix lint errors. (orchestrator resource: skill://orchestrator/orchestrator-skill/SKILL.md)",
|
||||
"- orchestrator-skill: Fix lint errors. (orchestrator package: orchestrator/orchestrator-skill)",
|
||||
),
|
||||
(
|
||||
"host_skills",
|
||||
@@ -1186,18 +1186,20 @@ async fn moderate_budget_pressure_keeps_every_catalog_entry() -> TestResult {
|
||||
let (executor, orchestrator) =
|
||||
skill_world_state_fragments(®istry, &session_store, &thread_store, "turn-1").await?;
|
||||
let description_lengths = [
|
||||
("executor", "environment", executor.body()),
|
||||
("orchestrator", "orchestrator", orchestrator.body()),
|
||||
("executor", executor.body()),
|
||||
("orchestrator", orchestrator.body()),
|
||||
]
|
||||
.into_iter()
|
||||
.flat_map(|(source, resource_kind, rendered)| {
|
||||
(0..5).map(move |index| (source, resource_kind, rendered, index))
|
||||
})
|
||||
.map(|(source, resource_kind, rendered, index)| {
|
||||
.flat_map(|(source, rendered)| (0..5).map(move |index| (source, rendered, index)))
|
||||
.map(|(source, rendered, index)| {
|
||||
let name = format!("{source}-skill-{index:02}");
|
||||
let package_id = format!("{source}/{name}");
|
||||
let line_prefix = format!("- {name}: ");
|
||||
let line_suffix = format!(" ({resource_kind} resource: skill://{package_id}/SKILL.md)");
|
||||
let line_suffix = if source == "executor" {
|
||||
format!(" (environment resource: skill://{package_id}/SKILL.md)")
|
||||
} else {
|
||||
format!(" (orchestrator package: {package_id})")
|
||||
};
|
||||
rendered
|
||||
.lines()
|
||||
.find_map(|line| {
|
||||
@@ -1320,7 +1322,7 @@ async fn extreme_budget_pressure_removes_descriptions_before_omitting_entries()
|
||||
assert!(
|
||||
orchestrator
|
||||
.body()
|
||||
.contains("- orchestrator-skill-000: (orchestrator resource:")
|
||||
.contains("- orchestrator-skill-000: (orchestrator package:")
|
||||
);
|
||||
assert!(!orchestrator.body().contains("- orchestrator-skill-159:"));
|
||||
for rendered in [executor.body(), orchestrator.body()] {
|
||||
|
||||
Reference in New Issue
Block a user