mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: Use remote installed plugin cache for skills and MCP (#20096)
- Fetches and caches remote /installed plugin state - Lets skills/list load skills from remote-installed cached plugins without requiring a local marketplace entry - Routes plugin list/startup/install/uninstall changes through async plugin cache invalidation and MCP refresh
This commit is contained in:
committed by
GitHub
Unverified
parent
5cf0adba93
commit
73cd831952
@@ -723,6 +723,35 @@ impl CodexMessageProcessor {
|
||||
self.clear_plugin_related_caches();
|
||||
}
|
||||
|
||||
pub(crate) fn effective_plugins_changed_callback(
|
||||
&self,
|
||||
config: Config,
|
||||
) -> Arc<dyn Fn() + Send + Sync> {
|
||||
let thread_manager = Arc::clone(&self.thread_manager);
|
||||
Arc::new(move || {
|
||||
Self::spawn_effective_plugins_changed_task(Arc::clone(&thread_manager), config.clone());
|
||||
})
|
||||
}
|
||||
|
||||
fn on_effective_plugins_changed(&self, config: Config) {
|
||||
Self::spawn_effective_plugins_changed_task(Arc::clone(&self.thread_manager), config);
|
||||
}
|
||||
|
||||
fn spawn_effective_plugins_changed_task(thread_manager: Arc<ThreadManager>, config: Config) {
|
||||
tokio::spawn(async move {
|
||||
thread_manager.plugins_manager().clear_cache();
|
||||
thread_manager.skills_manager().clear_cache();
|
||||
if thread_manager.list_thread_ids().await.is_empty() {
|
||||
return;
|
||||
}
|
||||
if let Err(err) =
|
||||
Self::queue_mcp_server_refresh_for_config(&thread_manager, &config).await
|
||||
{
|
||||
warn!("failed to queue MCP refresh after effective plugins changed: {err:?}");
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
fn clear_plugin_related_caches(&self) {
|
||||
self.thread_manager.plugins_manager().clear_cache();
|
||||
self.thread_manager.skills_manager().clear_cache();
|
||||
@@ -5372,7 +5401,7 @@ impl CodexMessageProcessor {
|
||||
async fn mcp_server_refresh(&self, request_id: ConnectionRequestId, _params: Option<()>) {
|
||||
let result = async {
|
||||
let config = self.load_latest_config(/*fallback_cwd*/ None).await?;
|
||||
self.queue_mcp_server_refresh_for_config(&config).await?;
|
||||
Self::queue_mcp_server_refresh_for_config(&self.thread_manager, &config).await?;
|
||||
Ok::<_, JSONRPCErrorError>(McpServerRefreshResponse {})
|
||||
}
|
||||
.await;
|
||||
@@ -5380,11 +5409,10 @@ impl CodexMessageProcessor {
|
||||
}
|
||||
|
||||
async fn queue_mcp_server_refresh_for_config(
|
||||
&self,
|
||||
thread_manager: &Arc<ThreadManager>,
|
||||
config: &Config,
|
||||
) -> Result<(), JSONRPCErrorError> {
|
||||
let configured_servers = self
|
||||
.thread_manager
|
||||
let configured_servers = thread_manager
|
||||
.mcp_manager()
|
||||
.configured_servers(config)
|
||||
.await;
|
||||
@@ -5420,7 +5448,6 @@ impl CodexMessageProcessor {
|
||||
|
||||
// Refresh requests are queued per thread; each thread rebuilds MCP connections on its next
|
||||
// active turn to avoid work for threads that never resume.
|
||||
let thread_manager = Arc::clone(&self.thread_manager);
|
||||
thread_manager.refresh_mcp_servers(refresh_config).await;
|
||||
Ok(())
|
||||
}
|
||||
@@ -6269,12 +6296,13 @@ impl CodexMessageProcessor {
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let effective_skill_roots = plugins_manager
|
||||
.effective_skill_roots_for_layer_stack(
|
||||
&config_layer_stack,
|
||||
config.features.enabled(Feature::Plugins) && workspace_codex_plugins_enabled,
|
||||
)
|
||||
.await;
|
||||
let effective_skill_roots = if workspace_codex_plugins_enabled {
|
||||
plugins_manager
|
||||
.effective_skill_roots_for_layer_stack(&config_layer_stack, &config)
|
||||
.await
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
let skills_input = codex_core::skills::SkillsLoadInput::new(
|
||||
cwd_abs.clone(),
|
||||
effective_skill_roots,
|
||||
|
||||
@@ -37,7 +37,12 @@ impl CodexMessageProcessor {
|
||||
{
|
||||
return Ok(empty_response());
|
||||
}
|
||||
plugins_manager.maybe_start_non_curated_plugin_cache_refresh(&roots);
|
||||
plugins_manager.maybe_start_plugin_list_background_tasks_for_config(
|
||||
&config,
|
||||
auth.clone(),
|
||||
&roots,
|
||||
Some(self.effective_plugins_changed_callback(config.clone())),
|
||||
);
|
||||
|
||||
let config_for_marketplace_listing = config.clone();
|
||||
let plugins_manager_for_marketplace_listing = plugins_manager.clone();
|
||||
@@ -362,17 +367,10 @@ impl CodexMessageProcessor {
|
||||
}
|
||||
};
|
||||
|
||||
self.clear_plugin_related_caches();
|
||||
self.on_effective_plugins_changed(config.clone());
|
||||
|
||||
let plugin_mcp_servers = load_plugin_mcp_servers(result.installed_path.as_path()).await;
|
||||
|
||||
if !plugin_mcp_servers.is_empty() {
|
||||
if let Err(err) = self.queue_mcp_server_refresh_for_config(&config).await {
|
||||
warn!(
|
||||
plugin = result.plugin_id.as_key(),
|
||||
"failed to queue MCP refresh after plugin install: {err:?}"
|
||||
);
|
||||
}
|
||||
self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers)
|
||||
.await;
|
||||
}
|
||||
@@ -464,19 +462,16 @@ impl CodexMessageProcessor {
|
||||
.await
|
||||
.map_err(|err| remote_plugin_catalog_error_to_jsonrpc(err, "install remote plugin"))?;
|
||||
|
||||
// TODO(remote plugins): remote marketplaces do not yet have a local
|
||||
// marketplace/read-path sync, so this install path reads MCP/apps directly
|
||||
// from the just-cached bundle.
|
||||
self.clear_plugin_related_caches();
|
||||
self.thread_manager
|
||||
.plugins_manager()
|
||||
.maybe_start_remote_installed_plugins_cache_refresh_after_mutation(
|
||||
&config,
|
||||
auth.clone(),
|
||||
Some(self.effective_plugins_changed_callback(config.clone())),
|
||||
);
|
||||
|
||||
let plugin_mcp_servers = load_plugin_mcp_servers(result.installed_path.as_path()).await;
|
||||
if !plugin_mcp_servers.is_empty() {
|
||||
if let Err(err) = self.queue_mcp_server_refresh_for_config(&config).await {
|
||||
warn!(
|
||||
plugin = result.plugin_id.as_key(),
|
||||
"failed to queue MCP refresh after remote plugin install: {err:?}"
|
||||
);
|
||||
}
|
||||
self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers)
|
||||
.await;
|
||||
}
|
||||
@@ -591,7 +586,15 @@ impl CodexMessageProcessor {
|
||||
.uninstall_plugin(plugin_id)
|
||||
.await
|
||||
.map_err(Self::plugin_uninstall_error)?;
|
||||
self.clear_plugin_related_caches();
|
||||
match self.load_latest_config(/*fallback_cwd*/ None).await {
|
||||
Ok(config) => self.on_effective_plugins_changed(config),
|
||||
Err(err) => {
|
||||
warn!(
|
||||
"failed to reload config after plugin uninstall, clearing plugin-related caches only: {err:?}"
|
||||
);
|
||||
self.clear_plugin_related_caches();
|
||||
}
|
||||
}
|
||||
Ok(PluginUninstallResponse {})
|
||||
}
|
||||
|
||||
@@ -675,16 +678,32 @@ impl CodexMessageProcessor {
|
||||
let remote_plugin_service_config = RemotePluginServiceConfig {
|
||||
chatgpt_base_url: config.chatgpt_base_url.clone(),
|
||||
};
|
||||
codex_core_plugins::remote::uninstall_remote_plugin(
|
||||
let uninstall_result = codex_core_plugins::remote::uninstall_remote_plugin(
|
||||
&remote_plugin_service_config,
|
||||
auth.as_ref(),
|
||||
config.codex_home.to_path_buf(),
|
||||
&plugin_id,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| remote_plugin_catalog_error_to_jsonrpc(err, "uninstall remote plugin"))?;
|
||||
.await;
|
||||
|
||||
self.clear_plugin_related_caches();
|
||||
if matches!(
|
||||
&uninstall_result,
|
||||
Ok(()) | Err(RemotePluginCatalogError::CacheRemove(_))
|
||||
) {
|
||||
let plugins_manager = self.thread_manager.plugins_manager();
|
||||
if plugins_manager.clear_remote_installed_plugins_cache() {
|
||||
self.on_effective_plugins_changed(config.clone());
|
||||
}
|
||||
plugins_manager.maybe_start_remote_installed_plugins_cache_refresh_after_mutation(
|
||||
&config,
|
||||
auth.clone(),
|
||||
Some(self.effective_plugins_changed_callback(config.clone())),
|
||||
);
|
||||
}
|
||||
|
||||
uninstall_result.map_err(|err| {
|
||||
remote_plugin_catalog_error_to_jsonrpc(err, "uninstall remote plugin")
|
||||
})?;
|
||||
Ok(PluginUninstallResponse {})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -317,10 +317,15 @@ impl MessageProcessor {
|
||||
});
|
||||
if matches!(plugin_startup_tasks, crate::PluginStartupTasks::Start) {
|
||||
// Keep plugin startup warmups aligned at app-server startup.
|
||||
// TODO(xl): Move into PluginManager once this no longer depends on config feature gating.
|
||||
let on_effective_plugins_changed =
|
||||
codex_message_processor.effective_plugins_changed_callback((*config).clone());
|
||||
thread_manager
|
||||
.plugins_manager()
|
||||
.maybe_start_plugin_startup_tasks_for_config(&config, auth_manager.clone());
|
||||
.maybe_start_plugin_startup_tasks_for_config(
|
||||
&config,
|
||||
auth_manager.clone(),
|
||||
Some(on_effective_plugins_changed),
|
||||
);
|
||||
}
|
||||
let config_api = ConfigApi::new(
|
||||
config_manager,
|
||||
|
||||
@@ -7,6 +7,8 @@ use app_test_support::McpProcess;
|
||||
use app_test_support::to_response;
|
||||
use app_test_support::write_chatgpt_auth;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
use codex_app_server_protocol::PluginListParams;
|
||||
use codex_app_server_protocol::PluginListResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::SkillsChangedNotification;
|
||||
use codex_app_server_protocol::SkillsListExtraRootsForCwd;
|
||||
@@ -24,6 +26,7 @@ use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::header;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
use wiremock::matchers::query_param;
|
||||
|
||||
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
const WATCHER_TIMEOUT: Duration = Duration::from_secs(20);
|
||||
@@ -52,6 +55,23 @@ plugins = true
|
||||
)
|
||||
}
|
||||
|
||||
fn write_remote_plugins_enabled_config_with_base_url(
|
||||
codex_home: &std::path::Path,
|
||||
base_url: &str,
|
||||
) -> std::io::Result<()> {
|
||||
std::fs::write(
|
||||
codex_home.join("config.toml"),
|
||||
format!(
|
||||
r#"chatgpt_base_url = "{base_url}"
|
||||
|
||||
[features]
|
||||
plugins = true
|
||||
remote_plugin = true
|
||||
"#,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
fn write_plugin_with_skill(
|
||||
repo_root: &std::path::Path,
|
||||
plugin_name: &str,
|
||||
@@ -93,6 +113,26 @@ fn write_plugin_with_skill(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_cached_remote_plugin_with_skill(
|
||||
codex_home: &std::path::Path,
|
||||
) -> Result<std::path::PathBuf> {
|
||||
let plugin_root = codex_home.join("plugins/cache/chatgpt-global/linear/local");
|
||||
std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?;
|
||||
std::fs::write(
|
||||
plugin_root.join(".codex-plugin/plugin.json"),
|
||||
r#"{"name":"linear"}"#,
|
||||
)?;
|
||||
|
||||
let skill_dir = plugin_root.join("skills/triage-issues");
|
||||
std::fs::create_dir_all(&skill_dir)?;
|
||||
let skill_path = skill_dir.join("SKILL.md");
|
||||
std::fs::write(
|
||||
&skill_path,
|
||||
"---\nname: triage-issues\ndescription: Triage Linear issues\n---\n\n# Body\n",
|
||||
)?;
|
||||
Ok(skill_path)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_list_includes_skills_from_per_cwd_extra_user_roots() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
@@ -131,6 +171,186 @@ async fn skills_list_includes_skills_from_per_cwd_extra_user_roots() -> Result<(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_list_loads_remote_installed_plugin_skills_from_cache() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cwd = TempDir::new()?;
|
||||
let server = MockServer::start().await;
|
||||
let expected_skill_path =
|
||||
std::fs::canonicalize(write_cached_remote_plugin_with_skill(codex_home.path())?)?;
|
||||
write_remote_plugins_enabled_config_with_base_url(
|
||||
codex_home.path(),
|
||||
&format!("{}/backend-api/", 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_directory_body = r#"{
|
||||
"plugins": [
|
||||
{
|
||||
"id": "plugins~Plugin_linear",
|
||||
"name": "linear",
|
||||
"scope": "GLOBAL",
|
||||
"installation_policy": "AVAILABLE",
|
||||
"authentication_policy": "ON_USE",
|
||||
"release": {
|
||||
"display_name": "Linear",
|
||||
"description": "Track work in Linear",
|
||||
"app_ids": [],
|
||||
"interface": {},
|
||||
"skills": []
|
||||
}
|
||||
}
|
||||
],
|
||||
"pagination": {
|
||||
"limit": 50,
|
||||
"next_page_token": null
|
||||
}
|
||||
}"#;
|
||||
let global_installed_body = r#"{
|
||||
"plugins": [
|
||||
{
|
||||
"id": "plugins~Plugin_linear",
|
||||
"name": "linear",
|
||||
"scope": "GLOBAL",
|
||||
"installation_policy": "AVAILABLE",
|
||||
"authentication_policy": "ON_USE",
|
||||
"release": {
|
||||
"display_name": "Linear",
|
||||
"description": "Track work in Linear",
|
||||
"app_ids": [],
|
||||
"interface": {},
|
||||
"skills": []
|
||||
},
|
||||
"enabled": true,
|
||||
"disabled_skill_names": []
|
||||
}
|
||||
],
|
||||
"pagination": {
|
||||
"limit": 50,
|
||||
"next_page_token": null
|
||||
}
|
||||
}"#;
|
||||
let empty_page_body = r#"{
|
||||
"plugins": [],
|
||||
"pagination": {
|
||||
"limit": 50,
|
||||
"next_page_token": null
|
||||
}
|
||||
}"#;
|
||||
|
||||
for (scope, body) in [
|
||||
("GLOBAL", global_directory_body),
|
||||
("WORKSPACE", empty_page_body),
|
||||
] {
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/ps/plugins/list"))
|
||||
.and(query_param("scope", scope))
|
||||
.and(query_param("limit", "200"))
|
||||
.and(header("authorization", "Bearer chatgpt-token"))
|
||||
.and(header("chatgpt-account-id", "account-123"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_string(body))
|
||||
.mount(&server)
|
||||
.await;
|
||||
}
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let stale_skills_list_request_id = mcp
|
||||
.send_skills_list_request(SkillsListParams {
|
||||
cwds: vec![cwd.path().to_path_buf()],
|
||||
force_reload: true,
|
||||
per_cwd_extra_user_roots: None,
|
||||
})
|
||||
.await?;
|
||||
let stale_skills_list_response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(stale_skills_list_request_id)),
|
||||
)
|
||||
.await??;
|
||||
let SkillsListResponse { data } = to_response(stale_skills_list_response)?;
|
||||
assert_eq!(data.len(), 1);
|
||||
assert!(
|
||||
data[0]
|
||||
.skills
|
||||
.iter()
|
||||
.all(|skill| skill.name != "linear:triage-issues"),
|
||||
"remote installed plugin cache has not been refreshed yet"
|
||||
);
|
||||
|
||||
for (scope, body) in [
|
||||
("GLOBAL", global_installed_body),
|
||||
("WORKSPACE", empty_page_body),
|
||||
] {
|
||||
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(200).set_body_string(body))
|
||||
.mount(&server)
|
||||
.await;
|
||||
}
|
||||
|
||||
let plugin_list_request_id = mcp
|
||||
.send_plugin_list_request(PluginListParams { cwds: None })
|
||||
.await?;
|
||||
let plugin_list_response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(plugin_list_request_id)),
|
||||
)
|
||||
.await??;
|
||||
let _: PluginListResponse = to_response(plugin_list_response)?;
|
||||
|
||||
let SkillsListResponse { data } = timeout(DEFAULT_TIMEOUT, async {
|
||||
loop {
|
||||
let skills_list_request_id = mcp
|
||||
.send_skills_list_request(SkillsListParams {
|
||||
cwds: vec![cwd.path().to_path_buf()],
|
||||
force_reload: false,
|
||||
per_cwd_extra_user_roots: None,
|
||||
})
|
||||
.await?;
|
||||
let skills_list_response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(skills_list_request_id)),
|
||||
)
|
||||
.await??;
|
||||
let response: SkillsListResponse = to_response(skills_list_response)?;
|
||||
if response.data.iter().any(|entry| {
|
||||
entry
|
||||
.skills
|
||||
.iter()
|
||||
.any(|skill| skill.name == "linear:triage-issues")
|
||||
}) {
|
||||
break Ok::<SkillsListResponse, anyhow::Error>(response);
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(50)).await;
|
||||
}
|
||||
})
|
||||
.await??;
|
||||
|
||||
assert_eq!(data.len(), 1);
|
||||
assert_eq!(data[0].errors, Vec::new());
|
||||
let skill = data[0]
|
||||
.skills
|
||||
.iter()
|
||||
.find(|skill| skill.name == "linear:triage-issues")
|
||||
.expect("expected skill from cached remote plugin");
|
||||
assert_eq!(
|
||||
std::fs::canonicalize(skill.path.as_path())?,
|
||||
expected_skill_path
|
||||
);
|
||||
assert_eq!(skill.enabled, true);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_list_excludes_plugin_skills_when_workspace_codex_plugins_disabled() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
Reference in New Issue
Block a user