fix(plugin): add dependency resolution and permission soft-check
Bug 3: install() and activate() now verify that declared dependencies are installed/active before proceeding. Raises ValueError which the service layer converts to HTTPException 400. Bug 5: activate() now performs a soft permission check — logs warnings for declared permissions not found in any discovered plugin.
This commit is contained in:
@@ -128,12 +128,96 @@ class PluginRegistry:
|
||||
"""Get the DB status record for a plugin."""
|
||||
return self._db_status.get(name)
|
||||
|
||||
# ── Dependency Resolution ──
|
||||
|
||||
async def _check_dependencies_installed(self, db: AsyncSession, name: str) -> None:
|
||||
"""Verify that all declared dependencies of a plugin are installed.
|
||||
|
||||
Raises ValueError listing missing dependencies.
|
||||
"""
|
||||
plugin = self.get_plugin(name)
|
||||
if plugin is None:
|
||||
raise ValueError(f"Plugin '{name}' not found in registry")
|
||||
|
||||
dependencies = plugin.manifest.dependencies
|
||||
if not dependencies:
|
||||
return
|
||||
|
||||
missing: list[str] = []
|
||||
for dep_name in dependencies:
|
||||
dep_record = await self._get_plugin_record(db, dep_name)
|
||||
if dep_record is None or not dep_record.installed:
|
||||
missing.append(dep_name)
|
||||
|
||||
if missing:
|
||||
raise ValueError(
|
||||
f"Plugin '{name}' has uninstalled dependencies: {', '.join(missing)}. "
|
||||
f"Install them first."
|
||||
)
|
||||
|
||||
async def _check_dependencies_active(self, db: AsyncSession, name: str) -> None:
|
||||
"""Verify that all declared dependencies of a plugin are active.
|
||||
|
||||
Raises ValueError listing inactive dependencies.
|
||||
"""
|
||||
plugin = self.get_plugin(name)
|
||||
if plugin is None:
|
||||
raise ValueError(f"Plugin '{name}' not found in registry")
|
||||
|
||||
dependencies = plugin.manifest.dependencies
|
||||
if not dependencies:
|
||||
return
|
||||
|
||||
inactive: list[str] = []
|
||||
for dep_name in dependencies:
|
||||
dep_record = await self._get_plugin_record(db, dep_name)
|
||||
if dep_record is None or not dep_record.active:
|
||||
inactive.append(dep_name)
|
||||
|
||||
if inactive:
|
||||
raise ValueError(
|
||||
f"Plugin '{name}' has inactive dependencies: {', '.join(inactive)}. "
|
||||
f"Activate them first."
|
||||
)
|
||||
|
||||
# ── Permission Validation (soft check) ──
|
||||
|
||||
def _check_permissions(self, name: str) -> list[str]:
|
||||
"""Soft-check that declared permissions exist in the system.
|
||||
|
||||
Returns a list of warning messages for permissions that are not
|
||||
recognised in the available system permissions set.
|
||||
Does not raise — this is a warning-only check.
|
||||
"""
|
||||
plugin = self.get_plugin(name)
|
||||
if plugin is None:
|
||||
return []
|
||||
|
||||
declared = plugin.manifest.permissions
|
||||
if not declared:
|
||||
return []
|
||||
|
||||
# Build a set of all known permission strings from every discovered plugin
|
||||
available: set[str] = set()
|
||||
for p in self._plugins.values():
|
||||
available.update(p.manifest.permissions)
|
||||
|
||||
warnings: list[str] = []
|
||||
for perm in declared:
|
||||
if perm not in available:
|
||||
warnings.append(
|
||||
f"Plugin '{name}' declares permission '{perm}' "
|
||||
f"which is not found in any discovered plugin's permissions."
|
||||
)
|
||||
return warnings
|
||||
|
||||
# ── Install / Activate / Deactivate / Uninstall ──
|
||||
|
||||
async def install(self, db: AsyncSession, name: str) -> PluginModel:
|
||||
"""Install a plugin: run migrations and create DB record.
|
||||
|
||||
Idempotent: if already installed, returns existing record.
|
||||
Checks that all declared dependencies are installed first.
|
||||
"""
|
||||
plugin = self.get_plugin(name)
|
||||
if plugin is None:
|
||||
@@ -144,6 +228,9 @@ class PluginRegistry:
|
||||
if existing is not None:
|
||||
return existing
|
||||
|
||||
# Check dependencies are installed
|
||||
await self._check_dependencies_installed(db, name)
|
||||
|
||||
# Run migrations
|
||||
if plugin.manifest.migrations:
|
||||
await self.migration_runner.run_all_migrations(db, name, plugin.manifest.migrations)
|
||||
@@ -170,6 +257,8 @@ class PluginRegistry:
|
||||
"""Activate a plugin: register routes, event listeners, set status=active.
|
||||
|
||||
Idempotent: if already active, returns existing record without error.
|
||||
Checks that all declared dependencies are active first.
|
||||
Performs a soft permission check and logs warnings for unknown permissions.
|
||||
"""
|
||||
plugin = self.get_plugin(name)
|
||||
if plugin is None:
|
||||
@@ -183,6 +272,14 @@ class PluginRegistry:
|
||||
if record.active and record.status == "active":
|
||||
return record
|
||||
|
||||
# Check dependencies are active
|
||||
await self._check_dependencies_active(db, name)
|
||||
|
||||
# Soft permission check — log warnings for unknown permissions
|
||||
perm_warnings = self._check_permissions(name)
|
||||
for warning in perm_warnings:
|
||||
logger.warning(warning)
|
||||
|
||||
# Call on_activate hook (registers event listeners)
|
||||
await plugin.on_activate(db, self._container, self._event_bus)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user