mirror of
https://github.com/openai/codex.git
synced 2026-09-05 15:18:41 +00:00
Hide ephemeral system threads from TUI routing (#40494)
## What changed - Ignore `thread/started` notifications for ephemeral threads whose feature source is `system`, preventing hidden helper threads from entering TUI thread routing or refreshing the agents overview. - Continue routing persisted system threads normally. ## Testing - Add coverage for hidden and persisted system-thread notifications in the agents overview. - Add generated-title normalization cases for wrapping quotes, trailing punctuation, and meaningful leading punctuation. GitOrigin-RevId: e1ea48502b4f9aa7d5a187c7407df69c55abe2a9
This commit is contained in:
@@ -11,6 +11,8 @@ use codex_app_server_protocol::CurrentTimeReadParams;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::SessionSource;
|
||||
use codex_app_server_protocol::ThreadActiveFlag;
|
||||
use codex_app_server_protocol::ThreadSource;
|
||||
use codex_app_server_protocol::ThreadStartedNotification;
|
||||
use codex_app_server_protocol::ThreadUnsubscribeParams;
|
||||
use codex_app_server_protocol::ThreadUnsubscribeResponse;
|
||||
use codex_app_server_protocol::ThreadUnsubscribeStatus;
|
||||
@@ -67,6 +69,92 @@ fn overview_thread(
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn hidden_system_thread_does_not_refresh_shared_overview() {
|
||||
let mut app = make_test_app().await;
|
||||
let app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
|
||||
.await
|
||||
.expect("embedded app server");
|
||||
let view = app.agents_overview_view(Vec::new(), /*selected_thread_id*/ None);
|
||||
app.chat_widget.show_bottom_pane_view(Box::new(view));
|
||||
|
||||
let request_id = uuid::Uuid::new_v4();
|
||||
app.agents_overview.request_id = Some(request_id);
|
||||
|
||||
let parent_thread_id = ThreadId::new();
|
||||
app.primary_thread_id = Some(parent_thread_id);
|
||||
app.active_thread_id = Some(parent_thread_id);
|
||||
app.ensure_thread_channel(parent_thread_id);
|
||||
app.agents_overview
|
||||
.dispatched_requests
|
||||
.insert(parent_thread_id, Vec::new());
|
||||
|
||||
let hidden_thread_id = ThreadId::new();
|
||||
let mut thread = overview_thread(
|
||||
hidden_thread_id,
|
||||
Some(parent_thread_id),
|
||||
"Generate thread title",
|
||||
ThreadStatus::Idle,
|
||||
);
|
||||
thread.ephemeral = true;
|
||||
thread.thread_source = Some(ThreadSource::Feature("system".to_string()));
|
||||
|
||||
app.handle_app_server_event(
|
||||
&app_server,
|
||||
AppServerEvent::ServerNotification(Box::new(ServerNotification::ThreadStarted(
|
||||
ThreadStartedNotification { thread },
|
||||
))),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
(
|
||||
app.agents_overview.request_id,
|
||||
app.agents_overview.refresh_pending,
|
||||
),
|
||||
(Some(request_id), false)
|
||||
);
|
||||
assert!(!app.thread_event_channels.contains_key(&hidden_thread_id));
|
||||
assert!(
|
||||
!app.agents_overview
|
||||
.dispatched_requests
|
||||
.contains_key(&hidden_thread_id)
|
||||
);
|
||||
|
||||
let visible_thread_id = ThreadId::new();
|
||||
let mut thread = overview_thread(
|
||||
visible_thread_id,
|
||||
Some(parent_thread_id),
|
||||
"Persisted system thread",
|
||||
ThreadStatus::Idle,
|
||||
);
|
||||
thread.thread_source = Some(ThreadSource::Feature("system".to_string()));
|
||||
|
||||
app.handle_app_server_event(
|
||||
&app_server,
|
||||
AppServerEvent::ServerNotification(Box::new(ServerNotification::ThreadStarted(
|
||||
ThreadStartedNotification { thread },
|
||||
))),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
(
|
||||
app.agents_overview.request_id,
|
||||
app.agents_overview.refresh_pending,
|
||||
),
|
||||
(Some(request_id), true)
|
||||
);
|
||||
assert!(app.thread_event_channels.contains_key(&visible_thread_id));
|
||||
assert!(
|
||||
app.agents_overview
|
||||
.dispatched_requests
|
||||
.contains_key(&visible_thread_id)
|
||||
);
|
||||
|
||||
app_server.shutdown().await.expect("shutdown app server");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shared_overview_shows_only_root_sessions() {
|
||||
assert_eq!(
|
||||
|
||||
@@ -20,6 +20,7 @@ use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::SessionSource;
|
||||
use codex_app_server_protocol::ThreadReadParams;
|
||||
use codex_app_server_protocol::ThreadReadResponse;
|
||||
use codex_app_server_protocol::ThreadSource;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
|
||||
@@ -92,6 +93,15 @@ impl App {
|
||||
let _ = self.dynamic_tool_status_updates.send(status.clone());
|
||||
}
|
||||
|
||||
if let ServerNotification::ThreadStarted(started) = ¬ification
|
||||
&& started.thread.ephemeral
|
||||
&& matches!(
|
||||
started.thread.thread_source.as_ref(),
|
||||
Some(ThreadSource::Feature(feature)) if feature == "system"
|
||||
)
|
||||
{
|
||||
return;
|
||||
}
|
||||
// Hidden helper threads must not enter visible thread routing or overview refreshes.
|
||||
if let ServerNotificationThreadTarget::Thread(thread_id) =
|
||||
server_notification_thread_target(¬ification)
|
||||
|
||||
@@ -99,6 +99,40 @@ fn normalizes_generated_title_whitespace() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn removes_wrapping_quotes_and_trailing_punctuation_from_generated_titles() {
|
||||
for title in [
|
||||
r#""Fix login errors!""#,
|
||||
"'Fix login errors?'",
|
||||
"`Fix login errors.`",
|
||||
"“Fix login errors!”",
|
||||
] {
|
||||
let response = serde_json::json!({ "title": title }).to_string();
|
||||
|
||||
assert_eq!(
|
||||
parse_thread_title(&response),
|
||||
Some("Fix login errors".to_string()),
|
||||
"response: {response}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_meaningful_leading_punctuation_in_generated_titles() {
|
||||
for (title, expected) in [
|
||||
(".NET migration.", ".NET migration"),
|
||||
("!important styling!", "!important styling"),
|
||||
] {
|
||||
let response = serde_json::json!({ "title": title }).to_string();
|
||||
|
||||
assert_eq!(
|
||||
parse_thread_title(&response),
|
||||
Some(expected.to_string()),
|
||||
"response: {response}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_invalid_or_empty_generated_titles() {
|
||||
for response in [
|
||||
|
||||
Reference in New Issue
Block a user