fix: TUI was not honoring --skip-git-repo-check correctly (#1105)

I discovered that if I ran `codex <PROMPT>` in a cwd that was not a Git
repo, Codex did not automatically run `<PROMPT>` after I accepted the
Git warning. It appears that we were not managing the `AppState`
transition correctly, so this fixes the bug and ensures the Codex
session does not start until the user accepts the Git warning.

In particular, we now create the `ChatWidget` lazily and store it in the
`AppState::Chat` variant.
This commit is contained in:
Michael Bolin
2025-05-24 08:33:49 -07:00
committed by GitHub
parent 4bf81373a7
commit 6b5b184f21

View File

@@ -7,6 +7,7 @@ use crate::mouse_capture::MouseCapture;
use crate::scroll_event_helper::ScrollEventHelper; use crate::scroll_event_helper::ScrollEventHelper;
use crate::slash_command::SlashCommand; use crate::slash_command::SlashCommand;
use crate::tui; use crate::tui;
// used by ChatWidgetArgs
use codex_core::config::Config; use codex_core::config::Config;
use codex_core::protocol::Event; use codex_core::protocol::Event;
use codex_core::protocol::Op; use codex_core::protocol::Op;
@@ -15,25 +16,43 @@ use crossterm::event::KeyCode;
use crossterm::event::KeyEvent; use crossterm::event::KeyEvent;
use crossterm::event::MouseEvent; use crossterm::event::MouseEvent;
use crossterm::event::MouseEventKind; use crossterm::event::MouseEventKind;
use std::path::PathBuf;
use std::sync::mpsc::Receiver; use std::sync::mpsc::Receiver;
use std::sync::mpsc::channel; use std::sync::mpsc::channel;
/// Toplevel application state which fullscreen view is currently active. /// Top-level application state: which full-screen view is currently active.
enum AppState { #[allow(clippy::large_enum_variant)]
enum AppState<'a> {
/// The main chat UI is visible. /// The main chat UI is visible.
Chat, Chat {
/// The startup warning that recommends running codex inside a Git repo. /// Boxed to avoid a large enum variant and reduce the overall size of
/// `AppState`.
widget: Box<ChatWidget<'a>>,
},
/// The start-up warning that recommends running codex inside a Git repo.
GitWarning { screen: GitWarningScreen }, GitWarning { screen: GitWarningScreen },
} }
pub(crate) struct App<'a> { pub(crate) struct App<'a> {
app_event_tx: AppEventSender, app_event_tx: AppEventSender,
app_event_rx: Receiver<AppEvent>, app_event_rx: Receiver<AppEvent>,
chat_widget: ChatWidget<'a>, app_state: AppState<'a>,
app_state: AppState,
/// Stored parameters needed to instantiate the ChatWidget later, e.g.,
/// after dismissing the Git-repo warning.
chat_args: Option<ChatWidgetArgs>,
} }
impl App<'_> { /// Aggregate parameters needed to create a `ChatWidget`, as creation may be
/// deferred until after the Git warning screen is dismissed.
#[derive(Clone)]
struct ChatWidgetArgs {
config: Config,
initial_prompt: Option<String>,
initial_images: Vec<PathBuf>,
}
impl<'a> App<'a> {
pub(crate) fn new( pub(crate) fn new(
config: Config, config: Config,
initial_prompt: Option<String>, initial_prompt: Option<String>,
@@ -94,26 +113,33 @@ impl App<'_> {
}); });
} }
let chat_widget = ChatWidget::new( let (app_state, chat_args) = if show_git_warning {
config, (
app_event_tx.clone(), AppState::GitWarning {
initial_prompt.clone(), screen: GitWarningScreen::new(),
initial_images, },
); Some(ChatWidgetArgs {
config,
let app_state = if show_git_warning { initial_prompt,
AppState::GitWarning { initial_images,
screen: GitWarningScreen::new(), }),
} )
} else { } else {
AppState::Chat let chat_widget =
ChatWidget::new(config, app_event_tx.clone(), initial_prompt, initial_images);
(
AppState::Chat {
widget: Box::new(chat_widget),
},
None,
)
}; };
Self { Self {
app_event_tx, app_event_tx,
app_event_rx, app_event_rx,
chat_widget,
app_state, app_state,
chat_args,
} }
} }
@@ -144,7 +170,15 @@ impl App<'_> {
modifiers: crossterm::event::KeyModifiers::CONTROL, modifiers: crossterm::event::KeyModifiers::CONTROL,
.. ..
} => { } => {
self.chat_widget.submit_op(Op::Interrupt); // Forward interrupt to ChatWidget when active.
match &mut self.app_state {
AppState::Chat { widget } => {
widget.submit_op(Op::Interrupt);
}
AppState::GitWarning { .. } => {
// No-op.
}
}
} }
KeyEvent { KeyEvent {
code: KeyCode::Char('d'), code: KeyCode::Char('d'),
@@ -167,20 +201,19 @@ impl App<'_> {
AppEvent::ExitRequest => { AppEvent::ExitRequest => {
break; break;
} }
AppEvent::CodexOp(op) => { AppEvent::CodexOp(op) => match &mut self.app_state {
if matches!(self.app_state, AppState::Chat) { AppState::Chat { widget } => widget.submit_op(op),
self.chat_widget.submit_op(op); AppState::GitWarning { .. } => {}
} },
} AppEvent::LatestLog(line) => match &mut self.app_state {
AppEvent::LatestLog(line) => { AppState::Chat { widget } => widget.update_latest_log(line),
if matches!(self.app_state, AppState::Chat) { AppState::GitWarning { .. } => {}
self.chat_widget.update_latest_log(line); },
}
}
AppEvent::DispatchCommand(command) => match command { AppEvent::DispatchCommand(command) => match command {
SlashCommand::Clear => { SlashCommand::Clear => match &mut self.app_state {
self.chat_widget.clear_conversation_history(); AppState::Chat { widget } => widget.clear_conversation_history(),
} AppState::GitWarning { .. } => {}
},
SlashCommand::ToggleMouseMode => { SlashCommand::ToggleMouseMode => {
if let Err(e) = mouse_capture.toggle() { if let Err(e) = mouse_capture.toggle() {
tracing::error!("Failed to toggle mouse mode: {e}"); tracing::error!("Failed to toggle mouse mode: {e}");
@@ -199,8 +232,8 @@ impl App<'_> {
fn draw_next_frame(&mut self, terminal: &mut tui::Tui) -> Result<()> { fn draw_next_frame(&mut self, terminal: &mut tui::Tui) -> Result<()> {
match &mut self.app_state { match &mut self.app_state {
AppState::Chat => { AppState::Chat { widget } => {
terminal.draw(|frame| frame.render_widget_ref(&self.chat_widget, frame.area()))?; terminal.draw(|frame| frame.render_widget_ref(&**widget, frame.area()))?;
} }
AppState::GitWarning { screen } => { AppState::GitWarning { screen } => {
terminal.draw(|frame| frame.render_widget_ref(&*screen, frame.area()))?; terminal.draw(|frame| frame.render_widget_ref(&*screen, frame.area()))?;
@@ -213,12 +246,24 @@ impl App<'_> {
/// with it. /// with it.
fn dispatch_key_event(&mut self, key_event: KeyEvent) { fn dispatch_key_event(&mut self, key_event: KeyEvent) {
match &mut self.app_state { match &mut self.app_state {
AppState::Chat => { AppState::Chat { widget } => {
self.chat_widget.handle_key_event(key_event); widget.handle_key_event(key_event);
} }
AppState::GitWarning { screen } => match screen.handle_key_event(key_event) { AppState::GitWarning { screen } => match screen.handle_key_event(key_event) {
GitWarningOutcome::Continue => { GitWarningOutcome::Continue => {
self.app_state = AppState::Chat; // User accepted switch to chat view.
let args = match self.chat_args.take() {
Some(args) => args,
None => panic!("ChatWidgetArgs already consumed"),
};
let widget = Box::new(ChatWidget::new(
args.config,
self.app_event_tx.clone(),
args.initial_prompt,
args.initial_images,
));
self.app_state = AppState::Chat { widget };
self.app_event_tx.send(AppEvent::Redraw); self.app_event_tx.send(AppEvent::Redraw);
} }
GitWarningOutcome::Quit => { GitWarningOutcome::Quit => {
@@ -232,14 +277,16 @@ impl App<'_> {
} }
fn dispatch_scroll_event(&mut self, scroll_delta: i32) { fn dispatch_scroll_event(&mut self, scroll_delta: i32) {
if matches!(self.app_state, AppState::Chat) { match &mut self.app_state {
self.chat_widget.handle_scroll_delta(scroll_delta); AppState::Chat { widget } => widget.handle_scroll_delta(scroll_delta),
AppState::GitWarning { .. } => {}
} }
} }
fn dispatch_codex_event(&mut self, event: Event) { fn dispatch_codex_event(&mut self, event: Event) {
if matches!(self.app_state, AppState::Chat) { match &mut self.app_state {
self.chat_widget.handle_codex_event(event); AppState::Chat { widget } => widget.handle_codex_event(event),
AppState::GitWarning { .. } => {}
} }
} }
} }