mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
Ignore stale workspace headline requests and archived messages (#61411)
## Why Workspace headline request counters restart when the chat widget is replaced, allowing a response for the previous widget to match a new pending request. Archived headlines can also be selected for the status line. ## What changed - Use UUIDs for workspace headline requests and reject refresh events and responses that do not match the current widget's pending request. - Skip archived messages when selecting the first non-empty workspace headline. ## Testing Add a regression test verifying that a replacement widget rejects the previous widget's response while retaining its own pending request. Extend headline selection coverage to include an archived headline before the active one. GitOrigin-RevId: 44b98ab8c6cb70b7cf8e8b9341f27c99c1e1490a
This commit is contained in:
@@ -220,8 +220,11 @@ impl App {
|
||||
pub(super) fn refresh_status_line_workspace_headline(
|
||||
&mut self,
|
||||
app_server: &AppServerSession,
|
||||
request_id: u64,
|
||||
request_id: Uuid,
|
||||
) {
|
||||
if !self.chat_widget.is_pending_workspace_headline(request_id) {
|
||||
return;
|
||||
}
|
||||
let request_handle = app_server.request_handle();
|
||||
let app_event_tx = self.app_event_tx.clone();
|
||||
tokio::spawn(async move {
|
||||
|
||||
@@ -978,7 +978,7 @@ pub(crate) enum AppEvent {
|
||||
|
||||
/// Fetch workspace messages for the status-line headline item.
|
||||
RefreshStatusLineWorkspaceHeadline {
|
||||
request_id: u64,
|
||||
request_id: Uuid,
|
||||
},
|
||||
|
||||
/// Commit settled asynchronous usage output after active-output barriers clear.
|
||||
@@ -1843,7 +1843,7 @@ pub(crate) enum AppEvent {
|
||||
},
|
||||
/// Async update of the workspace notification headline for status line rendering.
|
||||
StatusLineWorkspaceHeadlineUpdated {
|
||||
request_id: u64,
|
||||
request_id: Uuid,
|
||||
result: Result<crate::workspace_messages::WorkspaceHeadlineFetchResult, String>,
|
||||
},
|
||||
/// Apply a user-confirmed status-line item ordering/selection.
|
||||
|
||||
@@ -794,9 +794,7 @@ pub(crate) struct ChatWidget {
|
||||
// Cached workspace notification headline for the status line.
|
||||
status_line_workspace_headline: Option<String>,
|
||||
// Request ID for the async workspace headline fetch currently in flight.
|
||||
status_line_workspace_headline_pending_request_id: Option<u64>,
|
||||
// Request ID to assign to the next workspace headline fetch.
|
||||
next_status_line_workspace_headline_request_id: u64,
|
||||
status_line_workspace_headline_pending_request_id: Option<uuid::Uuid>,
|
||||
// Last time a workspace headline fetch was requested.
|
||||
status_line_workspace_headline_last_requested_at: Option<Instant>,
|
||||
// Set after the backend reports the workspace-message feature gate is disabled.
|
||||
|
||||
@@ -299,7 +299,6 @@ impl ChatWidget {
|
||||
status_line_git_summary_lookup_complete: false,
|
||||
status_line_workspace_headline: None,
|
||||
status_line_workspace_headline_pending_request_id: None,
|
||||
next_status_line_workspace_headline_request_id: 0,
|
||||
status_line_workspace_headline_last_requested_at: None,
|
||||
status_line_workspace_messages_disabled: false,
|
||||
thread_usage: thread_usage::ThreadUsageState::default(),
|
||||
|
||||
@@ -665,10 +665,7 @@ impl ChatWidget {
|
||||
if !self.status_line_workspace_headline_should_fetch(now) {
|
||||
return;
|
||||
}
|
||||
let request_id = self.next_status_line_workspace_headline_request_id;
|
||||
self.next_status_line_workspace_headline_request_id = self
|
||||
.next_status_line_workspace_headline_request_id
|
||||
.wrapping_add(/*rhs*/ 1);
|
||||
let request_id = uuid::Uuid::new_v4();
|
||||
self.status_line_workspace_headline_pending_request_id = Some(request_id);
|
||||
self.status_line_workspace_headline_last_requested_at = Some(now);
|
||||
self.app_event_tx
|
||||
@@ -704,12 +701,16 @@ impl ChatWidget {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn is_pending_workspace_headline(&self, request_id: uuid::Uuid) -> bool {
|
||||
self.status_line_workspace_headline_pending_request_id == Some(request_id)
|
||||
}
|
||||
|
||||
pub(crate) fn set_status_line_workspace_headline(
|
||||
&mut self,
|
||||
request_id: u64,
|
||||
request_id: uuid::Uuid,
|
||||
result: Result<crate::workspace_messages::WorkspaceHeadlineFetchResult, String>,
|
||||
) -> bool {
|
||||
if self.status_line_workspace_headline_pending_request_id != Some(request_id) {
|
||||
if !self.is_pending_workspace_headline(request_id) {
|
||||
return false;
|
||||
}
|
||||
self.status_line_workspace_headline_pending_request_id = None;
|
||||
|
||||
@@ -49,7 +49,7 @@ fn enable_test_ambient_pet(chat: &mut ChatWidget) {
|
||||
|
||||
fn take_workspace_headline_request_id(
|
||||
rx: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
|
||||
) -> u64 {
|
||||
) -> uuid::Uuid {
|
||||
match rx.try_recv() {
|
||||
Ok(AppEvent::RefreshStatusLineWorkspaceHeadline { request_id }) => request_id,
|
||||
event => panic!("expected workspace headline refresh, got {event:?}"),
|
||||
@@ -4010,7 +4010,7 @@ async fn workspace_headline_update_applies_feature_disabled_result() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.local_settings.tui.status_line = Some(vec!["workspace-headline".to_string()]);
|
||||
chat.status_line_workspace_headline = Some("Old headline".to_string());
|
||||
let request_id = 3;
|
||||
let request_id = uuid::Uuid::new_v4();
|
||||
chat.status_line_workspace_headline_pending_request_id = Some(request_id);
|
||||
|
||||
assert!(chat.set_status_line_workspace_headline(
|
||||
@@ -4026,7 +4026,7 @@ async fn workspace_headline_update_applies_feature_disabled_result() {
|
||||
async fn workspace_headline_update_applies_available_headline() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.local_settings.tui.status_line = Some(vec!["workspace-headline".to_string()]);
|
||||
let request_id = 4;
|
||||
let request_id = uuid::Uuid::new_v4();
|
||||
chat.status_line_workspace_headline_pending_request_id = Some(request_id);
|
||||
|
||||
assert!(chat.set_status_line_workspace_headline(
|
||||
@@ -4050,7 +4050,7 @@ async fn account_update_clears_workspace_headline_state() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.local_settings.tui.status_line = Some(vec!["workspace-headline".to_string()]);
|
||||
chat.status_line_workspace_headline = Some("Old workspace headline".to_string());
|
||||
chat.status_line_workspace_headline_pending_request_id = Some(5);
|
||||
chat.status_line_workspace_headline_pending_request_id = Some(uuid::Uuid::new_v4());
|
||||
chat.status_line_workspace_headline_last_requested_at = Some(Instant::now());
|
||||
chat.status_line_workspace_messages_disabled = true;
|
||||
|
||||
@@ -4087,6 +4087,25 @@ async fn workspace_headline_fetch_allows_backend_auth_without_chatgpt_account()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn replacement_widget_rejects_the_previous_widgets_workspace_headline() {
|
||||
let request_headline = async || {
|
||||
let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.local_settings.tui.status_line = Some(vec!["workspace-headline".into()]);
|
||||
chat.has_codex_backend_auth = true;
|
||||
chat.refresh_status_line();
|
||||
(chat, take_workspace_headline_request_id(&mut rx))
|
||||
};
|
||||
let (_, stale) = request_headline().await;
|
||||
let (mut replacement, current) = request_headline().await;
|
||||
|
||||
assert!(!replacement.set_status_line_workspace_headline(
|
||||
stale,
|
||||
Ok(crate::workspace_messages::WorkspaceHeadlineFetchResult::FeatureDisabled),
|
||||
));
|
||||
assert!(replacement.is_pending_workspace_headline(current));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn account_update_discards_stale_workspace_headline_results() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
@@ -18,7 +18,7 @@ pub(crate) fn workspace_headline_from_response(
|
||||
}
|
||||
|
||||
WorkspaceHeadlineFetchResult::Available(response.messages.into_iter().find_map(|message| {
|
||||
(message.message_type == WorkspaceMessageType::Headline)
|
||||
(message.archived_at.is_none() && message.message_type == WorkspaceMessageType::Headline)
|
||||
.then(|| message.message_body.trim().to_string())
|
||||
.filter(|headline| !headline.is_empty())
|
||||
}))
|
||||
|
||||
@@ -7,6 +7,13 @@ fn workspace_headline_from_response_uses_first_non_empty_headline() {
|
||||
let response = GetWorkspaceMessagesResponse {
|
||||
feature_enabled: true,
|
||||
messages: vec![
|
||||
WorkspaceMessage {
|
||||
message_id: "archived-headline".to_string(),
|
||||
message_type: WorkspaceMessageType::Headline,
|
||||
message_body: "Previous announcement".to_string(),
|
||||
created_at: None,
|
||||
archived_at: Some(1_700_000_000),
|
||||
},
|
||||
WorkspaceMessage {
|
||||
message_id: "announcement-id".to_string(),
|
||||
message_type: WorkspaceMessageType::Announcement,
|
||||
|
||||
Reference in New Issue
Block a user