Skip to content

Commit 6b5b184

Browse files
authored
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.
1 parent 4bf8137 commit 6b5b184

File tree

1 file changed

+90
-43
lines changed

1 file changed

+90
-43
lines changed

codex-rs/tui/src/app.rs

Lines changed: 90 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::mouse_capture::MouseCapture;
77
use crate::scroll_event_helper::ScrollEventHelper;
88
use crate::slash_command::SlashCommand;
99
use crate::tui;
10+
// used by ChatWidgetArgs
1011
use codex_core::config::Config;
1112
use codex_core::protocol::Event;
1213
use codex_core::protocol::Op;
@@ -15,25 +16,43 @@ use crossterm::event::KeyCode;
1516
use crossterm::event::KeyEvent;
1617
use crossterm::event::MouseEvent;
1718
use crossterm::event::MouseEventKind;
19+
use std::path::PathBuf;
1820
use std::sync::mpsc::Receiver;
1921
use std::sync::mpsc::channel;
2022

21-
/// Top‑level application state – which full‑screen view is currently active.
22-
enum AppState {
23+
/// Top-level application state: which full-screen view is currently active.
24+
#[allow(clippy::large_enum_variant)]
25+
enum AppState<'a> {
2326
/// The main chat UI is visible.
24-
Chat,
25-
/// The start‑up warning that recommends running codex inside a Git repo.
27+
Chat {
28+
/// Boxed to avoid a large enum variant and reduce the overall size of
29+
/// `AppState`.
30+
widget: Box<ChatWidget<'a>>,
31+
},
32+
/// The start-up warning that recommends running codex inside a Git repo.
2633
GitWarning { screen: GitWarningScreen },
2734
}
2835

2936
pub(crate) struct App<'a> {
3037
app_event_tx: AppEventSender,
3138
app_event_rx: Receiver<AppEvent>,
32-
chat_widget: ChatWidget<'a>,
33-
app_state: AppState,
39+
app_state: AppState<'a>,
40+
41+
/// Stored parameters needed to instantiate the ChatWidget later, e.g.,
42+
/// after dismissing the Git-repo warning.
43+
chat_args: Option<ChatWidgetArgs>,
44+
}
45+
46+
/// Aggregate parameters needed to create a `ChatWidget`, as creation may be
47+
/// deferred until after the Git warning screen is dismissed.
48+
#[derive(Clone)]
49+
struct ChatWidgetArgs {
50+
config: Config,
51+
initial_prompt: Option<String>,
52+
initial_images: Vec<PathBuf>,
3453
}
3554

36-
impl App<'_> {
55+
impl<'a> App<'a> {
3756
pub(crate) fn new(
3857
config: Config,
3958
initial_prompt: Option<String>,
@@ -94,26 +113,33 @@ impl App<'_> {
94113
});
95114
}
96115

97-
let chat_widget = ChatWidget::new(
98-
config,
99-
app_event_tx.clone(),
100-
initial_prompt.clone(),
101-
initial_images,
102-
);
103-
104-
let app_state = if show_git_warning {
105-
AppState::GitWarning {
106-
screen: GitWarningScreen::new(),
107-
}
116+
let (app_state, chat_args) = if show_git_warning {
117+
(
118+
AppState::GitWarning {
119+
screen: GitWarningScreen::new(),
120+
},
121+
Some(ChatWidgetArgs {
122+
config,
123+
initial_prompt,
124+
initial_images,
125+
}),
126+
)
108127
} else {
109-
AppState::Chat
128+
let chat_widget =
129+
ChatWidget::new(config, app_event_tx.clone(), initial_prompt, initial_images);
130+
(
131+
AppState::Chat {
132+
widget: Box::new(chat_widget),
133+
},
134+
None,
135+
)
110136
};
111137

112138
Self {
113139
app_event_tx,
114140
app_event_rx,
115-
chat_widget,
116141
app_state,
142+
chat_args,
117143
}
118144
}
119145

@@ -144,7 +170,15 @@ impl App<'_> {
144170
modifiers: crossterm::event::KeyModifiers::CONTROL,
145171
..
146172
} => {
147-
self.chat_widget.submit_op(Op::Interrupt);
173+
// Forward interrupt to ChatWidget when active.
174+
match &mut self.app_state {
175+
AppState::Chat { widget } => {
176+
widget.submit_op(Op::Interrupt);
177+
}
178+
AppState::GitWarning { .. } => {
179+
// No-op.
180+
}
181+
}
148182
}
149183
KeyEvent {
150184
code: KeyCode::Char('d'),
@@ -167,20 +201,19 @@ impl App<'_> {
167201
AppEvent::ExitRequest => {
168202
break;
169203
}
170-
AppEvent::CodexOp(op) => {
171-
if matches!(self.app_state, AppState::Chat) {
172-
self.chat_widget.submit_op(op);
173-
}
174-
}
175-
AppEvent::LatestLog(line) => {
176-
if matches!(self.app_state, AppState::Chat) {
177-
self.chat_widget.update_latest_log(line);
178-
}
179-
}
204+
AppEvent::CodexOp(op) => match &mut self.app_state {
205+
AppState::Chat { widget } => widget.submit_op(op),
206+
AppState::GitWarning { .. } => {}
207+
},
208+
AppEvent::LatestLog(line) => match &mut self.app_state {
209+
AppState::Chat { widget } => widget.update_latest_log(line),
210+
AppState::GitWarning { .. } => {}
211+
},
180212
AppEvent::DispatchCommand(command) => match command {
181-
SlashCommand::Clear => {
182-
self.chat_widget.clear_conversation_history();
183-
}
213+
SlashCommand::Clear => match &mut self.app_state {
214+
AppState::Chat { widget } => widget.clear_conversation_history(),
215+
AppState::GitWarning { .. } => {}
216+
},
184217
SlashCommand::ToggleMouseMode => {
185218
if let Err(e) = mouse_capture.toggle() {
186219
tracing::error!("Failed to toggle mouse mode: {e}");
@@ -199,8 +232,8 @@ impl App<'_> {
199232

200233
fn draw_next_frame(&mut self, terminal: &mut tui::Tui) -> Result<()> {
201234
match &mut self.app_state {
202-
AppState::Chat => {
203-
terminal.draw(|frame| frame.render_widget_ref(&self.chat_widget, frame.area()))?;
235+
AppState::Chat { widget } => {
236+
terminal.draw(|frame| frame.render_widget_ref(&**widget, frame.area()))?;
204237
}
205238
AppState::GitWarning { screen } => {
206239
terminal.draw(|frame| frame.render_widget_ref(&*screen, frame.area()))?;
@@ -213,12 +246,24 @@ impl App<'_> {
213246
/// with it.
214247
fn dispatch_key_event(&mut self, key_event: KeyEvent) {
215248
match &mut self.app_state {
216-
AppState::Chat => {
217-
self.chat_widget.handle_key_event(key_event);
249+
AppState::Chat { widget } => {
250+
widget.handle_key_event(key_event);
218251
}
219252
AppState::GitWarning { screen } => match screen.handle_key_event(key_event) {
220253
GitWarningOutcome::Continue => {
221-
self.app_state = AppState::Chat;
254+
// User accepted – switch to chat view.
255+
let args = match self.chat_args.take() {
256+
Some(args) => args,
257+
None => panic!("ChatWidgetArgs already consumed"),
258+
};
259+
260+
let widget = Box::new(ChatWidget::new(
261+
args.config,
262+
self.app_event_tx.clone(),
263+
args.initial_prompt,
264+
args.initial_images,
265+
));
266+
self.app_state = AppState::Chat { widget };
222267
self.app_event_tx.send(AppEvent::Redraw);
223268
}
224269
GitWarningOutcome::Quit => {
@@ -232,14 +277,16 @@ impl App<'_> {
232277
}
233278

234279
fn dispatch_scroll_event(&mut self, scroll_delta: i32) {
235-
if matches!(self.app_state, AppState::Chat) {
236-
self.chat_widget.handle_scroll_delta(scroll_delta);
280+
match &mut self.app_state {
281+
AppState::Chat { widget } => widget.handle_scroll_delta(scroll_delta),
282+
AppState::GitWarning { .. } => {}
237283
}
238284
}
239285

240286
fn dispatch_codex_event(&mut self, event: Event) {
241-
if matches!(self.app_state, AppState::Chat) {
242-
self.chat_widget.handle_codex_event(event);
287+
match &mut self.app_state {
288+
AppState::Chat { widget } => widget.handle_codex_event(event),
289+
AppState::GitWarning { .. } => {}
243290
}
244291
}
245292
}

0 commit comments

Comments
 (0)