feat: Allow duplicate and empty server prefixes, optimize config loading

Major changes:
- Allow duplicate prefixes across servers (including None/empty prefixes)
- Remove duplicate prefix validation from config reload
- Optimize config loading to use cached version when reload is enabled
- Make None the default prefix instead of auto-generating from server name

Key improvements:
- ServerConfig prefix field changed from str with default "" to str|None with default None
- ConfigManager.load_config() now returns cached config from ReloadManager when available
- Added cached_config property to ReloadManager for cleaner access
- Fixed self_prefix_ property to handle None prefixes properly
- Removed _unmount_from_fastmcp workaround implementation for FastMCP servers

Test updates:
- Updated tests to expect None as default prefix instead of auto-generated values
- Changed duplicate prefix validation test to verify duplicates are allowed
- Added test cases for duplicate None prefixes

Bug fixes:
- Fixed MAGG_DEBUG environment variable check to handle missing values
- Properly close FastMCP client connections on unmount to prevent pending tasks

This change makes server prefixes truly optional and allows multiple servers
to share the same prefix or have no prefix at all, providing more flexibility
in tool naming.

Signed-off-by: Phillip Sitbon <phillip.sitbon@gmail.com>

Author: sitbon

magg/cli.py

@@ -554,7 +554,7 @@ def main():
         sys.exit(130)  # Standard exit code for SIGINT
     except Exception as e:
         print_error(f"Unexpected error: {e}")
-        if os.getenv('MAGG_DEBUG').lower() in {'1', 'true', 'yes'}:
+        if os.getenv('MAGG_DEBUG', '').lower() in {'1', 'true', 'yes'}:
             import traceback
             traceback.print_exc()
         sys.exit(1)

magg/reload.py

@@ -187,7 +187,14 @@ def update_cached_config(self, config: MaggConfig) -> None:
         This keeps our internal state in sync when config is saved programmatically.
         """
         self._last_config = config
-        logger.debug("Updated cached config after programmatic save")
+    
+    def get_cached_config(self) -> MaggConfig | None:
+        """Get the cached configuration if available.
+        
+        Returns:
+            The cached config or None if no config has been loaded yet.
+        """
+        return self._last_config
 
     async def _watch_loop(self, poll_interval: float) -> None:
         """Main watch loop that handles both watchdog and polling modes."""
@@ -392,17 +399,6 @@ def _server_config_changed(self, old: ServerConfig, new: ServerConfig) -> bool:
     def _validate_config(self, config: MaggConfig) -> bool:
         """Validate that the configuration is valid."""
         try:
-            # Check for duplicate prefixes
-            prefixes = {}
-            for name, server in config.servers.items():
-                if server.prefix in prefixes:
-                    logger.error(
-                        "Duplicate prefix '%s' found in servers '%s' and '%s'",
-                        server.prefix, name, prefixes[server.prefix]
-                    )
-                    return False
-                prefixes[server.prefix] = name
-
             # Validate each server
             for name, server in config.servers.items():
                 if not server.command and not server.uri:
@@ -429,6 +425,13 @@ def __init__(self, config_manager: 'ConfigManager'):
         self.config_manager: ConfigManager = config_manager
         self._config_reloader: ConfigReloader | None = None
         self._reload_callback: ReloadCallback | None = None
+    
+    @property
+    def cached_config(self) -> MaggConfig | None:
+        """Get the cached configuration if available."""
+        if self._config_reloader:
+            return self._config_reloader._last_config
+        return None
 
     async def setup(self, reload_callback: ReloadCallback) -> None:
         """Setup config file watching with a callback.

magg/server/manager.py

@@ -112,35 +112,70 @@ async def mount_server(self, server: ServerConfig) -> bool:
                 logger.error("No command or URI specified for %s", server.name)
                 return False
 
-            # Create proxy and mount
-            proxy_server = FastMCP.as_proxy(client)
-            self.mcp.mount(server=proxy_server, prefix=server.prefix, as_proxy=True)
-            # Store both proxy and client for resource/prompt access
+            proxy_server = FastMCP.as_proxy(client, name=server.name)
+            self.mcp.mount(server=proxy_server, prefix=server.prefix)
+
             self.mounted_servers[server.name] = {
                 'proxy': proxy_server,
                 'client': client
             }
 
-            logger.debug("Mounted server %s with prefix %s", server.name, server.prefix)
+            logger.debug("Mounted server %s with prefix %r", server.name, server.prefix)
             return True
 
         except Exception as e:
             logger.error("Failed to mount server %s: %s", server.name, e)
             return False
 
     # noinspection PyProtectedMember
+    def _unmount_from_fastmcp(self, server_name: str) -> bool:
+        """Remove a mounted server from FastMCP's internal structures.
+        
+        This is a workaround until FastMCP provides an official unmount method.
+        Returns True if server was found and removed.
+        """
+        # We need to find and remove the MountedServer object from all managers
+        found = False
+        
+        # Check tool manager
+        if hasattr(self.mcp, '_tool_manager') and hasattr(self.mcp._tool_manager, '_mounted_servers'):
+            mounted_servers = self.mcp._tool_manager._mounted_servers
+            for i, ms in enumerate(mounted_servers):
+                if hasattr(ms, 'server') and hasattr(ms.server, 'name') and ms.server.name == server_name:
+                    mounted_servers.pop(i)
+                    found = True
+                    logger.debug("Removed server %s from tool manager", server_name)
+                    break
+        
+        # Check resource manager
+        if hasattr(self.mcp, '_resource_manager') and hasattr(self.mcp._resource_manager, '_mounted_servers'):
+            mounted_servers = self.mcp._resource_manager._mounted_servers
+            for i, ms in enumerate(mounted_servers):
+                if hasattr(ms, 'server') and hasattr(ms.server, 'name') and ms.server.name == server_name:
+                    mounted_servers.pop(i)
+                    logger.debug("Removed server %s from resource manager", server_name)
+                    break
+                    
+        # Check prompt manager
+        if hasattr(self.mcp, '_prompt_manager') and hasattr(self.mcp._prompt_manager, '_mounted_servers'):
+            mounted_servers = self.mcp._prompt_manager._mounted_servers
+            for i, ms in enumerate(mounted_servers):
+                if hasattr(ms, 'server') and hasattr(ms.server, 'name') and ms.server.name == server_name:
+                    mounted_servers.pop(i)
+                    logger.debug("Removed server %s from prompt manager", server_name)
+                    break
+                    
+        return found
+
     async def unmount_server(self, name: str) -> bool:
         """Unmount a server."""
         if name in self.mounted_servers:
-            # Get the server config to find the prefix
-            config = self.config
-            server = config.servers.get(name)
-            if server and server.prefix in self.mcp._tool_manager._mounted_servers:
-                # TODO: FastMCP doesn't have an unmount method yet
-                # For now, we just close the client and remove from our tracking
-                logger.debug("Would unmount prefix %s (FastMCP unmount not available)", server.prefix)
+            unmounted = self._unmount_from_fastmcp(name)
+            if unmounted:
+                logger.info("Unmounted server %s from FastMCP", name)
+            else:
+                logger.debug("Server %s was not found in FastMCP's mounted servers", name)
 
-            # Close the client to clean up background tasks
             server_info = self.mounted_servers.get(name, {})
             client: Client = server_info.get('client')
             if client:
@@ -150,7 +185,6 @@ async def unmount_server(self, name: str) -> bool:
                 except Exception as e:
                     logger.warning("Error closing client for server %s: %s", name, e)
 
-            # Remove from our tracking
             del self.mounted_servers[name]
             logger.info("Unmounted server %s", name)
             return True
@@ -320,9 +354,12 @@ def self_prefix(self) -> str:
 
     @cached_property
     def self_prefix_(self) -> str:
-        """self_prefix with trailing separator.
+        """self_prefix with trailing separator if prefix exists.
         """
-        return f"{self.self_prefix}{self.server_manager.prefix_separator}"
+        prefix = self.self_prefix
+        if prefix:
+            return f"{prefix}{self.server_manager.prefix_separator}"
+        return ""
 
     def _register_tools(self):
         pass

magg/server/server.py

@@ -289,7 +289,7 @@ async def add_server(
                 server = ServerConfig(
                     name=name,
                     source=source,
-                    prefix=prefix or "",
+                    prefix=prefix,
                     command=actual_command,
                     args=actual_args,
                     uri=uri,
@@ -739,7 +739,7 @@ async def status(self) -> MaggResponse:
                 "tools": {
                     "total": total_tools,
                 },
-                "prefixes": {s.name: s.prefix for s in config.servers.values()}
+                "prefixes": {s.name: s.prefix for s in config.servers.values() if s.prefix is not None}
             }
 
             return MaggResponse.success(status_data)

magg/settings.py

@@ -118,8 +118,8 @@ class ServerConfig(BaseSettings):
 
     name: str = Field(..., description="Unique server name - can contain any characters")
     source: str = Field(..., description="URL/URI/path of the server package, repository, or listing")
-    prefix: str = Field(
-        default="",
+    prefix: str | None = Field(
+        default=None,
         description=f"Tool prefix for this server - must be a valid Python identifier without {PREFIX_SEP!r}."
     )
     notes: str | None = Field(None, description="Setup notes for LLM and humans")
@@ -136,9 +136,7 @@ class ServerConfig(BaseSettings):
 
     @model_validator(mode='after')
     def set_default_prefix(self) -> 'ServerConfig':
-        """Set default prefix from name if not provided."""
-        if not self.prefix:
-            self.prefix = self.generate_prefix_from_name(self.name)
+        """No longer set default prefix - None is allowed."""
         return self
 
     @field_validator('prefix')
@@ -286,10 +284,16 @@ def logger(self) -> logging.Logger:
         return logging.getLogger(__name__)
 
     def load_config(self) -> MaggConfig:
-        """Load configuration from disk.
+        """Load configuration from disk or return cached version if reload is enabled.
 
         Note: The only dynamic part of the config is the servers.
         """
+        # If reload manager is active and has a cached config, return it
+        if self._reload_manager:
+            cached = self._reload_manager.cached_config
+            if cached:
+                return cached
+        
         config = MaggConfig()
 
         if not self.config_path.exists():

magg/test/test_config.py

@@ -39,26 +39,26 @@ def test_server_with_custom_prefix(self):
         assert server.prefix == "custom"
 
     def test_server_name_validation(self):
-        """Test that server names can be anything and prefix generation works."""
-        # Any name is now valid - prefix auto-generated from name
+        """Test that server names can be anything and prefix defaults to None."""
+        # Any name is now valid - prefix defaults to None
         server1 = ServerConfig(name="valid", source="test")
-        assert server1.prefix == "valid"
+        assert server1.prefix is None
 
         server2 = ServerConfig(name="valid123", source="test")
-        assert server2.prefix == "valid123"
+        assert server2.prefix is None
 
         server3 = ServerConfig(name="valid-name", source="test")
-        assert server3.prefix == "validname"  # Hyphens removed from prefix
+        assert server3.prefix is None
 
         # Names that would have been invalid before are now accepted
         server4 = ServerConfig(name="123invalid", source="test")
-        assert server4.prefix == "srv123invalid"  # Prefix adjusted to be valid
+        assert server4.prefix is None
 
         server5 = ServerConfig(name="invalid!", source="test")
-        assert server5.prefix == "invalid"  # Special chars removed
+        assert server5.prefix is None
 
         server6 = ServerConfig(name="@namespace/package", source="test")
-        assert server6.prefix == "namespacepackage"  # Cleaned up
+        assert server6.prefix is None
 
     def test_server_prefix_validation(self):
         """Test server prefix validation."""

magg/test/test_config_migration.py

@@ -57,7 +57,7 @@ def test_new_config_structure(self, temp_config_file):
 
         filesystem = loaded_config.servers["filesystemserver"]
         assert filesystem.uri == "http://localhost:8080"
-        assert filesystem.prefix == "filesystemserver"  # Default prefix
+        assert filesystem.prefix is None  # Default is None now
 
     def test_config_serialization_format(self, temp_config_file):
         """Test the actual JSON format of saved config."""
@@ -119,7 +119,7 @@ def test_minimal_server_config(self, temp_config_file):
 
         assert minimal.name == "minimal"
         assert minimal.source == "https://example.com"
-        assert minimal.prefix == "minimal"  # Defaults to name
+        assert minimal.prefix is None  # Default is None now
         assert minimal.enabled is True  # Default enabled
         assert minimal.command is None
         assert minimal.args is None
@@ -167,6 +167,6 @@ def test_invalid_server_in_config(self, temp_config_file):
         assert "valid" in config.servers
         assert "123invalid" in config.servers
 
-        # Check that the problematic server got a valid prefix
+        # Check that the problematic server now has None prefix
         assert config.servers["123invalid"].name == "123invalid"
-        assert config.servers["123invalid"].prefix == "srv123invalid"  # Auto-generated prefix
+        assert config.servers["123invalid"].prefix is None  # Default is None now

magg/test/test_config_reload.py

@@ -127,7 +127,7 @@ async def test_detect_changes_enable_disable(self):
 
     @pytest.mark.asyncio
     async def test_config_validation(self):
-        """Test config validation catches duplicate prefixes."""
+        """Test config validation allows duplicate prefixes."""
         config = MaggConfig()
         config.servers["server1"] = ServerConfig(
             name="server1",
@@ -139,11 +139,26 @@ async def test_config_validation(self):
             name="server2",
             source="https://example.com/2",
             command="echo",
-            prefix="test"  # Duplicate prefix
+            prefix="test"  # Duplicate prefix is now allowed
         )
 
         reloader = ConfigReloader(Path("/fake/path"), lambda x: None)
-        assert not reloader._validate_config(config)
+        assert reloader._validate_config(config)  # Should pass validation
+        
+        # Test with None/empty prefixes
+        config.servers["server3"] = ServerConfig(
+            name="server3",
+            source="https://example.com/3",
+            command="echo",
+            prefix=None
+        )
+        config.servers["server4"] = ServerConfig(
+            name="server4",
+            source="https://example.com/4",
+            command="echo",
+            prefix=None  # Duplicate None prefix is also allowed
+        )
+        assert reloader._validate_config(config)  # Should still pass
 
     @pytest.mark.asyncio
     async def test_reload_callback(self, temp_config_file):

magg/test/test_tool_delegation.py

@@ -55,13 +55,13 @@ def test_tool_prefix_handling(self):
         # Test prefix validation
         assert server.prefix == "custom"
 
-        # Test that default prefix uses cleaned name
+        # Test that default prefix is now None
         server2 = ServerConfig(
             name="test-server",
             source="https://example.com",
             command="echo"
         )
-        assert server2.prefix == "testserver"  # Hyphens removed
+        assert server2.prefix is None  # Default is None now
 
     def test_tool_name_collision_handling(self):
         """Test handling of tool name collisions."""