agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

tui: use thread_id for resume/fork cwd resolution (#12727)

Browse Source 
## Summary
- make resume/fork targets explicit and typed as `SessionTarget { path,
thread_id }` (non-optional `thread_id`)
- resolve `thread_id` centrally via `resolve_session_thread_id(...)`:
- use CLI input directly when it is a UUID (`--resume <uuid>` / `--fork
<uuid>`)
- otherwise read `thread_id` from rollout `SessionMeta` for path-based
selections (picker, `--resume-last`, name-based resume/fork)
- use `thread_id` to read cwd from SQLite first during resume/fork cwd
resolution
- keep rollout fallback for cwd resolution when SQLite is unavailable or
does not return thread metadata (`TurnContext` tail, then `SessionMeta`)
- keep the resume picker open when a selected row has unreadable session
metadata, and show an inline recoverable error instead of aborting the
TUI

## Why
This removes ad-hoc rollout filename parsing and makes resume/fork
target identity explicit. The resume/fork cwd check can use indexed
SQLite lookup by `thread_id` in the common path, while preserving
rollout-based fallback behavior. It also keeps malformed legacy rows
recoverable in the picker instead of letting a selection failure unwind
the app.

## Notes
- minimal TUI-only change; no schema/protocol changes
- includes TUI test coverage for SQLite cwd precedence when `thread_id`
is available
- includes TUI regression coverage for picker inline error rendering /
non-fatal unreadable session rows

## Codex author
`codex resume 019c9205-7f8b-7173-a2a2-f082d4df3de3`
This commit is contained in:
Charley Cunningham
2026-02-26 12:52:31 -08:00
committed by GitHub
parent a6065d30f4
commit c1afb8815a
4 changed files with 367 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/app.rs
+41 -18
View File
@@ -1414,12 +1414,16 @@ impl App {
};
ChatWidget::new(init, thread_manager.clone())
}
SessionSelection::Resume(path) => {
SessionSelection::Resume(target_session) => {
let resumed = thread_manager
.resume_thread_from_rollout(config.clone(), path.clone(), auth_manager.clone())
.resume_thread_from_rollout(
config.clone(),
target_session.path.clone(),
auth_manager.clone(),
)
.await
.wrap_err_with(|| {
let path_display = path.display();
let path_display = target_session.path.display();
format!("Failed to resume session from {path_display}")
})?;
let init = crate::chatwidget::ChatWidgetInit {
@@ -1444,13 +1448,18 @@ impl App {
};
ChatWidget::new_from_existing(init, resumed.thread, resumed.session_configured)
}
SessionSelection::Fork(path) => {
SessionSelection::Fork(target_session) => {
otel_manager.counter("codex.thread.fork", 1, &[("source", "cli_subcommand")]);
let forked = thread_manager
.fork_thread(usize::MAX, config.clone(), path.clone(), false)
.fork_thread(
usize::MAX,
config.clone(),
target_session.path.clone(),
false,
)
.await
.wrap_err_with(|| {
let path_display = path.display();
let path_display = target_session.path.display();
format!("Failed to fork session from {path_display}")
})?;
let init = crate::chatwidget::ChatWidgetInit {
@@ -1713,12 +1722,14 @@ impl App {
}
AppEvent::OpenResumePicker => {
match crate::resume_picker::run_resume_picker(tui, &self.config, false).await? {
SessionSelection::Resume(path) => {
SessionSelection::Resume(target_session) => {
let current_cwd = self.config.cwd.clone();
let resume_cwd = match crate::resolve_cwd_for_resume_or_fork(
tui,
&self.config,
&current_cwd,
&path,
target_session.thread_id,
&target_session.path,
CwdPromptAction::Resume,
true,
)
@@ -1754,7 +1765,7 @@ impl App {
.server
.resume_thread_from_rollout(
resume_config.clone(),
path.clone(),
target_session.path.clone(),
self.auth_manager.clone(),
)
.await
@@ -1788,7 +1799,7 @@ impl App {
}
}
Err(err) => {
let path_display = path.display();
let path_display = target_session.path.display();
self.chat_widget.add_error_message(format!(
"Failed to resume session from {path_display}: {err}"
));
@@ -3400,15 +3411,21 @@ mod tests {
true
);
assert_eq!(
App::should_wait_for_initial_session(&SessionSelection::Resume(PathBuf::from(
"/tmp/restore"
))),
App::should_wait_for_initial_session(&SessionSelection::Resume(
crate::resume_picker::SessionTarget {
path: PathBuf::from("/tmp/restore"),
thread_id: ThreadId::new(),
}
)),
false
);
assert_eq!(
App::should_wait_for_initial_session(&SessionSelection::Fork(PathBuf::from(
"/tmp/fork"
))),
App::should_wait_for_initial_session(&SessionSelection::Fork(
crate::resume_picker::SessionTarget {
path: PathBuf::from("/tmp/fork"),
thread_id: ThreadId::new(),
}
)),
false
);
}
@@ -3444,14 +3461,20 @@ mod tests {
#[test]
fn startup_waiting_gate_not_applied_for_resume_or_fork_session_selection() {
let wait_for_resume = App::should_wait_for_initial_session(&SessionSelection::Resume(
PathBuf::from("/tmp/restore"),
crate::resume_picker::SessionTarget {
path: PathBuf::from("/tmp/restore"),
thread_id: ThreadId::new(),
},
));
assert_eq!(
App::should_handle_active_thread_events(wait_for_resume, true),
true
);
let wait_for_fork = App::should_wait_for_initial_session(&SessionSelection::Fork(
PathBuf::from("/tmp/fork"),
crate::resume_picker::SessionTarget {
path: PathBuf::from("/tmp/fork"),
thread_id: ThreadId::new(),
},
));
assert_eq!(
App::should_handle_active_thread_events(wait_for_fork, true),
codex-rs/tui/src/lib.rs
+205 -25
View File
@@ -31,8 +31,10 @@ use codex_core::find_thread_path_by_name_str;
use codex_core::format_exec_policy_error_with_source;
use codex_core::path_utils;
use codex_core::read_session_meta_line;
use codex_core::state_db::get_state_db;
use codex_core::terminal::Multiplexer;
use codex_core::windows_sandbox::WindowsSandboxLevelExt;
use codex_protocol::ThreadId;
use codex_protocol::config_types::AltScreenMode;
use codex_protocol::config_types::SandboxMode;
use codex_protocol::config_types::WindowsSandboxLevel;
@@ -665,7 +667,19 @@ async fn run_ratatui_app(
find_thread_path_by_name_str(&config.codex_home, id_str).await?
};
match path {
Some(path) => resume_picker::SessionSelection::Fork(path),
Some(path) => {
let thread_id =
match resolve_session_thread_id(path.as_path(), is_uuid.then_some(id_str))
.await
{
Some(thread_id) => thread_id,
None => return missing_session_exit(id_str, "fork"),
};
resume_picker::SessionSelection::Fork(resume_picker::SessionTarget {
path,
thread_id,
})
}
None => return missing_session_exit(id_str, "fork"),
}
} else if cli.fork_last {
@@ -682,11 +696,37 @@ async fn run_ratatui_app(
)
.await
{
Ok(page) => page
.items
.first()
.map(|it| resume_picker::SessionSelection::Fork(it.path.clone()))
.unwrap_or(resume_picker::SessionSelection::StartFresh),
Ok(page) => match page.items.first() {
Some(item) => {
match resolve_session_thread_id(item.path.as_path(), None).await {
Some(thread_id) => resume_picker::SessionSelection::Fork(
resume_picker::SessionTarget {
path: item.path.clone(),
thread_id,
},
),
None => {
let rollout_path = item.path.display();
error!(
"Error reading session metadata from latest rollout: {rollout_path}"
);
restore();
session_log::log_session_end();
let _ = tui.terminal.clear();
return Ok(AppExitInfo {
token_usage: codex_protocol::protocol::TokenUsage::default(),
thread_id: None,
thread_name: None,
update_action: None,
exit_reason: ExitReason::Fatal(format!(
"Found latest saved session at {rollout_path}, but failed to read its metadata. Run `codex fork` to choose from existing sessions."
)),
});
}
}
}
None => resume_picker::SessionSelection::StartFresh,
},
Err(_) => resume_picker::SessionSelection::StartFresh,
}
} else if cli.fork_picker {
@@ -715,7 +755,21 @@ async fn run_ratatui_app(
find_thread_path_by_name_str(&config.codex_home, id_str).await?
};
match path {
Some(path) => resume_picker::SessionSelection::Resume(path),
Some(path) => {
let thread_id = match resolve_session_thread_id(
path.as_path(),
is_uuid.then_some(id_str),
)
.await
{
Some(thread_id) => thread_id,
None => return missing_session_exit(id_str, "resume"),
};
resume_picker::SessionSelection::Resume(resume_picker::SessionTarget {
path,
thread_id,
})
}
None => return missing_session_exit(id_str, "resume"),
}
} else if cli.resume_last {
@@ -737,7 +791,30 @@ async fn run_ratatui_app(
)
.await
{
Ok(Some(path)) => resume_picker::SessionSelection::Resume(path),
Ok(Some(path)) => match resolve_session_thread_id(path.as_path(), None).await {
Some(thread_id) => {
resume_picker::SessionSelection::Resume(resume_picker::SessionTarget {
path,
thread_id,
})
}
None => {
let rollout_path = path.display();
error!("Error reading session metadata from latest rollout: {rollout_path}");
restore();
session_log::log_session_end();
let _ = tui.terminal.clear();
return Ok(AppExitInfo {
token_usage: codex_protocol::protocol::TokenUsage::default(),
thread_id: None,
thread_name: None,
update_action: None,
exit_reason: ExitReason::Fatal(format!(
"Found latest saved session at {rollout_path}, but failed to read its metadata. Run `codex resume` to choose from existing sessions."
)),
});
}
},
_ => resume_picker::SessionSelection::StartFresh,
}
} else if cli.resume_picker {
@@ -761,15 +838,27 @@ async fn run_ratatui_app(
let current_cwd = config.cwd.clone();
let allow_prompt = cli.cwd.is_none();
let action_and_path_if_resume_or_fork = match &session_selection {
resume_picker::SessionSelection::Resume(path) => Some((CwdPromptAction::Resume, path)),
resume_picker::SessionSelection::Fork(path) => Some((CwdPromptAction::Fork, path)),
let action_and_target_session_if_resume_or_fork = match &session_selection {
resume_picker::SessionSelection::Resume(target_session) => {
Some((CwdPromptAction::Resume, target_session))
}
resume_picker::SessionSelection::Fork(target_session) => {
Some((CwdPromptAction::Fork, target_session))
}
_ => None,
};
let fallback_cwd = match action_and_path_if_resume_or_fork {
Some((action, path)) => {
match resolve_cwd_for_resume_or_fork(&mut tui, &current_cwd, path, action, allow_prompt)
.await?
let fallback_cwd = match action_and_target_session_if_resume_or_fork {
Some((action, target_session)) => {
match resolve_cwd_for_resume_or_fork(
&mut tui,
&config,
&current_cwd,
target_session.thread_id,
&target_session.path,
action,
allow_prompt,
)
.await?
{
ResolveCwdOutcome::Continue(cwd) => cwd,
ResolveCwdOutcome::Exit => {
@@ -851,12 +940,35 @@ async fn run_ratatui_app(
app_result
}
pub(crate) async fn read_session_cwd(path: &Path) -> Option<PathBuf> {
pub(crate) async fn resolve_session_thread_id(
path: &Path,
id_str_if_uuid: Option<&str>,
) -> Option<ThreadId> {
match id_str_if_uuid {
Some(id_str) => ThreadId::from_string(id_str).ok(),
None => read_session_meta_line(path)
.await
.ok()
.map(|meta_line| meta_line.meta.id),
}
}
pub(crate) async fn read_session_cwd(
config: &Config,
thread_id: ThreadId,
path: &Path,
) -> Option<PathBuf> {
if let Some(state_db_ctx) = get_state_db(config, None).await
&& let Ok(Some(metadata)) = state_db_ctx.get_thread(thread_id).await
{
return Some(metadata.cwd);
}
// Prefer the latest TurnContext cwd so resume/fork reflects the most recent
// session directory (for the changed-cwd prompt). The alternative would be
// mutating the SessionMeta line when the session cwd changes, but the rollout
// is an append-only JSONL log and rewriting the head would be error-prone.
// When rollouts move to SQLite, we can drop this scan.
// session directory (for the changed-cwd prompt) when DB data is unavailable.
// The alternative would be mutating the SessionMeta line when the session cwd
// changes, but the rollout is an append-only JSONL log and rewriting the head
// would be error-prone.
if let Some(cwd) = parse_latest_turn_context_cwd(path).await {
return Some(cwd);
}
@@ -908,12 +1020,14 @@ pub(crate) enum ResolveCwdOutcome {
pub(crate) async fn resolve_cwd_for_resume_or_fork(
tui: &mut Tui,
config: &Config,
current_cwd: &Path,
thread_id: ThreadId,
path: &Path,
action: CwdPromptAction,
allow_prompt: bool,
) -> color_eyre::Result<ResolveCwdOutcome> {
let Some(history_cwd) = read_session_cwd(path).await else {
let Some(history_cwd) = read_session_cwd(config, thread_id, path).await else {
return Ok(ResolveCwdOutcome::Continue(None));
};
if allow_prompt && cwds_differ(current_cwd, &history_cwd) {
@@ -1071,11 +1185,14 @@ mod tests {
use codex_core::config::ConfigBuilder;
use codex_core::config::ConfigOverrides;
use codex_core::config::ProjectConfig;
use codex_core::features::Feature;
use codex_protocol::ThreadId;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::RolloutLine;
use codex_protocol::protocol::SessionMeta;
use codex_protocol::protocol::SessionMetaLine;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::TurnContextItem;
use serial_test::serial;
use tempfile::TempDir;
@@ -1196,7 +1313,9 @@ mod tests {
}
std::fs::write(&rollout_path, text)?;
let cwd = read_session_cwd(&rollout_path).await.expect("expected cwd");
let cwd = read_session_cwd(&config, ThreadId::new(), &rollout_path)
.await
.expect("expected cwd");
assert_eq!(cwd, second);
Ok(())
}
@@ -1236,7 +1355,9 @@ mod tests {
}
std::fs::write(&rollout_path, text)?;
let session_cwd = read_session_cwd(&rollout_path).await.expect("expected cwd");
let session_cwd = read_session_cwd(&config, ThreadId::new(), &rollout_path)
.await
.expect("expected cwd");
assert_eq!(session_cwd, latest);
assert!(cwds_differ(&current, &session_cwd));
Ok(())
@@ -1341,7 +1462,7 @@ trust_level = "untrusted"
#[tokio::test]
async fn read_session_cwd_falls_back_to_session_meta() -> std::io::Result<()> {
let temp_dir = TempDir::new()?;
let _config = build_config(&temp_dir).await?;
let config = build_config(&temp_dir).await?;
let session_cwd = temp_dir.path().join("session");
std::fs::create_dir_all(&session_cwd)?;
@@ -1363,8 +1484,67 @@ trust_level = "untrusted"
);
std::fs::write(&rollout_path, text)?;
let cwd = read_session_cwd(&rollout_path).await.expect("expected cwd");
let cwd = read_session_cwd(&config, ThreadId::new(), &rollout_path)
.await
.expect("expected cwd");
assert_eq!(cwd, session_cwd);
Ok(())
}
#[tokio::test]
async fn read_session_cwd_prefers_sqlite_when_thread_id_present() -> std::io::Result<()> {
let temp_dir = TempDir::new()?;
let mut config = build_config(&temp_dir).await?;
config.features.enable(Feature::Sqlite);
let thread_id = ThreadId::new();
let rollout_cwd = temp_dir.path().join("rollout-cwd");
let sqlite_cwd = temp_dir.path().join("sqlite-cwd");
std::fs::create_dir_all(&rollout_cwd)?;
std::fs::create_dir_all(&sqlite_cwd)?;
let rollout_path = temp_dir.path().join("rollout.jsonl");
let rollout_line = RolloutLine {
timestamp: "t0".to_string(),
item: RolloutItem::TurnContext(build_turn_context(&config, rollout_cwd)),
};
std::fs::write(
&rollout_path,
format!(
"{}\n",
serde_json::to_string(&rollout_line).expect("serialize rollout")
),
)?;
let runtime = codex_state::StateRuntime::init(
config.codex_home.clone(),
config.model_provider_id.clone(),
None,
)
.await
.map_err(std::io::Error::other)?;
runtime
.mark_backfill_complete(None)
.await
.map_err(std::io::Error::other)?;
let mut builder = codex_state::ThreadMetadataBuilder::new(
thread_id,
rollout_path.clone(),
chrono::Utc::now(),
SessionSource::Cli,
);
builder.cwd = sqlite_cwd.clone();
let metadata = builder.build(config.model_provider_id.as_str());
runtime
.upsert_thread(&metadata)
.await
.map_err(std::io::Error::other)?;
let cwd = read_session_cwd(&config, thread_id, &rollout_path)
.await
.expect("expected cwd");
assert_eq!(cwd, sqlite_cwd);
Ok(())
}
}
codex-rs/tui/src/resume_picker.rs
+116 -12
View File
@@ -39,11 +39,18 @@ use unicode_width::UnicodeWidthStr;
const PAGE_SIZE: usize = 25;
const LOAD_NEAR_THRESHOLD: usize = 5;
#[derive(Debug, Clone)]
pub struct SessionTarget {
pub path: PathBuf,
pub thread_id: ThreadId,
}
#[derive(Debug, Clone)]
pub enum SessionSelection {
StartFresh,
Resume(PathBuf),
Fork(PathBuf),
Resume(SessionTarget),
Fork(SessionTarget),
Exit,
}
@@ -68,10 +75,11 @@ impl SessionPickerAction {
}
}
fn selection(self, path: PathBuf) -> SessionSelection {
fn selection(self, path: PathBuf, thread_id: ThreadId) -> SessionSelection {
let target_session = SessionTarget { path, thread_id };
match self {
SessionPickerAction::Resume => SessionSelection::Resume(path),
SessionPickerAction::Fork => SessionSelection::Fork(path),
SessionPickerAction::Resume => SessionSelection::Resume(target_session),
SessionPickerAction::Fork => SessionSelection::Fork(target_session),
}
}
}
@@ -266,6 +274,7 @@ struct PickerState {
action: SessionPickerAction,
sort_key: ThreadSortKey,
thread_name_cache: HashMap<ThreadId, Option<String>>,
inline_error: Option<String>,
}
struct PaginationState {
@@ -383,6 +392,7 @@ impl PickerState {
action,
sort_key: ThreadSortKey::CreatedAt,
thread_name_cache: HashMap::new(),
inline_error: None,
}
}
@@ -391,6 +401,7 @@ impl PickerState {
}
async fn handle_key(&mut self, key: KeyEvent) -> Result<Option<SessionSelection>> {
self.inline_error = None;
match key.code {
KeyCode::Esc => return Ok(Some(SessionSelection::StartFresh)),
KeyCode::Char('c')
@@ -402,7 +413,19 @@ impl PickerState {
}
KeyCode::Enter => {
if let Some(row) = self.filtered_rows.get(self.selected) {
return Ok(Some(self.action.selection(row.path.clone())));
let path = row.path.clone();
let thread_id = match row.thread_id {
Some(thread_id) => Some(thread_id),
None => crate::resolve_session_thread_id(path.as_path(), None).await,
};
if let Some(thread_id) = thread_id {
return Ok(Some(self.action.selection(path, thread_id)));
}
self.inline_error = Some(format!(
"Failed to read session metadata from {}",
path.display()
));
self.request_frame();
}
}
KeyCode::Up => {
@@ -866,12 +889,7 @@ fn draw_picker(tui: &mut Tui, state: &PickerState) -> std::io::Result<()> {
frame.render_widget_ref(header_line, header);
// Search line
let q = if state.query.is_empty() {
"Type to search".dim().to_string()
} else {
format!("Search: {}", state.query)
};
frame.render_widget_ref(Line::from(q), search);
frame.render_widget_ref(search_line(state), search);
let metrics = calculate_column_metrics(&state.filtered_rows, state.show_all);
@@ -904,6 +922,16 @@ fn draw_picker(tui: &mut Tui, state: &PickerState) -> std::io::Result<()> {
})
}
fn search_line(state: &PickerState) -> Line<'_> {
if let Some(error) = state.inline_error.as_deref() {
return Line::from(error.red());
}
if state.query.is_empty() {
return Line::from("Type to search".dim());
}
Line::from(format!("Search: {}", state.query))
}
fn render_list(
frame: &mut crate::custom_terminal::Frame,
area: Rect,
@@ -1607,6 +1635,42 @@ mod tests {
assert_snapshot!("resume_picker_table", snapshot);
}
#[test]
fn resume_search_error_snapshot() {
use crate::custom_terminal::Terminal;
use crate::test_backend::VT100Backend;
let loader: PageLoader = Arc::new(|_| {});
let mut state = PickerState::new(
PathBuf::from("/tmp"),
FrameRequester::test_dummy(),
loader,
String::from("openai"),
true,
None,
SessionPickerAction::Resume,
);
state.inline_error = Some(String::from(
"Failed to read session metadata from /tmp/missing.jsonl",
));
let width: u16 = 80;
let height: u16 = 1;
let backend = VT100Backend::new(width, height);
let mut terminal = Terminal::with_options(backend).expect("terminal");
terminal.set_viewport_area(Rect::new(0, 0, width, height));
{
let mut frame = terminal.get_frame();
let line = search_line(&state);
frame.render_widget_ref(line, frame.area());
}
terminal.flush().expect("flush");
let snapshot = terminal.backend().to_string();
assert_snapshot!("resume_picker_search_error", snapshot);
}
// TODO(jif) fix
// #[tokio::test]
// async fn resume_picker_screen_snapshot() {
@@ -2102,6 +2166,46 @@ mod tests {
assert_eq!(state.selected, 5);
}
#[tokio::test]
async fn enter_on_row_without_resolvable_thread_id_shows_inline_error() {
let loader: PageLoader = Arc::new(|_| {});
let mut state = PickerState::new(
PathBuf::from("/tmp"),
FrameRequester::test_dummy(),
loader,
String::from("openai"),
true,
None,
SessionPickerAction::Resume,
);
let row = Row {
path: PathBuf::from("/tmp/missing.jsonl"),
preview: String::from("missing metadata"),
thread_id: None,
thread_name: None,
created_at: None,
updated_at: None,
cwd: None,
git_branch: None,
};
state.all_rows = vec![row.clone()];
state.filtered_rows = vec![row];
let selection = state
.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE))
.await
.expect("enter should not abort the picker");
assert!(selection.is_none());
assert_eq!(
state.inline_error,
Some(String::from(
"Failed to read session metadata from /tmp/missing.jsonl"
))
);
}
#[tokio::test]
async fn up_at_bottom_does_not_scroll_when_visible() {
let loader: PageLoader = Arc::new(|_| {});
codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_search_error.snap
+5
View File
@@ -0,0 +1,5 @@
---
source: tui/src/resume_picker.rs
expression: snapshot
---
Failed to read session metadata from /tmp/missing.jsonl