Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
235 changes: 235 additions & 0 deletions codex-rs/app-server/tests/suite/v2/plugin_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2226,6 +2226,230 @@ plugin_sharing = true
Ok(())
}

#[tokio::test]
async fn plugin_installed_keeps_global_remote_plugins_when_workspace_fetch_fails() -> Result<()> {
let codex_home = TempDir::new()?;
let server = MockServer::start().await;
std::fs::write(
codex_home.path().join("config.toml"),
format!(
r#"chatgpt_base_url = "{}/backend-api/"

[features]
plugins = true
remote_plugin = true
plugin_sharing = false
"#,
server.uri()
),
)?;
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 global_installed_body = remote_installed_plugin_body("", "1.2.3", /*enabled*/ true);
mount_remote_installed_plugins(&server, "GLOBAL", &global_installed_body).await;
mount_remote_installed_plugins_error(&server, "WORKSPACE").await;

let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;

let request_id = mcp
.send_plugin_installed_request(PluginInstalledParams {
cwds: None,
install_suggestion_plugin_names: None,
})
.await?;

let response: PluginInstalledResponse = to_response(
timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
)
.await??,
)?;

assert_eq!(response.marketplaces.len(), 1);
assert_eq!(response.marketplaces[0].name, "openai-curated-remote");
assert_eq!(
response.marketplaces[0]
.plugins
.iter()
.map(|plugin| (plugin.id.clone(), plugin.installed, plugin.enabled))
.collect::<Vec<_>>(),
vec![("linear@openai-curated-remote".to_string(), true, true)]
);
wait_for_remote_installed_scope_request(&server, "GLOBAL").await?;
wait_for_remote_installed_scope_request(&server, "WORKSPACE").await?;
Ok(())
}

#[tokio::test]
async fn plugin_installed_keeps_workspace_remote_plugins_when_global_fetch_fails() -> Result<()> {
let codex_home = TempDir::new()?;
let server = MockServer::start().await;
std::fs::write(
codex_home.path().join("config.toml"),
format!(
r#"chatgpt_base_url = "{}/backend-api/"

[features]
plugins = true
remote_plugin = false
plugin_sharing = true
"#,
server.uri()
),
)?;
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 workspace_installed_body = workspace_remote_plugin_page_body(
"plugins~Plugin_22222222222222222222222222222222",
"shared-linear",
"Shared Linear",
"PRIVATE",
/*enabled*/ Some(true),
);
mount_remote_installed_plugins_error(&server, "GLOBAL").await;
mount_remote_installed_plugins(&server, "WORKSPACE", &workspace_installed_body).await;

let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;

let request_id = mcp
.send_plugin_installed_request(PluginInstalledParams {
cwds: None,
install_suggestion_plugin_names: None,
})
.await?;

let response: PluginInstalledResponse = to_response(
timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
)
.await??,
)?;

assert_eq!(response.marketplaces.len(), 1);
assert_eq!(response.marketplaces[0].name, "workspace-shared-with-me");
assert_eq!(
response.marketplaces[0]
.plugins
.iter()
.map(|plugin| (plugin.id.clone(), plugin.installed, plugin.enabled))
.collect::<Vec<_>>(),
vec![(
"shared-linear@workspace-shared-with-me".to_string(),
true,
true
)]
);
wait_for_remote_installed_scope_request(&server, "GLOBAL").await?;
wait_for_remote_installed_scope_request(&server, "WORKSPACE").await?;
Ok(())
}

#[tokio::test]
async fn plugin_installed_skips_malformed_remote_plugin_within_successful_scope() -> Result<()> {
let codex_home = TempDir::new()?;
let server = MockServer::start().await;
std::fs::write(
codex_home.path().join("config.toml"),
format!(
r#"chatgpt_base_url = "{}/backend-api/"

[features]
plugins = true
remote_plugin = false
plugin_sharing = true
"#,
server.uri()
),
)?;
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 workspace_installed_body: serde_json::Value =
serde_json::from_str(&workspace_remote_plugin_page_body(
"plugins~Plugin_22222222222222222222222222222222",
"shared-linear",
"Shared Linear",
"PRIVATE",
/*enabled*/ Some(true),
))?;
let mut malformed_installed_body: serde_json::Value =
serde_json::from_str(&workspace_remote_plugin_page_body(
"plugins~Plugin_33333333333333333333333333333333",
"malformed-linear",
"Malformed Linear",
"PRIVATE",
/*enabled*/ Some(true),
))?;
malformed_installed_body["plugins"][0]
.as_object_mut()
.expect("installed plugin should be an object")
.remove("discoverability");
workspace_installed_body["plugins"]
.as_array_mut()
.expect("installed plugins should be an array")
.push(malformed_installed_body["plugins"][0].clone());
let workspace_installed_body = serde_json::to_string(&workspace_installed_body)?;
mount_remote_installed_plugins(&server, "GLOBAL", empty_remote_installed_plugins_body()).await;
mount_remote_installed_plugins(&server, "WORKSPACE", &workspace_installed_body).await;

let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;

let request_id = mcp
.send_plugin_installed_request(PluginInstalledParams {
cwds: None,
install_suggestion_plugin_names: None,
})
.await?;

let response: PluginInstalledResponse = to_response(
timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
)
.await??,
)?;

assert_eq!(response.marketplaces.len(), 1);
assert_eq!(response.marketplaces[0].name, "workspace-shared-with-me");
assert_eq!(
response.marketplaces[0]
.plugins
.iter()
.map(|plugin| (plugin.id.clone(), plugin.installed, plugin.enabled))
.collect::<Vec<_>>(),
vec![(
"shared-linear@workspace-shared-with-me".to_string(),
true,
true
)]
);
wait_for_remote_installed_scope_request(&server, "GLOBAL").await?;
wait_for_remote_installed_scope_request(&server, "WORKSPACE").await?;
Ok(())
}

#[tokio::test]
async fn plugin_installed_starts_remote_installed_bundle_sync() -> Result<()> {
let codex_home = TempDir::new()?;
Expand Down Expand Up @@ -3049,6 +3273,17 @@ async fn mount_remote_installed_plugins(server: &MockServer, scope: &str, body:
.await;
}

async fn mount_remote_installed_plugins_error(server: &MockServer, scope: &str) {
Mock::given(method("GET"))
.and(path("/backend-api/ps/plugins/installed"))
.and(query_param("scope", scope))
.and(header("authorization", "Bearer chatgpt-token"))
.and(header("chatgpt-account-id", "account-123"))
.respond_with(ResponseTemplate::new(500).set_body_string("remote installed fetch failed"))
.mount(server)
.await;
}

fn empty_remote_installed_plugins_body() -> &'static str {
r#"{
"plugins": [],
Expand Down
49 changes: 43 additions & 6 deletions codex-rs/core-plugins/src/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -689,12 +689,49 @@ pub(crate) async fn fetch_remote_installed_plugins(
Ok::<_, RemotePluginCatalogError>((scope, installed_plugins))
};

let (global, workspace) = tokio::try_join!(global, workspace)?;
let mut installed_plugins = [global, workspace]
.into_iter()
.flat_map(|(_scope, plugins)| plugins)
.map(|plugin| remote_installed_plugin_to_cache_entry(&plugin))
.collect::<Result<Vec<_>, _>>()?;
let (global, workspace) = tokio::join!(global, workspace);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bound per-scope installed fetches

Switching from try_join! to join! means a quick failure from one scope no longer completes the request while the other in-flight scope is still pending. The default remote-plugin reqwest client does not set an application timeout, so during a partial backend outage where (for example) GLOBAL returns 500 but WORKSPACE stalls, plugin/installed can now block until the transport/OS gives up instead of returning the error promptly as it did before; add a per-scope timeout or other cancellation path before waiting for both results.

Useful? React with 👍 / 👎.


let mut installed_plugins = Vec::new();
let mut error_if_all_failed = None;
let mut any_scope_succeeded = false;
for (scope, result) in [
(RemotePluginScope::Global, global),
(RemotePluginScope::Workspace, workspace),
] {
match result {
Ok((_scope, plugins)) => {
any_scope_succeeded = true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Don't let hidden scopes mask visible-scope failures

When only one remote installed scope is enabled for the client (for example remote_plugin = true and plugin_sharing = false), this marks the whole fetch successful if the disabled/hidden workspace scope succeeds, even when the visible global scope failed; load_remote_installed_plugins then filters the successful workspace result out via visible_scopes and returns no remote marketplaces instead of surfacing the global fetch failure. The same happens in reverse for workspace-only configurations, so partial success should be evaluated against the scopes that can actually be shown or the fetch should not include disabled scopes.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is expected

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve cached plugins for failed scopes

When one scope succeeds and the other scope returns an error, this marks the whole fetch as successful and returns only the plugins from the successful scope. Both build_and_cache_remote_installed_plugin_marketplaces and the background refresh path write that returned vector back to remote_installed_plugins_cache wholesale, so with both remote scopes enabled a transient 500/timeout for WORKSPACE after GLOBAL succeeds will drop all previously cached workspace-installed plugins and fire the effective-plugins-changed callback, effectively disabling those plugins until a later successful refresh. The partial response should not overwrite the failed scope's cached entries unless the fetch actually proved they are gone.

Useful? React with 👍 / 👎.

for plugin in plugins {
match remote_installed_plugin_to_cache_entry(&plugin) {
Ok(plugin) => installed_plugins.push(plugin),
Err(err) => {
tracing::warn!(
scope = scope.api_value(),
plugin_id = %plugin.plugin.id,
plugin_name = %plugin.plugin.name,
error = %err,
"skipping malformed remote installed plugin"
);
Comment on lines +707 to +714
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve cached plugins skipped as malformed

When a successful scope contains one malformed installed-plugin entry, this branch drops only that entry and still returns Ok for the shortened list. The manager then writes that shortened vector back to remote_installed_plugins_cache, so a transient bad payload for a single previously cached plugin, such as a workspace plugin missing discoverability, makes that plugin disappear from effective plugins until a later clean refresh instead of preserving the last known-good entry.

Useful? React with 👍 / 👎.

}
}
}
}
Err(err) => {
tracing::warn!(
scope = scope.api_value(),
error = %err,
"failed to fetch remote installed plugins for scope"
);
if error_if_all_failed.is_none() {
error_if_all_failed = Some(err);
}
}
}
}
if !any_scope_succeeded && let Some(err) = error_if_all_failed {
return Err(err);
}

installed_plugins.sort_by(|left, right| {
left.marketplace_name
.cmp(&right.marketplace_name)
Expand Down
Loading