Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 32 additions & 3 deletions codex-rs/tui/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ pub(crate) struct App {
primary_session_configured: Option<ThreadSessionState>,
pending_primary_events: VecDeque<ThreadBufferedEvent>,
pending_app_server_requests: PendingAppServerRequests,
pending_startup_thread_start: bool,
// Serialize plugin enablement writes per plugin so stale completions cannot
// overwrite a newer toggle, even if the plugin is toggled from different
// cwd contexts.
Expand Down Expand Up @@ -575,6 +576,27 @@ async fn resolve_runtime_model_provider_base_url(provider: &ModelProviderInfo) -
}
}

fn spawn_startup_thread_start(
app_server: &AppServerSession,
config: Config,
app_event_tx: AppEventSender,
) {
let request_handle = app_server.request_handle();
let thread_params_mode = app_server.thread_params_mode();
let remote_cwd_override = app_server.remote_cwd_override().map(Path::to_path_buf);
tokio::spawn(async move {
let result = crate::app_server_session::start_thread_with_request_handle(
request_handle,
config,
thread_params_mode,
remote_cwd_override,
)
.await
.map_err(|err| format!("{err:#}"));
app_event_tx.send(AppEvent::StartupThreadStarted { result });
});
}

#[derive(Debug, Clone, PartialEq, Eq)]
enum ActiveTurnSteerRace {
Missing,
Expand Down Expand Up @@ -778,10 +800,14 @@ impl App {
&initial_images,
);
let thread_and_widget_started_at = Instant::now();
let pending_startup_thread_start = matches!(
&session_selection,
SessionSelection::StartFresh | SessionSelection::Exit
);
let (mut chat_widget, initial_started_thread) = match session_selection {
SessionSelection::StartFresh | SessionSelection::Exit => {
let started = app_server.start_thread(&config).await?;
// Only count a startup tooltip once the fresh thread can actually render it.
spawn_startup_thread_start(&app_server, config.clone(), app_event_tx.clone());
// Count a startup tooltip once the initial chat widget can render it.
let startup_tooltip_override =
prepare_startup_tooltip_override(&mut config, &available_models, is_first_run)
.await;
Expand Down Expand Up @@ -811,7 +837,9 @@ impl App {
.clone(),
session_telemetry: session_telemetry.clone(),
};
(ChatWidget::new_with_app_event(init), Some(started))
let mut chat_widget = ChatWidget::new_with_app_event(init);
chat_widget.set_queue_submissions_until_session_configured(/*queue*/ true);
(chat_widget, None)
}
SessionSelection::Resume(target_session) => {
let resumed = app_server
Expand Down Expand Up @@ -956,6 +984,7 @@ See the Codex keymap documentation for supported actions and examples."
primary_session_configured: None,
pending_primary_events: VecDeque::new(),
pending_app_server_requests: PendingAppServerRequests::default(),
pending_startup_thread_start,
pending_plugin_enabled_writes: HashMap::new(),
pending_hook_enabled_writes: HashMap::new(),
};
Expand Down
4 changes: 4 additions & 0 deletions codex-rs/tui/src/app/event_dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ impl App {
)
.await;
}
AppEvent::StartupThreadStarted { result } => {
self.handle_startup_thread_started(app_server, result)
.await?;
}
AppEvent::ClearUi => {
self.clear_terminal_ui(tui, /*redraw_header*/ false)?;
self.reset_app_ui_state_after_clear();
Expand Down
38 changes: 38 additions & 0 deletions codex-rs/tui/src/app/session_lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,10 +418,48 @@ impl App {
self.primary_session_configured = None;
self.pending_primary_events.clear();
self.pending_app_server_requests.clear();
self.pending_startup_thread_start = false;
self.chat_widget.set_pending_thread_approvals(Vec::new());
self.sync_active_agent_label();
}

pub(super) async fn handle_startup_thread_started(
&mut self,
app_server: &mut AppServerSession,
result: Result<AppServerStartedThread, String>,
) -> Result<()> {
if !self.pending_startup_thread_start {
if let Ok(started) = result {
let thread_id = started.session.thread_id;
if let Err(err) = app_server.thread_unsubscribe(thread_id).await {
tracing::warn!(
thread_id = %thread_id,
"failed to unsubscribe stale startup thread: {err}"
);
}
self.discard_thread_local_state(thread_id).await;
}
return Ok(());
}

self.pending_startup_thread_start = false;
self.chat_widget
.set_queue_submissions_until_session_configured(/*queue*/ false);
match result {
Ok(started) => {
self.enqueue_primary_thread_session(started.session, started.turns)
.await?;
Comment thread
etraut-openai marked this conversation as resolved.
self.chat_widget.maybe_send_next_queued_input();
}
Err(err) => {
return Err(color_eyre::eyre::eyre!(
"Failed to start a fresh session through the app server: {err}"
));
Comment thread
etraut-openai marked this conversation as resolved.
}
}
Ok(())
}

pub(super) async fn start_fresh_session_with_summary_hint(
&mut self,
tui: &mut tui::Tui,
Expand Down
6 changes: 3 additions & 3 deletions codex-rs/tui/src/app/side.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,15 +369,15 @@ impl App {
self.chat_widget.add_error_message(message);
return false;
}
self.discard_side_thread_local(thread_id).await;
self.discard_thread_local_state(thread_id).await;
true
}

pub(super) async fn discard_closed_side_thread(&mut self, thread_id: ThreadId) {
self.discard_side_thread_local(thread_id).await;
self.discard_thread_local_state(thread_id).await;
}

async fn discard_side_thread_local(&mut self, thread_id: ThreadId) {
pub(super) async fn discard_thread_local_state(&mut self, thread_id: ThreadId) {
self.abort_thread_event_listener(thread_id);
self.thread_event_channels.remove(&thread_id);
self.side_threads.remove(&thread_id);
Expand Down
1 change: 1 addition & 0 deletions codex-rs/tui/src/app/test_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ pub(super) async fn make_test_app() -> App {
primary_session_configured: None,
pending_primary_events: VecDeque::new(),
pending_app_server_requests: PendingAppServerRequests::default(),
pending_startup_thread_start: false,
pending_plugin_enabled_writes: HashMap::new(),
pending_hook_enabled_writes: HashMap::new(),
}
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/tui/src/app/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3882,6 +3882,7 @@ async fn make_test_app() -> App {
primary_session_configured: None,
pending_primary_events: VecDeque::new(),
pending_app_server_requests: PendingAppServerRequests::default(),
pending_startup_thread_start: false,
pending_plugin_enabled_writes: HashMap::new(),
pending_hook_enabled_writes: HashMap::new(),
}
Expand Down Expand Up @@ -3945,6 +3946,7 @@ async fn make_test_app_with_channels() -> (
primary_session_configured: None,
pending_primary_events: VecDeque::new(),
pending_app_server_requests: PendingAppServerRequests::default(),
pending_startup_thread_start: false,
pending_plugin_enabled_writes: HashMap::new(),
pending_hook_enabled_writes: HashMap::new(),
},
Expand Down
118 changes: 118 additions & 0 deletions codex-rs/tui/src/app/tests/startup.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use super::*;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
use pretty_assertions::assert_eq;

#[test]
Expand Down Expand Up @@ -132,6 +135,121 @@ fn startup_waiting_gate_not_applied_for_resume_or_fork_session_selection() {
);
}

#[tokio::test]
async fn startup_thread_started_submits_queued_startup_input() {
let (mut app, _app_event_rx, mut op_rx) = make_test_app_with_channels().await;
app.pending_startup_thread_start = true;
app.chat_widget
.set_queue_submissions_until_session_configured(/*queue*/ true);
app.chat_widget
.apply_external_edit("queued before startup completes".to_string());
app.chat_widget
.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));

assert_eq!(
app.chat_widget.queued_user_message_texts(),
vec!["queued before startup completes".to_string()]
);

let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(
app.chat_widget.config_ref(),
))
.await
.expect("embedded app server");
let thread_id = ThreadId::new();
app.handle_startup_thread_started(
&mut app_server,
Ok(AppServerStartedThread {
session: test_thread_session(thread_id, test_path_buf("/tmp/project")),
turns: Vec::new(),
}),
)
.await
.expect("startup thread should attach");

match next_user_turn_op(&mut op_rx) {
Op::UserTurn { items, .. } => assert_eq!(
items,
vec![UserInput::Text {
text: "queued before startup completes".to_string(),
text_elements: Vec::new(),
}]
),
other => panic!("expected queued startup input submission, got {other:?}"),
}
}

#[tokio::test]
async fn startup_thread_start_failure_returns_error() {
let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await;
app.pending_startup_thread_start = true;

let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(
app.chat_widget.config_ref(),
))
.await
.expect("embedded app server");
let err = app
.handle_startup_thread_started(&mut app_server, Err("boom".to_string()))
.await
.expect_err("startup thread failure should exit instead of leaving chat unconfigured");

assert!(
err.to_string()
.contains("Failed to start a fresh session through the app server: boom")
);
assert!(!app.pending_startup_thread_start);
assert_eq!(app.primary_thread_id, None);
}

#[test]
fn stale_startup_thread_started_removes_local_routing_state() -> Result<()> {
tokio::runtime::Builder::new_multi_thread()
.worker_threads(1)
.thread_stack_size(8 * 1024 * 1024)
.enable_all()
.build()?
.block_on(async {
let mut app = make_test_app().await;
let mut app_server =
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()).await?;
let primary_thread_id = ThreadId::new();
let stale_thread_id = ThreadId::new();
app.primary_thread_id = Some(primary_thread_id);
app.thread_event_channels.insert(
primary_thread_id,
ThreadEventChannel::new(THREAD_EVENT_CHANNEL_CAPACITY),
);
app.activate_thread_channel(primary_thread_id).await;
app.thread_event_channels.insert(
stale_thread_id,
ThreadEventChannel::new(THREAD_EVENT_CHANNEL_CAPACITY),
);
app.agent_navigation.upsert(
stale_thread_id,
/*agent_nickname*/ None,
/*agent_role*/ None,
/*is_closed*/ false,
);
assert!(app.thread_event_channels.contains_key(&stale_thread_id));
assert!(app.agent_navigation.get(&stale_thread_id).is_some());

app.handle_startup_thread_started(
&mut app_server,
Ok(AppServerStartedThread {
session: test_thread_session(stale_thread_id, test_path_buf("/tmp/project")),
turns: Vec::new(),
}),
)
.await?;

assert!(!app.thread_event_channels.contains_key(&stale_thread_id));
assert_eq!(app.agent_navigation.get(&stale_thread_id), None);
assert_eq!(app.active_thread_id, Some(primary_thread_id));
Ok(())
})
}

#[tokio::test]
async fn ignore_same_thread_resume_reports_noop_for_current_thread() {
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
Expand Down
6 changes: 6 additions & 0 deletions codex-rs/tui/src/app_event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_approval_presets::ApprovalPreset;

use crate::app_command::AppCommand;
use crate::app_server_session::AppServerStartedThread;
use crate::bottom_pane::ApprovalRequest;
use crate::bottom_pane::StatusLineItem;
use crate::bottom_pane::TerminalTitleItem;
Expand Down Expand Up @@ -180,6 +181,11 @@ pub(crate) enum AppEvent {
/// Start a new session.
NewSession,

/// Result of the fresh startup thread that is attached after the input UI is live.
StartupThreadStarted {
result: Result<AppServerStartedThread, String>,
},

/// Clear the terminal UI (screen + scrollback), start a fresh session, and keep the
/// previous chat resumable.
ClearUi,
Expand Down
26 changes: 25 additions & 1 deletion codex-rs/tui/src/app_server_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ use color_eyre::eyre::Result;
use color_eyre::eyre::WrapErr;
use std::collections::HashMap;
use std::path::PathBuf;
use uuid::Uuid;

fn bootstrap_request_error(context: &'static str, err: TypedRequestError) -> color_eyre::Report {
color_eyre::eyre::eyre!("{context}: {err}")
Expand Down Expand Up @@ -167,6 +168,7 @@ impl ThreadParamsMode {
}
}

#[derive(Debug)]
pub(crate) struct AppServerStartedThread {
pub(crate) session: ThreadSessionState,
pub(crate) turns: Vec<Turn>,
Expand Down Expand Up @@ -337,6 +339,7 @@ impl AppServerSession {
self.client.next_event().await
}

#[cfg(test)]
pub(crate) async fn start_thread(&mut self, config: &Config) -> Result<AppServerStartedThread> {
self.start_thread_with_session_start_source(config, /*session_start_source*/ None)
.await
Expand Down Expand Up @@ -427,7 +430,7 @@ impl AppServerSession {
Ok(started)
}

fn thread_params_mode(&self) -> ThreadParamsMode {
pub(crate) fn thread_params_mode(&self) -> ThreadParamsMode {
self.thread_params_mode
}

Expand Down Expand Up @@ -1002,6 +1005,27 @@ impl AppServerSession {
}
}

pub(crate) async fn start_thread_with_request_handle(
request_handle: AppServerRequestHandle,
config: Config,
thread_params_mode: ThreadParamsMode,
remote_cwd_override: Option<PathBuf>,
) -> Result<AppServerStartedThread> {
let response: ThreadStartResponse = request_handle
.request_typed(ClientRequest::ThreadStart {
request_id: RequestId::String(format!("startup-thread-start-{}", Uuid::new_v4())),
params: thread_start_params_from_config(
&config,
thread_params_mode,
remote_cwd_override.as_deref(),
/*session_start_source*/ None,
),
})
.await
.map_err(|err| bootstrap_request_error("thread/start failed during TUI bootstrap", err))?;
started_thread_from_start_response(response, &config, thread_params_mode).await
}

fn thread_realtime_start_params(
thread_id: ThreadId,
transport: Option<ThreadRealtimeStartTransport>,
Expand Down
Loading
Loading