fix: always use cache_by_config for skills cache.

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

@@ -147,7 +147,7 @@ async fn skills_list_uses_cached_result_until_force_reload() -> Result<()> {
     let mut mcp = McpProcess::new(codex_home.path()).await?;
     timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
 
-    // Seed the cwd cache first without extra roots.
+    // Seed the config-aware cache first without extra roots.
     let first_request_id = mcp
         .send_skills_list_request(SkillsListParams {
             cwds: vec![cwd.path().to_path_buf()],

codex-rs/core/src/skills/loader.rs

@@ -176,6 +176,7 @@ impl fmt::Display for SkillParseError {
 
 impl Error for SkillParseError {}
 
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub(crate) struct SkillRoot {
     pub(crate) path: PathBuf,
     pub(crate) scope: SkillScope,

codex-rs/core/src/skills/manager.rs

@@ -15,6 +15,7 @@ use tracing::warn;
 use crate::config::Config;
 use crate::config::types::SkillsConfig;
 use crate::config_loader::CloudRequirementsLoader;
+use crate::config_loader::ConfigLayerStack;
 use crate::config_loader::ConfigLayerStackOrdering;
 use crate::config_loader::LoaderOverrides;
 use crate::config_loader::load_config_layers_state;
@@ -30,7 +31,6 @@ use crate::skills::system::uninstall_system_skills;
 pub struct SkillsManager {
     codex_home: PathBuf,
     plugins_manager: Arc<PluginsManager>,
-    cache_by_cwd: RwLock<HashMap<PathBuf, SkillLoadOutcome>>,
     cache_by_config: RwLock<HashMap<ConfigSkillsCacheKey, SkillLoadOutcome>>,
 }
 
@@ -43,7 +43,6 @@ impl SkillsManager {
         let manager = Self {
             codex_home,
             plugins_manager,
-            cache_by_cwd: RwLock::new(HashMap::new()),
             cache_by_config: RwLock::new(HashMap::new()),
         };
         if !bundled_skills_enabled {
@@ -65,18 +64,12 @@ impl SkillsManager {
     pub fn skills_for_config(&self, config: &Config) -> SkillLoadOutcome {
         let roots = self.skill_roots_for_config(config);
         let cache_key = config_skills_cache_key(&roots, &config.config_layer_stack);
-        if let Some(outcome) = self.cached_outcome_for_config(&cache_key) {
-            return outcome;
-        }
-
-        let outcome =
-            finalize_skill_outcome(load_skills_from_roots(roots), &config.config_layer_stack);
-        let mut cache = self
-            .cache_by_config
-            .write()
-            .unwrap_or_else(std::sync::PoisonError::into_inner);
-        cache.insert(cache_key, outcome.clone());
-        outcome
+        self.load_outcome_for_cache_key(
+            cache_key,
+            roots,
+            &config.config_layer_stack,
+            /*force_reload*/ false,
+        )
     }
 
     pub(crate) fn skill_roots_for_config(&self, config: &Config) -> Vec<SkillRoot> {
@@ -93,10 +86,6 @@ impl SkillsManager {
     }
 
     pub async fn skills_for_cwd(&self, cwd: &Path, force_reload: bool) -> SkillLoadOutcome {
-        if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(cwd) {
-            return outcome;
-        }
-
         self.skills_for_cwd_with_extra_user_roots(cwd, force_reload, &[])
             .await
     }
@@ -107,26 +96,89 @@ impl SkillsManager {
         force_reload: bool,
         extra_user_roots: &[PathBuf],
     ) -> SkillLoadOutcome {
-        if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(cwd) {
+        let normalized_extra_user_roots = normalize_extra_user_roots(extra_user_roots);
+        let resolved = match self.resolve_skill_context_for_cwd(cwd, force_reload).await {
+            Ok(resolved) => resolved,
+            Err(outcome) => return outcome,
+        };
+        let cache_key = config_skills_cache_key(&resolved.roots, &resolved.config_layer_stack);
+        let mut load_roots = resolved.roots.clone();
+        load_roots.extend(
+            normalized_extra_user_roots
+                .iter()
+                .cloned()
+                .map(|path| SkillRoot {
+                    path,
+                    scope: SkillScope::User,
+                }),
+        );
+        self.load_outcome_for_cache_key(
+            cache_key,
+            load_roots,
+            &resolved.config_layer_stack,
+            force_reload,
+        )
+    }
+
+    pub fn clear_cache(&self) {
+        let cleared_config = {
+            let mut cache = self
+                .cache_by_config
+                .write()
+                .unwrap_or_else(std::sync::PoisonError::into_inner);
+            let cleared = cache.len();
+            cache.clear();
+            cleared
+        };
+        let cleared = cleared_config;
+        info!("skills cache cleared ({cleared} entries)");
+    }
+
+    fn cached_outcome_for_config(
+        &self,
+        cache_key: &ConfigSkillsCacheKey,
+    ) -> Option<SkillLoadOutcome> {
+        match self.cache_by_config.read() {
+            Ok(cache) => cache.get(cache_key).cloned(),
+            Err(err) => err.into_inner().get(cache_key).cloned(),
+        }
+    }
+
+    fn load_outcome_for_cache_key(
+        &self,
+        cache_key: ConfigSkillsCacheKey,
+        roots: Vec<SkillRoot>,
+        config_layer_stack: &ConfigLayerStack,
+        force_reload: bool,
+    ) -> SkillLoadOutcome {
+        if !force_reload && let Some(outcome) = self.cached_outcome_for_config(&cache_key) {
             return outcome;
         }
-        let normalized_extra_user_roots = normalize_extra_user_roots(extra_user_roots);
 
-        let cwd_abs = match AbsolutePathBuf::try_from(cwd) {
-            Ok(cwd_abs) => cwd_abs,
-            Err(err) => {
-                return SkillLoadOutcome {
-                    errors: vec![crate::skills::model::SkillError {
-                        path: cwd.to_path_buf(),
-                        message: err.to_string(),
-                    }],
-                    ..Default::default()
-                };
-            }
-        };
+        let outcome = finalize_skill_outcome(load_skills_from_roots(roots), config_layer_stack);
+        let mut cache = self
+            .cache_by_config
+            .write()
+            .unwrap_or_else(std::sync::PoisonError::into_inner);
+        cache.insert(cache_key, outcome.clone());
+        outcome
+    }
+
+    async fn resolve_skill_context_for_cwd(
+        &self,
+        cwd: &Path,
+        force_reload: bool,
+    ) -> Result<ResolvedSkillContext, SkillLoadOutcome> {
+        let cwd_abs = AbsolutePathBuf::try_from(cwd).map_err(|err| SkillLoadOutcome {
+            errors: vec![crate::skills::model::SkillError {
+                path: cwd.to_path_buf(),
+                message: err.to_string(),
+            }],
+            ..Default::default()
+        })?;
 
         let cli_overrides: Vec<(String, TomlValue)> = Vec::new();
-        let config_layer_stack = match load_config_layers_state(
+        let config_layer_stack = load_config_layers_state(
             &self.codex_home,
             Some(cwd_abs),
             &cli_overrides,
@@ -134,18 +186,13 @@ impl SkillsManager {
             CloudRequirementsLoader::default(),
         )
         .await
-        {
-            Ok(config_layer_stack) => config_layer_stack,
-            Err(err) => {
-                return SkillLoadOutcome {
-                    errors: vec![crate::skills::model::SkillError {
-                        path: cwd.to_path_buf(),
-                        message: err.to_string(),
-                    }],
-                    ..Default::default()
-                };
-            }
-        };
+        .map_err(|err| SkillLoadOutcome {
+            errors: vec![crate::skills::model::SkillError {
+                path: cwd.to_path_buf(),
+                message: err.to_string(),
+            }],
+            ..Default::default()
+        })?;
 
         let loaded_plugins =
             self.plugins_manager
@@ -158,64 +205,17 @@ impl SkillsManager {
         if !bundled_skills_enabled_from_stack(&config_layer_stack) {
             roots.retain(|root| root.scope != SkillScope::System);
         }
-        roots.extend(
-            normalized_extra_user_roots
-                .iter()
-                .cloned()
-                .map(|path| SkillRoot {
-                    path,
-                    scope: SkillScope::User,
-                }),
-        );
-        let outcome = load_skills_from_roots(roots);
-        let outcome = finalize_skill_outcome(outcome, &config_layer_stack);
-        let mut cache = self
-            .cache_by_cwd
-            .write()
-            .unwrap_or_else(std::sync::PoisonError::into_inner);
-        cache.insert(cwd.to_path_buf(), outcome.clone());
-        outcome
-    }
 
-    pub fn clear_cache(&self) {
-        let cleared_cwd = {
-            let mut cache = self
-                .cache_by_cwd
-                .write()
-                .unwrap_or_else(std::sync::PoisonError::into_inner);
-            let cleared = cache.len();
-            cache.clear();
-            cleared
-        };
-        let cleared_config = {
-            let mut cache = self
-                .cache_by_config
-                .write()
-                .unwrap_or_else(std::sync::PoisonError::into_inner);
-            let cleared = cache.len();
-            cache.clear();
-            cleared
-        };
-        let cleared = cleared_cwd + cleared_config;
-        info!("skills cache cleared ({cleared} entries)");
+        Ok(ResolvedSkillContext {
+            roots,
+            config_layer_stack,
+        })
     }
+}
 
-    fn cached_outcome_for_cwd(&self, cwd: &Path) -> Option<SkillLoadOutcome> {
-        match self.cache_by_cwd.read() {
-            Ok(cache) => cache.get(cwd).cloned(),
-            Err(err) => err.into_inner().get(cwd).cloned(),
-        }
-    }
-
-    fn cached_outcome_for_config(
-        &self,
-        cache_key: &ConfigSkillsCacheKey,
-    ) -> Option<SkillLoadOutcome> {
-        match self.cache_by_config.read() {
-            Ok(cache) => cache.get(cache_key).cloned(),
-            Err(err) => err.into_inner().get(cache_key).cloned(),
-        }
-    }
+struct ResolvedSkillContext {
+    roots: Vec<SkillRoot>,
+    config_layer_stack: ConfigLayerStack,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]

codex-rs/core/src/skills/manager_tests.rs

@@ -69,7 +69,7 @@ async fn skills_for_config_reuses_cache_for_same_effective_config() {
 }
 
 #[tokio::test]
-async fn skills_for_cwd_reuses_cached_entry_even_when_entry_has_extra_roots() {
+async fn skills_for_cwd_reuses_cached_entry_even_when_seeded_with_extra_roots() {
     let codex_home = tempfile::tempdir().expect("tempdir");
     let cwd = tempfile::tempdir().expect("tempdir");
     let extra_root = tempfile::tempdir().expect("tempdir");
@@ -110,13 +110,54 @@ async fn skills_for_cwd_reuses_cached_entry_even_when_entry_has_extra_roots() {
             .any(|skill| skill.scope == SkillScope::System)
     );
 
-    // The cwd-only API returns the current cached entry for this cwd, even when that entry
-    // was produced with extra roots.
+    // A cwd-based lookup with extra roots warms the config-aware cache entry for the same
+    // effective skill configuration, so the cwd API reads back the same result.
     let outcome_without_extra = skills_manager.skills_for_cwd(cwd.path(), false).await;
     assert_eq!(outcome_without_extra.skills, outcome_with_extra.skills);
     assert_eq!(outcome_without_extra.errors, outcome_with_extra.errors);
 }
 
+#[tokio::test]
+async fn skills_for_config_reads_cache_seeded_via_cwd_with_extra_roots() {
+    let codex_home = tempfile::tempdir().expect("tempdir");
+    let cwd = tempfile::tempdir().expect("tempdir");
+    let extra_root = tempfile::tempdir().expect("tempdir");
+
+    let config = ConfigBuilder::default()
+        .codex_home(codex_home.path().to_path_buf())
+        .harness_overrides(ConfigOverrides {
+            cwd: Some(cwd.path().to_path_buf()),
+            ..Default::default()
+        })
+        .build()
+        .await
+        .expect("defaults for test should always succeed");
+
+    let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf()));
+    let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), plugins_manager, true);
+    let _ = skills_manager.skills_for_config(&config);
+
+    write_user_skill(&extra_root, "x", "extra-skill", "from extra root");
+    let extra_root_path = extra_root.path().to_path_buf();
+    let outcome_with_extra = skills_manager
+        .skills_for_cwd_with_extra_user_roots(
+            cwd.path(),
+            true,
+            std::slice::from_ref(&extra_root_path),
+        )
+        .await;
+    assert!(
+        outcome_with_extra
+            .skills
+            .iter()
+            .any(|skill| skill.name == "extra-skill")
+    );
+
+    let outcome_for_config = skills_manager.skills_for_config(&config);
+    assert_eq!(outcome_for_config.skills, outcome_with_extra.skills);
+    assert_eq!(outcome_for_config.errors, outcome_with_extra.errors);
+}
+
 #[tokio::test]
 async fn skills_for_config_excludes_bundled_skills_when_disabled_in_config() {
     let codex_home = tempfile::tempdir().expect("tempdir");
@@ -357,7 +398,7 @@ enabled = false
 
 #[cfg_attr(windows, ignore)]
 #[tokio::test]
-async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill() {
+async fn skills_for_config_ignores_parent_cached_entry_when_session_flags_reenable_skill() {
     let codex_home = tempfile::tempdir().expect("tempdir");
     let cwd = tempfile::tempdir().expect("tempdir");
     let skill_dir = codex_home.path().join("skills").join("demo");