From 94a5bf105cc93c496556ae0c61c82ef5d7e98884 Mon Sep 17 00:00:00 2001
From: Leopoldadmin <leopold.hammerschmidt@media-on.de>
Date: Fri, 3 Jul 2026 16:25:31 +0000
Subject: [PATCH] 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.
---
 app/plugins/registry.py | 42 +++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/app/plugins/registry.py b/app/plugins/registry.py
index 364619c..5c8946f 100644
--- a/app/plugins/registry.py
+++ b/app/plugins/registry.py
@@ -31,8 +31,9 @@ class PluginRegistry:
         self._plugins: dict[str, BasePlugin] = {}
         # name -> PluginModel DB record (installed status)
         self._db_status: dict[str, PluginModel] = {}
-        # name -> list of mounted APIRouter references (for unregistration)
-        self._mounted_routers: dict[str, list[Any]] = {}
+        # name -> list of actual route objects added to the FastAPI app
+        # (tracked by object identity to avoid cross-plugin route removal)
+        self._mounted_routes: dict[str, list[Any]] = {}
         self._engine: AsyncEngine | None = None
         self._event_bus: EventBus = get_event_bus()
         self._container: ServiceContainer = get_container()
@@ -59,7 +60,7 @@ class PluginRegistry:
             raise RuntimeError("Registry not initialized — call initialize() first")
         return self._engine
 
-    # ─── Discovery ───
+    # ── Discovery ──
 
     def discover_builtins(self) -> list[str]:
         """Discover and load built-in plugins from app.plugins.builtins.
@@ -116,7 +117,7 @@ class PluginRegistry:
         """List all discovered plugin names."""
         return list(self._plugins.keys())
 
-    # ─── DB Status Sync ───
+    # ── DB Status Sync ──
 
     async def sync_db_status(self, db: AsyncSession) -> None:
         """Load plugin status records from the database."""
@@ -127,7 +128,7 @@ class PluginRegistry:
         """Get the DB status record for a plugin."""
         return self._db_status.get(name)
 
-    # ─── Install / Activate / Deactivate / Uninstall ───
+    # ── Install / Activate / Deactivate / Uninstall ──
 
     async def install(self, db: AsyncSession, name: str) -> PluginModel:
         """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)
 
         # Register routes on FastAPI app if available
+        # Track actual route objects by identity to avoid cross-plugin removal
         if self._app is not None:
             routers = plugin.get_routes()
-            mounted = []
+            mounted_routes: list[Any] = []
             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)
-                mounted.append(router)
-            self._mounted_routers[name] = mounted
+                # Collect newly added route objects
+                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
         record.status = "active"
@@ -222,20 +229,15 @@ class PluginRegistry:
         # Call on_deactivate hook (unregisters event listeners)
         await plugin.on_deactivate(db, self._container, self._event_bus)
 
-        # Unregister routes from FastAPI app if available
-        if self._app is not None and name in self._mounted_routers:
-            mounted = self._mounted_routers.pop(name, [])
-            # Collect all route paths from mounted routers for removal
-            paths_to_remove: set[str] = set()
-            for router in mounted:
-                for route in router.routes:
-                    if hasattr(route, "path"):
-                        paths_to_remove.add(route.path)
-            # Remove matching routes from app
+        # Unregister only the specific route objects that belong to this plugin
+        # (by object identity, not by path — prevents cross-plugin route removal)
+        if self._app is not None and name in self._mounted_routes:
+            mounted_routes = self._mounted_routes.pop(name, [])
+            mounted_ids = {id(r) for r in mounted_routes}
             self._app.router.routes = [
                 r
                 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
@@ -362,7 +364,7 @@ class PluginRegistry:
 
         return plugins_list
 
-    # ─── Internal Helpers ───
+    # ── Internal Helpers ──
 
     async def _get_plugin_record(self, db: AsyncSession, name: str) -> PluginModel | None:
         """Fetch a plugin record from DB by name."""