From 5664a5c07c64ca5eff1dccf51da68255d6f30b80 Mon Sep 17 00:00:00 2001 From: jif Date: Wed, 12 Aug 2026 22:49:30 +0000 Subject: [PATCH] Expose executor skill roots from `skills.read` (#38268) ## Why Executor-backed skills can include bundled scripts, so skill readers need the skill directory in the executor filesystem to locate them. ## What changed - Add `skill_root` to `skills.read` responses for executor-backed skills. - Derive the root from the parent directory of the skill's main resource and omit the field for other skill authorities. ## Testing - Verify executor skill reads return the expected root for both `SKILL.md` and a referenced resource. GitOrigin-RevId: b7035c7e2e62155890795fb508fefe3a1c17805b --- .../tests/suite/v2/executor_skills.rs | 20 ++++++++++++++---- codex-rs/ext/skills/src/tools/read.rs | 21 +++++++++++++++++-- 2 files changed, 35 insertions(+), 6 deletions(-) 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 7b6c421d52..943b1c615a 100644 --- a/codex-rs/app-server/tests/suite/v2/executor_skills.rs +++ b/codex-rs/app-server/tests/suite/v2/executor_skills.rs @@ -531,11 +531,19 @@ stream_max_retries = 0 assert_eq!(list_output["skills"], json!([])); } } - assert!( - requests[2] + let main_output = serde_json::from_str::( + &requests[2] .function_call_output_text("main") - .expect("main skill output") - .contains(SKILL_MARKER) + .expect("main skill output"), + )?; + assert!( + main_output["contents"] + .as_str() + .is_some_and(|contents| contents.contains(SKILL_MARKER)) + ); + assert_eq!( + main_output["skill_root"], + json!(skill_dir.inferred_native_path_string()) ); let reference_output_text = requests[3] .function_call_output_text("reference") @@ -555,6 +563,10 @@ stream_max_retries = 0 .as_str() .is_some_and(|contents| contents.contains(REFERENCE_MARKER)) ); + assert_eq!( + reference_output["skill_root"], + json!(skill_dir.inferred_native_path_string()) + ); match scenario { ExecutorSkillScenario::VisibleWithBudgetWarning => { assert!(reference_output["next_cursor"].is_string()); diff --git a/codex-rs/ext/skills/src/tools/read.rs b/codex-rs/ext/skills/src/tools/read.rs index 00a3d8d459..fc092008bc 100644 --- a/codex-rs/ext/skills/src/tools/read.rs +++ b/codex-rs/ext/skills/src/tools/read.rs @@ -40,6 +40,8 @@ struct ReadArgs { struct ReadResponse { resource: String, contents: String, + #[serde(skip_serializing_if = "Option::is_none")] + skill_root: Option, next_cursor: Option, } @@ -56,7 +58,7 @@ impl ToolExecutor for ReadTool { fn spec(&self) -> ToolSpec { skill_function_tool::( TOOL_NAME, - "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.", + "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. For executor-backed skills, skill_root is the skill's absolute directory in the executor filesystem and can be used to locate bundled scripts. If the package is not provided, use skills.list to find it. Pass next_cursor back as cursor to continue.", ) } @@ -189,7 +191,20 @@ impl ToolExecutor for ReadTool { "skills.read cursor is invalid".to_string(), )); } - let response = page_response(result.resource.as_str(), &result.contents, start)?; + let skill_root = if output_authority == super::SkillToolAuthoritySelector::Executor { + main_prompt + .environment_path() + .and_then(|(_, path)| path.parent()) + .map(|path| path.inferred_native_path_string()) + } else { + None + }; + let response = page_response( + result.resource.as_str(), + &result.contents, + skill_root.as_deref(), + start, + )?; let output = skill_json_output(&response, output_authority)?; if requested_resource == main_prompt @@ -212,11 +227,13 @@ impl ToolExecutor for ReadTool { fn page_response( resource: &str, contents: &str, + skill_root: Option<&str>, start: usize, ) -> Result { let response = |end, next_cursor| ReadResponse { resource: resource.to_string(), contents: contents[start..end].to_string(), + skill_root: skill_root.map(str::to_string), next_cursor, }; let complete = response(contents.len(), None);