mirror of
https://github.com/openai/codex.git
synced 2026-11-13 21:30:47 +00:00
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
This commit is contained in:
@@ -1422,6 +1422,8 @@ impl App {
|
||||
) -> Option<ThreadSessionState> {
|
||||
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(),
|
||||
|
||||
@@ -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)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user