Enable resource reads for explicit executor skills (#35198)

## Why

Executor skills that disallow implicit invocation are omitted from `skills.list`, so an explicitly selected skill did not provide the authority and package identifiers needed to read its referenced resources.

## What changed

- Add `resource_access` metadata to injected instructions for explicit-only executor skills.
- Teach the `skills.read` description to accept that metadata as well as values returned by `skills.list`.
- Keep discoverable executor skill instructions unchanged.

## Testing

Add an app-server integration scenario that selects an explicit-only executor skill, confirms it remains absent from `skills.list`, and reads a referenced file through `skills.read`.

GitOrigin-RevId: 97d3b93d1618c013363788452303c8a31e32cc56

codex-rs/app-server/tests/suite/v2/executor_skills.rs

@@ -28,8 +28,23 @@ const SKILL_MARKER: &str = "EXECUTOR_SKILL_BODY_MARKER";
 const LOCAL_SKILL_MARKER: &str = "LOCAL_SKILL_BODY_MARKER";
 const REFERENCE_MARKER: &str = "EXECUTOR_SKILL_REFERENCE_MARKER";
 
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+enum ExecutorSkillScenario {
+    VisibleWithBudgetWarning,
+    ExplicitOnly,
+}
+
 #[tokio::test]
 async fn selected_executor_root_exposes_plugin_skill_and_forwards_budget_warning() -> Result<()> {
+    exercise_executor_skill(ExecutorSkillScenario::VisibleWithBudgetWarning).await
+}
+
+#[tokio::test]
+async fn explicit_executor_skill_can_read_referenced_file() -> Result<()> {
+    exercise_executor_skill(ExecutorSkillScenario::ExplicitOnly).await
+}
+
+async fn exercise_executor_skill(scenario: ExecutorSkillScenario) -> Result<()> {
     let server = responses::start_mock_server().await;
     let codex_home = TempDir::new()?;
     std::fs::write(
@@ -71,9 +86,10 @@ stream_max_retries = 0
     let plugin_dir = auto_env.selection().cwd.join("plugin")?;
     let manifest_dir = plugin_dir.join(".codex-plugin")?;
     let skill_dir = plugin_dir.join("skills/deploy")?;
+    let agents_dir = skill_dir.join("agents")?;
     let reference_dir = skill_dir.join("references")?;
     let file_system = auto_env.environment().get_filesystem();
-    for directory in [&manifest_dir, &reference_dir] {
+    for directory in [&manifest_dir, &agents_dir, &reference_dir] {
         file_system
             .create_directory(
                 directory,
@@ -84,8 +100,14 @@ stream_max_retries = 0
     }
     let manifest_path = manifest_dir.join("plugin.json")?;
     let skill_path = skill_dir.join("SKILL.md")?;
+    let openai_yaml_path = agents_dir.join("openai.yaml")?;
     let reference_path = reference_dir.join("details.md")?;
-    let reference_contents = format!("{REFERENCE_MARKER}\n{}", "x".repeat(600 * 1024));
+    let reference_size = match scenario {
+        ExecutorSkillScenario::VisibleWithBudgetWarning => 600 * 1024,
+        ExecutorSkillScenario::ExplicitOnly => 40 * 1024,
+    };
+    let allow_implicit_invocation = scenario == ExecutorSkillScenario::VisibleWithBudgetWarning;
+    let reference_contents = format!("{REFERENCE_MARKER}\n{}", "x".repeat(reference_size));
     tokio::try_join!(
         file_system.write_file(
             &manifest_path,
@@ -100,43 +122,53 @@ stream_max_retries = 0
             .into_bytes(),
             /*sandbox*/ None,
         ),
+        file_system.write_file(
+            &openai_yaml_path,
+            format!(
+                "policy:\n  allow_implicit_invocation: {allow_implicit_invocation}\n"
+            )
+            .into_bytes(),
+            /*sandbox*/ None,
+        ),
         file_system.write_file(
             &reference_path,
             reference_contents.into_bytes(),
             /*sandbox*/ None,
         ),
     )?;
-    futures::stream::iter(0..200)
-        .map(|index| {
-            let file_system = file_system.clone();
-            let plugin_dir = plugin_dir.clone();
-            async move {
-                let relative = format!("skills/skill-{index:03}");
-                let skill_dir = plugin_dir.join(&relative)?;
-                file_system
-                    .create_directory(
-                        &skill_dir,
-                        CreateDirectoryOptions { recursive: true },
-                        /*sandbox*/ None,
-                    )
-                    .await?;
-                file_system
-                    .write_file(
-                        &skill_dir.join("SKILL.md")?,
-                        format!(
-                            "---\nname: skill-{index:03}\ndescription: {}\n---\n",
-                            "x".repeat(1_025)
+    if scenario == ExecutorSkillScenario::VisibleWithBudgetWarning {
+        futures::stream::iter(0..200)
+            .map(|index| {
+                let file_system = file_system.clone();
+                let plugin_dir = plugin_dir.clone();
+                async move {
+                    let relative = format!("skills/skill-{index:03}");
+                    let skill_dir = plugin_dir.join(&relative)?;
+                    file_system
+                        .create_directory(
+                            &skill_dir,
+                            CreateDirectoryOptions { recursive: true },
+                            /*sandbox*/ None,
                         )
-                        .into_bytes(),
-                        /*sandbox*/ None,
-                    )
-                    .await?;
-                Ok::<(), anyhow::Error>(())
-            }
-        })
-        .buffer_unordered(16)
-        .try_collect::<Vec<_>>()
-        .await?;
+                        .await?;
+                    file_system
+                        .write_file(
+                            &skill_dir.join("SKILL.md")?,
+                            format!(
+                                "---\nname: skill-{index:03}\ndescription: {}\n---\n",
+                                "x".repeat(1_025)
+                            )
+                            .into_bytes(),
+                            /*sandbox*/ None,
+                        )
+                        .await?;
+                    Ok::<(), anyhow::Error>(())
+                }
+            })
+            .buffer_unordered(16)
+            .try_collect::<Vec<_>>()
+            .await?;
+    }
 
     let authority_id = "demo-plugin@1";
     let locator = |path: &PathUri| {
@@ -231,24 +263,26 @@ stream_max_retries = 0
         app_server.read_stream_until_response_message(RequestId::Integer(request_id)),
     )
     .await??;
-    let warning = timeout(READ_TIMEOUT, async {
-        loop {
-            let warning: WarningNotification = app_server.read_notification("warning").await?;
-            if warning
+    if scenario == ExecutorSkillScenario::VisibleWithBudgetWarning {
+        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.")
+                {
+                    return Ok::<WarningNotification, anyhow::Error>(warning);
+                }
+            }
+        })
+        .await??;
+        assert_eq!(warning.thread_id, Some(thread_id));
+        assert!(
+            warning
                 .message
                 .starts_with("Exceeded skills context budget.")
-            {
-                return Ok::<WarningNotification, anyhow::Error>(warning);
-            }
-        }
-    })
-    .await??;
-    assert_eq!(warning.thread_id, Some(thread_id));
-    assert!(
-        warning
-            .message
-            .starts_with("Exceeded skills context budget.")
-    );
+        );
+    }
     timeout(
         READ_TIMEOUT,
         app_server.read_stream_until_notification_message("turn/completed"),
@@ -275,26 +309,54 @@ stream_max_retries = 0
     assert!(skill_fragment.contains(&format!("<name>{SKILL_NAME}</name>")));
     assert!(skill_fragment.contains(SKILL_MARKER));
     assert!(!skill_fragment.contains(LOCAL_SKILL_MARKER));
+    match scenario {
+        ExecutorSkillScenario::VisibleWithBudgetWarning => {
+            assert!(!skill_fragment.contains("<resource_access>"));
+        }
+        ExecutorSkillScenario::ExplicitOnly => {
+            let resource_access = skill_fragment
+                .split_once("<resource_access>")
+                .and_then(|(_, rest)| rest.split_once("</resource_access>"))
+                .map(|(metadata, _)| serde_json::from_str::<serde_json::Value>(metadata))
+                .transpose()?
+                .expect("explicit executor skill should include resource access metadata");
+            assert_eq!(
+                resource_access,
+                json!({
+                    "authority": {"kind": "executor", "id": authority_id},
+                    "package": package,
+                    "main_resource": main_resource,
+                })
+            );
+        }
+    }
     let list_output = serde_json::from_str::<serde_json::Value>(
         &requests[1]
             .function_call_output_text("list")
             .expect("skills.list output"),
     )?;
-    let deploy_skill = list_output["skills"]
-        .as_array()
-        .and_then(|skills| skills.iter().find(|skill| skill["name"] == SKILL_NAME))
-        .expect("skills.list should include the selected executor skill");
-    assert_eq!(
-        deploy_skill,
-        &json!({
-            "authority": {"kind": "executor", "id": authority_id},
-            "package": package,
-            "name": SKILL_NAME,
-            "description": "Deploy through the executor.",
-            "main_resource": main_resource,
-        })
-    );
-    assert!(list_output["next_cursor"].is_string());
+    match scenario {
+        ExecutorSkillScenario::VisibleWithBudgetWarning => {
+            let deploy_skill = list_output["skills"]
+                .as_array()
+                .and_then(|skills| skills.iter().find(|skill| skill["name"] == SKILL_NAME))
+                .expect("skills.list should include the selected executor skill");
+            assert_eq!(
+                deploy_skill,
+                &json!({
+                    "authority": {"kind": "executor", "id": authority_id},
+                    "package": package,
+                    "name": SKILL_NAME,
+                    "description": "Deploy through the executor.",
+                    "main_resource": main_resource,
+                })
+            );
+            assert!(list_output["next_cursor"].is_string());
+        }
+        ExecutorSkillScenario::ExplicitOnly => {
+            assert_eq!(list_output["skills"], json!([]));
+        }
+    }
     assert!(
         requests[2]
             .function_call_output_text("main")
@@ -311,7 +373,14 @@ stream_max_retries = 0
             .as_str()
             .is_some_and(|contents| contents.contains(REFERENCE_MARKER))
     );
-    assert!(reference_output["next_cursor"].is_string());
+    match scenario {
+        ExecutorSkillScenario::VisibleWithBudgetWarning => {
+            assert!(reference_output["next_cursor"].is_string());
+        }
+        ExecutorSkillScenario::ExplicitOnly => {
+            assert!(reference_output["next_cursor"].is_null());
+        }
+    }
 
     Ok(())
 }

codex-rs/ext/skills/src/extension.rs

@@ -36,6 +36,7 @@ use crate::catalog::SkillCatalogEntry;
 use crate::catalog::SkillReadResult;
 use crate::catalog::SkillSourceKind;
 use crate::fragments::AvailableSkillsInstructions;
+use crate::fragments::ExecutorSkillResourceAccess;
 use crate::fragments::SkillInstructions;
 use crate::provider::HostSkillProvider;
 use crate::provider::SkillListQuery;
@@ -464,6 +465,13 @@ where
                             )
                             .0,
                             contents,
+                            executor_resource_access: (!entry.prompt_visible
+                                && entry.authority.kind == SkillSourceKind::Executor)
+                                .then(|| ExecutorSkillResourceAccess {
+                                    authority_id: entry.authority.id.clone(),
+                                    package: entry.id.0.clone(),
+                                    main_resource: entry.main_prompt.as_str().to_string(),
+                                }),
                         };
                         fragments.push(Box::new(fragment));
                         main_prompts_injected = true;

codex-rs/ext/skills/src/fragments.rs

@@ -77,6 +77,14 @@ pub(crate) struct SkillInstructions {
     pub(crate) name: String,
     pub(crate) path: String,
     pub(crate) contents: String,
+    pub(crate) executor_resource_access: Option<ExecutorSkillResourceAccess>,
+}
+
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub(crate) struct ExecutorSkillResourceAccess {
+    pub(crate) authority_id: String,
+    pub(crate) package: String,
+    pub(crate) main_resource: String,
 }
 
 impl ContextualUserFragment for SkillInstructions {
@@ -96,6 +104,21 @@ impl ContextualUserFragment for SkillInstructions {
         let name = &self.name;
         let path = &self.path;
         let contents = &self.contents;
-        format!("\n<name>{name}</name>\n<path>{path}</path>\n{contents}\n")
+        let resource_access = self
+            .executor_resource_access
+            .as_ref()
+            .map(|access| {
+                let metadata = serde_json::json!({
+                    "authority": {
+                        "kind": "executor",
+                        "id": access.authority_id,
+                    },
+                    "package": access.package,
+                    "main_resource": access.main_resource,
+                });
+                format!("\n<resource_access>{metadata}</resource_access>")
+            })
+            .unwrap_or_default();
+        format!("\n<name>{name}</name>\n<path>{path}</path>{resource_access}\n{contents}\n")
     }
 }

codex-rs/ext/skills/src/tools/read.rs

@@ -56,7 +56,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 authority and package from skills.list, 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 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.",
         )
     }