agent/codex
1
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Require remote plugin detail before uninstall (#19966)

Browse Source 
## Summary
- Fetch remote plugin detail before sending the uninstall request.
- Use the detail response to derive the marketplace namespace and plugin
name for cache cleanup.
- Stop the uninstall before the backend POST if detail lookup fails, so
backend state and local cache state do not diverge.

## Testing
- `just fmt`
- `cargo test -p codex-app-server plugin_uninstall`
- `cargo test -p codex-core-plugins`
- `git diff --check`
This commit is contained in:
xli-oai
2026-04-29 14:01:11 -07:00
committed by GitHub
Unverified
parent 973c5c823e
commit afbddabc8b
2 changed files with 53 additions and 125 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/tests/suite/v2/plugin_uninstall.rs
+14 -40
View File
@@ -196,7 +196,6 @@ async fn plugin_uninstall_writes_remote_plugin_to_cloud_when_remote_plugin_enabl
)?;
mount_remote_plugin_detail(&server, REMOTE_PLUGIN_ID, "1.0.0", "GLOBAL").await;
mount_empty_remote_installed_plugins(&server).await;
Mock::given(method("POST"))
.and(path(format!(
@@ -269,7 +268,6 @@ async fn plugin_uninstall_uses_detail_scope_for_cache_namespace() -> Result<()>
AuthCredentialsStoreMode::File,
)?;
mount_remote_plugin_detail(&server, REMOTE_PLUGIN_ID, "1.0.0", "WORKSPACE").await;
mount_empty_remote_installed_plugins(&server).await;
Mock::given(method("POST"))
.and(path(format!(
@@ -326,7 +324,7 @@ async fn plugin_uninstall_uses_detail_scope_for_cache_namespace() -> Result<()>
}
#[tokio::test]
async fn plugin_uninstall_posts_even_when_remote_detail_fetch_fails() -> Result<()> {
async fn plugin_uninstall_rejects_before_post_when_remote_detail_fetch_fails() -> Result<()> {
let codex_home = TempDir::new()?;
let server = MockServer::start().await;
write_remote_plugin_catalog_config(
@@ -342,19 +340,6 @@ async fn plugin_uninstall_posts_even_when_remote_detail_fetch_fails() -> Result<
AuthCredentialsStoreMode::File,
)?;
Mock::given(method("POST"))
.and(path(format!(
"/backend-api/plugins/{REMOTE_PLUGIN_ID}/uninstall"
)))
.and(header("authorization", "Bearer chatgpt-token"))
.and(header("chatgpt-account-id", "account-123"))
.respond_with(
ResponseTemplate::new(200)
.set_body_string(format!(r#"{{"id":"{REMOTE_PLUGIN_ID}","enabled":false}}"#)),
)
.mount(&server)
.await;
let legacy_remote_plugin_cache_root = codex_home
.path()
.join(format!("plugins/cache/chatgpt-global/{REMOTE_PLUGIN_ID}"));
@@ -368,22 +353,29 @@ async fn plugin_uninstall_posts_even_when_remote_detail_fetch_fails() -> Result<
plugin_id: REMOTE_PLUGIN_ID.to_string(),
})
.await?;
let response: JSONRPCResponse = timeout(
let err = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
mcp.read_stream_until_error_message(RequestId::Integer(request_id)),
)
.await??;
let response: PluginUninstallResponse = to_response(response)?;
assert_eq!(response, PluginUninstallResponse {});
assert_eq!(err.error.code, -32600);
assert!(err.error.message.contains("remote plugin catalog request"));
wait_for_remote_plugin_request_count(
&server,
"GET",
&format!("/ps/plugins/{REMOTE_PLUGIN_ID}"),
/*expected_count*/ 1,
)
.await?;
wait_for_remote_plugin_request_count(
&server,
"POST",
&format!("/plugins/{REMOTE_PLUGIN_ID}/uninstall"),
/*expected_count*/ 1,
/*expected_count*/ 0,
)
.await?;
assert!(!legacy_remote_plugin_cache_root.exists());
assert!(legacy_remote_plugin_cache_root.exists());
Ok(())
}
@@ -557,24 +549,6 @@ async fn mount_remote_plugin_detail(
.await;
}
async fn mount_empty_remote_installed_plugins(server: &MockServer) {
Mock::given(method("GET"))
.and(path("/backend-api/ps/plugins/installed"))
.and(header("authorization", "Bearer chatgpt-token"))
.and(header("chatgpt-account-id", "account-123"))
.respond_with(ResponseTemplate::new(200).set_body_string(
r#"{
"plugins": [],
"pagination": {
"limit": 50,
"next_page_token": null
}
}"#,
))
.mount(server)
.await;
}
async fn wait_for_remote_plugin_request_count(
server: &MockServer,
method_name: &str,
codex-rs/core-plugins/src/remote.rs
+39 -85
View File
@@ -13,10 +13,8 @@ use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::collections::HashSet;
use std::fs;
use std::path::Path;
use std::path::PathBuf;
use std::time::Duration;
use tracing::warn;
pub const REMOTE_GLOBAL_MARKETPLACE_NAME: &str = "chatgpt-global";
pub const REMOTE_WORKSPACE_MARKETPLACE_NAME: &str = "chatgpt-workspace";
@@ -476,27 +474,6 @@ async fn fetch_remote_plugin_detail_with_download_url_option(
.await
}
async fn fetch_remote_plugin_detail_by_id(
config: &RemotePluginServiceConfig,
auth: &CodexAuth,
plugin_id: &str,
) -> Result<RemotePluginDetail, RemotePluginCatalogError> {
let plugin = fetch_plugin_detail(
config, auth, plugin_id, /*include_download_urls*/ false,
)
.await?;
let scope = plugin.scope;
build_remote_plugin_detail(
config,
auth,
scope,
scope.marketplace_name().to_string(),
plugin_id,
plugin,
)
.await
}
async fn build_remote_plugin_detail(
config: &RemotePluginServiceConfig,
auth: &CodexAuth,
@@ -589,6 +566,12 @@ pub async fn uninstall_remote_plugin(
plugin_id: &str,
) -> Result<(), RemotePluginCatalogError> {
let auth = ensure_chatgpt_auth(auth)?;
let plugin = fetch_plugin_detail(
config, auth, plugin_id, /*include_download_urls*/ false,
)
.await?;
let marketplace_name = plugin.scope.marketplace_name().to_string();
let plugin_name = plugin.name;
let base_url = config.chatgpt_base_url.trim_end_matches('/');
let url = format!("{base_url}/plugins/{plugin_id}/uninstall");
@@ -609,19 +592,9 @@ pub async fn uninstall_remote_plugin(
});
}
let remote_detail = match fetch_remote_plugin_detail_by_id(config, auth, plugin_id).await {
Ok(remote_detail) => Some(remote_detail),
Err(err) => {
warn!(
plugin_id,
"failed to read remote plugin details after uninstall; skipping named cache removal: {err}"
);
None
}
};
let legacy_plugin_id = plugin_id.to_string();
tokio::task::spawn_blocking(move || {
remove_remote_plugin_cache(codex_home, remote_detail, legacy_plugin_id)
remove_remote_plugin_cache(codex_home, marketplace_name, plugin_name, legacy_plugin_id)
})
.await
.map_err(|err| {
@@ -636,65 +609,46 @@ pub async fn uninstall_remote_plugin(
fn remove_remote_plugin_cache(
codex_home: PathBuf,
remote_detail: Option<RemotePluginDetail>,
marketplace_name: String,
plugin_name: String,
legacy_plugin_id: String,
) -> Result<(), String> {
if let Some(remote_detail) = remote_detail {
let marketplace_name = remote_detail.marketplace_name;
let plugin_name = remote_detail.summary.name;
let store = PluginStore::try_new(codex_home.clone())
.map_err(|err| format!("failed to resolve remote plugin cache root: {err}"))?;
let plugin_id = PluginId::new(plugin_name.clone(), marketplace_name.clone()).map_err(
|err| {
format!(
"invalid remote plugin cache id for `{plugin_name}` in `{marketplace_name}`: {err}"
)
},
)?;
let plugin_cache_root = store.plugin_base_root(&plugin_id);
store.uninstall(&plugin_id).map_err(|err| {
let store = PluginStore::try_new(codex_home.clone())
.map_err(|err| format!("failed to resolve remote plugin cache root: {err}"))?;
let plugin_id =
PluginId::new(plugin_name.clone(), marketplace_name.clone()).map_err(|err| {
format!(
"failed to remove remote plugin cache entry {}: {err}",
plugin_cache_root.display()
"invalid remote plugin cache id for `{plugin_name}` in `{marketplace_name}`: {err}"
)
})?;
let legacy_remote_plugin_cache_root = codex_home
.join(PLUGINS_CACHE_DIR)
.join(marketplace_name)
.join(legacy_plugin_id);
if legacy_remote_plugin_cache_root != plugin_cache_root.as_path() {
remove_path_if_exists(&legacy_remote_plugin_cache_root)?;
}
return Ok(());
}
for scope in RemotePluginScope::all() {
let legacy_remote_plugin_cache_root = codex_home
.join(PLUGINS_CACHE_DIR)
.join(scope.marketplace_name())
.join(&legacy_plugin_id);
remove_path_if_exists(&legacy_remote_plugin_cache_root)?;
}
Ok(())
}
fn remove_path_if_exists(path: &Path) -> Result<(), String> {
if !path.exists() {
return Ok(());
}
let result = if path.is_dir() {
fs::remove_dir_all(path)
} else {
fs::remove_file(path)
};
result.map_err(|err| {
let plugin_cache_root = store.plugin_base_root(&plugin_id);
store.uninstall(&plugin_id).map_err(|err| {
format!(
"failed to remove remote plugin cache entry {}: {err}",
path.display()
plugin_cache_root.display()
)
})
})?;
let legacy_remote_plugin_cache_root = codex_home
.join(PLUGINS_CACHE_DIR)
.join(marketplace_name)
.join(legacy_plugin_id);
if legacy_remote_plugin_cache_root != plugin_cache_root.as_path()
&& legacy_remote_plugin_cache_root.exists()
{
let result = if legacy_remote_plugin_cache_root.is_dir() {
fs::remove_dir_all(&legacy_remote_plugin_cache_root)
} else {
fs::remove_file(&legacy_remote_plugin_cache_root)
};
result.map_err(|err| {
format!(
"failed to remove remote plugin cache entry {}: {err}",
legacy_remote_plugin_cache_root.display()
)
})?;
}
Ok(())
}
fn build_remote_plugin_summary(