mirror of
https://github.com/openai/codex.git
synced 2026-11-05 16:29:31 +00:00
Allow naming sessions with /new and /clear (#34605)
## What changed - Accept an optional session name after `/new` or `/clear`. - Set the requested name through the app server when starting the new thread and update the attached session with that name. - Report naming failures in the chat while still attaching the newly created thread. ## Testing - Cover named `/new` and `/clear` dispatch, app-server name persistence, and the existing unnamed `/clear` behavior. GitOrigin-RevId: 570886abb4af522123f0693ce05d2415e1c0d902
This commit is contained in:
1 parent
2497972808
commit
25d2dfcad0
8 files changed
+149
-21
No files matched your search
@@ -22,10 +22,10 @@ impl App {
|
|||||||
event: AppEvent,
|
event: AppEvent,
|
||||||
) -> Result<AppRunControl> {
|
) -> Result<AppRunControl> {
|
||||||
match event {
|
match event {
|
||||||
AppEvent::NewSession => {
|
AppEvent::NewSession { name } => {
|
||||||
self.start_fresh_session_with_summary_hint(
|
self.start_fresh_session_with_summary_hint(
|
||||||
tui, app_server, /*session_start_source*/ None,
|
tui, app_server, /*session_start_source*/ None,
|
||||||
/*initial_user_message*/ None,
|
/*initial_user_message*/ None, name,
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
}
|
}
|
||||||
@@ -33,7 +33,7 @@ impl App {
|
|||||||
self.handle_startup_thread_started(app_server, result)
|
self.handle_startup_thread_started(app_server, result)
|
||||||
.await?;
|
.await?;
|
||||||
}
|
}
|
||||||
AppEvent::ClearUi => {
|
AppEvent::ClearUi { name } => {
|
||||||
self.clear_terminal_ui(tui, /*redraw_header*/ false)?;
|
self.clear_terminal_ui(tui, /*redraw_header*/ false)?;
|
||||||
self.reset_app_ui_state_after_clear();
|
self.reset_app_ui_state_after_clear();
|
||||||
|
|
||||||
@@ -42,6 +42,7 @@ impl App {
|
|||||||
app_server,
|
app_server,
|
||||||
Some(ThreadStartSource::Clear),
|
Some(ThreadStartSource::Clear),
|
||||||
/*initial_user_message*/ None,
|
/*initial_user_message*/ None,
|
||||||
|
name,
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
}
|
}
|
||||||
@@ -61,6 +62,7 @@ impl App {
|
|||||||
Vec::new(),
|
Vec::new(),
|
||||||
Vec::new(),
|
Vec::new(),
|
||||||
),
|
),
|
||||||
|
/*new_thread_name*/ None,
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -604,6 +604,7 @@ impl App {
|
|||||||
app_server: &mut AppServerSession,
|
app_server: &mut AppServerSession,
|
||||||
session_start_source: Option<ThreadStartSource>,
|
session_start_source: Option<ThreadStartSource>,
|
||||||
initial_user_message: Option<crate::chatwidget::UserMessage>,
|
initial_user_message: Option<crate::chatwidget::UserMessage>,
|
||||||
|
new_thread_name: Option<String>,
|
||||||
) {
|
) {
|
||||||
// Start a fresh in-memory session while preserving resumability via persisted rollout
|
// Start a fresh in-memory session while preserving resumability via persisted rollout
|
||||||
// history. If an initial message is provided, `enqueue_primary_thread_session` suppresses it
|
// history. If an initial message is provided, `enqueue_primary_thread_session` suppresses it
|
||||||
@@ -637,7 +638,21 @@ impl App {
|
|||||||
.start_thread_with_session_start_source(&config, session_start_source)
|
.start_thread_with_session_start_source(&config, session_start_source)
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
Ok(started) => {
|
Ok(mut started) => {
|
||||||
|
let name_error = if let Some(name) = new_thread_name {
|
||||||
|
match app_server
|
||||||
|
.thread_set_name(started.session.thread_id, name.clone())
|
||||||
|
.await
|
||||||
|
{
|
||||||
|
Ok(()) => {
|
||||||
|
started.session.thread_name = Some(name);
|
||||||
|
None
|
||||||
|
}
|
||||||
|
Err(err) => Some(format!("Failed to name the new session: {err}")),
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
};
|
||||||
if let Err(err) = self
|
if let Err(err) = self
|
||||||
.replace_chat_widget_with_app_server_thread(
|
.replace_chat_widget_with_app_server_thread(
|
||||||
tui,
|
tui,
|
||||||
@@ -650,16 +665,22 @@ impl App {
|
|||||||
self.chat_widget.add_error_message(format!(
|
self.chat_widget.add_error_message(format!(
|
||||||
"Failed to attach to fresh app-server thread: {err}"
|
"Failed to attach to fresh app-server thread: {err}"
|
||||||
));
|
));
|
||||||
} else if let Some(summary) = summary {
|
} else {
|
||||||
let mut lines: Vec<Line<'static>> = Vec::new();
|
if let Some(err) = name_error {
|
||||||
if let Some(usage_line) = summary.usage_line {
|
self.chat_widget.add_error_message(err);
|
||||||
lines.push(usage_line.into());
|
|
||||||
}
|
}
|
||||||
if let Some(command) = summary.resume_hint {
|
if let Some(summary) = summary {
|
||||||
let spans = vec!["To continue this session, run ".into(), command.cyan()];
|
let mut lines: Vec<Line<'static>> = Vec::new();
|
||||||
lines.push(spans.into());
|
if let Some(usage_line) = summary.usage_line {
|
||||||
|
lines.push(usage_line.into());
|
||||||
|
}
|
||||||
|
if let Some(command) = summary.resume_hint {
|
||||||
|
let spans =
|
||||||
|
vec!["To continue this session, run ".into(), command.cyan()];
|
||||||
|
lines.push(spans.into());
|
||||||
|
}
|
||||||
|
self.chat_widget.add_plain_history_lines(lines);
|
||||||
}
|
}
|
||||||
self.chat_widget.add_plain_history_lines(lines);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Err(err) => {
|
Err(err) => {
|
||||||
|
|||||||
@@ -137,6 +137,62 @@ async fn start_recording_app_server(
|
|||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn fresh_session_applies_requested_name() -> Result<()> {
|
||||||
|
const TEST_STACK_SIZE_BYTES: usize = 8 * 1024 * 1024;
|
||||||
|
|
||||||
|
std::thread::Builder::new()
|
||||||
|
.name("tui-named-fresh-session".to_string())
|
||||||
|
.stack_size(TEST_STACK_SIZE_BYTES)
|
||||||
|
.spawn(|| {
|
||||||
|
let runtime = tokio::runtime::Builder::new_current_thread()
|
||||||
|
.enable_all()
|
||||||
|
.build()?;
|
||||||
|
runtime.block_on(async {
|
||||||
|
let mut app = make_test_app().await;
|
||||||
|
let codex_home = tempdir()?;
|
||||||
|
app.config.codex_home = codex_home.path().to_path_buf().abs();
|
||||||
|
app.config.sqlite_home = codex_home.path().to_path_buf();
|
||||||
|
let (mut app_server, requests, proxy) =
|
||||||
|
start_recording_app_server(&app.config).await?;
|
||||||
|
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||||
|
|
||||||
|
app.start_fresh_session_with_summary_hint(
|
||||||
|
&mut tui,
|
||||||
|
&mut app_server,
|
||||||
|
/*session_start_source*/ None,
|
||||||
|
/*initial_user_message*/ None,
|
||||||
|
/*new_thread_name*/ Some("Add User".to_string()),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
|
||||||
|
let thread_id = app
|
||||||
|
.chat_widget
|
||||||
|
.thread_id()
|
||||||
|
.expect("fresh session should have a thread id");
|
||||||
|
assert_eq!(app.chat_widget.thread_name(), Some("Add User".to_string()));
|
||||||
|
assert!(
|
||||||
|
requests
|
||||||
|
.lock()
|
||||||
|
.expect("request recorder lock")
|
||||||
|
.iter()
|
||||||
|
.any(|method| method == "thread/name/set"),
|
||||||
|
"fresh session should be named through the app server"
|
||||||
|
);
|
||||||
|
let thread = app_server
|
||||||
|
.thread_read(thread_id, /*include_turns*/ false)
|
||||||
|
.await?;
|
||||||
|
assert_eq!(thread.name.as_deref(), Some("Add User"));
|
||||||
|
|
||||||
|
app_server.shutdown().await?;
|
||||||
|
proxy.await??;
|
||||||
|
Ok(())
|
||||||
|
})
|
||||||
|
})?
|
||||||
|
.join()
|
||||||
|
.expect("named fresh session test thread")
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn session_lifecycle_avoids_redundant_subagent_metadata_reads() -> Result<()> {
|
fn session_lifecycle_avoids_redundant_subagent_metadata_reads() -> Result<()> {
|
||||||
const TEST_STACK_SIZE_BYTES: usize = 8 * 1024 * 1024;
|
const TEST_STACK_SIZE_BYTES: usize = 8 * 1024 * 1024;
|
||||||
@@ -231,6 +287,7 @@ fn session_lifecycle_avoids_redundant_subagent_metadata_reads() -> Result<()> {
|
|||||||
&mut app_server,
|
&mut app_server,
|
||||||
/*session_start_source*/ None,
|
/*session_start_source*/ None,
|
||||||
/*initial_user_message*/ None,
|
/*initial_user_message*/ None,
|
||||||
|
/*new_thread_name*/ None,
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
|
|
||||||
|
|||||||
@@ -235,8 +235,10 @@ pub(crate) enum AppEvent {
|
|||||||
log_id: u64,
|
log_id: u64,
|
||||||
},
|
},
|
||||||
|
|
||||||
/// Start a new session.
|
/// Start a new session, optionally assigning it a name.
|
||||||
NewSession,
|
NewSession {
|
||||||
|
name: Option<String>,
|
||||||
|
},
|
||||||
|
|
||||||
/// Result of the fresh startup thread that is attached after the input UI is live.
|
/// Result of the fresh startup thread that is attached after the input UI is live.
|
||||||
StartupThreadStarted {
|
StartupThreadStarted {
|
||||||
@@ -245,7 +247,9 @@ pub(crate) enum AppEvent {
|
|||||||
|
|
||||||
/// Clear the terminal UI (screen + scrollback), start a fresh session, and keep the
|
/// Clear the terminal UI (screen + scrollback), start a fresh session, and keep the
|
||||||
/// previous chat resumable.
|
/// previous chat resumable.
|
||||||
ClearUi,
|
ClearUi {
|
||||||
|
name: Option<String>,
|
||||||
|
},
|
||||||
|
|
||||||
/// Re-render the transcript using the selected scrollback rendering mode.
|
/// Re-render the transcript using the selected scrollback rendering mode.
|
||||||
RawOutputModeChanged {
|
RawOutputModeChanged {
|
||||||
|
|||||||
@@ -171,7 +171,7 @@ impl ChatWidget {
|
|||||||
self.request_redraw();
|
self.request_redraw();
|
||||||
}
|
}
|
||||||
SlashCommand::New => {
|
SlashCommand::New => {
|
||||||
self.app_event_tx.send(AppEvent::NewSession);
|
self.app_event_tx.send(AppEvent::NewSession { name: None });
|
||||||
}
|
}
|
||||||
SlashCommand::Archive => {
|
SlashCommand::Archive => {
|
||||||
self.bottom_pane.show_selection_view(SelectionViewParams {
|
self.bottom_pane.show_selection_view(SelectionViewParams {
|
||||||
@@ -231,7 +231,7 @@ impl ChatWidget {
|
|||||||
self.request_redraw();
|
self.request_redraw();
|
||||||
}
|
}
|
||||||
SlashCommand::Clear => {
|
SlashCommand::Clear => {
|
||||||
self.app_event_tx.send(AppEvent::ClearUi);
|
self.app_event_tx.send(AppEvent::ClearUi { name: None });
|
||||||
}
|
}
|
||||||
SlashCommand::Resume => {
|
SlashCommand::Resume => {
|
||||||
self.app_event_tx.send(AppEvent::OpenResumePicker);
|
self.app_event_tx.send(AppEvent::OpenResumePicker);
|
||||||
@@ -725,6 +725,16 @@ impl ChatWidget {
|
|||||||
};
|
};
|
||||||
self.app_event_tx.set_thread_name(name);
|
self.app_event_tx.set_thread_name(name);
|
||||||
}
|
}
|
||||||
|
SlashCommand::New if !trimmed.is_empty() => {
|
||||||
|
self.app_event_tx.send(AppEvent::NewSession {
|
||||||
|
name: Some(trimmed.to_string()),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
SlashCommand::Clear if !trimmed.is_empty() => {
|
||||||
|
self.app_event_tx.send(AppEvent::ClearUi {
|
||||||
|
name: Some(trimmed.to_string()),
|
||||||
|
});
|
||||||
|
}
|
||||||
SlashCommand::Plan if !trimmed.is_empty() => {
|
SlashCommand::Plan if !trimmed.is_empty() => {
|
||||||
if !self.apply_plan_slash_command() {
|
if !self.apply_plan_slash_command() {
|
||||||
return;
|
return;
|
||||||
|
|||||||
@@ -2104,7 +2104,39 @@ async fn slash_clear_requests_ui_clear_when_idle() {
|
|||||||
|
|
||||||
chat.dispatch_command(SlashCommand::Clear);
|
chat.dispatch_command(SlashCommand::Clear);
|
||||||
|
|
||||||
assert_matches!(rx.try_recv(), Ok(AppEvent::ClearUi));
|
assert_matches!(rx.try_recv(), Ok(AppEvent::ClearUi { name: None }));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn slash_new_with_name_requests_named_session() {
|
||||||
|
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||||
|
chat.bottom_pane
|
||||||
|
.set_composer_text("/new Add User ".to_string(), Vec::new(), Vec::new());
|
||||||
|
|
||||||
|
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||||
|
|
||||||
|
assert_matches!(
|
||||||
|
rx.try_recv(),
|
||||||
|
Ok(AppEvent::NewSession {
|
||||||
|
name: Some(name)
|
||||||
|
}) if name == "Add User"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn slash_clear_with_name_requests_named_session() {
|
||||||
|
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||||
|
chat.bottom_pane
|
||||||
|
.set_composer_text("/clear Add User ".to_string(), Vec::new(), Vec::new());
|
||||||
|
|
||||||
|
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||||
|
|
||||||
|
assert_matches!(
|
||||||
|
rx.try_recv(),
|
||||||
|
Ok(AppEvent::ClearUi {
|
||||||
|
name: Some(name)
|
||||||
|
}) if name == "Add User"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
@@ -2130,7 +2162,7 @@ async fn slash_clear_after_ctrl_c_keeps_stashed_draft_recallable() {
|
|||||||
.set_composer_text("/clear".to_string(), Vec::new(), Vec::new());
|
.set_composer_text("/clear".to_string(), Vec::new(), Vec::new());
|
||||||
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||||
|
|
||||||
assert_matches!(rx.try_recv(), Ok(AppEvent::ClearUi));
|
assert_matches!(rx.try_recv(), Ok(AppEvent::ClearUi { name: None }));
|
||||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
||||||
assert_eq!(chat.bottom_pane.composer_text(), stashed_draft);
|
assert_eq!(chat.bottom_pane.composer_text(), stashed_draft);
|
||||||
|
|
||||||
|
|||||||
@@ -126,7 +126,7 @@ pub(crate) fn log_inbound_app_event(event: &AppEvent) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
match event {
|
match event {
|
||||||
AppEvent::NewSession => {
|
AppEvent::NewSession { .. } => {
|
||||||
let value = json!({
|
let value = json!({
|
||||||
"ts": now_ts(),
|
"ts": now_ts(),
|
||||||
"dir": "to_tui",
|
"dir": "to_tui",
|
||||||
@@ -134,7 +134,7 @@ pub(crate) fn log_inbound_app_event(event: &AppEvent) {
|
|||||||
});
|
});
|
||||||
LOGGER.write_json_line(value);
|
LOGGER.write_json_line(value);
|
||||||
}
|
}
|
||||||
AppEvent::ClearUi => {
|
AppEvent::ClearUi { .. } => {
|
||||||
let value = json!({
|
let value = json!({
|
||||||
"ts": now_ts(),
|
"ts": now_ts(),
|
||||||
"dir": "to_tui",
|
"dir": "to_tui",
|
||||||
|
|||||||
@@ -155,6 +155,8 @@ impl SlashCommand {
|
|||||||
self,
|
self,
|
||||||
SlashCommand::Review
|
SlashCommand::Review
|
||||||
| SlashCommand::Rename
|
| SlashCommand::Rename
|
||||||
|
| SlashCommand::New
|
||||||
|
| SlashCommand::Clear
|
||||||
| SlashCommand::Plan
|
| SlashCommand::Plan
|
||||||
| SlashCommand::Goal
|
| SlashCommand::Goal
|
||||||
| SlashCommand::Ide
|
| SlashCommand::Ide
|
||||||
|
|||||||
Reference in new issue
Block a user