Address ordering finding

codex-rs/tui/src/app.rs

@@ -456,13 +456,20 @@ fn rollout_path_is_resumable(rollout_path: &Path) -> bool {
     std::fs::metadata(rollout_path).is_ok_and(|metadata| metadata.is_file() && metadata.len() > 0)
 }
 
-fn errors_for_cwd(cwd: &Path, response: &SkillsListResponse) -> Vec<SkillErrorInfo> {
+fn errors_for_cwd(cwd: &Path, response: &SkillsListResponse) -> Option<Vec<SkillErrorInfo>> {
     response
         .data
         .iter()
         .find(|entry| entry.cwd.as_path() == cwd)
-        .map(|entry| entry.errors.clone())
-        .unwrap_or_default()
+        .map(|entry| {
+            let mut errors = entry.errors.clone();
+            errors.sort_by(|left, right| {
+                left.path
+                    .cmp(&right.path)
+                    .then_with(|| left.message.cmp(&right.message))
+            });
+            errors
+        })
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -512,6 +519,7 @@ pub(crate) struct App {
     status_line_invalid_items_warned: Arc<AtomicBool>,
     // Shared across ChatWidget instances so invalid terminal-title config warnings only emit once.
     terminal_title_invalid_items_warned: Arc<AtomicBool>,
+    skill_load_warnings_by_cwd: HashMap<PathBuf, Vec<SkillErrorInfo>>,
 
     // Esc-backtracking state grouped
     pub(crate) backtrack: crate::app_backtrack::BacktrackState,
@@ -991,6 +999,7 @@ See the Codex keymap documentation for supported actions and examples."
             commit_anim_running: Arc::new(AtomicBool::new(false)),
             status_line_invalid_items_warned: status_line_invalid_items_warned.clone(),
             terminal_title_invalid_items_warned: terminal_title_invalid_items_warned.clone(),
+            skill_load_warnings_by_cwd: HashMap::new(),
             backtrack: BacktrackState::default(),
             backtrack_render_pending: false,
             feedback: feedback.clone(),

codex-rs/tui/src/app/test_support.rs

@@ -39,6 +39,7 @@ pub(super) async fn make_test_app() -> App {
         commit_anim_running: Arc::new(AtomicBool::new(false)),
         status_line_invalid_items_warned: Arc::new(AtomicBool::new(false)),
         terminal_title_invalid_items_warned: Arc::new(AtomicBool::new(false)),
+        skill_load_warnings_by_cwd: HashMap::new(),
         backtrack: BacktrackState::default(),
         backtrack_render_pending: false,
         feedback: codex_feedback::CodexFeedback::new(),

codex-rs/tui/src/app/tests.rs

@@ -2281,6 +2281,75 @@ async fn replay_snapshot_with_pending_request_suppresses_replay_notices() {
     );
 }
 
+#[tokio::test]
+async fn repeated_skill_load_warnings_emit_once_until_errors_clear() {
+    let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
+    let cwd = app.chat_widget.config_ref().cwd.to_path_buf();
+    let expected_path = test_path_display("/tmp/user/skills/planning/SKILL.md");
+    let error = codex_app_server_protocol::SkillErrorInfo {
+        path: test_path_buf("/tmp/user/skills/planning/SKILL.md"),
+        message: "invalid YAML".to_string(),
+    };
+    let response = codex_app_server_protocol::SkillsListResponse {
+        data: vec![codex_app_server_protocol::SkillsListEntry {
+            cwd: cwd.clone(),
+            skills: Vec::new(),
+            errors: vec![error],
+        }],
+    };
+
+    app.handle_skills_list_response(response.clone());
+    app.handle_skills_list_response(response.clone());
+
+    let warnings = drain_insert_history_cells(&mut app_event_rx);
+    assert_eq!(warnings.len(), 2);
+    app.transcript_cells.extend(warnings);
+    let rendered = lines_to_single_string(&crate::terminal_hyperlinks::visible_lines(
+        app.render_transcript_lines_for_reflow(/*width*/ 80).lines,
+    ))
+    .replace(&expected_path, "/tmp/user/skills/planning/SKILL.md");
+    assert_app_snapshot!(
+        "repeated_skill_load_warnings_emit_once_until_errors_clear",
+        rendered,
+    );
+
+    app.handle_skills_list_response(codex_app_server_protocol::SkillsListResponse {
+        data: vec![codex_app_server_protocol::SkillsListEntry {
+            cwd: test_path_buf("/tmp/other-project"),
+            skills: Vec::new(),
+            errors: Vec::new(),
+        }],
+    });
+    assert!(drain_insert_history_cells(&mut app_event_rx).is_empty());
+
+    app.handle_skills_list_response(response.clone());
+    assert!(drain_insert_history_cells(&mut app_event_rx).is_empty());
+
+    app.handle_skills_list_response(codex_app_server_protocol::SkillsListResponse {
+        data: vec![codex_app_server_protocol::SkillsListEntry {
+            cwd,
+            skills: Vec::new(),
+            errors: Vec::new(),
+        }],
+    });
+    assert!(drain_insert_history_cells(&mut app_event_rx).is_empty());
+
+    app.handle_skills_list_response(response);
+    assert_eq!(drain_insert_history_cells(&mut app_event_rx).len(), 2);
+}
+
+fn drain_insert_history_cells(
+    app_event_rx: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
+) -> Vec<Arc<dyn HistoryCell>> {
+    let mut cells = Vec::new();
+    while let Ok(event) = app_event_rx.try_recv() {
+        if let AppEvent::InsertHistoryCell(cell) = event {
+            cells.push(cell.into());
+        }
+    }
+    cells
+}
+
 #[tokio::test]
 async fn side_defers_subagent_approval_overlay_until_side_exits() -> Result<()> {
     let mut app = make_test_app().await;
@@ -3781,6 +3850,7 @@ async fn make_test_app() -> App {
         commit_anim_running: Arc::new(AtomicBool::new(false)),
         status_line_invalid_items_warned: Arc::new(AtomicBool::new(false)),
         terminal_title_invalid_items_warned: Arc::new(AtomicBool::new(false)),
+        skill_load_warnings_by_cwd: HashMap::new(),
         backtrack: BacktrackState::default(),
         backtrack_render_pending: false,
         feedback: codex_feedback::CodexFeedback::new(),
@@ -3844,6 +3914,7 @@ async fn make_test_app_with_channels() -> (
             commit_anim_running: Arc::new(AtomicBool::new(false)),
             status_line_invalid_items_warned: Arc::new(AtomicBool::new(false)),
             terminal_title_invalid_items_warned: Arc::new(AtomicBool::new(false)),
+            skill_load_warnings_by_cwd: HashMap::new(),
             backtrack: BacktrackState::default(),
             backtrack_render_pending: false,
             feedback: codex_feedback::CodexFeedback::new(),

codex-rs/tui/src/app/thread_routing.rs

@@ -1339,9 +1339,20 @@ impl App {
 
     #[allow(clippy::too_many_arguments)]
     pub(super) fn handle_skills_list_response(&mut self, response: SkillsListResponse) {
-        let cwd = self.chat_widget.config_ref().cwd.clone();
-        let errors = errors_for_cwd(&cwd, &response);
-        emit_skill_load_warnings(&self.app_event_tx, &errors);
+        let cwd = self.chat_widget.config_ref().cwd.to_path_buf();
+        if let Some(errors) = errors_for_cwd(&cwd, &response) {
+            if errors.is_empty() {
+                self.skill_load_warnings_by_cwd.remove(&cwd);
+            } else if !self
+                .skill_load_warnings_by_cwd
+                .get(&cwd)
+                .is_some_and(|previous_errors| previous_errors == &errors)
+            {
+                emit_skill_load_warnings(&self.app_event_tx, &errors);
+                self.skill_load_warnings_by_cwd.insert(cwd, errors);
+            }
+        }
+
         self.chat_widget.handle_skills_list_response(response);
     }
 

codex-rs/tui/src/snapshots/codex_tui__app__tests__repeated_skill_load_warnings_emit_once_until_errors_clear.snap

@@ -0,0 +1,7 @@
+---
+source: tui/src/app/tests.rs
+expression: rendered
+---
+⚠ Skipped loading 1 skill(s) due to invalid SKILL.md files.
+
+⚠ /tmp/user/skills/planning/SKILL.md: invalid YAML