codex: address PR review feedback (#16274)

codex-rs/core/src/codex.rs

@@ -24,6 +24,9 @@ use crate::compact::run_inline_auto_compact_task;
 use crate::compact::should_use_remote_compact_task;
 use crate::compact_remote::run_inline_remote_auto_compact_task;
 use crate::config::ManagedFeatures;
+use crate::config_loader::CloudRequirementsLoader;
+use crate::config_loader::LoaderOverrides;
+use crate::config_loader::load_config_layers_state;
 use crate::connectors;
 use crate::exec_policy::ExecPolicyManager;
 #[cfg(test)]
@@ -35,6 +38,7 @@ use crate::parse_turn_item;
 use crate::path_utils::normalize_for_native_workdir;
 use crate::personalities::PersonalityCatalog;
 use crate::personalities::catalog_for_config;
+use crate::personalities::catalog_from_layer_stack;
 use crate::realtime_conversation::RealtimeConversationManager;
 use crate::realtime_conversation::handle_audio as handle_realtime_conversation_audio;
 use crate::realtime_conversation::handle_close as handle_realtime_conversation_close;
@@ -2625,7 +2629,9 @@ impl Session {
         };
         let shell = self.user_shell();
         let exec_policy = self.services.exec_policy.current();
-        let personality_catalog = self.personality_catalog_for_turn_context(current_context);
+        let personality_catalog = self
+            .personality_catalog_for_turn_context(current_context)
+            .await;
         crate::context_manager::updates::build_settings_update_items(
             reference_context_item,
             previous_turn_settings.as_ref(),
@@ -2637,7 +2643,7 @@ impl Session {
         )
     }
 
-    fn personality_catalog_for_turn_context(
+    async fn personality_catalog_for_turn_context(
         &self,
         turn_context: &TurnContext,
     ) -> PersonalityCatalog {
@@ -2646,7 +2652,25 @@ impl Session {
             .as_ref()
             .is_some_and(|personality| !personality.is_builtin())
         {
-            catalog_for_config(turn_context.config.as_ref())
+            match load_config_layers_state(
+                &turn_context.config.codex_home,
+                Some(turn_context.cwd.clone()),
+                &[],
+                LoaderOverrides::default(),
+                CloudRequirementsLoader::default(),
+            )
+            .await
+            {
+                Ok(config_layer_stack) => catalog_from_layer_stack(&config_layer_stack),
+                Err(err) => {
+                    warn!(
+                        ?turn_context.cwd,
+                        error = %err,
+                        "failed to reload personality config layers; falling back to current config"
+                    );
+                    catalog_for_config(turn_context.config.as_ref())
+                }
+            }
         } else {
             self.services.personality_catalog.as_ref().clone()
         }
@@ -3636,7 +3660,9 @@ impl Session {
             && let Some(personality) = turn_context.personality.clone()
         {
             let model_info = turn_context.model_info.clone();
-            let personality_catalog = self.personality_catalog_for_turn_context(turn_context);
+            let personality_catalog = self
+                .personality_catalog_for_turn_context(turn_context)
+                .await;
             let has_baked_personality = personality.is_builtin()
                 && model_info.supports_personality()
                 && base_instructions

codex-rs/core/src/codex_tests.rs

@@ -3917,6 +3917,55 @@ async fn build_settings_update_items_reload_custom_personality_catalog_for_runni
     );
 }
 
+#[tokio::test]
+async fn build_settings_update_items_reload_custom_personality_catalog_for_cwd_override() {
+    let (session, previous_context) = make_session_and_context().await;
+    let repo = tempfile::tempdir().expect("create repo");
+    let personalities_dir = repo.path().join(".codex/personalities");
+    let personality_name = "repo-captain";
+    let personality_body = "Ahoy from the overridden repo.";
+    std::fs::create_dir_all(&personalities_dir).expect("create repo personalities dir");
+    std::fs::write(
+        personalities_dir.join("repo-captain.md"),
+        format!(
+            "---\nname: {personality_name}\ndescription: repo personality\n---\n\n{personality_body}\n"
+        ),
+    )
+    .expect("write repo personality");
+
+    let mut current_context = previous_context
+        .with_model(
+            previous_context.model_info.slug.clone(),
+            &session.services.models_manager,
+        )
+        .await;
+    let overridden_cwd: codex_utils_absolute_path::AbsolutePathBuf = repo
+        .path()
+        .to_path_buf()
+        .try_into()
+        .expect("override cwd should be absolute");
+    let mut overridden_config = (*current_context.config).clone();
+    overridden_config.cwd = overridden_cwd.clone();
+    current_context.cwd = overridden_cwd;
+    current_context.config = Arc::new(overridden_config);
+    current_context.personality = Some(Personality::from(personality_name));
+
+    let update_items = session
+        .build_settings_update_items(
+            Some(&previous_context.to_turn_context_item()),
+            &current_context,
+        )
+        .await;
+
+    let developer_texts = developer_input_texts(&update_items);
+    assert!(
+        developer_texts
+            .iter()
+            .any(|text| text.contains(personality_body)),
+        "expected repo personality after cwd override, got {developer_texts:?}"
+    );
+}
+
 #[tokio::test]
 async fn build_settings_update_items_uses_previous_turn_settings_for_realtime_end() {
     let (session, previous_context) = make_session_and_context().await;

codex-rs/core/src/personalities.rs

@@ -235,8 +235,13 @@ fn discover_personalities_under_root(
         }
     };
 
-    for entry in entries.flatten() {
-        let path = entry.path();
+    let mut paths = entries
+        .filter_map(Result::ok)
+        .map(|entry| entry.path())
+        .collect::<Vec<_>>();
+    paths.sort();
+
+    for path in paths {
         if !path.is_file()
             || !path
                 .extension()
@@ -484,4 +489,40 @@ mod tests {
             vec!["friendly", "none", "pragmatic", "alpha", "zebra"]
         );
     }
+
+    #[tokio::test]
+    async fn duplicate_personality_names_in_one_directory_are_resolved_by_path_order() {
+        let temp = TempDir::new().unwrap();
+        let codex_home = temp.path().join(".codex-home");
+        let repo_root = temp.path().join("repo");
+        let repo_personalities = repo_root.join(".codex/personalities");
+        fs::create_dir_all(&repo_personalities).unwrap();
+        fs::create_dir_all(&codex_home).unwrap();
+
+        fs::write(
+            repo_personalities.join("a-first.md"),
+            "---\nname: duped\ndescription: first\n---\n\nfirst body\n",
+        )
+        .unwrap();
+        fs::write(
+            repo_personalities.join("z-second.md"),
+            "---\nname: duped\ndescription: second\n---\n\nsecond body\n",
+        )
+        .unwrap();
+
+        let stack = load_config_layers_state(
+            &codex_home,
+            Some(repo_root.clone().try_into().unwrap()),
+            &[],
+            LoaderOverrides::default(),
+            CloudRequirementsLoader::default(),
+        )
+        .await
+        .unwrap();
+
+        let catalog = catalog_from_layer_stack(&stack);
+        let personality = catalog.get(&Personality::from("duped")).unwrap();
+        assert_eq!(personality.description, "first");
+        assert_eq!(personality.instructions.as_deref(), Some("first body"));
+    }
 }