mirror of
https://github.com/openai/codex.git
synced 2026-04-26 15:45:02 +00:00
[apps] Improve app loading. (#10994)
There are two concepts of apps that we load in the harness: - Directory apps, which is all the apps that the user can install. - Accessible apps, which is what the user actually installed and can be $ inserted and be used by the model. These are extracted from the tools that are loaded through the gateway MCP. Previously we wait for both sets of apps before returning the full apps list. Which causes many issues because accessible apps won't be available to the UI or the model if directory apps aren't loaded or failed to load. In this PR we are separating them so that accessible apps can be loaded separately and are instantly available to be shown in the UI and to be provided in model context. We also added an app-server event so that clients can subscribe to also get accessible apps without being blocked on the full app list. - [x] Separate accessible apps and directory apps loading. - [x] `app/list` request will also emit `app/list/updated` notifications that app-server clients can subscribe. Which allows clients to get accessible apps list to render in the $ menu without being blocked by directory apps. - [x] Cache both accessible and directory apps with 1 hour TTL to avoid reloading them when creating new threads. - [x] TUI improvements to redraw $ menu and /apps menu when app list is updated.
This commit is contained in:
@@ -3,6 +3,7 @@ use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::Result;
|
||||
use anyhow::bail;
|
||||
use app_test_support::ChatGptAuthFixture;
|
||||
use app_test_support::McpProcess;
|
||||
use app_test_support::to_response;
|
||||
@@ -15,10 +16,13 @@ use axum::http::StatusCode;
|
||||
use axum::http::header::AUTHORIZATION;
|
||||
use axum::routing::get;
|
||||
use codex_app_server_protocol::AppInfo;
|
||||
use codex_app_server_protocol::AppListUpdatedNotification;
|
||||
use codex_app_server_protocol::AppsListParams;
|
||||
use codex_app_server_protocol::AppsListResponse;
|
||||
use codex_app_server_protocol::JSONRPCError;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_core::auth::AuthCredentialsStoreMode;
|
||||
use pretty_assertions::assert_eq;
|
||||
use rmcp::handler::server::ServerHandler;
|
||||
@@ -51,6 +55,7 @@ async fn list_apps_returns_empty_when_connectors_disabled() -> Result<()> {
|
||||
.send_apps_list_request(AppsListParams {
|
||||
limit: Some(50),
|
||||
cursor: None,
|
||||
force_refetch: false,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -67,6 +72,120 @@ async fn list_apps_returns_empty_when_connectors_disabled() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn list_apps_emits_updates_and_returns_after_both_lists_load() -> Result<()> {
|
||||
let connectors = vec![
|
||||
AppInfo {
|
||||
id: "alpha".to_string(),
|
||||
name: "Alpha".to_string(),
|
||||
description: Some("Alpha connector".to_string()),
|
||||
logo_url: Some("https://example.com/alpha.png".to_string()),
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: None,
|
||||
is_accessible: false,
|
||||
},
|
||||
AppInfo {
|
||||
id: "beta".to_string(),
|
||||
name: "beta".to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: None,
|
||||
is_accessible: false,
|
||||
},
|
||||
];
|
||||
|
||||
let tools = vec![connector_tool("beta", "Beta App")?];
|
||||
let (server_url, server_handle) = start_apps_server_with_delays(
|
||||
connectors.clone(),
|
||||
tools,
|
||||
Duration::from_millis(300),
|
||||
Duration::ZERO,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
write_connectors_config(codex_home.path(), &server_url)?;
|
||||
write_chatgpt_auth(
|
||||
codex_home.path(),
|
||||
ChatGptAuthFixture::new("chatgpt-token")
|
||||
.account_id("account-123")
|
||||
.chatgpt_user_id("user-123")
|
||||
.chatgpt_account_id("account-123"),
|
||||
AuthCredentialsStoreMode::File,
|
||||
)?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let request_id = mcp
|
||||
.send_apps_list_request(AppsListParams {
|
||||
limit: None,
|
||||
cursor: None,
|
||||
force_refetch: false,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let expected_accessible = vec![AppInfo {
|
||||
id: "beta".to_string(),
|
||||
name: "Beta App".to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: Some("https://chatgpt.com/apps/beta-app/beta".to_string()),
|
||||
is_accessible: true,
|
||||
}];
|
||||
|
||||
let first_update = read_app_list_updated_notification(&mut mcp).await?;
|
||||
assert_eq!(first_update.data, expected_accessible);
|
||||
|
||||
let expected_merged = vec![
|
||||
AppInfo {
|
||||
id: "beta".to_string(),
|
||||
name: "Beta App".to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: Some("https://chatgpt.com/apps/beta/beta".to_string()),
|
||||
is_accessible: true,
|
||||
},
|
||||
AppInfo {
|
||||
id: "alpha".to_string(),
|
||||
name: "Alpha".to_string(),
|
||||
description: Some("Alpha connector".to_string()),
|
||||
logo_url: Some("https://example.com/alpha.png".to_string()),
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()),
|
||||
is_accessible: false,
|
||||
},
|
||||
];
|
||||
|
||||
let second_update = read_app_list_updated_notification(&mut mcp).await?;
|
||||
assert_eq!(second_update.data, expected_merged);
|
||||
|
||||
let response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let AppsListResponse {
|
||||
data: response_data,
|
||||
next_cursor,
|
||||
} = to_response(response)?;
|
||||
assert_eq!(response_data, expected_merged);
|
||||
assert!(next_cursor.is_none());
|
||||
|
||||
server_handle.abort();
|
||||
let _ = server_handle.await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn list_apps_returns_connectors_with_accessible_flags() -> Result<()> {
|
||||
let connectors = vec![
|
||||
@@ -93,7 +212,13 @@ async fn list_apps_returns_connectors_with_accessible_flags() -> Result<()> {
|
||||
];
|
||||
|
||||
let tools = vec![connector_tool("beta", "Beta App")?];
|
||||
let (server_url, server_handle) = start_apps_server(connectors.clone(), tools).await?;
|
||||
let (server_url, server_handle) = start_apps_server_with_delays(
|
||||
connectors.clone(),
|
||||
tools,
|
||||
Duration::ZERO,
|
||||
Duration::from_millis(300),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
write_connectors_config(codex_home.path(), &server_url)?;
|
||||
@@ -113,16 +238,36 @@ async fn list_apps_returns_connectors_with_accessible_flags() -> Result<()> {
|
||||
.send_apps_list_request(AppsListParams {
|
||||
limit: None,
|
||||
cursor: None,
|
||||
force_refetch: false,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let AppsListResponse { data, next_cursor } = to_response(response)?;
|
||||
let first_update = read_app_list_updated_notification(&mut mcp).await?;
|
||||
assert_eq!(
|
||||
first_update.data,
|
||||
vec![
|
||||
AppInfo {
|
||||
id: "alpha".to_string(),
|
||||
name: "Alpha".to_string(),
|
||||
description: Some("Alpha connector".to_string()),
|
||||
logo_url: Some("https://example.com/alpha.png".to_string()),
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()),
|
||||
is_accessible: false,
|
||||
},
|
||||
AppInfo {
|
||||
id: "beta".to_string(),
|
||||
name: "beta".to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: Some("https://chatgpt.com/apps/beta/beta".to_string()),
|
||||
is_accessible: false,
|
||||
},
|
||||
]
|
||||
);
|
||||
|
||||
let expected = vec![
|
||||
AppInfo {
|
||||
@@ -147,6 +292,15 @@ async fn list_apps_returns_connectors_with_accessible_flags() -> Result<()> {
|
||||
},
|
||||
];
|
||||
|
||||
let second_update = read_app_list_updated_notification(&mut mcp).await?;
|
||||
assert_eq!(second_update.data, expected);
|
||||
|
||||
let response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
let AppsListResponse { data, next_cursor } = to_response(response)?;
|
||||
assert_eq!(data, expected);
|
||||
assert!(next_cursor.is_none());
|
||||
|
||||
@@ -180,7 +334,13 @@ async fn list_apps_paginates_results() -> Result<()> {
|
||||
];
|
||||
|
||||
let tools = vec![connector_tool("beta", "Beta App")?];
|
||||
let (server_url, server_handle) = start_apps_server(connectors.clone(), tools).await?;
|
||||
let (server_url, server_handle) = start_apps_server_with_delays(
|
||||
connectors.clone(),
|
||||
tools,
|
||||
Duration::ZERO,
|
||||
Duration::from_millis(300),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
write_connectors_config(codex_home.path(), &server_url)?;
|
||||
@@ -200,6 +360,7 @@ async fn list_apps_paginates_results() -> Result<()> {
|
||||
.send_apps_list_request(AppsListParams {
|
||||
limit: Some(1),
|
||||
cursor: None,
|
||||
force_refetch: false,
|
||||
})
|
||||
.await?;
|
||||
let first_response: JSONRPCResponse = timeout(
|
||||
@@ -226,10 +387,18 @@ async fn list_apps_paginates_results() -> Result<()> {
|
||||
assert_eq!(first_page, expected_first);
|
||||
let next_cursor = first_cursor.ok_or_else(|| anyhow::anyhow!("missing cursor"))?;
|
||||
|
||||
loop {
|
||||
let update = read_app_list_updated_notification(&mut mcp).await?;
|
||||
if update.data.len() == 2 && update.data.iter().any(|connector| connector.is_accessible) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
let second_request = mcp
|
||||
.send_apps_list_request(AppsListParams {
|
||||
limit: Some(1),
|
||||
cursor: Some(next_cursor),
|
||||
force_refetch: false,
|
||||
})
|
||||
.await?;
|
||||
let second_response: JSONRPCResponse = timeout(
|
||||
@@ -260,21 +429,134 @@ async fn list_apps_paginates_results() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn list_apps_force_refetch_preserves_previous_cache_on_failure() -> Result<()> {
|
||||
let connectors = vec![AppInfo {
|
||||
id: "beta".to_string(),
|
||||
name: "Beta App".to_string(),
|
||||
description: Some("Beta connector".to_string()),
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
distribution_channel: None,
|
||||
install_url: None,
|
||||
is_accessible: false,
|
||||
}];
|
||||
let tools = vec![connector_tool("beta", "Beta App")?];
|
||||
let (server_url, server_handle) =
|
||||
start_apps_server_with_delays(connectors, tools, Duration::ZERO, Duration::ZERO).await?;
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
write_connectors_config(codex_home.path(), &server_url)?;
|
||||
write_chatgpt_auth(
|
||||
codex_home.path(),
|
||||
ChatGptAuthFixture::new("chatgpt-token")
|
||||
.account_id("account-123")
|
||||
.chatgpt_user_id("user-123")
|
||||
.chatgpt_account_id("account-123"),
|
||||
AuthCredentialsStoreMode::File,
|
||||
)?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let initial_request = mcp
|
||||
.send_apps_list_request(AppsListParams {
|
||||
limit: None,
|
||||
cursor: None,
|
||||
force_refetch: false,
|
||||
})
|
||||
.await?;
|
||||
let initial_response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(initial_request)),
|
||||
)
|
||||
.await??;
|
||||
let AppsListResponse {
|
||||
data: initial_data,
|
||||
next_cursor: initial_next_cursor,
|
||||
} = to_response(initial_response)?;
|
||||
assert!(initial_next_cursor.is_none());
|
||||
assert_eq!(initial_data.len(), 1);
|
||||
assert!(initial_data.iter().all(|app| app.is_accessible));
|
||||
|
||||
write_chatgpt_auth(
|
||||
codex_home.path(),
|
||||
ChatGptAuthFixture::new("chatgpt-token-invalid")
|
||||
.account_id("account-123")
|
||||
.chatgpt_user_id("user-123")
|
||||
.chatgpt_account_id("account-123"),
|
||||
AuthCredentialsStoreMode::File,
|
||||
)?;
|
||||
|
||||
let refetch_request = mcp
|
||||
.send_apps_list_request(AppsListParams {
|
||||
limit: None,
|
||||
cursor: None,
|
||||
force_refetch: true,
|
||||
})
|
||||
.await?;
|
||||
let refetch_error: JSONRPCError = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_error_message(RequestId::Integer(refetch_request)),
|
||||
)
|
||||
.await??;
|
||||
assert!(refetch_error.error.message.contains("failed to"));
|
||||
|
||||
let cached_request = mcp
|
||||
.send_apps_list_request(AppsListParams {
|
||||
limit: None,
|
||||
cursor: None,
|
||||
force_refetch: false,
|
||||
})
|
||||
.await?;
|
||||
let cached_response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(cached_request)),
|
||||
)
|
||||
.await??;
|
||||
let AppsListResponse {
|
||||
data: cached_data,
|
||||
next_cursor: cached_next_cursor,
|
||||
} = to_response(cached_response)?;
|
||||
|
||||
assert_eq!(cached_data, initial_data);
|
||||
assert!(cached_next_cursor.is_none());
|
||||
server_handle.abort();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn read_app_list_updated_notification(
|
||||
mcp: &mut McpProcess,
|
||||
) -> Result<AppListUpdatedNotification> {
|
||||
let notification = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("app/list/updated"),
|
||||
)
|
||||
.await??;
|
||||
let parsed: ServerNotification = notification.try_into()?;
|
||||
let ServerNotification::AppListUpdated(payload) = parsed else {
|
||||
bail!("unexpected notification variant");
|
||||
};
|
||||
Ok(payload)
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct AppsServerState {
|
||||
expected_bearer: String,
|
||||
expected_account_id: String,
|
||||
response: serde_json::Value,
|
||||
directory_delay: Duration,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct AppListMcpServer {
|
||||
tools: Arc<Vec<Tool>>,
|
||||
tools_delay: Duration,
|
||||
}
|
||||
|
||||
impl AppListMcpServer {
|
||||
fn new(tools: Arc<Vec<Tool>>) -> Self {
|
||||
Self { tools }
|
||||
fn new(tools: Arc<Vec<Tool>>, tools_delay: Duration) -> Self {
|
||||
Self { tools, tools_delay }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -293,7 +575,11 @@ impl ServerHandler for AppListMcpServer {
|
||||
) -> impl std::future::Future<Output = Result<ListToolsResult, rmcp::ErrorData>> + Send + '_
|
||||
{
|
||||
let tools = self.tools.clone();
|
||||
let tools_delay = self.tools_delay;
|
||||
async move {
|
||||
if tools_delay > Duration::ZERO {
|
||||
tokio::time::sleep(tools_delay).await;
|
||||
}
|
||||
Ok(ListToolsResult {
|
||||
tools: (*tools).clone(),
|
||||
next_cursor: None,
|
||||
@@ -303,14 +589,17 @@ impl ServerHandler for AppListMcpServer {
|
||||
}
|
||||
}
|
||||
|
||||
async fn start_apps_server(
|
||||
async fn start_apps_server_with_delays(
|
||||
connectors: Vec<AppInfo>,
|
||||
tools: Vec<Tool>,
|
||||
directory_delay: Duration,
|
||||
tools_delay: Duration,
|
||||
) -> Result<(String, JoinHandle<()>)> {
|
||||
let state = AppsServerState {
|
||||
expected_bearer: "Bearer chatgpt-token".to_string(),
|
||||
expected_account_id: "account-123".to_string(),
|
||||
response: json!({ "apps": connectors, "next_token": null }),
|
||||
directory_delay,
|
||||
};
|
||||
let state = Arc::new(state);
|
||||
let tools = Arc::new(tools);
|
||||
@@ -321,7 +610,7 @@ async fn start_apps_server(
|
||||
let mcp_service = StreamableHttpService::new(
|
||||
{
|
||||
let tools = tools.clone();
|
||||
move || Ok(AppListMcpServer::new(tools.clone()))
|
||||
move || Ok(AppListMcpServer::new(tools.clone(), tools_delay))
|
||||
},
|
||||
Arc::new(LocalSessionManager::default()),
|
||||
StreamableHttpServerConfig::default(),
|
||||
@@ -347,6 +636,10 @@ async fn list_directory_connectors(
|
||||
State(state): State<Arc<AppsServerState>>,
|
||||
headers: HeaderMap,
|
||||
) -> Result<impl axum::response::IntoResponse, StatusCode> {
|
||||
if state.directory_delay > Duration::ZERO {
|
||||
tokio::time::sleep(state.directory_delay).await;
|
||||
}
|
||||
|
||||
let bearer_ok = headers
|
||||
.get(AUTHORIZATION)
|
||||
.and_then(|value| value.to_str().ok())
|
||||
|
||||
Reference in New Issue
Block a user