mirror of
https://github.com/openai/codex.git
synced 2026-10-30 19:18:49 +00:00
Preserve unrelated side conversations when archiving or deleting threads (#58237)
## Why Archiving or deleting a thread in the command center shuts down side conversations belonging to other threads. ## What changed Limit side conversation shutdown to those whose parents belong to the affected thread's known subtree. Proceed once those conversations close successfully, even if unrelated side conversations remain. Continue shutting down all side conversations on exit. ## Testing Extend the delete lifecycle test to verify that an unrelated side conversation remains open after deletion. GitOrigin-RevId: f27efcd1fc49cf5c617c9fa454d83eed91ae185f
This commit is contained in:
@@ -237,6 +237,36 @@ impl App {
|
||||
});
|
||||
}
|
||||
|
||||
pub(super) fn known_thread_subtree(&self, thread_id: ThreadId) -> HashSet<ThreadId> {
|
||||
let mut removed = HashSet::from([thread_id]);
|
||||
loop {
|
||||
let previous_len = removed.len();
|
||||
for (id, thread) in &self.agents_overview.threads {
|
||||
let Some(thread) = thread else { continue };
|
||||
let parent = thread
|
||||
.parent_thread_id
|
||||
.as_deref()
|
||||
.and_then(|id| ThreadId::from_string(id).ok())
|
||||
.or(match &thread.source {
|
||||
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
..
|
||||
}) => Some(*parent_thread_id),
|
||||
_ => None,
|
||||
});
|
||||
if thread.session_id == thread_id.to_string()
|
||||
|| parent.is_some_and(|parent| removed.contains(&parent))
|
||||
{
|
||||
removed.insert(*id);
|
||||
}
|
||||
}
|
||||
if removed.len() == previous_len {
|
||||
break;
|
||||
}
|
||||
}
|
||||
removed
|
||||
}
|
||||
|
||||
pub(super) async fn run_agents_overview_action(
|
||||
&mut self,
|
||||
tui: &mut tui::Tui,
|
||||
@@ -339,32 +369,7 @@ impl App {
|
||||
}
|
||||
if result.is_ok() {
|
||||
// Invalidate cached details for the displayed group, without inferring child outcomes.
|
||||
let mut removed = HashSet::from([thread_id]);
|
||||
loop {
|
||||
let previous_len = removed.len();
|
||||
for (id, thread) in &self.agents_overview.threads {
|
||||
let Some(thread) = thread else { continue };
|
||||
let parent = thread
|
||||
.parent_thread_id
|
||||
.as_deref()
|
||||
.and_then(|id| ThreadId::from_string(id).ok())
|
||||
.or(match &thread.source {
|
||||
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
..
|
||||
}) => Some(*parent_thread_id),
|
||||
_ => None,
|
||||
});
|
||||
if thread.session_id == thread_id.to_string()
|
||||
|| parent.is_some_and(|parent| removed.contains(&parent))
|
||||
{
|
||||
removed.insert(*id);
|
||||
}
|
||||
}
|
||||
if removed.len() == previous_len {
|
||||
break;
|
||||
}
|
||||
}
|
||||
let mut removed = self.known_thread_subtree(thread_id);
|
||||
if let Some(primary) = self.primary_thread_id
|
||||
&& removes_primary
|
||||
{
|
||||
|
||||
@@ -3859,7 +3859,7 @@ impl App {
|
||||
if !self.side_threads.is_empty()
|
||||
&& tokio::time::timeout(
|
||||
SHUTDOWN_FIRST_EXIT_TIMEOUT,
|
||||
self.shutdown_side_threads(app_server),
|
||||
self.shutdown_side_threads(app_server, /*root*/ None),
|
||||
)
|
||||
.await
|
||||
.is_err()
|
||||
@@ -3917,11 +3917,12 @@ impl App {
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
|
||||
if self.app_server_target.supports_command_center() {
|
||||
self.shutdown_side_threads(app_server).await;
|
||||
if !self.side_threads.is_empty() {
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
if self.app_server_target.supports_command_center()
|
||||
&& !self
|
||||
.shutdown_side_threads(app_server, Some(thread_id))
|
||||
.await
|
||||
{
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
|
||||
let result = async {
|
||||
@@ -3985,11 +3986,12 @@ impl App {
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
|
||||
if self.app_server_target.supports_command_center() {
|
||||
self.shutdown_side_threads(app_server).await;
|
||||
if !self.side_threads.is_empty() {
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
if self.app_server_target.supports_command_center()
|
||||
&& !self
|
||||
.shutdown_side_threads(app_server, Some(thread_id))
|
||||
.await
|
||||
{
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
|
||||
let result = async {
|
||||
|
||||
@@ -1089,6 +1089,9 @@ async fn delete_current_thread_navigates_only_after_success() -> Result<()> {
|
||||
let side_id = side.session.thread_id;
|
||||
app.side_threads
|
||||
.insert(side_id, SideThreadState::new(thread_id));
|
||||
let unrelated_side = ThreadId::new();
|
||||
app.side_threads
|
||||
.insert(unrelated_side, SideThreadState::new(ThreadId::new()));
|
||||
if !matches!(app.app_server_target, AppServerTarget::Embedded) {
|
||||
// The recording proxy rejects the next unsubscribe after a failed fork.
|
||||
side_config.cwd = side_config.cwd.join("failure");
|
||||
@@ -1128,7 +1131,10 @@ async fn delete_current_thread_navigates_only_after_success() -> Result<()> {
|
||||
assert!(recorded_params(&requests, "thread/unsubscribe").is_empty());
|
||||
} else {
|
||||
assert_matches!(control, AppRunControl::Continue);
|
||||
assert!(app.side_threads.is_empty());
|
||||
assert_eq!(
|
||||
app.side_threads.keys().copied().collect::<Vec<_>>(),
|
||||
vec![unrelated_side]
|
||||
);
|
||||
assert_eq!(
|
||||
recorded_params(&requests, "thread/unsubscribe"),
|
||||
vec![serde_json::json!({"threadId": side_id.to_string()}); 2]
|
||||
|
||||
@@ -49,11 +49,30 @@ impl App {
|
||||
.await;
|
||||
}
|
||||
|
||||
pub(super) async fn shutdown_side_threads(&mut self, app_server: &mut AppServerSession) {
|
||||
let side_thread_ids: Vec<ThreadId> = self.side_threads.keys().copied().collect();
|
||||
for side_thread_id in side_thread_ids {
|
||||
self.discard_side_thread(app_server, side_thread_id).await;
|
||||
pub(super) async fn shutdown_side_threads(
|
||||
&mut self,
|
||||
app_server: &mut AppServerSession,
|
||||
root: Option<ThreadId>,
|
||||
) -> bool {
|
||||
if self.side_threads.is_empty() {
|
||||
return true;
|
||||
}
|
||||
let subtree = root.map(|id| self.known_thread_subtree(id));
|
||||
let ids: Vec<_> = self
|
||||
.side_threads
|
||||
.iter()
|
||||
.filter(|(_, side)| {
|
||||
subtree
|
||||
.as_ref()
|
||||
.is_none_or(|ids| ids.contains(&side.parent_thread_id))
|
||||
})
|
||||
.map(|(id, _)| *id)
|
||||
.collect();
|
||||
let mut closed = true;
|
||||
for id in ids {
|
||||
closed &= self.discard_side_thread(app_server, id).await;
|
||||
}
|
||||
closed
|
||||
}
|
||||
|
||||
pub(super) fn abort_thread_event_listener(&mut self, thread_id: ThreadId) {
|
||||
|
||||
Reference in New Issue
Block a user