Skip to content

fix: report an appropriate error in the TUI for malformed rules #9011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bolinfest merged 1 commit into from
Jan 13, 2026
Merged

fix: report an appropriate error in the TUI for malformed rules #9011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions codex-rs/cli/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use codex_execpolicy::ExecPolicyCheckCommand;
use codex_responses_api_proxy::Args as ResponsesApiProxyArgs;
use codex_tui::AppExitInfo;
use codex_tui::Cli as TuiCli;
use codex_tui::ExitReason;
use codex_tui::update_action::UpdateAction;
use codex_tui2 as tui2;
use owo_colors::OwoColorize;
Expand Down Expand Up @@ -353,6 +354,14 @@ fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec<Stri

/// Handle the app exit and print the results. Optionally run the update action.
fn handle_app_exit(exit_info: AppExitInfo) -> anyhow::Result<()> {
match exit_info.exit_reason {
Copy link
Collaborator

@etraut-openai etraut-openai Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this runs only in interactive mode. What about non-interactive mode (codex exec)? I think we'd want this same error printed in that case.

Copy link
Collaborator Author

@bolinfest bolinfest Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This already appears to work as intended for codex exec:

$ ./codex-rs/target/debug/codex exec hi
Error: Fatal error: failed to load execpolicy: failed to read execpolicy files from /Users/mbolin/.codex/rules: Not a directory (os error 20)

ExitReason::Fatal(message) => {
eprintln!("ERROR: {message}");
std::process::exit(1);
}
ExitReason::UserRequested => { /* normal exit */ }
}

let update_action = exit_info.update_action;
let color_enabled = supports_color::on(Stream::Stdout).is_some();
for line in format_exit_messages(exit_info, color_enabled) {
Expand Down Expand Up @@ -947,6 +956,7 @@ mod tests {
token_usage,
thread_id: conversation.map(ThreadId::from_string).map(Result::unwrap),
update_action: None,
exit_reason: ExitReason::UserRequested,
}
}

Expand All @@ -956,6 +966,7 @@ mod tests {
token_usage: TokenUsage::default(),
thread_id: None,
update_action: None,
exit_reason: ExitReason::UserRequested,
};
let lines = format_exit_messages(exit_info, false);
assert!(lines.is_empty());
Expand Down
87 changes: 60 additions & 27 deletions codex-rs/tui/src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,19 @@ pub struct AppExitInfo {
pub token_usage: TokenUsage,
pub thread_id: Option<ThreadId>,
pub update_action: Option<UpdateAction>,
pub exit_reason: ExitReason,
}

#[derive(Debug)]
pub(crate) enum AppRunControl {
Continue,
Exit(ExitReason),
}

#[derive(Debug, Clone)]
pub enum ExitReason {
UserRequested,
Fatal(String),
}

fn session_summary(token_usage: TokenUsage, thread_id: Option<ThreadId>) -> Option<SessionSummary> {
Expand Down Expand Up @@ -289,6 +302,7 @@ async fn handle_model_migration_prompt_if_needed(
token_usage: TokenUsage::default(),
thread_id: None,
update_action: None,
exit_reason: ExitReason::UserRequested,
});
}
}
Expand Down Expand Up @@ -506,42 +520,58 @@ impl App {

#[cfg(not(debug_assertions))]
if let Some(latest_version) = upgrade_version {
app.handle_event(
tui,
AppEvent::InsertHistoryCell(Box::new(UpdateAvailableHistoryCell::new(
latest_version,
crate::update_action::get_update_action(),
))),
)
.await?;
let control = app
.handle_event(
tui,
AppEvent::InsertHistoryCell(Box::new(UpdateAvailableHistoryCell::new(
latest_version,
crate::update_action::get_update_action(),
))),
)
.await?;
if let AppRunControl::Exit(exit_reason) = control {
return Ok(AppExitInfo {
token_usage: app.token_usage(),
thread_id: app.chat_widget.thread_id(),
update_action: app.pending_update_action,
exit_reason,
});
}
}

let tui_events = tui.event_stream();
tokio::pin!(tui_events);

tui.frame_requester().schedule_frame();

while select! {
Some(event) = app_event_rx.recv() => {
app.handle_event(tui, event).await?
}
Some(event) = tui_events.next() => {
app.handle_tui_event(tui, event).await?
let exit_reason = loop {
let control = select! {
Some(event) = app_event_rx.recv() => {
app.handle_event(tui, event).await?
}
Some(event) = tui_events.next() => {
app.handle_tui_event(tui, event).await?
}
};
match control {
AppRunControl::Continue => {}
AppRunControl::Exit(reason) => break reason,
}
} {}
};
tui.terminal.clear()?;
Ok(AppExitInfo {
token_usage: app.token_usage(),
thread_id: app.chat_widget.thread_id(),
update_action: app.pending_update_action,
exit_reason,
})
}

pub(crate) async fn handle_tui_event(
&mut self,
tui: &mut tui::Tui,
event: TuiEvent,
) -> Result<bool> {
) -> Result<AppRunControl> {
if self.overlay.is_some() {
let _ = self.handle_backtrack_overlay_event(tui, event).await?;
} else {
Expand All @@ -563,7 +593,7 @@ impl App {
.chat_widget
.handle_paste_burst_tick(tui.frame_requester())
{
return Ok(true);
return Ok(AppRunControl::Continue);
}
tui.draw(
self.chat_widget.desired_height(tui.terminal.size()?.width),
Expand All @@ -582,10 +612,10 @@ impl App {
}
}
}
Ok(true)
Ok(AppRunControl::Continue)
}

async fn handle_event(&mut self, tui: &mut tui::Tui, event: AppEvent) -> Result<bool> {
async fn handle_event(&mut self, tui: &mut tui::Tui, event: AppEvent) -> Result<AppRunControl> {
let model_info = self
.server
.get_models_manager()
Expand Down Expand Up @@ -816,7 +846,7 @@ impl App {
&& matches!(event.msg, EventMsg::ShutdownComplete)
{
self.suppress_shutdown_complete = false;
return Ok(true);
return Ok(AppRunControl::Continue);
}
if let EventMsg::ListSkillsResponse(response) = &event.msg {
let cwd = self.chat_widget.config_ref().cwd.clone();
Expand All @@ -826,7 +856,10 @@ impl App {
self.chat_widget.handle_codex_event(event);
}
AppEvent::ExitRequest => {
return Ok(false);
return Ok(AppRunControl::Exit(ExitReason::UserRequested));
}
AppEvent::FatalExitRequest(message) => {
return Ok(AppRunControl::Exit(ExitReason::Fatal(message)));
}
AppEvent::CodexOp(op) => self.chat_widget.submit_op(op),
AppEvent::DiffResult(text) => {
Expand Down Expand Up @@ -926,7 +959,7 @@ impl App {
preset,
mode: WindowsSandboxEnableMode::Elevated,
});
return Ok(true);
return Ok(AppRunControl::Continue);
}

self.chat_widget.show_windows_sandbox_setup_status();
Expand Down Expand Up @@ -1095,7 +1128,7 @@ impl App {
tracing::warn!(%err, "failed to set sandbox policy on app config");
self.chat_widget
.add_error_message(format!("Failed to set sandbox policy: {err}"));
return Ok(true);
return Ok(AppRunControl::Continue);
}
#[cfg(target_os = "windows")]
if !matches!(&policy, codex_core::protocol::SandboxPolicy::ReadOnly)
Expand All @@ -1107,7 +1140,7 @@ impl App {
tracing::warn!(%err, "failed to set sandbox policy on chat config");
self.chat_widget
.add_error_message(format!("Failed to set sandbox policy: {err}"));
return Ok(true);
return Ok(AppRunControl::Continue);
}

// If sandbox policy becomes workspace-write or read-only, run the Windows world-writable scan.
Expand All @@ -1116,7 +1149,7 @@ impl App {
// One-shot suppression if the user just confirmed continue.
if self.skip_world_writable_scan_once {
self.skip_world_writable_scan_once = false;
return Ok(true);
return Ok(AppRunControl::Continue);
}

let should_check = codex_core::get_platform_sandbox().is_some()
Expand All @@ -1141,7 +1174,7 @@ impl App {
}
AppEvent::UpdateFeatureFlags { updates } => {
if updates.is_empty() {
return Ok(true);
return Ok(AppRunControl::Continue);
}
let mut builder = ConfigEditsBuilder::new(&self.config.codex_home)
.with_profile(self.active_profile.as_deref());
Expand Down Expand Up @@ -1300,7 +1333,7 @@ impl App {
}
},
}
Ok(true)
Ok(AppRunControl::Continue)
}

fn reasoning_label(reasoning_effort: Option<ReasoningEffortConfig>) -> &'static str {
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/tui/src/app_event.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ pub(crate) enum AppEvent {
/// Request to exit the application gracefully.
ExitRequest,

/// Request to exit the application due to a fatal error.
FatalExitRequest(String),

/// Forward an `Op` to the Agent. Using an `AppEvent` for this avoids
/// bubbling channels through layers of widgets.
CodexOp(codex_core::protocol::Op),
Expand Down
8 changes: 3 additions & 5 deletions codex-rs/tui/src/chatwidget/agent.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,14 @@ pub(crate) fn spawn_agent(
..
} = match server.start_thread(config).await {
Copy link
Collaborator Author

@bolinfest bolinfest Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@etraut-openai to answer your question about "Will this also work for api service clients like the IDE extension?" I believe the crux of the issue is how the UI deals with an Err from start_thread().

I believe it is already doing the right thing here:

codex/codex-rs/app-server/src/codex_message_processor.rs

Lines 1333 to 1356 in 3c711f3

match self.thread_manager.start_thread(config).await {
Ok(new_thread) => {
let NewThread {
thread_id,
session_configured,
..
} = new_thread;
let response = NewConversationResponse {
conversation_id: thread_id,
model: session_configured.model,
reasoning_effort: session_configured.reasoning_effort,
rollout_path: session_configured.rollout_path,
};
self.outgoing.send_response(request_id, response).await;
}
Err(err) => {
let error = JSONRPCErrorError {
code: INTERNAL_ERROR_CODE,
message: format!("error creating conversation: {err}"),
data: None,
};
self.outgoing.send_error(request_id, error).await;
}
}

Copy link
Collaborator

@etraut-openai etraut-openai Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working on a new PR that surfaces config parsing errors correctly to app server clients. I'll extend it to handle malformed rules errors as well.

Ok(v) => v,
#[allow(clippy::print_stderr)]
Err(err) => {
let message = err.to_string();
eprintln!("{message}");
let message = format!("Failed to initialize codex: {err}");
tracing::error!("{message}");
app_event_tx_clone.send(AppEvent::CodexEvent(Event {
id: "".to_string(),
msg: EventMsg::Error(err.to_error_event(None)),
}));
app_event_tx_clone.send(AppEvent::ExitRequest);
tracing::error!("failed to initialize codex: {err}");
app_event_tx_clone.send(AppEvent::FatalExitRequest(message));
return;
}
};
Expand Down
15 changes: 8 additions & 7 deletions codex-rs/tui/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use additional_dirs::add_dir_warning_message;
use app::App;
pub use app::AppExitInfo;
pub use app::ExitReason;
use codex_app_server_protocol::AuthMode;
use codex_common::oss::ensure_oss_provider_ready;
use codex_common::oss::get_default_model_for_oss_provider;
Expand Down Expand Up @@ -99,7 +100,6 @@ pub use cli::Cli;
pub use markdown_render::render_markdown_text;
pub use public_widgets::composer_input::ComposerAction;
pub use public_widgets::composer_input::ComposerInput;
use std::io::Write as _;
// (tests access modules directly within the crate)

pub async fn run_main(
Expand Down Expand Up @@ -377,6 +377,7 @@ async fn run_ratatui_app(
token_usage: codex_core::protocol::TokenUsage::default(),
thread_id: None,
update_action: Some(action),
exit_reason: ExitReason::UserRequested,
});
}
}
Expand Down Expand Up @@ -416,6 +417,7 @@ async fn run_ratatui_app(
token_usage: codex_core::protocol::TokenUsage::default(),
thread_id: None,
update_action: None,
exit_reason: ExitReason::UserRequested,
});
}
// if the user acknowledged windows or made an explicit decision ato trust the directory, reload the config accordingly
Expand Down Expand Up @@ -444,16 +446,13 @@ async fn run_ratatui_app(
restore();
session_log::log_session_end();
let _ = tui.terminal.clear();
if let Err(err) = writeln!(
std::io::stdout(),
"No saved session found with ID {id_str}. Run `codex {action}` without an ID to choose from existing sessions."
) {
error!("Failed to write session error message: {err}");
}
Ok(AppExitInfo {
token_usage: codex_core::protocol::TokenUsage::default(),
thread_id: None,
update_action: None,
exit_reason: ExitReason::Fatal(format!(
"No saved session found with ID {id_str}. Run `codex {action}` without an ID to choose from existing sessions."
)),
})
};

Expand Down Expand Up @@ -499,6 +498,7 @@ async fn run_ratatui_app(
token_usage: codex_core::protocol::TokenUsage::default(),
thread_id: None,
update_action: None,
exit_reason: ExitReason::UserRequested,
});
}
other => other,
Expand Down Expand Up @@ -546,6 +546,7 @@ async fn run_ratatui_app(
token_usage: codex_core::protocol::TokenUsage::default(),
thread_id: None,
update_action: None,
exit_reason: ExitReason::UserRequested,
});
}
other => other,
Expand Down
Loading
Loading