fix(plugin): track routes by object identity to prevent cross-plugin removal

Bug 2: deactivate() removed routes by path-matching, which caused
routes from other plugins sharing the same prefix (e.g. /api/v1/dms)
to be removed when deactivating one plugin. Now tracks actual route
objects by identity in _mounted_routes and removes only those specific
objects during deactivation.
This commit is contained in:
2026-07-03 16:25:31 +00:00
parent 18a0861a96
commit 94a5bf105c
+22 -20
View File
@@ -31,8 +31,9 @@ class PluginRegistry:
self._plugins: dict[str, BasePlugin] = {} self._plugins: dict[str, BasePlugin] = {}
# name -> PluginModel DB record (installed status) # name -> PluginModel DB record (installed status)
self._db_status: dict[str, PluginModel] = {} self._db_status: dict[str, PluginModel] = {}
# name -> list of mounted APIRouter references (for unregistration) # name -> list of actual route objects added to the FastAPI app
self._mounted_routers: dict[str, list[Any]] = {} # (tracked by object identity to avoid cross-plugin route removal)
self._mounted_routes: dict[str, list[Any]] = {}
self._engine: AsyncEngine | None = None self._engine: AsyncEngine | None = None
self._event_bus: EventBus = get_event_bus() self._event_bus: EventBus = get_event_bus()
self._container: ServiceContainer = get_container() self._container: ServiceContainer = get_container()
@@ -59,7 +60,7 @@ class PluginRegistry:
raise RuntimeError("Registry not initialized — call initialize() first") raise RuntimeError("Registry not initialized — call initialize() first")
return self._engine return self._engine
# ── Discovery ── # ── Discovery ──
def discover_builtins(self) -> list[str]: def discover_builtins(self) -> list[str]:
"""Discover and load built-in plugins from app.plugins.builtins. """Discover and load built-in plugins from app.plugins.builtins.
@@ -116,7 +117,7 @@ class PluginRegistry:
"""List all discovered plugin names.""" """List all discovered plugin names."""
return list(self._plugins.keys()) return list(self._plugins.keys())
# ── DB Status Sync ── # ── DB Status Sync ──
async def sync_db_status(self, db: AsyncSession) -> None: async def sync_db_status(self, db: AsyncSession) -> None:
"""Load plugin status records from the database.""" """Load plugin status records from the database."""
@@ -127,7 +128,7 @@ class PluginRegistry:
"""Get the DB status record for a plugin.""" """Get the DB status record for a plugin."""
return self._db_status.get(name) return self._db_status.get(name)
# ── Install / Activate / Deactivate / Uninstall ── # ── Install / Activate / Deactivate / Uninstall ──
async def install(self, db: AsyncSession, name: str) -> PluginModel: async def install(self, db: AsyncSession, name: str) -> PluginModel:
"""Install a plugin: run migrations and create DB record. """Install a plugin: run migrations and create DB record.
@@ -186,13 +187,19 @@ class PluginRegistry:
await plugin.on_activate(db, self._container, self._event_bus) await plugin.on_activate(db, self._container, self._event_bus)
# Register routes on FastAPI app if available # Register routes on FastAPI app if available
# Track actual route objects by identity to avoid cross-plugin removal
if self._app is not None: if self._app is not None:
routers = plugin.get_routes() routers = plugin.get_routes()
mounted = [] mounted_routes: list[Any] = []
for router in routers: for router in routers:
# Snapshot existing route object IDs before inclusion
existing_ids = {id(r) for r in self._app.router.routes}
self._app.include_router(router) self._app.include_router(router)
mounted.append(router) # Collect newly added route objects
self._mounted_routers[name] = mounted for r in self._app.router.routes:
if id(r) not in existing_ids:
mounted_routes.append(r)
self._mounted_routes[name] = mounted_routes
# Update DB record # Update DB record
record.status = "active" record.status = "active"
@@ -222,20 +229,15 @@ class PluginRegistry:
# Call on_deactivate hook (unregisters event listeners) # Call on_deactivate hook (unregisters event listeners)
await plugin.on_deactivate(db, self._container, self._event_bus) await plugin.on_deactivate(db, self._container, self._event_bus)
# Unregister routes from FastAPI app if available # Unregister only the specific route objects that belong to this plugin
if self._app is not None and name in self._mounted_routers: # (by object identity, not by path — prevents cross-plugin route removal)
mounted = self._mounted_routers.pop(name, []) if self._app is not None and name in self._mounted_routes:
# Collect all route paths from mounted routers for removal mounted_routes = self._mounted_routes.pop(name, [])
paths_to_remove: set[str] = set() mounted_ids = {id(r) for r in mounted_routes}
for router in mounted:
for route in router.routes:
if hasattr(route, "path"):
paths_to_remove.add(route.path)
# Remove matching routes from app
self._app.router.routes = [ self._app.router.routes = [
r r
for r in self._app.router.routes for r in self._app.router.routes
if not (hasattr(r, "path") and r.path in paths_to_remove) if id(r) not in mounted_ids
] ]
# Update DB record # Update DB record
@@ -362,7 +364,7 @@ class PluginRegistry:
return plugins_list return plugins_list
# ── Internal Helpers ── # ── Internal Helpers ──
async def _get_plugin_record(self, db: AsyncSession, name: str) -> PluginModel | None: async def _get_plugin_record(self, db: AsyncSession, name: str) -> PluginModel | None:
"""Fetch a plugin record from DB by name.""" """Fetch a plugin record from DB by name."""