Follow-up: address CodeRabbit feedback for #308 – centralize GetToolsFolderIdentifier, fix tools copy dir, and limit scan scope

Author: JohanHoltby

MCPForUnity/Editor/Helpers/ServerInstaller.cs

@@ -406,6 +406,7 @@ private static bool TryGetEmbeddedServerSource(out string srcPath)
         /// <summary>
         /// Searches Unity project for MCPForUnityTools folders and copies .py files to server tools directory.
         /// Only copies if the tool's version.txt has changed (or doesn't exist).
+        /// Files are copied into per-folder subdirectories to avoid conflicts.
         /// </summary>
         private static void CopyUnityProjectTools(string destToolsDir)
         {
@@ -418,18 +419,51 @@ private static void CopyUnityProjectTools(string destToolsDir)
                     return;
                 }
 
-                // Find all MCPForUnityTools folders
-                var toolsFolders = Directory.GetDirectories(projectRoot, "MCPForUnityTools", SearchOption.AllDirectories);
+                // Ensure destToolsDir exists
+                Directory.CreateDirectory(destToolsDir);
+
+                // Limit scan to specific directories to avoid deep recursion
+                var searchRoots = new List<string>();
+                var assetsPath = Path.Combine(projectRoot, "Assets");
+                var packagesPath = Path.Combine(projectRoot, "Packages");
+                var packageCachePath = Path.Combine(projectRoot, "Library", "PackageCache");
+
+                if (Directory.Exists(assetsPath)) searchRoots.Add(assetsPath);
+                if (Directory.Exists(packagesPath)) searchRoots.Add(packagesPath);
+                if (Directory.Exists(packageCachePath)) searchRoots.Add(packageCachePath);
+
+                // Find all MCPForUnityTools folders in limited search roots
+                var toolsFolders = new List<string>();
+                foreach (var searchRoot in searchRoots)
+                {
+                    try
+                    {
+                        toolsFolders.AddRange(Directory.GetDirectories(searchRoot, "MCPForUnityTools", SearchOption.AllDirectories));
+                    }
+                    catch (Exception ex)
+                    {
+                        McpLog.Warn($"Failed to search {searchRoot}: {ex.Message}");
+                    }
+                }
 
                 int copiedCount = 0;
                 int skippedCount = 0;
 
+                // Track all active folder identifiers (for cleanup)
+                var activeFolderIdentifiers = new HashSet<string>();
+
                 foreach (var folder in toolsFolders)
                 {
                     // Generate unique identifier for this tools folder based on its parent directory structure
                     // e.g., "MooseRunner_MCPForUnityTools" or "MyPackage_MCPForUnityTools"
                     string folderIdentifier = GetToolsFolderIdentifier(folder);
-                    string versionTrackingFile = Path.Combine(destToolsDir, $"{folderIdentifier}_version.txt");
+                    activeFolderIdentifiers.Add(folderIdentifier);
+
+                    // Create per-folder subdirectory in destToolsDir
+                    string destFolderSubdir = Path.Combine(destToolsDir, folderIdentifier);
+                    Directory.CreateDirectory(destFolderSubdir);
+
+                    string versionTrackingFile = Path.Combine(destFolderSubdir, "version.txt");
 
                     // Read source version
                     string sourceVersionFile = Path.Combine(folder, "version.txt");
@@ -450,7 +484,7 @@ private static void CopyUnityProjectTools(string destToolsDir)
                         foreach (var pyFile in pyFiles)
                         {
                             string fileName = Path.GetFileName(pyFile);
-                            string destFile = Path.Combine(destToolsDir, fileName);
+                            string destFile = Path.Combine(destFolderSubdir, fileName);
 
                             try
                             {
@@ -480,6 +514,9 @@ private static void CopyUnityProjectTools(string destToolsDir)
                     }
                 }
 
+                // Clean up stale subdirectories (folders removed from upstream)
+                CleanupStaleToolFolders(destToolsDir, activeFolderIdentifiers);
+
                 if (copiedCount > 0)
                 {
                     McpLog.Info($"Copied {copiedCount} Unity project tool(s) to server");
@@ -491,6 +528,43 @@ private static void CopyUnityProjectTools(string destToolsDir)
             }
         }
 
+        /// <summary>
+        /// Removes stale tool subdirectories that are no longer present in the Unity project.
+        /// </summary>
+        private static void CleanupStaleToolFolders(string destToolsDir, HashSet<string> activeFolderIdentifiers)
+        {
+            try
+            {
+                if (!Directory.Exists(destToolsDir)) return;
+
+                // Get all subdirectories in destToolsDir
+                var existingSubdirs = Directory.GetDirectories(destToolsDir);
+
+                foreach (var subdir in existingSubdirs)
+                {
+                    string subdirName = Path.GetFileName(subdir);
+
+                    // Check if this subdirectory corresponds to an active tools folder
+                    if (!activeFolderIdentifiers.Contains(subdirName))
+                    {
+                        try
+                        {
+                            Directory.Delete(subdir, recursive: true);
+                            McpLog.Info($"Cleaned up stale tools folder: {subdirName}");
+                        }
+                        catch (Exception ex)
+                        {
+                            McpLog.Warn($"Failed to delete stale folder {subdirName}: {ex.Message}");
+                        }
+                    }
+                }
+            }
+            catch (Exception ex)
+            {
+                McpLog.Warn($"Failed to cleanup stale tool folders: {ex.Message}");
+            }
+        }
+
         /// <summary>
         /// Generates a unique identifier for a MCPForUnityTools folder based on its parent directory.
         /// Example: "Assets/MooseRunner/Editor/MCPForUnityTools" → "MooseRunner_MCPForUnityTools"

MCPForUnity/UnityMcpServer~/src/tools/__init__.py

@@ -21,13 +21,14 @@ def register_all_tools(mcp: FastMCP):
     """
     Auto-discover and register all tools in the tools/ directory.
 
-    Any .py file in this directory with @mcp_for_unity_tool decorated
+    Any .py file in this directory or subdirectories with @mcp_for_unity_tool decorated
     functions will be automatically registered.
     """
     logger.info("Auto-discovering MCP for Unity Server tools...")
     # Dynamic import of all modules in this directory
     tools_dir = Path(__file__).parent
 
+    # Discover modules in the top level
     for _, module_name, _ in pkgutil.iter_modules([str(tools_dir)]):
         # Skip private modules and __init__
         if module_name.startswith('_'):
@@ -38,6 +39,24 @@ def register_all_tools(mcp: FastMCP):
         except Exception as e:
             logger.warning(f"Failed to import tool module {module_name}: {e}")
 
+    # Discover modules in subdirectories (one level deep)
+    for subdir in tools_dir.iterdir():
+        if not subdir.is_dir() or subdir.name.startswith('_') or subdir.name.startswith('.'):
+            continue
+
+        # Check if subdirectory contains Python modules
+        for _, module_name, _ in pkgutil.iter_modules([str(subdir)]):
+            # Skip private modules and __init__
+            if module_name.startswith('_'):
+                continue
+
+            try:
+                # Import as tools.subdirname.modulename
+                full_module_name = f'.{subdir.name}.{module_name}'
+                importlib.import_module(full_module_name, __package__)
+            except Exception as e:
+                logger.warning(f"Failed to import tool module {subdir.name}.{module_name}: {e}")
+
     tools = get_registered_tools()
 
     if not tools: