core/core-agent-ide
8
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

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

Browse source 
The underlying issue is that when we encountered an error starting a
conversation (any sort of error, though making `$CODEX_HOME/rules` a
file rather than folder was the example in #8803), then we were writing
the message to stderr, but this could be printed over by our UI
framework so the user would not see it. In general, we disallow the use
of `eprintln!()` in this part of the code for exactly this reason,
though this was suppressed by an `#[allow(clippy::print_stderr)]`.

This attempts to clean things up by changing `handle_event()` and
`handle_tui_event()` to return a `Result<AppRunControl>` instead of a
`Result<bool>`, which is a new type introduced in this PR (and depends
on `ExitReason`, also a new type):

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

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

This makes it possible to exit the primary control flow of the TUI with
richer information. This PR adds `ExitReason` to the existing
`AppExitInfo` struct and updates `handle_app_exit()` to print the error
and exit code `1` in the event of `ExitReason::Fatal`.

I tried to create an integration test for this, but it was a bit
involved, so I published it as a separate PR:
https://github.com/openai/codex/pull/9166. For this PR, please have
faith in my manual testing!

Fixes https://github.com/openai/codex/issues/8803.




---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/9011).
* #9166
* __->__ #9011
This commit is contained in:
Michael Bolin 2026-01-13 15:21:32 -08:00 • committed by GitHub
parent 9f8d3c14ce
commit 2cd1a0a45e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 164 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
codex-rs/cli/src/main.rs
View file

@ -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;
@ -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 {
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) {
@ -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,
}
}
@ -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());

87
codex-rs/tui/src/app.rs
View file

@ -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> {
@ -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,
});
}
}
@ -506,14 +520,23 @@ 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();
@ -521,19 +544,26 @@ impl App {
tui.frame_requester().schedule_frame();
while select! {
Some(event) = app_event_rx.recv() => {
app.handle_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,
}
Some(event) = tui_events.next() => {
app.handle_tui_event(tui, event).await?
}
} {}
};
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,
})
}
@ -541,7 +571,7 @@ impl App {
&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 {
@ -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),
@ -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()
@ -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();
@ -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) => {
@ -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();
@ -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)
@ -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.
@ -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()
@ -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());
@ -1300,7 +1333,7 @@ impl App {
}
},
}
Ok(true)
Ok(AppRunControl::Continue)
}
fn reasoning_label(reasoning_effort: Option<ReasoningEffortConfig>) -> &'static str {

3
codex-rs/tui/src/app_event.rs
View file

@ -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),

8
codex-rs/tui/src/chatwidget/agent.rs
View file

@ -30,16 +30,14 @@ pub(crate) fn spawn_agent(
..
} = match server.start_thread(config).await {
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;
}
};

15
codex-rs/tui/src/lib.rs
View file

@ -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;
@ -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(
@ -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,
});
}
}
@ -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
@ -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."
)),
})
};
@ -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,
@ -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,

91
codex-rs/tui2/src/app.rs
View file

@ -98,6 +98,7 @@ pub struct AppExitInfo {
pub token_usage: TokenUsage,
pub conversation_id: Option<ThreadId>,
pub update_action: Option<UpdateAction>,
pub exit_reason: ExitReason,
/// ANSI-styled transcript lines to print after the TUI exits.
///
/// These lines are rendered against the same width as the final TUI
@ -106,12 +107,29 @@ pub struct AppExitInfo {
pub session_lines: Vec<String>,
}
#[derive(Debug)]
pub(crate) enum AppRunControl {
Continue,
Exit(ExitReason),
}
#[derive(Debug, Clone)]
pub enum ExitReason {
UserRequested,
Fatal(String),
}
impl From<AppExitInfo> for codex_tui::AppExitInfo {
fn from(info: AppExitInfo) -> Self {
let exit_reason = match info.exit_reason {
ExitReason::UserRequested => codex_tui::ExitReason::UserRequested,
ExitReason::Fatal(message) => codex_tui::ExitReason::Fatal(message),
};
codex_tui::AppExitInfo {
token_usage: info.token_usage,
thread_id: info.conversation_id,
update_action: info.update_action.map(Into::into),
exit_reason,
}
}
}
@ -326,6 +344,7 @@ async fn handle_model_migration_prompt_if_needed(
token_usage: TokenUsage::default(),
conversation_id: None,
update_action: None,
exit_reason: ExitReason::UserRequested,
session_lines: Vec::new(),
});
}
@ -596,14 +615,24 @@ 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(),
conversation_id: app.chat_widget.conversation_id(),
update_action: app.pending_update_action,
exit_reason,
session_lines: Vec::new(),
});
}
}
let tui_events = tui.event_stream();
@ -611,14 +640,20 @@ impl App {
tui.frame_requester().schedule_frame();
while select! {
Some(event) = app_event_rx.recv() => {
app.handle_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,
}
Some(event) = tui_events.next() => {
app.handle_tui_event(tui, event).await?
}
} {}
};
let width = tui.terminal.last_known_screen_size.width;
let session_lines = if width == 0 {
Vec::new()
@ -639,6 +674,7 @@ impl App {
token_usage: app.token_usage(),
conversation_id: app.chat_widget.conversation_id(),
update_action: app.pending_update_action,
exit_reason,
session_lines,
})
}
@ -647,7 +683,7 @@ impl App {
&mut self,
tui: &mut tui::Tui,
event: TuiEvent,
) -> Result<bool> {
) -> Result<AppRunControl> {
if matches!(&event, TuiEvent::Draw) {
self.handle_scroll_tick(tui);
}
@ -676,7 +712,7 @@ impl App {
.chat_widget
.handle_paste_burst_tick(tui.frame_requester())
{
return Ok(true);
return Ok(AppRunControl::Continue);
}
let cells = self.transcript_cells.clone();
tui.draw(tui.terminal.size()?.height, |frame| {
@ -736,7 +772,7 @@ impl App {
}
}
}
Ok(true)
Ok(AppRunControl::Continue)
}
pub(crate) fn render_transcript_cells(
@ -1387,7 +1423,7 @@ impl App {
Some(TranscriptSelectionPoint { line_index, column })
}
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> {
match event {
AppEvent::NewSession => {
let summary = session_summary(
@ -1597,7 +1633,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();
@ -1607,7 +1643,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) => {
@ -1702,7 +1741,7 @@ impl App {
preset,
mode: WindowsSandboxEnableMode::Elevated,
});
return Ok(true);
return Ok(AppRunControl::Continue);
}
self.chat_widget.show_windows_sandbox_setup_status();
@ -1871,7 +1910,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)
@ -1883,7 +1922,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.
@ -1892,7 +1931,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()
@ -2040,7 +2079,7 @@ impl App {
}
},
}
Ok(true)
Ok(AppRunControl::Continue)
}
fn reasoning_label(reasoning_effort: Option<ReasoningEffortConfig>) -> &'static str {

3
codex-rs/tui2/src/app_event.rs
View file

@ -43,6 +43,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),

8
codex-rs/tui2/src/chatwidget/agent.rs
View file

@ -30,16 +30,14 @@ pub(crate) fn spawn_agent(
thread_id: _,
} = match server.start_thread(config).await {
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;
}
};

14
codex-rs/tui2/src/lib.rs
View file

@ -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;
@ -395,6 +396,7 @@ async fn run_ratatui_app(
token_usage: codex_core::protocol::TokenUsage::default(),
conversation_id: None,
update_action: Some(action),
exit_reason: ExitReason::UserRequested,
session_lines: Vec::new(),
});
}
@ -435,6 +437,7 @@ async fn run_ratatui_app(
token_usage: codex_core::protocol::TokenUsage::default(),
conversation_id: None,
update_action: None,
exit_reason: ExitReason::UserRequested,
session_lines: Vec::new(),
});
}
@ -464,16 +467,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(),
conversation_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."
)),
session_lines: Vec::new(),
})
};
@ -520,6 +520,7 @@ async fn run_ratatui_app(
token_usage: codex_core::protocol::TokenUsage::default(),
conversation_id: None,
update_action: None,
exit_reason: ExitReason::UserRequested,
session_lines: Vec::new(),
});
}
@ -568,6 +569,7 @@ async fn run_ratatui_app(
token_usage: codex_core::protocol::TokenUsage::default(),
conversation_id: None,
update_action: None,
exit_reason: ExitReason::UserRequested,
session_lines: Vec::new(),
});
}