Make loading malformed skills fail-open (#8243)

Instead of failing to start Codex, clearly call out that N skills did
not load and provide warnings so that the user may fix them.

<img width="3548" height="874" alt="image"
src="https://github.com/user-attachments/assets/6ce041b2-1373-4007-a6dd-0194e58fafe4"
/>
This commit is contained in:
Gav Verma
2025-12-17 23:41:04 -08:00
committed by GitHub
Unverified
parent da3869eeb6
commit 50dafbc31b
8 changed files with 44 additions and 455 deletions
+22 -25
View File
@@ -14,8 +14,6 @@ use crate::pager_overlay::Overlay;
use crate::render::highlight::highlight_bash_to_lines;
use crate::render::renderable::Renderable;
use crate::resume_picker::ResumeSelection;
use crate::skill_error_prompt::SkillErrorPromptOutcome;
use crate::skill_error_prompt::run_skill_error_prompt;
use crate::tui;
use crate::tui::TuiEvent;
use crate::update_action::UpdateAction;
@@ -37,7 +35,6 @@ use codex_core::protocol::Op;
use codex_core::protocol::SessionSource;
use codex_core::protocol::SkillErrorInfo;
use codex_core::protocol::TokenUsage;
use codex_core::skills::SkillError;
use codex_protocol::ConversationId;
use codex_protocol::openai_models::ModelPreset;
use codex_protocol::openai_models::ModelUpgrade;
@@ -89,16 +86,6 @@ fn session_summary(
})
}
fn skill_errors_from_info(errors: &[SkillErrorInfo]) -> Vec<SkillError> {
errors
.iter()
.map(|err| SkillError {
path: err.path.clone(),
message: err.message.clone(),
})
.collect()
}
fn errors_for_cwd(cwd: &Path, response: &ListSkillsResponseEvent) -> Vec<SkillErrorInfo> {
response
.skills
@@ -108,6 +95,27 @@ fn errors_for_cwd(cwd: &Path, response: &ListSkillsResponseEvent) -> Vec<SkillEr
.unwrap_or_default()
}
fn emit_skill_load_warnings(app_event_tx: &AppEventSender, errors: &[SkillErrorInfo]) {
if errors.is_empty() {
return;
}
let error_count = errors.len();
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
crate::history_cell::new_warning_event(format!(
"Skipped loading {error_count} skill(s) due to invalid SKILL.md files."
)),
)));
for error in errors {
let path = error.path.display();
let message = error.message.as_str();
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
crate::history_cell::new_warning_event(format!("{path}: {message}")),
)));
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
struct SessionSummary {
usage_line: String,
@@ -705,18 +713,7 @@ impl App {
if let EventMsg::ListSkillsResponse(response) = &event.msg {
let cwd = self.chat_widget.config_ref().cwd.clone();
let errors = errors_for_cwd(&cwd, response);
if errors.is_empty() {
self.chat_widget.handle_codex_event(event);
return Ok(true);
}
let errors = skill_errors_from_info(&errors);
match run_skill_error_prompt(tui, &errors).await {
SkillErrorPromptOutcome::Exit => {
self.chat_widget.submit_op(Op::Shutdown);
return Ok(false);
}
SkillErrorPromptOutcome::Continue => {}
}
emit_skill_load_warnings(&self.app_event_tx, &errors);
}
self.chat_widget.handle_codex_event(event);
}
-1
View File
@@ -67,7 +67,6 @@ mod resume_picker;
mod selection_list;
mod session_log;
mod shimmer;
mod skill_error_prompt;
mod slash_command;
mod status;
mod status_indicator_widget;
-193
View File
@@ -1,193 +0,0 @@
use crate::tui::FrameRequester;
use crate::tui::Tui;
use crate::tui::TuiEvent;
use crate::wrapping::RtOptions;
use crate::wrapping::word_wrap_line;
use codex_core::skills::SkillError;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyEventKind;
use crossterm::event::KeyModifiers;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::prelude::Stylize as _;
use ratatui::text::Line;
use ratatui::widgets::Block;
use ratatui::widgets::Borders;
use ratatui::widgets::Clear;
use ratatui::widgets::Paragraph;
use ratatui::widgets::Widget;
use ratatui::widgets::WidgetRef;
use tokio_stream::StreamExt;
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum SkillErrorPromptOutcome {
Continue,
Exit,
}
pub(crate) async fn run_skill_error_prompt(
tui: &mut Tui,
errors: &[SkillError],
) -> SkillErrorPromptOutcome {
struct AltScreenGuard<'a> {
tui: &'a mut Tui,
stashed_history_lines: Vec<Line<'static>>,
}
impl<'a> AltScreenGuard<'a> {
fn enter(tui: &'a mut Tui) -> Self {
let _ = tui.enter_alt_screen();
let stashed_history_lines = tui.stash_pending_history_lines();
Self {
tui,
stashed_history_lines,
}
}
}
impl Drop for AltScreenGuard<'_> {
fn drop(&mut self) {
let _ = self.tui.leave_alt_screen();
let stashed_history_lines = std::mem::take(&mut self.stashed_history_lines);
self.tui
.restore_pending_history_lines(stashed_history_lines);
}
}
let alt = AltScreenGuard::enter(tui);
let mut screen = SkillErrorScreen::new(alt.tui.frame_requester(), errors);
let _ = alt.tui.draw(u16::MAX, |frame| {
frame.render_widget_ref(&screen, frame.area());
});
let events = alt.tui.event_stream();
tokio::pin!(events);
while !screen.is_done() {
if let Some(event) = events.next().await {
match event {
TuiEvent::Key(key_event) => screen.handle_key(key_event),
TuiEvent::Paste(_) => {}
TuiEvent::Draw => {
let _ = alt.tui.draw(u16::MAX, |frame| {
frame.render_widget_ref(&screen, frame.area());
});
}
}
} else {
screen.confirm_continue();
break;
}
}
screen.outcome()
}
struct SkillErrorScreen {
request_frame: FrameRequester,
errors: Vec<SkillError>,
done: bool,
exit: bool,
}
impl SkillErrorScreen {
fn new(request_frame: FrameRequester, errors: &[SkillError]) -> Self {
Self {
request_frame,
errors: errors.to_vec(),
done: false,
exit: false,
}
}
fn is_done(&self) -> bool {
self.done
}
fn confirm_continue(&mut self) {
self.done = true;
self.exit = false;
self.request_frame.schedule_frame();
}
fn confirm_exit(&mut self) {
self.done = true;
self.exit = true;
self.request_frame.schedule_frame();
}
fn outcome(&self) -> SkillErrorPromptOutcome {
if self.exit {
SkillErrorPromptOutcome::Exit
} else {
SkillErrorPromptOutcome::Continue
}
}
fn handle_key(&mut self, key_event: KeyEvent) {
if key_event.kind == KeyEventKind::Release {
return;
}
if key_event
.modifiers
.intersects(KeyModifiers::CONTROL | KeyModifiers::META)
&& matches!(key_event.code, KeyCode::Char('c') | KeyCode::Char('d'))
{
self.confirm_exit();
return;
}
match key_event.code {
KeyCode::Enter | KeyCode::Esc | KeyCode::Char(' ') | KeyCode::Char('q') => {
self.confirm_continue();
}
_ => {}
}
}
}
impl WidgetRef for &SkillErrorScreen {
fn render_ref(&self, area: Rect, buf: &mut Buffer) {
Clear.render(area, buf);
let block = Block::default()
.title("Skill errors".bold())
.borders(Borders::ALL);
let inner = block.inner(area);
let width = usize::from(inner.width).max(1);
let mut base_lines: Vec<Line<'static>> = vec![
Line::from("Skill validation errors detected".bold()),
Line::from("Fix these SKILL.md files and restart."),
Line::from("Invalid skills are ignored until resolved."),
Line::from("Press enter or esc to continue. Ctrl+C or Ctrl+D to exit."),
Line::from(""),
];
let error_start = base_lines.len();
for error in &self.errors {
base_lines.push(Line::from(vec![
error.path.display().to_string().dim(),
": ".into(),
error.message.clone().red(),
]));
}
let error_wrap_opts = RtOptions::new(width)
.initial_indent(Line::from("- "))
.subsequent_indent(Line::from(" "));
let mut lines: Vec<Line<'_>> = Vec::new();
for (idx, line) in base_lines.iter().enumerate() {
if idx < error_start {
lines.extend(word_wrap_line(line, width));
} else {
lines.extend(word_wrap_line(line, error_wrap_opts.clone()));
}
}
Paragraph::new(lines).block(block).render(area, buf);
}
}
-8
View File
@@ -329,14 +329,6 @@ impl Tui {
self.frame_requester().schedule_frame();
}
pub(crate) fn stash_pending_history_lines(&mut self) -> Vec<Line<'static>> {
std::mem::take(&mut self.pending_history_lines)
}
pub(crate) fn restore_pending_history_lines(&mut self, lines: Vec<Line<'static>>) {
self.pending_history_lines = lines;
}
pub fn draw(
&mut self,
height: u16,
+22 -25
View File
@@ -17,8 +17,6 @@ use crate::pager_overlay::Overlay;
use crate::render::highlight::highlight_bash_to_lines;
use crate::render::renderable::Renderable;
use crate::resume_picker::ResumeSelection;
use crate::skill_error_prompt::SkillErrorPromptOutcome;
use crate::skill_error_prompt::run_skill_error_prompt;
use crate::tui;
use crate::tui::TuiEvent;
use crate::tui::scrolling::TranscriptLineMeta;
@@ -44,7 +42,6 @@ use codex_core::protocol::Op;
use codex_core::protocol::SessionSource;
use codex_core::protocol::SkillErrorInfo;
use codex_core::protocol::TokenUsage;
use codex_core::skills::SkillError;
use codex_protocol::ConversationId;
use codex_protocol::openai_models::ModelPreset;
use codex_protocol::openai_models::ModelUpgrade;
@@ -118,16 +115,6 @@ fn session_summary(
})
}
fn skill_errors_from_info(errors: &[SkillErrorInfo]) -> Vec<SkillError> {
errors
.iter()
.map(|err| SkillError {
path: err.path.clone(),
message: err.message.clone(),
})
.collect()
}
fn errors_for_cwd(cwd: &Path, response: &ListSkillsResponseEvent) -> Vec<SkillErrorInfo> {
response
.skills
@@ -137,6 +124,27 @@ fn errors_for_cwd(cwd: &Path, response: &ListSkillsResponseEvent) -> Vec<SkillEr
.unwrap_or_default()
}
fn emit_skill_load_warnings(app_event_tx: &AppEventSender, errors: &[SkillErrorInfo]) {
if errors.is_empty() {
return;
}
let error_count = errors.len();
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
crate::history_cell::new_warning_event(format!(
"Skipped loading {error_count} skill(s) due to invalid SKILL.md files."
)),
)));
for error in errors {
let path = error.path.display();
let message = error.message.as_str();
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
crate::history_cell::new_warning_event(format!("{path}: {message}")),
)));
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
struct SessionSummary {
usage_line: String,
@@ -1547,18 +1555,7 @@ impl App {
if let EventMsg::ListSkillsResponse(response) = &event.msg {
let cwd = self.chat_widget.config_ref().cwd.clone();
let errors = errors_for_cwd(&cwd, response);
if errors.is_empty() {
self.chat_widget.handle_codex_event(event);
return Ok(true);
}
let errors = skill_errors_from_info(&errors);
match run_skill_error_prompt(tui, &errors).await {
SkillErrorPromptOutcome::Exit => {
self.chat_widget.submit_op(Op::Shutdown);
return Ok(false);
}
SkillErrorPromptOutcome::Continue => {}
}
emit_skill_load_warnings(&self.app_event_tx, &errors);
}
self.chat_widget.handle_codex_event(event);
}
-1
View File
@@ -68,7 +68,6 @@ mod resume_picker;
mod selection_list;
mod session_log;
mod shimmer;
mod skill_error_prompt;
mod slash_command;
mod status;
mod status_indicator_widget;
-194
View File
@@ -1,194 +0,0 @@
use crate::tui::FrameRequester;
use crate::tui::Tui;
use crate::tui::TuiEvent;
use crate::wrapping::RtOptions;
use crate::wrapping::word_wrap_line;
use codex_core::skills::SkillError;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyEventKind;
use crossterm::event::KeyModifiers;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::prelude::Stylize as _;
use ratatui::text::Line;
use ratatui::widgets::Block;
use ratatui::widgets::Borders;
use ratatui::widgets::Clear;
use ratatui::widgets::Paragraph;
use ratatui::widgets::Widget;
use ratatui::widgets::WidgetRef;
use tokio_stream::StreamExt;
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum SkillErrorPromptOutcome {
Continue,
Exit,
}
pub(crate) async fn run_skill_error_prompt(
tui: &mut Tui,
errors: &[SkillError],
) -> SkillErrorPromptOutcome {
struct AltScreenGuard<'a> {
tui: &'a mut Tui,
stashed_history_lines: Vec<Line<'static>>,
}
impl<'a> AltScreenGuard<'a> {
fn enter(tui: &'a mut Tui) -> Self {
let _ = tui.enter_alt_screen();
let stashed_history_lines = tui.stash_pending_history_lines();
Self {
tui,
stashed_history_lines,
}
}
}
impl Drop for AltScreenGuard<'_> {
fn drop(&mut self) {
let _ = self.tui.leave_alt_screen();
let stashed_history_lines = std::mem::take(&mut self.stashed_history_lines);
self.tui
.restore_pending_history_lines(stashed_history_lines);
}
}
let alt = AltScreenGuard::enter(tui);
let mut screen = SkillErrorScreen::new(alt.tui.frame_requester(), errors);
let _ = alt.tui.draw(u16::MAX, |frame| {
frame.render_widget_ref(&screen, frame.area());
});
let events = alt.tui.event_stream();
tokio::pin!(events);
while !screen.is_done() {
if let Some(event) = events.next().await {
match event {
TuiEvent::Key(key_event) => screen.handle_key(key_event),
TuiEvent::Mouse(_) => {}
TuiEvent::Paste(_) => {}
TuiEvent::Draw => {
let _ = alt.tui.draw(u16::MAX, |frame| {
frame.render_widget_ref(&screen, frame.area());
});
}
}
} else {
screen.confirm_continue();
break;
}
}
screen.outcome()
}
struct SkillErrorScreen {
request_frame: FrameRequester,
errors: Vec<SkillError>,
done: bool,
exit: bool,
}
impl SkillErrorScreen {
fn new(request_frame: FrameRequester, errors: &[SkillError]) -> Self {
Self {
request_frame,
errors: errors.to_vec(),
done: false,
exit: false,
}
}
fn is_done(&self) -> bool {
self.done
}
fn confirm_continue(&mut self) {
self.done = true;
self.exit = false;
self.request_frame.schedule_frame();
}
fn confirm_exit(&mut self) {
self.done = true;
self.exit = true;
self.request_frame.schedule_frame();
}
fn outcome(&self) -> SkillErrorPromptOutcome {
if self.exit {
SkillErrorPromptOutcome::Exit
} else {
SkillErrorPromptOutcome::Continue
}
}
fn handle_key(&mut self, key_event: KeyEvent) {
if key_event.kind == KeyEventKind::Release {
return;
}
if key_event
.modifiers
.intersects(KeyModifiers::CONTROL | KeyModifiers::META)
&& matches!(key_event.code, KeyCode::Char('c') | KeyCode::Char('d'))
{
self.confirm_exit();
return;
}
match key_event.code {
KeyCode::Enter | KeyCode::Esc | KeyCode::Char(' ') | KeyCode::Char('q') => {
self.confirm_continue();
}
_ => {}
}
}
}
impl WidgetRef for &SkillErrorScreen {
fn render_ref(&self, area: Rect, buf: &mut Buffer) {
Clear.render(area, buf);
let block = Block::default()
.title("Skill errors".bold())
.borders(Borders::ALL);
let inner = block.inner(area);
let width = usize::from(inner.width).max(1);
let mut base_lines: Vec<Line<'static>> = vec![
Line::from("Skill validation errors detected".bold()),
Line::from("Fix these SKILL.md files and restart."),
Line::from("Invalid skills are ignored until resolved."),
Line::from("Press enter or esc to continue. Ctrl+C or Ctrl+D to exit."),
Line::from(""),
];
let error_start = base_lines.len();
for error in &self.errors {
base_lines.push(Line::from(vec![
error.path.display().to_string().dim(),
": ".into(),
error.message.clone().red(),
]));
}
let error_wrap_opts = RtOptions::new(width)
.initial_indent(Line::from("- "))
.subsequent_indent(Line::from(" "));
let mut lines: Vec<Line<'_>> = Vec::new();
for (idx, line) in base_lines.iter().enumerate() {
if idx < error_start {
lines.extend(word_wrap_line(line, width));
} else {
lines.extend(word_wrap_line(line, error_wrap_opts.clone()));
}
}
Paragraph::new(lines).block(block).render(area, buf);
}
}
-8
View File
@@ -335,14 +335,6 @@ impl Tui {
self.frame_requester().schedule_frame();
}
pub(crate) fn stash_pending_history_lines(&mut self) -> Vec<Line<'static>> {
std::mem::take(&mut self.pending_history_lines)
}
pub(crate) fn restore_pending_history_lines(&mut self, lines: Vec<Line<'static>>) {
self.pending_history_lines = lines;
}
pub fn draw(
&mut self,
height: u16,