fix(plugins): isole les plugins non servables et durcit la réconciliation MCP
Un bundle_url invalide ou un plugin dont l'asset n'est pas réellement servable passe désormais en état Invalid isolé au lieu de faire échouer globalement la réconciliation MCP au démarrage — cause racine de la perte d'affichage à l'installation de hello-plugin (#116/#120). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@ -507,7 +507,7 @@ async fn install_from_staged(
|
||||
.await
|
||||
.map_err(map_store)?;
|
||||
let mut registry = registry_store.load_registry().await.map_err(map_registry)?;
|
||||
let entry = PluginRegistryEntry {
|
||||
let mut entry = PluginRegistryEntry {
|
||||
id: plugin_id.clone(),
|
||||
lifecycle_state: PluginLifecycleState::Enabled,
|
||||
source: review.source.clone(),
|
||||
@ -516,6 +516,15 @@ async fn install_from_staged(
|
||||
restart_required: true,
|
||||
error: None,
|
||||
};
|
||||
if let Err(err) = runtime_plugin_from_entry(packages, validator, entry.clone()).await {
|
||||
let message = format!(
|
||||
"runtime contributions disabled: plugin `{}` is not servable: {err}",
|
||||
plugin_id.as_str()
|
||||
);
|
||||
crate::diag!("[plugins] {message}");
|
||||
entry.lifecycle_state = PluginLifecycleState::Invalid;
|
||||
entry.error = Some(message);
|
||||
}
|
||||
registry.upsert(entry.clone());
|
||||
registry_store
|
||||
.save_registry(®istry)
|
||||
@ -525,9 +534,10 @@ async fn install_from_staged(
|
||||
plugin_id: plugin_id.clone(),
|
||||
version: review.manifest.version.clone(),
|
||||
});
|
||||
let _ = mcp
|
||||
.reconcile(active_mcp_specs(packages, validator, ®istry).await?)
|
||||
.await;
|
||||
let (active_servers, invalid_plugins) =
|
||||
active_mcp_specs(packages, validator, ®istry).await?;
|
||||
persist_invalid_runtime_plugins(registry_store, &mut registry, invalid_plugins).await;
|
||||
let _ = mcp.reconcile(active_servers).await;
|
||||
let admin = admin_from_descriptor(
|
||||
PluginDescriptor {
|
||||
manifest: review.manifest.clone(),
|
||||
@ -611,13 +621,11 @@ impl SetPluginEnabled {
|
||||
restart_required: true,
|
||||
});
|
||||
}
|
||||
let _ = self
|
||||
.mcp
|
||||
.reconcile(
|
||||
active_mcp_specs(self.packages.as_ref(), self.validator.as_ref(), ®istry)
|
||||
.await?,
|
||||
)
|
||||
let (active_servers, invalid_plugins) =
|
||||
active_mcp_specs(self.packages.as_ref(), self.validator.as_ref(), ®istry).await?;
|
||||
persist_invalid_runtime_plugins(self.registry.as_ref(), &mut registry, invalid_plugins)
|
||||
.await;
|
||||
let _ = self.mcp.reconcile(active_servers).await;
|
||||
let descriptor =
|
||||
descriptor_for(self.packages.as_ref(), self.validator.as_ref(), saved).await?;
|
||||
admin_from_descriptor(descriptor, self.packages.as_ref())
|
||||
@ -833,9 +841,11 @@ impl ReconcilePluginMcpServers {
|
||||
|
||||
/// Executes the use case.
|
||||
pub async fn execute(&self) -> Result<domain::PluginMcpStatusSet, AppError> {
|
||||
let registry = self.registry.load_registry().await.map_err(map_registry)?;
|
||||
let specs =
|
||||
let mut registry = self.registry.load_registry().await.map_err(map_registry)?;
|
||||
let (specs, invalid_plugins) =
|
||||
active_mcp_specs(self.packages.as_ref(), self.validator.as_ref(), ®istry).await?;
|
||||
persist_invalid_runtime_plugins(self.registry.as_ref(), &mut registry, invalid_plugins)
|
||||
.await;
|
||||
self.mcp.reconcile(specs).await.map_err(map_mcp)
|
||||
}
|
||||
}
|
||||
@ -844,7 +854,7 @@ async fn active_mcp_specs(
|
||||
packages: &dyn PluginPackageStore,
|
||||
validator: &dyn PluginManifestValidator,
|
||||
registry: &domain::PluginRegistry,
|
||||
) -> Result<Vec<PluginMcpServerSpec>, AppError> {
|
||||
) -> Result<(Vec<PluginMcpServerSpec>, Vec<(PluginId, String)>), AppError> {
|
||||
let installed_roots = packages
|
||||
.list_installed()
|
||||
.await
|
||||
@ -854,11 +864,23 @@ async fn active_mcp_specs(
|
||||
.collect::<std::collections::HashMap<_, _>>();
|
||||
let app_data_dir = packages.app_data_dir_label();
|
||||
let mut specs = Vec::new();
|
||||
let mut invalid_plugins = Vec::new();
|
||||
for entry in ®istry.plugins {
|
||||
if !entry.lifecycle_state.is_runtime_active() {
|
||||
continue;
|
||||
}
|
||||
let descriptor = descriptor_for(packages, validator, entry.clone()).await?;
|
||||
let descriptor = match descriptor_for(packages, validator, entry.clone()).await {
|
||||
Ok(descriptor) => descriptor,
|
||||
Err(err) => {
|
||||
let message = format!(
|
||||
"MCP servers disabled: plugin `{}` is not servable: {err}",
|
||||
entry.id.as_str()
|
||||
);
|
||||
crate::diag!("[plugins] {message}");
|
||||
invalid_plugins.push((entry.id.clone(), message));
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let plugin_root = installed_roots
|
||||
.get(&descriptor.manifest.id)
|
||||
.cloned()
|
||||
@ -907,7 +929,30 @@ async fn active_mcp_specs(
|
||||
});
|
||||
}
|
||||
}
|
||||
Ok(specs)
|
||||
Ok((specs, invalid_plugins))
|
||||
}
|
||||
|
||||
async fn persist_invalid_runtime_plugins(
|
||||
registry_store: &dyn PluginRegistryStore,
|
||||
registry: &mut domain::PluginRegistry,
|
||||
invalid_plugins: Vec<(PluginId, String)>,
|
||||
) {
|
||||
if invalid_plugins.is_empty() {
|
||||
return;
|
||||
}
|
||||
for (plugin_id, message) in invalid_plugins {
|
||||
if let Some(entry) = registry.plugins.iter_mut().find(|p| p.id == plugin_id) {
|
||||
entry.lifecycle_state = PluginLifecycleState::Invalid;
|
||||
entry.error = Some(message);
|
||||
}
|
||||
}
|
||||
if let Err(err) = registry_store
|
||||
.save_registry(registry)
|
||||
.await
|
||||
.map_err(map_registry)
|
||||
{
|
||||
crate::diag!("[plugins] failed to persist invalid runtime plugin state: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
fn substitute_vars(raw: &str, plugin_root: &str, app_data_dir: Option<&str>) -> String {
|
||||
@ -1668,6 +1713,38 @@ mod tests {
|
||||
assert!(reconciles[0].is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reconcile_mcp_marks_invalid_active_plugin_and_keeps_reconcile_alive() {
|
||||
let packages = Arc::new(FakePackages::with_manifest(br#"{"broken":true}"#.to_vec()));
|
||||
let registry = Arc::new(FakeRegistry {
|
||||
registry: Mutex::new(registry_with(PluginLifecycleState::Enabled)),
|
||||
});
|
||||
let mcp = Arc::new(FakeMcp::default());
|
||||
let usecase = ReconcilePluginMcpServers::new(
|
||||
packages,
|
||||
registry.clone(),
|
||||
Arc::new(validator()),
|
||||
mcp.clone(),
|
||||
);
|
||||
|
||||
let statuses = usecase.execute().await.unwrap();
|
||||
|
||||
assert!(statuses.servers.is_empty());
|
||||
assert_eq!(mcp.reconciles.lock().unwrap().len(), 1);
|
||||
assert!(mcp.reconciles.lock().unwrap()[0].is_empty());
|
||||
let saved = registry.load_registry().await.unwrap();
|
||||
let entry = saved.find(&plugin_id()).unwrap();
|
||||
assert_eq!(entry.lifecycle_state, PluginLifecycleState::Invalid);
|
||||
assert!(
|
||||
entry
|
||||
.error
|
||||
.as_deref()
|
||||
.unwrap_or_default()
|
||||
.contains("MCP servers disabled"),
|
||||
"registry must carry a confined MCP error: {entry:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reconcile_mcp_substitutes_app_data_dir_in_plugin_server_specs() {
|
||||
let mut manifest: serde_json::Value = serde_json::from_slice(&valid_manifest()).unwrap();
|
||||
|
||||
@ -255,6 +255,7 @@ impl PluginPackageStore for FsPluginPackageStore {
|
||||
entry: &RelativePath,
|
||||
hash: &ContentHash,
|
||||
) -> Result<PluginBundleUrl, PluginStoreError> {
|
||||
self.resolve_asset_path(plugin_id, entry)?;
|
||||
Ok(PluginBundleUrl::new(format!(
|
||||
"idea-plugin://{}/current/{}{}{}",
|
||||
plugin_id.as_str(),
|
||||
|
||||
@ -146,3 +146,77 @@ async fn installs_sdk_hello_plugin_and_loads_runtime_catalog() {
|
||||
|
||||
let _ = fs::remove_dir_all(app_data);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn installed_plugin_with_missing_main_is_isolated_from_runtime_catalog() {
|
||||
let app_data = temp_dir("missing-main-app-data");
|
||||
let source = temp_dir("missing-main-source");
|
||||
fs::write(
|
||||
source.join("idea-plugin.json"),
|
||||
r#"{
|
||||
"ideaPluginManifestVersion": 1,
|
||||
"id": "dev.idea.fixtures.missing-main",
|
||||
"displayName": "Missing Main Plugin",
|
||||
"version": "0.1.0",
|
||||
"main": "dist/index.js",
|
||||
"trustLevel": "full",
|
||||
"capabilities": ["ui"],
|
||||
"contributes": {}
|
||||
}"#,
|
||||
)
|
||||
.unwrap();
|
||||
let packages = Arc::new(FsPluginPackageStore::new(&app_data));
|
||||
let registry = Arc::new(FsPluginRegistryStore::new(&app_data));
|
||||
let validator = Arc::new(JsonPluginManifestValidator::new("0.3.0"));
|
||||
let events = Arc::new(TokioBroadcastEventBus::new());
|
||||
let mcp = Arc::new(ExternalMcpPluginSupervisor::new());
|
||||
|
||||
let install = InstallPluginFromDirectory::new(
|
||||
packages.clone(),
|
||||
registry.clone(),
|
||||
validator.clone(),
|
||||
events,
|
||||
mcp,
|
||||
);
|
||||
let result = install
|
||||
.execute(source.to_string_lossy().into_owned())
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(result.plugin.id, "dev.idea.fixtures.missing-main");
|
||||
assert_eq!(
|
||||
result.plugin.lifecycle_state,
|
||||
domain::PluginLifecycleState::Invalid
|
||||
);
|
||||
assert!(result
|
||||
.plugin
|
||||
.error
|
||||
.as_deref()
|
||||
.unwrap_or_default()
|
||||
.contains("not servable"));
|
||||
|
||||
let catalog = ListPluginRuntimeContributions::new(packages, registry.clone(), validator)
|
||||
.execute()
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(catalog.plugins.is_empty());
|
||||
let admin = ListPlugins::new(
|
||||
Arc::new(FsPluginPackageStore::new(&app_data)),
|
||||
registry,
|
||||
Arc::new(JsonPluginManifestValidator::new("0.3.0")),
|
||||
)
|
||||
.execute()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
admin[0].lifecycle_state,
|
||||
domain::PluginLifecycleState::Invalid
|
||||
);
|
||||
assert!(admin[0]
|
||||
.error
|
||||
.as_deref()
|
||||
.unwrap_or_default()
|
||||
.contains("not servable"));
|
||||
|
||||
let _ = fs::remove_dir_all(source);
|
||||
let _ = fs::remove_dir_all(app_data);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user