From 6059a80c2062829a07310cd9a513a9ced374d567 Mon Sep 17 00:00:00 2001
From: starr-openai <starr@openai.com>
Date: Thu, 28 May 2026 19:27:03 -0700
Subject: [PATCH] codex: document local skill path choices

---
 codex-rs/core-plugins/src/loader.rs  |  2 ++
 codex-rs/core-plugins/src/manager.rs |  2 ++
 codex-rs/core-skills/src/loader.rs   | 12 ++++++++++++
 3 files changed, 16 insertions(+)

diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs
index 4fc7cfa235..ae71568ae9 100644
--- a/codex-rs/core-plugins/src/loader.rs
+++ b/codex-rs/core-plugins/src/loader.rs
@@ -563,6 +563,8 @@ async fn load_plugin(
         .or_else(|| Some(manifest.name.clone()));
     loaded_plugin.manifest_description = manifest.description.clone();
     loaded_plugin.skill_roots = plugin_skill_roots(&plugin_root, manifest_paths);
+    // TODO: Support multi-env plugin skill loading. Plugin discovery is local-only today, so bind
+    // plugin skill reads to the local environment instead of the selected turn environment.
     let resolved_skills = load_plugin_skills(
         EnvironmentPathRef::local(plugin_root.clone()),
         &loaded_plugin_id,
diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs
index e387607c85..5d4b497951 100644
--- a/codex-rs/core-plugins/src/manager.rs
+++ b/codex-rs/core-plugins/src/manager.rs
@@ -1419,6 +1419,8 @@ impl PluginsManager {
             manifest.interface.clone(),
             marketplace_category,
         );
+        // TODO: Support multi-env plugin skill loading. Marketplace plugins are installed and
+        // scanned locally today, so bind plugin skill reads to the local environment.
         let resolved_skills = load_plugin_skills(
             codex_exec_server::EnvironmentPathRef::local(source_path.clone()),
             &plugin_id,
diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs
index d1d8ab6817..aebfc0be62 100644
--- a/codex-rs/core-skills/src/loader.rs
+++ b/codex-rs/core-skills/src/loader.rs
@@ -236,6 +236,9 @@ pub(crate) async fn skill_roots(
     plugin_skill_roots: Vec<PluginSkillRoot>,
     extra_skill_roots: Vec<AbsolutePathBuf>,
 ) -> Vec<SkillRoot> {
+    // `env_path` owns workspace/repo-relative skill discovery for the selected environment.
+    // `local_file_system` owns client-local system/user/plugin roots and is absent when local exec
+    // is disabled, in which case those local roots intentionally do not load.
     let home_dir =
         home_dir().and_then(|path| AbsolutePathBuf::from_absolute_path_checked(path).ok());
     skill_roots_with_home_dir(
@@ -257,6 +260,8 @@ async fn skill_roots_with_home_dir(
     plugin_skill_roots: Vec<PluginSkillRoot>,
     extra_skill_roots: Vec<AbsolutePathBuf>,
 ) -> Vec<SkillRoot> {
+    // Assemble one precedence-ordered root list from the two authorities above, then dedupe before
+    // any reads happen so downstream loading stays oblivious to local-vs-selected-env routing.
     let mut roots = repo_config_skill_roots(env_path, config_layer_stack);
     roots.extend(local_skill_roots(
         local_file_system,
@@ -277,6 +282,9 @@ fn local_skill_roots(
     plugin_skill_roots: Vec<PluginSkillRoot>,
     extra_skill_roots: Vec<AbsolutePathBuf>,
 ) -> Vec<SkillRoot> {
+    // These roots are absolute paths on the client machine, not paths inside the selected
+    // workspace environment. Bind them to the available local exec filesystem so remote turns
+    // keep loading local installed/bundled/plugin skills without reading those paths remotely.
     let Some(local_file_system) = local_file_system else {
         // Local exec can be disabled. In that case, keep repo/env skill loading intact but skip
         // local absolute roots instead of falling back to the process-global local filesystem.
@@ -366,6 +374,8 @@ fn repo_config_skill_roots(
     env_path: &EnvironmentPathRef,
     config_layer_stack: &ConfigLayerStack,
 ) -> Vec<SkillRoot> {
+    // Project config layers describe workspace-local skill directories. Their absolute paths only
+    // make sense inside the selected environment, so keep them bound to `env_path`.
     config_layer_stack
         .get_layers(
             ConfigLayerStackOrdering::HighestPrecedenceFirst,
@@ -397,6 +407,8 @@ async fn repo_agents_skill_roots(
     env_path: &EnvironmentPathRef,
     config_layer_stack: &ConfigLayerStack,
 ) -> Vec<SkillRoot> {
+    // Discover repo `.agents/skills` folders by walking from the selected environment's project
+    // root to its cwd; this must never consult the local filesystem for remote workspaces.
     let project_root_markers = project_root_markers_from_stack(config_layer_stack);
     let project_root = find_project_root(env_path, &project_root_markers).await;
     let dirs = dirs_between_project_root_and_cwd(env_path.path(), project_root.path());