From 2bb2e20d6aa8ded4d6da6babebb3b61b3c50efb7 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Thu, 12 Nov 2026 19:44:13 +0000 Subject: [PATCH] Fix fork parent metadata in reconstructed TUI sessions (#61183) ## Why Reconstructing a thread session from the primary session could carry over another thread's fork parent and cached parent title. ## What changed Use the target thread's `forked_from_id` when reconstructing sessions from `thread/read` responses and `ThreadStarted` notifications. Clear the cached parent title when the parent changes, and treat absent or invalid parent IDs as no parent. ## Testing Extend the session reconstruction test to cover replacing an inherited fork parent and clearing inherited fork metadata for a thread with no parent, while retaining coverage of active permission settings. GitOrigin-RevId: 860779b40dc21fb4cc2ec1ac392523da5e439a94 --- codex-rs/tui/src/app/thread_routing.rs | 2 ++ codex-rs/tui/src/app/thread_session_state.rs | 35 ++++++++++++++++++-- codex-rs/tui/src/session_state.rs | 9 +++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index abe4441d4b..4aee17d93f 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -1422,6 +1422,8 @@ impl App { ) -> Option { let mut session = self.primary_session_configured.clone()?; session.thread_id = thread_id; + // The primary session supplies defaults. The new thread supplies its own fork parent. + session.set_fork_parent(notification.thread.forked_from_id.as_deref()); session.daybreak_enabled = notification.thread.daybreak_enabled.unwrap_or(false); session.windows_sandbox_host = crate::windows_sandbox::host_from_environments( notification.thread.environments.as_deref(), diff --git a/codex-rs/tui/src/app/thread_session_state.rs b/codex-rs/tui/src/app/thread_session_state.rs index 983b95b25a..b1ab8ad1af 100644 --- a/codex-rs/tui/src/app/thread_session_state.rs +++ b/codex-rs/tui/src/app/thread_session_state.rs @@ -115,6 +115,7 @@ impl App { session.windows_sandbox_host = crate::windows_sandbox::host_from_environments(thread.environments.as_deref()); session.thread_id = thread_id; + session.set_fork_parent(thread.forked_from_id.as_deref()); session.daybreak_enabled = thread.daybreak_enabled.unwrap_or(false); session.thread_name = thread.name.clone(); session.model_provider_id = thread.model_provider.clone(); @@ -401,23 +402,26 @@ mod tests { } #[tokio::test] - async fn thread_read_fallback_uses_active_permission_settings() { + async fn reconstructed_session_uses_target_lineage_and_active_permissions() { let mut app = make_test_app().await; let primary_thread_id = ThreadId::from_string("00000000-0000-0000-0000-000000000404").expect("valid thread"); let read_thread_id = ThreadId::from_string("00000000-0000-0000-0000-000000000405").expect("valid thread"); + let other_parent = ThreadId::new(); let primary_session = ThreadSessionState { + forked_from_id: Some(ThreadId::new()), + fork_parent_title: Some("Previous parent".into()), permission_profile: PermissionProfile::workspace_write(), ..test_thread_session(primary_thread_id, test_path_buf("/tmp/primary")) }; - let read_thread = Thread { + let mut read_thread = Thread { originator: None, environments: None, id: read_thread_id.to_string(), extra: None, session_id: read_thread_id.to_string(), - forked_from_id: None, + forked_from_id: Some(other_parent.to_string()), attached_to_thread_id: None, parent_thread_id: None, preview: "read thread".to_string(), @@ -454,6 +458,10 @@ mod tests { .session_state_for_thread_read(read_thread_id, &read_thread) .await; + assert_eq!( + (session.forked_from_id, session.fork_parent_title), + (Some(other_parent), None) + ); let expected_permission_profile = app .chat_widget .config_ref() @@ -467,5 +475,26 @@ mod tests { "thread/read fallback must use the active widget permissions rather than stale app \ config defaults" ); + + read_thread.forked_from_id = None; + app.enqueue_thread_notification( + read_thread_id, + codex_app_server_protocol::ServerNotification::ThreadStarted( + codex_app_server_protocol::ThreadStartedNotification { + thread: read_thread, + }, + ), + ) + .await + .expect("thread started"); + let store = app.thread_event_channels[&read_thread_id] + .store + .lock() + .await; + let started = store.session.as_ref().expect("inferred session"); + assert_eq!( + (started.forked_from_id, &started.fork_parent_title), + (None, &None) + ); } } diff --git a/codex-rs/tui/src/session_state.rs b/codex-rs/tui/src/session_state.rs index b448243695..62e55d2732 100644 --- a/codex-rs/tui/src/session_state.rs +++ b/codex-rs/tui/src/session_state.rs @@ -58,6 +58,15 @@ pub(crate) struct ThreadSessionState { } impl ThreadSessionState { + pub(crate) fn set_fork_parent(&mut self, parent: Option<&str>) { + let parent = parent.and_then(|id| ThreadId::from_string(id).ok()); + // The cached title belongs to the old parent. Drop it if the parent changes or is absent. + if self.forked_from_id != parent { + self.fork_parent_title = None; + } + self.forked_from_id = parent; + } + pub(crate) fn set_cwd_retargeting_implicit_runtime_workspace_root( &mut self, cwd: AbsolutePathBuf,