mirror of
https://github.com/openai/codex.git
synced 2026-11-02 19:43:24 +00:00
Deduplicate invalid skill load warnings
This commit is contained in:
@@ -464,6 +464,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,
|
||||
@@ -886,6 +887,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(),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -2459,6 +2459,61 @@ 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 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());
|
||||
|
||||
assert_eq!(
|
||||
drain_insert_history_transcripts(&mut app_event_rx),
|
||||
vec![
|
||||
"⚠ Skipped loading 1 skill(s) due to invalid SKILL.md files.".to_string(),
|
||||
"⚠ /tmp/user/skills/planning/SKILL.md: invalid YAML".to_string(),
|
||||
],
|
||||
);
|
||||
|
||||
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_eq!(
|
||||
drain_insert_history_transcripts(&mut app_event_rx),
|
||||
Vec::<String>::new(),
|
||||
);
|
||||
|
||||
app.handle_skills_list_response(response);
|
||||
assert_eq!(drain_insert_history_transcripts(&mut app_event_rx).len(), 2);
|
||||
}
|
||||
|
||||
fn drain_insert_history_transcripts(
|
||||
app_event_rx: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
|
||||
) -> Vec<String> {
|
||||
let mut transcripts = Vec::new();
|
||||
while let Ok(event) = app_event_rx.try_recv() {
|
||||
if let AppEvent::InsertHistoryCell(cell) = event {
|
||||
transcripts.push(lines_to_single_string(&cell.transcript_lines(/*width*/ 80)));
|
||||
}
|
||||
}
|
||||
transcripts
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn side_defers_subagent_approval_overlay_until_side_exits() -> Result<()> {
|
||||
let mut app = make_test_app().await;
|
||||
@@ -3803,6 +3858,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(),
|
||||
@@ -3866,6 +3922,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(),
|
||||
|
||||
@@ -1319,10 +1319,29 @@ impl App {
|
||||
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);
|
||||
self.emit_skill_load_warnings_if_changed(&cwd, errors);
|
||||
self.chat_widget.handle_skills_list_response(response);
|
||||
}
|
||||
|
||||
fn emit_skill_load_warnings_if_changed(&mut self, cwd: &Path, errors: Vec<SkillErrorInfo>) {
|
||||
if errors.is_empty() {
|
||||
self.skill_load_warnings_by_cwd.remove(cwd);
|
||||
return;
|
||||
}
|
||||
|
||||
if self
|
||||
.skill_load_warnings_by_cwd
|
||||
.get(cwd)
|
||||
.is_some_and(|previous_errors| previous_errors == &errors)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
emit_skill_load_warnings(&self.app_event_tx, &errors);
|
||||
self.skill_load_warnings_by_cwd
|
||||
.insert(cwd.to_path_buf(), errors);
|
||||
}
|
||||
|
||||
pub(super) async fn handle_thread_rollback_response(
|
||||
&mut self,
|
||||
thread_id: ThreadId,
|
||||
|
||||
Reference in New Issue
Block a user