mirror of
https://github.com/openai/codex.git
synced 2026-11-08 20:40:33 +00:00
codex: address PR review feedback (#21085)
This commit is contained in:
@@ -284,10 +284,15 @@
|
||||
"$ref": "#/definitions/AppInfo"
|
||||
},
|
||||
"type": "array"
|
||||
},
|
||||
"isFinal": {
|
||||
"description": "Whether the notification includes the complete merged app list.",
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"data"
|
||||
"data",
|
||||
"isFinal"
|
||||
],
|
||||
"type": "object"
|
||||
},
|
||||
|
||||
+6
-1
@@ -5919,10 +5919,15 @@
|
||||
"$ref": "#/definitions/v2/AppInfo"
|
||||
},
|
||||
"type": "array"
|
||||
},
|
||||
"isFinal": {
|
||||
"description": "Whether the notification includes the complete merged app list.",
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"data"
|
||||
"data",
|
||||
"isFinal"
|
||||
],
|
||||
"title": "AppListUpdatedNotification",
|
||||
"type": "object"
|
||||
|
||||
+6
-1
@@ -479,10 +479,15 @@
|
||||
"$ref": "#/definitions/AppInfo"
|
||||
},
|
||||
"type": "array"
|
||||
},
|
||||
"isFinal": {
|
||||
"description": "Whether the notification includes the complete merged app list.",
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"data"
|
||||
"data",
|
||||
"isFinal"
|
||||
],
|
||||
"title": "AppListUpdatedNotification",
|
||||
"type": "object"
|
||||
|
||||
+6
-1
@@ -266,10 +266,15 @@
|
||||
"$ref": "#/definitions/AppInfo"
|
||||
},
|
||||
"type": "array"
|
||||
},
|
||||
"isFinal": {
|
||||
"description": "Whether the notification includes the complete merged app list.",
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"data"
|
||||
"data",
|
||||
"isFinal"
|
||||
],
|
||||
"title": "AppListUpdatedNotification",
|
||||
"type": "object"
|
||||
|
||||
+5
-1
@@ -6,4 +6,8 @@ import type { AppInfo } from "./AppInfo";
|
||||
/**
|
||||
* EXPERIMENTAL - notification emitted when the app list changes.
|
||||
*/
|
||||
export type AppListUpdatedNotification = { data: Array<AppInfo>, };
|
||||
export type AppListUpdatedNotification = { data: Array<AppInfo>,
|
||||
/**
|
||||
* Whether the notification includes the complete merged app list.
|
||||
*/
|
||||
isFinal: boolean, };
|
||||
|
||||
@@ -143,4 +143,6 @@ pub struct AppsListResponse {
|
||||
/// EXPERIMENTAL - notification emitted when the app list changes.
|
||||
pub struct AppListUpdatedNotification {
|
||||
pub data: Vec<AppInfo>,
|
||||
/// Whether the notification includes the complete merged app list.
|
||||
pub is_final: bool,
|
||||
}
|
||||
|
||||
@@ -194,7 +194,12 @@ impl AppsRequestProcessor {
|
||||
accessible_loaded,
|
||||
all_loaded,
|
||||
) {
|
||||
send_app_list_updated_notification(outgoing, merged.clone()).await;
|
||||
send_app_list_updated_notification(
|
||||
outgoing,
|
||||
merged.clone(),
|
||||
/*is_final*/ false,
|
||||
)
|
||||
.await;
|
||||
last_notified_apps = Some(merged);
|
||||
}
|
||||
}
|
||||
@@ -254,7 +259,12 @@ impl AppsRequestProcessor {
|
||||
all_loaded,
|
||||
) && last_notified_apps.as_ref() != Some(&merged)
|
||||
{
|
||||
send_app_list_updated_notification(outgoing, merged.clone()).await;
|
||||
send_app_list_updated_notification(
|
||||
outgoing,
|
||||
merged.clone(),
|
||||
/*is_final*/ accessible_loaded && all_loaded,
|
||||
)
|
||||
.await;
|
||||
last_notified_apps = Some(merged.clone());
|
||||
}
|
||||
|
||||
@@ -381,10 +391,11 @@ fn paginate_apps(
|
||||
async fn send_app_list_updated_notification(
|
||||
outgoing: &Arc<OutgoingMessageSender>,
|
||||
data: Vec<AppInfo>,
|
||||
is_final: bool,
|
||||
) {
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::AppListUpdated(
|
||||
AppListUpdatedNotification { data },
|
||||
AppListUpdatedNotification { data, is_final },
|
||||
))
|
||||
.await;
|
||||
}
|
||||
@@ -428,6 +439,6 @@ async fn send_force_refetched_app_list_updated_notification(
|
||||
&config,
|
||||
);
|
||||
if data != previous_data {
|
||||
send_app_list_updated_notification(outgoing, data).await;
|
||||
send_app_list_updated_notification(outgoing, data, /*is_final*/ true).await;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -272,7 +272,10 @@ impl ConfigRequestProcessor {
|
||||
);
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::AppListUpdated(
|
||||
AppListUpdatedNotification { data },
|
||||
AppListUpdatedNotification {
|
||||
data,
|
||||
is_final: true,
|
||||
},
|
||||
))
|
||||
.await;
|
||||
});
|
||||
|
||||
@@ -87,6 +87,7 @@ use crate::version::CODEX_CLI_VERSION;
|
||||
use codex_app_server_protocol::AddCreditsNudgeCreditType;
|
||||
use codex_app_server_protocol::AddCreditsNudgeEmailStatus;
|
||||
use codex_app_server_protocol::AppInfo;
|
||||
use codex_app_server_protocol::AppListUpdatedNotification;
|
||||
use codex_app_server_protocol::AppSummary;
|
||||
use codex_app_server_protocol::CodexErrorInfo as AppServerCodexErrorInfo;
|
||||
use codex_app_server_protocol::CollabAgentTool;
|
||||
@@ -10034,10 +10035,16 @@ impl ChatWidget {
|
||||
}
|
||||
}
|
||||
|
||||
fn on_app_list_updated(&mut self, connectors: Vec<AppInfo>) {
|
||||
fn on_app_list_updated(&mut self, notification: AppListUpdatedNotification) {
|
||||
if !notification.is_final && !self.connectors.prefetch_in_flight {
|
||||
return;
|
||||
}
|
||||
let is_final = notification.is_final && !self.connectors.prefetch_in_flight;
|
||||
self.on_connectors_loaded(
|
||||
Ok(ConnectorsSnapshot { connectors }),
|
||||
/*is_final*/ !self.connectors.prefetch_in_flight,
|
||||
Ok(ConnectorsSnapshot {
|
||||
connectors: notification.data,
|
||||
}),
|
||||
is_final,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -159,7 +159,7 @@ impl ChatWidget {
|
||||
}
|
||||
ServerNotification::AppListUpdated(notification) => {
|
||||
if !from_replay {
|
||||
self.on_app_list_updated(notification.data);
|
||||
self.on_app_list_updated(notification);
|
||||
}
|
||||
}
|
||||
ServerNotification::ItemGuardianApprovalReviewStarted(notification) => {
|
||||
|
||||
@@ -1659,7 +1659,7 @@ async fn apps_refresh_failure_with_cached_snapshot_triggers_pending_force_refetc
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn apps_popup_keeps_existing_full_snapshot_while_partial_refresh_loads() {
|
||||
async fn apps_popup_ignores_stale_partial_app_list_update_after_refresh() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
set_chatgpt_auth(&mut chat);
|
||||
chat.config
|
||||
@@ -1742,6 +1742,7 @@ async fn apps_popup_keeps_existing_full_snapshot_while_partial_refresh_loads() {
|
||||
plugin_display_names: Vec::new(),
|
||||
},
|
||||
],
|
||||
is_final: false,
|
||||
}),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user