From c41646b190c1f0170d649fcd997ec53fb8d1f520 Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Fri, 25 Sep 2026 19:55:26 +0000
Subject: [PATCH 01/14] Add backend_init_timeout setting

---
 magg/settings.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/magg/settings.py b/magg/settings.py
index 903a239..72f3c59 100644
--- a/magg/settings.py
+++ b/magg/settings.py
@@ -213,6 +213,11 @@ class MaggConfig(BaseSettings):
     stderr_show: bool = Field(
         default=False, description="Show stderr output from subprocess MCP servers (env: MAGG_STDERR_SHOW)"
     )
+    backend_init_timeout: float = Field(
+        default=30.0,
+        description="Seconds to wait for a backend server to connect and initialize, 0 to wait forever "
+        "(env: MAGG_BACKEND_INIT_TIMEOUT)",
+    )
     servers: dict[str, ServerConfig] = Field(
         default_factory=dict, description="Servers configuration (loaded from config_path)"
     )

From 0b961e4e596d8c486b80c5d37f37e4606d4ea2b8 Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Fri, 25 Sep 2026 20:01:54 +0000
Subject: [PATCH 02/14] Stop config saves from silently deleting user data

- A config.json that fails to parse is no longer overwritten by the next
  save; the save is refused and the file left as-is. An empty file is
  still treated as new.
- Server entries that fail validation, and top-level settings like
  self_prefix, are written back unchanged instead of being dropped.
- The reloader now reads the file with the same loader as
  ConfigManager, so its cached config includes kits and settings. With
  auto-reload on (the default), the first save from a running server
  used to erase the kits section.
- ServerConfig, KitConfig, AuthConfig and BearerAuthConfig are plain
  models. As BaseSettings with no env prefix they read unrelated env
  vars like PREFIX (set by Termux), URI, ENV and KEY_PATH, which made
  valid servers fail to load and then get dropped on save.
- Config and auth.json writes go through a temp file and rename, keeping
  the existing file's mode and following symlinks.
- The watchdog handler also reacts to files created or renamed over the
  config, which is how many editors and tools save.
- A disabled server without a command or uri no longer blocks every
  subsequent reload.
---
 magg/kit.py                     |   7 +-
 magg/reload.py                  |  64 +++++----
 magg/settings.py                | 229 +++++++++++++++++++++---------
 test/magg/test_config_safety.py | 242 ++++++++++++++++++++++++++++++++
 4 files changed, 438 insertions(+), 104 deletions(-)
 create mode 100644 test/magg/test_config_safety.py

diff --git a/magg/kit.py b/magg/kit.py
index 819fa42..32cebae 100644
--- a/magg/kit.py
+++ b/magg/kit.py
@@ -5,18 +5,17 @@
 from pathlib import Path
 from typing import Any
 
-from pydantic import Field, field_validator
-from pydantic_settings import BaseSettings, SettingsConfigDict
+from pydantic import BaseModel, ConfigDict, Field, field_validator
 
 from .settings import ConfigManager, KitInfo, MaggConfig, ServerConfig
 
 logger = logging.getLogger(__name__)
 
 
-class KitConfig(BaseSettings):
+class KitConfig(BaseModel):
     """Configuration for a kit - a bundle of related MCP servers."""
 
-    model_config = SettingsConfigDict(
+    model_config = ConfigDict(
         extra="allow",
         validate_assignment=True,
     )
diff --git a/magg/reload.py b/magg/reload.py
index ffb5601..4a12e23 100644
--- a/magg/reload.py
+++ b/magg/reload.py
@@ -3,19 +3,16 @@
 from __future__ import annotations
 
 import asyncio
-import json
 import logging
+import os
 from dataclasses import dataclass, field
 from pathlib import Path
-from typing import TYPE_CHECKING, Callable, Coroutine, TypeAlias
+from typing import Callable, Coroutine, TypeAlias
 
 from watchdog.events import FileSystemEventHandler
 from watchdog.observers import Observer
 
-from .settings import MaggConfig, ServerConfig
-
-if TYPE_CHECKING:
-    from .settings import ConfigManager
+from .settings import ConfigFileError, ConfigManager, MaggConfig, ServerConfig
 
 logger = logging.getLogger(__name__)
 
@@ -78,25 +75,41 @@ def __init__(self, config_path: Path, reload_event: asyncio.Event):
             # Not in an async context, get the event loop
             self._loop = asyncio.get_event_loop()
 
-    def on_modified(self, event):
-        """Handle file modification events."""
-        if not event.is_directory and Path(event.src_path) == self.config_path:
+    def _notify(self, path: str | bytes) -> None:
+        if Path(os.fsdecode(path)) == self.config_path:
             # Schedule the reload event in the asyncio loop
             self._loop.call_soon_threadsafe(self.reload_event.set)
 
+    def on_modified(self, event):
+        """Handle file modification events."""
+        if not event.is_directory:
+            self._notify(event.src_path)
+
+    def on_created(self, event):
+        """Handle file creation, e.g. an editor writing a new copy of the file."""
+        if not event.is_directory:
+            self._notify(event.src_path)
+
+    def on_moved(self, event):
+        """Handle a file renamed over the config, as editors and atomic writes do."""
+        if not event.is_directory:
+            self._notify(event.dest_path)
+
 
 class ConfigReloader:
     """Manages configuration reloading with file watching and diff detection."""
 
-    def __init__(self, config_path: Path, reload_callback: ReloadCallback):
+    def __init__(self, config_path: Path, reload_callback: ReloadCallback, config_manager: ConfigManager | None = None):
         """Initialize the config reloader.
 
         Args:
             config_path: Path to the configuration file
             reload_callback: Async callback to handle config changes
+            config_manager: Manager used to read the config file (default: one for config_path)
         """
         self.config_path = config_path
         self.reload_callback = reload_callback
+        self._config_manager = config_manager or ConfigManager(config_path)
         self._last_mtime: float | None = None
         self._last_config: MaggConfig | None = None
         self._watch_task: asyncio.Task | None = None
@@ -295,24 +308,9 @@ async def reload_config(self) -> ConfigChange | None:
     def _load_config(self) -> MaggConfig | None:
         """Load configuration from disk."""
         try:
-            with self.config_path.open("r") as f:
-                data = json.load(f)
-
-            servers = {}
-            for name, server_data in data.get("servers", {}).items():
-                try:
-                    server_data["name"] = name
-                    servers[name] = ServerConfig.model_validate(server_data)
-                except Exception as e:
-                    logger.error("Error loading server '%s': %s", name, e)
-                    continue
-
-            config = MaggConfig()
-            config.servers = servers
-            return config
-
-        except Exception as e:
-            logger.error("Error loading config file: %s", e)
+            return self._config_manager.read_config()
+        except ConfigFileError as e:
+            logger.error("%s", e)
             return None
 
     def _detect_changes(self, old_config: MaggConfig, new_config: MaggConfig) -> ConfigChange:
@@ -367,7 +365,7 @@ def _validate_config(self, config: MaggConfig) -> bool:
         """Validate that the configuration is valid."""
         try:
             for name, server in config.servers.items():
-                if not server.command and not server.uri:
+                if server.enabled and not server.command and not server.uri:
                     logger.error("Server '%s' has neither command nor uri", name)
                     return False
 
@@ -410,7 +408,9 @@ async def setup(self, reload_callback: ReloadCallback) -> None:
         if config.auto_reload and not self._config_reloader:
             if self.config_manager.config_path.exists():
                 self._config_reloader = ConfigReloader(
-                    config_path=self.config_manager.config_path, reload_callback=reload_callback
+                    config_path=self.config_manager.config_path,
+                    reload_callback=reload_callback,
+                    config_manager=self.config_manager,
                 )
                 await self._config_reloader.start_watching(poll_interval=config.reload_poll_interval)
 
@@ -436,7 +436,9 @@ async def reload(self) -> bool:
                 return False
 
             reloader = ConfigReloader(
-                config_path=self.config_manager.config_path, reload_callback=self._reload_callback
+                config_path=self.config_manager.config_path,
+                reload_callback=self._reload_callback,
+                config_manager=self.config_manager,
             )
             change = await reloader.reload_config()
             return change is not None
diff --git a/magg/settings.py b/magg/settings.py
index 72f3c59..7cb23a4 100644
--- a/magg/settings.py
+++ b/magg/settings.py
@@ -3,6 +3,8 @@
 import json
 import logging
 import os
+import stat
+import tempfile
 from functools import cached_property
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, Callable, Coroutine
@@ -10,13 +12,22 @@
 if TYPE_CHECKING:
     from .reload import ConfigChange
 
-from pydantic import AnyUrl, BaseModel, Field, field_validator, model_validator
+from pydantic import AnyUrl, BaseModel, ConfigDict, Field, field_validator, model_validator
 from pydantic_settings import BaseSettings, SettingsConfigDict
 
 from .util.paths import get_contrib_paths
 from .util.system import get_project_root
 
-__all__ = "ServerConfig", "MaggConfig", "ConfigManager", "AuthConfig", "BearerAuthConfig", "ClientSettings", "KitInfo"
+__all__ = (
+    "ServerConfig",
+    "MaggConfig",
+    "ConfigManager",
+    "ConfigFileError",
+    "AuthConfig",
+    "BearerAuthConfig",
+    "ClientSettings",
+    "KitInfo",
+)
 
 logger = logging.getLogger(__name__)
 
@@ -48,10 +59,10 @@ class ClientSettings(BaseSettings):
     jwt: str | None = Field(default=None, description="JWT token for authentication (env: MAGG_JWT)")
 
 
-class BearerAuthConfig(BaseSettings):
+class BearerAuthConfig(BaseModel):
     """Bearer token authentication configuration."""
 
-    model_config = SettingsConfigDict(
+    model_config = ConfigDict(
         extra="allow",
         validate_assignment=True,
     )
@@ -108,20 +119,20 @@ def public_key_exists(self) -> bool:
         return self.public_key_path.exists()
 
 
-class AuthConfig(BaseSettings):
+class AuthConfig(BaseModel):
     """Top-level authentication configuration."""
 
-    model_config = SettingsConfigDict(
+    model_config = ConfigDict(
         extra="allow",
         validate_assignment=True,
     )
     bearer: BearerAuthConfig = Field(default_factory=BearerAuthConfig, description="Bearer token authentication config")
 
 
-class ServerConfig(BaseSettings):
+class ServerConfig(BaseModel):
     """Server configuration - defines how to run an MCP server."""
 
-    model_config = SettingsConfigDict(
+    model_config = ConfigDict(
         extra="allow",
         validate_assignment=True,
         arbitrary_types_allowed=True,
@@ -310,6 +321,44 @@ def get_enabled_servers(self) -> dict[str, ServerConfig]:
         return {name: server for name, server in self.servers.items() if server.enabled}
 
 
+class ConfigFileError(ValueError):
+    """The config file exists but couldn't be read or parsed."""
+
+
+def _is_valid_server(name: str, server_data: Any) -> bool:
+    try:
+        ServerConfig.model_validate({**server_data, "name": name})
+        return True
+    except Exception:
+        return False
+
+
+def _write_json_atomic(path: Path, data: Any) -> None:
+    """Write JSON through a temp file and rename, so a crash can't leave a truncated file."""
+    target = Path(os.path.realpath(path))  # Replace a symlink's target, not the link
+    fd, tmp_path = tempfile.mkstemp(dir=target.parent, prefix=f".{target.name}.", suffix=".tmp")
+
+    try:
+        with os.fdopen(fd, "w") as f:
+            json.dump(data, f, indent=2)
+            f.flush()
+            os.fsync(f.fileno())
+
+        try:
+            os.chmod(tmp_path, stat.S_IMODE(target.stat().st_mode))
+        except FileNotFoundError:
+            pass  # New file: keep mkstemp's owner-only permissions
+
+        os.replace(tmp_path, target)
+
+    except BaseException:
+        try:
+            os.unlink(tmp_path)
+        except OSError:
+            pass
+        raise
+
+
 class ConfigManager:
     """Manages Magg configuration persistence."""
 
@@ -346,94 +395,137 @@ def load_config(self) -> MaggConfig:
             if cached:
                 return cached
 
+        try:
+            return self.read_config()
+        except ConfigFileError as e:
+            self.logger.error("%s", e)
+            return MaggConfig()
+
+    def read_config(self) -> MaggConfig:
+        """Read configuration from disk, bypassing the reload cache.
+
+        Entries that fail validation are logged and skipped. Raises ConfigFileError
+        if the file exists but can't be parsed.
+        """
         config = MaggConfig()
+        data = self._read_config_data()
 
-        if not self.config_path.exists():
+        if data is None:
             return config
 
-        try:
-            with self.config_path.open("r") as f:
-                data = json.load(f)
+        servers = {}
 
-            servers = {}
-
-            for name, server_data in data.pop("servers", {}).items():
-                try:
-                    server_data["name"] = name
-                    servers[name] = ServerConfig.model_validate(server_data)
-                except Exception as e:
-                    self.logger.error("Error loading server %r: %s", name, e)
-                    continue
-
-            config.servers = servers
-
-            if "kits" in data:
-                # Handle both old format (list of strings) and new format (dict)
-                kits_data = data.pop("kits", {})
-                if isinstance(kits_data, list):
-                    # Convert old format to new format
-                    config.kits = {kit_name: KitInfo(name=kit_name, source="legacy") for kit_name in kits_data}
-                else:
-                    # Load new format
-                    config.kits = {
-                        name: KitInfo.model_validate(kit_data) if isinstance(kit_data, dict) else KitInfo(name=name)
-                        for name, kit_data in kits_data.items()
-                    }
-
-            for key, value in data.items():
-                if not hasattr(config, key):
-                    self.logger.warning("Setting unknown config key %r in %s.", key, self.config_path)
+        for name, server_data in data.pop("servers", {}).items():
+            try:
+                servers[name] = ServerConfig.model_validate({**server_data, "name": name})
+            except Exception as e:
+                self.logger.error("Error loading server %r: %s", name, e)
+
+        config.servers = servers
+
+        # Handle both old format (list of strings) and new format (dict)
+        kits_data = data.pop("kits", {})
+        if isinstance(kits_data, list):
+            kits_data = {kit_name: {"name": kit_name, "source": "legacy"} for kit_name in kits_data}
+
+        kits = {}
+
+        for name, kit_data in kits_data.items():
+            try:
+                kits[name] = KitInfo.model_validate(kit_data) if isinstance(kit_data, dict) else KitInfo(name=name)
+            except Exception as e:
+                self.logger.error("Error loading kit %r: %s", name, e)
+
+        config.kits = kits
+
+        for key, value in data.items():
+            if not hasattr(config, key):
+                self.logger.warning("Setting unknown config key %r in %s.", key, self.config_path)
+            try:
                 setattr(config, key, value)
+            except Exception as e:
+                self.logger.error("Error loading config key %r: %s", key, e)
 
-            return config
+        return config
 
-        except Exception as e:
-            self.logger.error("Error loading config: %s", e)
-            return config
+    def _read_config_data(self) -> dict | None:
+        """Read the raw config file, or return None if it doesn't exist."""
+        try:
+            text = self.config_path.read_text()
+            if not text.strip():
+                return None
+            data = json.loads(text)
+        except FileNotFoundError:
+            return None
+        except (OSError, ValueError) as e:
+            raise ConfigFileError(f"Error loading config {self.config_path}: {e}") from e
+
+        if (
+            not isinstance(data, dict)
+            or not isinstance(data.get("servers", {}), dict)
+            or not isinstance(data.get("kits", {}), dict | list)
+        ):
+            raise ConfigFileError(
+                f'Error loading config {self.config_path}: expected a JSON object with a "servers" object'
+            )
+
+        return data
 
     def save_config(self, config: MaggConfig) -> bool:
-        """Save configuration to disk."""
-        if config.read_only:
+        """Save configuration to disk.
+
+        Settings and server entries in the file that couldn't be loaded are written back
+        unchanged, and a file that can't be parsed is never overwritten.
+        """
+        if config.read_only or self.read_only:
             self.logger.warning("Config is read-only, not saving.")
             return False
 
-        if self.read_only:
-            raise RuntimeError("Config read_only value cannot be changed after initialization.")
-
         try:
-            # Notify the reloader to ignore the next file change since we're making it
-            if self._reload_manager:
-                self._reload_manager.ignore_next_change()
+            data = self._read_config_data() or {}
+        except ConfigFileError as e:
+            self.logger.error("%s - not saving to avoid losing its contents. Fix or remove the file first.", e)
+            return False
 
-            data = {
-                "servers": {
-                    name: server.model_dump(
-                        mode="json",
-                        exclude_unset=True,
-                        exclude_none=True,
-                        exclude_defaults=True,
-                        by_alias=True,
-                        exclude={"name"},
-                    )
-                    for name, server in config.servers.items()
-                }
+        try:
+            existing_servers = data.get("servers", {})
+
+            data["servers"] = {
+                name: server.model_dump(
+                    mode="json",
+                    exclude_unset=True,
+                    exclude_none=True,
+                    exclude_defaults=True,
+                    by_alias=True,
+                    exclude={"name"},
+                )
+                for name, server in config.servers.items()
             }
 
+            # An entry that failed validation was never loaded, so it isn't missing because it was removed
+            for name, server_data in existing_servers.items():
+                if name not in data["servers"] and not _is_valid_server(name, server_data):
+                    self.logger.warning("Keeping invalid server %r in %s as-is", name, self.config_path)
+                    data["servers"][name] = server_data
+
             if config.kits:
                 data["kits"] = {
                     name: kit_info.model_dump(mode="json", exclude_unset=True, exclude_none=True, exclude_defaults=True)
                     for name, kit_info in config.kits.items()
                 }
+            else:
+                data.pop("kits", None)
 
             if not self.config_path.parent.exists():
                 self.logger.warning("Creating new directory: %s", self.config_path.parent)
                 self.config_path.parent.mkdir(parents=True, exist_ok=True)
 
-            with self.config_path.open("w") as f:
-                json.dump(data, f, indent=2)
+            _write_json_atomic(self.config_path, data)
 
-            # Update the reload manager's cached config to stay in sync
+            # Notify the reloader to ignore the change we just made
             if self._reload_manager:
+                self._reload_manager.ignore_next_change()
+                # Update the reload manager's cached config to stay in sync
                 self._reload_manager.update_cached_config(config)
 
             return True
@@ -509,8 +601,7 @@ def save_auth_config(self, auth_config: AuthConfig) -> bool:
 
             data = auth_config.model_dump(mode="json", exclude_none=True)
 
-            with self.auth_config_path.open("w") as f:
-                json.dump(data, f, indent=2)
+            _write_json_atomic(self.auth_config_path, data)
 
             self.auth_config = auth_config
             return True
diff --git a/test/magg/test_config_safety.py b/test/magg/test_config_safety.py
new file mode 100644
index 0000000..a7e0cff
--- /dev/null
+++ b/test/magg/test_config_safety.py
@@ -0,0 +1,242 @@
+"""Tests that saving the config never loses data the user wrote."""
+
+import asyncio
+import json
+import os
+import stat
+
+import pytest
+from watchdog.events import FileMovedEvent
+
+from magg.reload import ConfigReloader, WatchdogHandler
+from magg.settings import BearerAuthConfig, ConfigFileError, ConfigManager, MaggConfig, ServerConfig
+
+
+@pytest.fixture
+def config_path(tmp_path):
+    return tmp_path / "config.json"
+
+
+def write_config(path, data):
+    path.write_text(json.dumps(data, indent=2))
+
+
+def read_config(path):
+    return json.loads(path.read_text())
+
+
+class TestEnvironmentIsolation:
+    """Nested config models must not read unrelated environment variables."""
+
+    def test_server_config_ignores_env(self, monkeypatch):
+        # Termux sets PREFIX=/usr, POSIX shells use ENV, and URI/ENABLED are common names
+        monkeypatch.setenv("PREFIX", "/usr")
+        monkeypatch.setenv("URI", "http://evil.example/mcp")
+        monkeypatch.setenv("ENABLED", "false")
+        monkeypatch.setenv("ENV", "/home/user/.shrc")
+
+        server = ServerConfig.model_validate({"name": "calc", "source": "https://x", "command": "echo"})
+
+        assert server.prefix is None
+        assert server.uri is None
+        assert server.enabled is True
+        assert server.env is None
+
+    def test_bearer_auth_config_ignores_env(self, monkeypatch, tmp_path):
+        monkeypatch.setenv("AUDIENCE", "other")
+        monkeypatch.setenv("KEY_PATH", str(tmp_path))
+
+        config = BearerAuthConfig()
+
+        assert config.audience == "magg"
+        assert config.key_path != tmp_path
+
+
+class TestSaveConfig:
+    """ConfigManager.save_config preserves what it didn't load."""
+
+    def test_refuses_to_overwrite_unparseable_file(self, config_path):
+        original = '{"servers": {"a": {"source": "s", "command": "echo"}},}'
+        config_path.write_text(original)
+        manager = ConfigManager(str(config_path))
+
+        config = manager.load_config()
+        assert config.servers == {}
+
+        config.add_server(ServerConfig(name="b", source="s", command="echo"))
+        assert manager.save_config(config) is False
+        assert config_path.read_text() == original
+
+    def test_read_config_raises_on_unparseable_file(self, config_path):
+        config_path.write_text("[1, 2, 3]")
+
+        with pytest.raises(ConfigFileError):
+            ConfigManager(str(config_path)).read_config()
+
+    def test_empty_file_is_treated_as_new(self, config_path):
+        config_path.write_text("")
+        manager = ConfigManager(str(config_path))
+
+        config = manager.load_config()
+        config.add_server(ServerConfig(name="a", source="s", command="echo"))
+
+        assert manager.save_config(config) is True
+        assert list(read_config(config_path)["servers"]) == ["a"]
+
+    def test_invalid_server_is_kept(self, config_path):
+        write_config(
+            config_path,
+            {
+                "servers": {
+                    "good": {"source": "s", "command": "echo"},
+                    "bad": {"source": "s", "command": "echo", "prefix": "not-valid!"},
+                }
+            },
+        )
+        manager = ConfigManager(str(config_path))
+
+        config = manager.load_config()
+        assert list(config.servers) == ["good"]
+
+        config.add_server(ServerConfig(name="new", source="s", command="echo"))
+        assert manager.save_config(config) is True
+
+        servers = read_config(config_path)["servers"]
+        assert set(servers) == {"good", "bad", "new"}
+        assert servers["bad"]["prefix"] == "not-valid!"
+
+    def test_removed_server_is_removed(self, config_path):
+        write_config(
+            config_path,
+            {"servers": {"a": {"source": "s", "command": "echo"}, "b": {"source": "s", "command": "echo"}}},
+        )
+        manager = ConfigManager(str(config_path))
+
+        config = manager.load_config()
+        config.remove_server("a")
+        assert manager.save_config(config) is True
+
+        assert list(read_config(config_path)["servers"]) == ["b"]
+
+    def test_top_level_settings_are_kept(self, config_path):
+        write_config(
+            config_path,
+            {"self_prefix": "hub", "servers": {"a": {"source": "s", "command": "echo"}}, "custom": {"x": 1}},
+        )
+        manager = ConfigManager(str(config_path))
+
+        config = manager.load_config()
+        assert config.self_prefix == "hub"
+
+        config.remove_server("a")
+        assert manager.save_config(config) is True
+
+        data = read_config(config_path)
+        assert list(data) == ["self_prefix", "servers", "custom"]
+        assert data["self_prefix"] == "hub"
+        assert data["custom"] == {"x": 1}
+
+    def test_invalid_setting_does_not_discard_servers(self, config_path):
+        write_config(
+            config_path,
+            {"auto_reload": "maybe", "servers": {"a": {"source": "s", "command": "echo"}}},
+        )
+
+        config = ConfigManager(str(config_path)).load_config()
+
+        assert list(config.servers) == ["a"]
+
+    def test_write_preserves_mode_and_symlink(self, tmp_path):
+        real_path = tmp_path / "real.json"
+        write_config(real_path, {"servers": {}})
+        os.chmod(real_path, 0o640)
+        link_path = tmp_path / "config.json"
+        link_path.symlink_to(real_path)
+        manager = ConfigManager(str(link_path))
+
+        config = manager.load_config()
+        config.add_server(ServerConfig(name="a", source="s", command="echo"))
+        assert manager.save_config(config) is True
+
+        assert link_path.is_symlink()
+        assert list(read_config(real_path)["servers"]) == ["a"]
+        assert stat.S_IMODE(real_path.stat().st_mode) == 0o640
+        assert sorted(p.name for p in tmp_path.iterdir()) == ["config.json", "real.json"]
+
+    @pytest.mark.asyncio
+    async def test_kits_survive_save_with_reload_enabled(self, config_path):
+        write_config(
+            config_path,
+            {
+                "servers": {"calc": {"source": "s", "command": "echo", "enabled": False, "kits": ["mykit"]}},
+                "kits": {"mykit": {"name": "mykit", "description": "d", "source": "file"}},
+            },
+        )
+        manager = ConfigManager(str(config_path))
+
+        async def callback(change):
+            pass
+
+        await manager.setup_config_reload(callback)
+        try:
+            await asyncio.sleep(0.1)  # Let the watcher load its baseline
+
+            config = manager.load_config()
+            assert list(config.kits) == ["mykit"]
+
+            config.add_server(ServerConfig(name="other", source="s", command="echo", enabled=False))
+            assert manager.save_config(config) is True
+        finally:
+            await manager.stop_config_reload()
+
+        data = read_config(config_path)
+        assert list(data["kits"]) == ["mykit"]
+        assert set(data["servers"]) == {"calc", "other"}
+
+
+class TestReload:
+    """Config reload handles files replaced by rename and placeholder servers."""
+
+    @pytest.mark.asyncio
+    async def test_watchdog_handles_rename_over_config(self, config_path, tmp_path):
+        event = asyncio.Event()
+        handler = WatchdogHandler(config_path, event)
+
+        handler.on_moved(FileMovedEvent(str(tmp_path / ".config.json.tmp"), str(config_path)))
+        await asyncio.sleep(0)
+
+        assert event.is_set()
+
+    @pytest.mark.asyncio
+    async def test_watchdog_ignores_other_files(self, config_path, tmp_path):
+        event = asyncio.Event()
+        handler = WatchdogHandler(config_path, event)
+
+        handler.on_moved(FileMovedEvent(str(config_path), str(tmp_path / "backup.json")))
+        await asyncio.sleep(0)
+
+        assert not event.is_set()
+
+    def test_disabled_placeholder_does_not_block_reload(self, config_path):
+        config = MaggConfig()
+        config.servers["placeholder"] = ServerConfig(name="placeholder", source="s", enabled=False)
+        config.servers["real"] = ServerConfig(name="real", source="s", command="echo")
+
+        reloader = ConfigReloader(config_path, lambda change: None)
+
+        assert reloader._validate_config(config)
+
+    @pytest.mark.asyncio
+    async def test_unparseable_file_is_not_applied(self, config_path):
+        write_config(config_path, {"servers": {"a": {"source": "s", "command": "echo"}}})
+        changes = []
+
+        async def callback(change):
+            changes.append(change)
+
+        reloader = ConfigReloader(config_path, callback)
+        reloader._last_config = reloader._load_config()
+
+        config_path.write_text('{"servers": {')
+        assert await reloader.reload_config() is None
+        assert changes == []

From 0609c982fd2ab56ea4f10a22a2f94ad98eb655f7 Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Fri, 25 Sep 2026 20:04:16 +0000
Subject: [PATCH 03/14] Fix auth CLI crashes and harden server search

Auth:
- `magg auth init` creates missing parent dirs (e.g. no ~/.ssh) and
  reports "key already exists" as an error instead of a traceback.
- The private key file is created 0600 rather than chmod'ed after write.
- `--key-path` is saved to auth.json; previously only a custom issuer or
  audience was, so auth status/token couldn't find the new key.
- `auth private-key --export` quotes the key so the output can be eval'd.
- `auth status` no longer prints the MAGG_PRIVATE_KEY line twice.

Search:
- A GitHub repo with a null description, or an npm package with a null
  author, no longer throws away all results from that source.
- Sources are searched concurrently, with a 15s session timeout instead
  of aiohttp's 300s default, and proxy env vars are honored.
- The ranking bonus for Glama results now matches their source name.
---
 magg/auth.py                |   9 +--
 magg/cli.py                 |  13 ++--
 magg/discovery/metadata.py  |   2 +-
 magg/discovery/search.py    |  30 +++++----
 test/magg/test_cli_fixes.py | 127 ++++++++++++++++++++++++++++++++++++
 5 files changed, 158 insertions(+), 23 deletions(-)
 create mode 100644 test/magg/test_cli_fixes.py

diff --git a/magg/auth.py b/magg/auth.py
index b43cd7c..cedb78b 100644
--- a/magg/auth.py
+++ b/magg/auth.py
@@ -1,6 +1,7 @@
 """Authentication support for Magg."""
 
 import logging
+import os
 import time
 from functools import cached_property
 
@@ -60,7 +61,7 @@ def generate_keys(self) -> None:
 
         private_key = self._generate_keypair()
         if private_key is None:
-            raise RuntimeError("Failed to generate keypair")
+            raise RuntimeError(f"Failed to generate keypair in {self.bearer_config.key_path} (see log for details)")
 
         self._private_key = private_key
         self._public_key = self._derive_public_key(private_key)
@@ -86,11 +87,12 @@ def _generate_keypair(self) -> rsa.RSAPrivateKey | None:
         try:
             private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048, backend=default_backend())
 
-            self.bearer_config.key_path.mkdir(mode=0o700, exist_ok=True)
+            self.bearer_config.key_path.mkdir(mode=0o700, parents=True, exist_ok=True)
 
             private_path = self.bearer_config.private_key_path
 
-            with private_path.open("wb") as f:
+            # Create owner-only from the start so the key is never readable by others
+            with os.fdopen(os.open(private_path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600), "wb") as f:
                 f.write(
                     private_key.private_bytes(
                         encoding=serialization.Encoding.PEM,
@@ -98,7 +100,6 @@ def _generate_keypair(self) -> rsa.RSAPrivateKey | None:
                         encryption_algorithm=serialization.NoEncryption(),
                     )
                 )
-            private_path.chmod(0o600)
 
             ssh_public_path = self.bearer_config.public_key_path
             public_key = private_key.public_key()
diff --git a/magg/cli.py b/magg/cli.py
index 9205c35..95d5690 100644
--- a/magg/cli.py
+++ b/magg/cli.py
@@ -737,7 +737,12 @@ async def cmd_auth(args) -> int:
 
             auth_manager = BearerAuthManager(auth_config.bearer)
 
-            auth_manager.generate_keys()
+            try:
+                auth_manager.generate_keys()
+            except RuntimeError as e:
+                print_error(str(e))
+                return 1
+
             print_success(f"Generated new RSA keypair for audience '{auth_config.bearer.audience}'")
             print_text(
                 f"Private key: {auth_config.bearer.key_path}/{auth_config.bearer.audience}.key\n"
@@ -748,6 +753,7 @@ async def cmd_auth(args) -> int:
             if (
                 auth_config.bearer.issuer != default_config.issuer
                 or auth_config.bearer.audience != default_config.audience
+                or auth_config.bearer.key_path != default_config.key_path
             ):
                 if config_manager.save_auth_config(auth_config):
                     print_info(f"Auth config saved to: {config_manager.auth_config_path}")
@@ -775,9 +781,6 @@ async def cmd_auth(args) -> int:
 
                 if auth_config.bearer.public_key_exists:
                     print_info(f"SSH public key exists: {auth_config.bearer.public_key_path}")
-
-                if auth_config.bearer.private_key_env:
-                    print_info("Private key also available via MAGG_PRIVATE_KEY env var")
             else:
                 print_info("Authentication is DISABLED")
                 print_text("Run 'magg auth init' to enable authentication")
@@ -843,7 +846,7 @@ async def cmd_auth(args) -> int:
 
                     if args.export:
                         single_line = pem.replace("\n", "\\n")
-                        print(f"export MAGG_PRIVATE_KEY={single_line}")
+                        print(f"export MAGG_PRIVATE_KEY={shlex.quote(single_line)}")
                     elif args.oneline:
                         single_line = pem.replace("\n", "\\n")
                         print(single_line)
diff --git a/magg/discovery/metadata.py b/magg/discovery/metadata.py
index 3c6b653..12a0c11 100644
--- a/magg/discovery/metadata.py
+++ b/magg/discovery/metadata.py
@@ -237,7 +237,7 @@ async def _collect_github_metadata(self, url: str) -> dict[str, Any]:
             owner, repo = path_parts[0], path_parts[1]
 
             # Fetch from GitHub API
-            async with aiohttp.ClientSession() as session:
+            async with aiohttp.ClientSession(timeout=aiohttp.ClientTimeout(total=15)) as session:
                 api_url = f"https://api.github.com/repos/{owner}/{repo}"
 
                 async with session.get(api_url) as response:
diff --git a/magg/discovery/search.py b/magg/discovery/search.py
index 24cd4e3..8ee1d04 100644
--- a/magg/discovery/search.py
+++ b/magg/discovery/search.py
@@ -3,6 +3,7 @@
 TODO: Add support for mcpservers.org.
 """
 
+import asyncio
 import logging
 from dataclasses import dataclass
 from typing import Any
@@ -35,7 +36,8 @@ def __init__(self):
         self.session: aiohttp.ClientSession | None = None
 
     async def __aenter__(self):
-        self.session = aiohttp.ClientSession()
+        # Bounded so an unreachable source can't stall a search, and honor proxy env vars
+        self.session = aiohttp.ClientSession(timeout=aiohttp.ClientTimeout(total=15), trust_env=True)
         return self
 
     async def __aexit__(self, exc_type, exc_val, exc_tb):
@@ -92,7 +94,7 @@ def _parse_registry_results(cls, data: dict[str, Any]) -> list[ToolSearchResult]
 
             result = ToolSearchResult(
                 name=server.get("name", ""),
-                description=server.get("description", ""),
+                description=server.get("description") or "",
                 source="mcp-registry",
                 url=url,
                 tags=tags,
@@ -168,7 +170,7 @@ def _parse_glama_results(self, data: dict[str, Any]) -> list[ToolSearchResult]:
             # Convert glama result to our format
             result = ToolSearchResult(
                 name=server.get("name", ""),
-                description=server.get("description", ""),
+                description=server.get("description") or "",
                 source="glama",
                 url=server.get("url", ""),
                 tags=self._extract_tags(server),
@@ -282,14 +284,14 @@ def _parse_github_results(cls, data: dict[str, Any]) -> list[ToolSearchResult]:
         for item in data.get("items", []):
             # Try to determine if this is an MCP server
             is_mcp_server = any(
-                keyword in item.get("description", "").lower()
+                keyword in (item.get("description") or "").lower()
                 for keyword in ["mcp", "model context protocol", "mcp server"]
             )
 
             if is_mcp_server:
                 result = ToolSearchResult(
                     name=item.get("name", ""),
-                    description=item.get("description", ""),
+                    description=item.get("description") or "",
                     source="github",
                     url=item.get("html_url"),
                     tags=item.get("topics", []),
@@ -338,14 +340,14 @@ def _parse_npm_results(cls, data: dict[str, Any]) -> list[ToolSearchResult]:
 
             result = ToolSearchResult(
                 name=package.get("name", ""),
-                description=package.get("description", ""),
+                description=package.get("description") or "",
                 source="npm",
                 url=f"https://www.npmjs.com/package/{package.get('name')}",
                 tags=package.get("keywords", []),
                 install_command=f"npm install {package.get('name')}",
                 metadata={
                     "version": package.get("version"),
-                    "author": package.get("author", {}).get("name"),
+                    "author": (package.get("author") or {}).get("name"),
                     "license": package.get("license"),
                 },
             )
@@ -362,13 +364,15 @@ async def search_all(self, query: str, limit_per_source: int = 5) -> dict[str, l
             ("npm", self.search_npm(query, limit_per_source)),
         ]
 
+        outcomes = await asyncio.gather(*(task for _, task in tasks), return_exceptions=True)
+
         results = {}
-        for source, task in tasks:
-            try:
-                results[source] = await task
-            except Exception as e:
-                self.logger.error("Error searching %s: %s", source, e)
+        for (source, _), outcome in zip(tasks, outcomes):
+            if isinstance(outcome, BaseException):
+                self.logger.error("Error searching %s: %s", source, outcome)
                 results[source] = []
+            else:
+                results[source] = outcome
 
         return results
 
@@ -384,7 +388,7 @@ def calculate_score(result: ToolSearchResult) -> float:
                 score += result.rating * 10
 
             # Bonus for certain sources
-            source_bonus = {"mcp-registry": 6.0, "glama.ai": 5.0, "github": 3.0, "npm": 2.0}
+            source_bonus = {"mcp-registry": 6.0, "glama": 5.0, "github": 3.0, "npm": 2.0}
             score += source_bonus.get(result.source, 0.0)
 
             # Bonus for having install command
diff --git a/test/magg/test_cli_fixes.py b/test/magg/test_cli_fixes.py
new file mode 100644
index 0000000..2b6e28f
--- /dev/null
+++ b/test/magg/test_cli_fixes.py
@@ -0,0 +1,127 @@
+"""Tests for auth CLI and server search robustness fixes."""
+
+import asyncio
+import json
+import os
+import subprocess
+import time
+
+import pytest
+
+from magg.auth import BearerAuthManager
+from magg.cli import cmd_auth, create_parser
+from magg.discovery.search import ToolSearchEngine
+from magg.settings import BearerAuthConfig
+
+
+class TestAuthCLI:
+    """magg auth commands."""
+
+    @pytest.fixture
+    def auth_env(self, tmp_path, monkeypatch):
+        """Point HOME and the config at a temp dir with no ~/.ssh."""
+        home = tmp_path / "home"
+        home.mkdir()
+        monkeypatch.setenv("HOME", str(home))
+        monkeypatch.delenv("MAGG_PRIVATE_KEY", raising=False)
+        return tmp_path / "config" / "config.json"
+
+    async def run_auth(self, config_path, *argv):
+        args = create_parser().parse_args(["--config", str(config_path), "auth", *argv])
+        return await cmd_auth(args)
+
+    def test_generate_keypair_creates_missing_parents(self, tmp_path):
+        key_path = tmp_path / "home" / ".ssh" / "magg"
+        manager = BearerAuthManager(BearerAuthConfig(key_path=key_path))
+
+        manager.generate_keys()
+
+        private_path = key_path / "magg.key"
+        assert private_path.exists()
+        assert oct(private_path.stat().st_mode)[-3:] == "600"
+
+    @pytest.mark.asyncio
+    async def test_init_twice_reports_error(self, auth_env, tmp_path, capsys):
+        key_path = tmp_path / "keys"
+
+        assert await self.run_auth(auth_env, "init", "--key-path", str(key_path)) == 0
+        assert await self.run_auth(auth_env, "init", "--key-path", str(key_path)) == 1
+
+        assert "already exists" in capsys.readouterr().err
+
+    @pytest.mark.asyncio
+    async def test_init_saves_custom_key_path(self, auth_env, tmp_path):
+        key_path = tmp_path / "keys"
+
+        assert await self.run_auth(auth_env, "init", "--key-path", str(key_path)) == 0
+
+        saved = json.loads((auth_env.parent / "auth.json").read_text())
+        assert saved["bearer"]["key_path"] == str(key_path)
+
+    @pytest.mark.asyncio
+    async def test_private_key_export_is_shell_safe(self, auth_env, tmp_path, capsys):
+        key_path = tmp_path / "keys"
+        assert await self.run_auth(auth_env, "init", "--key-path", str(key_path)) == 0
+        capsys.readouterr()
+
+        assert await self.run_auth(auth_env, "private-key", "--export") == 0
+        line = capsys.readouterr().out.strip()
+
+        result = subprocess.run(
+            ["sh", "-c", f'{line}; printf %s "$MAGG_PRIVATE_KEY"'], capture_output=True, text=True, check=True
+        )
+        pem = result.stdout.replace("\\n", "\n")
+        assert pem == (key_path / "magg.key").read_text()
+
+
+class TestSearch:
+    """ToolSearchEngine handles missing fields and slow sources."""
+
+    def test_github_null_description(self):
+        data = {
+            "items": [
+                {"name": "no-desc", "description": None, "html_url": "https://github.com/a/b"},
+                {"name": "mcp-thing", "description": "An MCP server", "html_url": "https://github.com/a/c"},
+            ]
+        }
+
+        results = ToolSearchEngine._parse_github_results(data)
+
+        assert [r.name for r in results] == ["mcp-thing"]
+
+    def test_npm_null_author_and_description(self):
+        data = {"objects": [{"package": {"name": "pkg", "description": None, "author": None}}]}
+
+        results = ToolSearchEngine._parse_npm_results(data)
+
+        assert results[0].description == ""
+        assert results[0].metadata["author"] is None
+
+    @pytest.mark.asyncio
+    async def test_search_all_runs_sources_concurrently(self, monkeypatch):
+        async def slow(self, query, limit):
+            await asyncio.sleep(0.3)
+            return []
+
+        async def broken(self, query, limit):
+            raise RuntimeError("offline")
+
+        monkeypatch.setattr(ToolSearchEngine, "search_registry", slow)
+        monkeypatch.setattr(ToolSearchEngine, "search_glama", slow)
+        monkeypatch.setattr(ToolSearchEngine, "search_github", slow)
+        monkeypatch.setattr(ToolSearchEngine, "search_npm", broken)
+
+        engine = ToolSearchEngine()
+        start = time.monotonic()
+        results = await engine.search_all("x")
+        elapsed = time.monotonic() - start
+
+        assert elapsed < 0.8
+        assert results == {"mcp-registry": [], "glama": [], "github": [], "npm": []}
+
+
+@pytest.fixture(autouse=True)
+def _no_real_private_key(monkeypatch):
+    # A MAGG_PRIVATE_KEY in the developer's environment would change auth behavior
+    if "MAGG_PRIVATE_KEY" in os.environ:
+        monkeypatch.delenv("MAGG_PRIVATE_KEY")

From d432c197877473c68915782417c7d5ffd1c37bac Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Fri, 25 Sep 2026 20:06:19 +0000
Subject: [PATCH 04/14] Make documented kit and reload settings real; fix docs
 that drifted

- Kits in a kit.d next to the config file are found even when the config
  is set with MAGG_CONFIG_PATH or --config, as the docs described.
- MAGG_RELOAD_USE_WATCHDOG was documented but ignored; false now forces
  polling, which helps where file notifications don't arrive (network
  filesystems, Docker Desktop bind mounts).
- Docs: --env-pass/--env-set are global options (magg --env-pass serve);
  kit discovery uses MAGG_PATH, not the nonexistent MAGG_KITD_PATH; an
  omitted prefix means no prefix, not the server name; MAGG_PREFIX_SEP
  only applies to Magg's own tools; magg_check's timeout default is 2.5;
  the kit example is valid JSON; document MAGG_BACKEND_INIT_TIMEOUT and
  how reload treats invalid files.
---
 docs/config-reload.md           |  7 ++++---
 docs/index.md                   | 29 +++++++++++++++++------------
 docs/kits.md                    |  9 +++++----
 magg/kit.py                     |  4 ++++
 magg/reload.py                  | 32 +++++++++++++++++++-------------
 magg/settings.py                |  5 +++++
 readme.md                       |  3 ++-
 test/magg/test_config_safety.py | 29 +++++++++++++++++++++++++++++
 8 files changed, 85 insertions(+), 33 deletions(-)

diff --git a/docs/config-reload.md b/docs/config-reload.md
index e89d76e..32d1207 100644
--- a/docs/config-reload.md
+++ b/docs/config-reload.md
@@ -33,7 +33,7 @@ When a configuration change is detected, Magg:
 
 - `MAGG_AUTO_RELOAD`: Enable/disable automatic config reloading (default: `true`)
 - `MAGG_RELOAD_POLL_INTERVAL`: File check interval in seconds when polling (default: `1.0`)
-- `MAGG_RELOAD_USE_WATCHDOG`: Force watchdog on/off, or auto-detect (default: `null` for auto)
+- `MAGG_RELOAD_USE_WATCHDOG`: `true` to use file system notifications, `false` to always poll (default: auto-detect)
 - `MAGG_READ_ONLY`: When `true`, Magg cannot modify config but can still reload external changes
 
 ### Example
@@ -203,11 +203,12 @@ INFO: Configuration reload complete
 
 The logs will show which validation failed:
 ```
-ERROR: Duplicate prefix 'test' found in servers 'server1' and 'server2'
 ERROR: Server 'myserver' has neither command nor uri
 ```
 
-Fix the configuration issue and save again.
+A file that isn't valid JSON is not applied, and Magg won't save over it (so a typo can't
+wipe your configuration). Server entries that fail validation are skipped when loading but
+kept in the file when Magg saves. Fix the configuration issue and save again.
 
 ### Server not responding after reload
 
diff --git a/docs/index.md b/docs/index.md
index aa92a55..bfdc642 100644
--- a/docs/index.md
+++ b/docs/index.md
@@ -239,7 +239,7 @@ Add a new MCP server to the configuration.
 - `name` (str, required): Unique server name
 - `source` (str, required): URL or URI of the server package/repository (can be a local path as well)
 - `command` (str, optional): Full command to run (e.g., "npx @playwright/mcp@latest")
-- `prefix` (str, optional): Tool prefix (defaults to server name)
+- `prefix` (str, optional): Tool prefix (default: none, tools keep their original names)
 - `uri` (str, optional): URI for HTTP servers
 - `env` (dict|str, optional): Environment variables (can be a dict or JSON string)
 - `cwd` (str, optional): Working directory
@@ -345,7 +345,7 @@ Perform health checks on configured servers with optional remediation actions.
 
 **Parameters:**
 - `action` (str, optional): Action to perform - "report" (default), "remount", "unmount", or "disable"
-- `timeout` (float, optional): Health check timeout in seconds (default: 0.5)
+- `timeout` (float, optional): Health check timeout in seconds (default: 2.5)
 
 **Actions:**
 - `report`: Check server health and return status report
@@ -565,9 +565,10 @@ Kits are a way to bundle related MCP servers together for easy installation and
 
 ### Kit Discovery
 
-Magg looks for kits in these locations:
-1. `$MAGG_KITD_PATH` (defaults to `~/.magg/kit.d`)
-2. `.magg/kit.d` in the same directory as your `config.json`
+Magg looks for kits in these locations (the first kit found with a given name wins):
+1. `kit.d` in the same directory as your `config.json`
+2. `kit.d` in each directory on `MAGG_PATH` (colon-separated, default: `./.magg`, then `~/.magg`)
+3. Example kits bundled with Magg
 
 ### Creating Kits
 
@@ -738,29 +739,32 @@ When configuration changes are detected, Magg:
 
 - `MAGG_AUTO_RELOAD` (default: `true`) - Enable/disable automatic configuration reloading
 - `MAGG_RELOAD_POLL_INTERVAL` (default: `1.0`) - Polling interval in seconds when watchdog unavailable
-- `MAGG_RELOAD_USE_WATCHDOG` (default: auto-detect) - Force watchdog on/off or let it auto-detect
+- `MAGG_RELOAD_USE_WATCHDOG` (default: auto-detect) - `true` to require file system notifications, `false` to always poll (useful for network filesystems and Docker Desktop bind mounts)
 - `MAGG_STDERR_SHOW` (default: `false`) - Show stderr output from subprocess MCP servers
-- `MAGG_PREFIX_SEP` (default: `_`) - Separator between prefix and tool name
+- `MAGG_PREFIX_SEP` (default: `_`) - Separator between Magg's prefix and its own tool names
+- `MAGG_BACKEND_INIT_TIMEOUT` (default: `30`) - Seconds to wait for a backend server to start, `0` to wait forever
 - `MAGG_SELF_PREFIX` (default: `magg`) - Prefix for Magg's own tools
 
 ### Environment Variable Inheritance
 
 By default, stdio-based MCP servers run with isolated environments. While the recommended approach is to configure environment variables in the server configuration (see Advanced Configuration), you can also pass environment variables at runtime to all stdio subprocesses:
 
+These are global options, so they go before the `serve` command.
+
 **Pass current environment to servers:**
 ```bash
-magg serve --env-pass
+magg --env-pass serve
 ```
 
 **Set specific environment variables:**
 ```bash
-magg serve --env-set API_KEY mykey --env-set LANG en_US.UTF-8
+magg --env-set API_KEY mykey --env-set LANG en_US.UTF-8 serve
 ```
 
 **Combined usage:**
 ```bash
 # Pass current env and add specific variables
-magg serve --env-pass --env-set DEBUG true
+magg --env-pass --env-set DEBUG true serve
 ```
 
 This is useful for:
@@ -771,6 +775,7 @@ This is useful for:
 
 **Note:** 
 - Environment inheritance only affects stdio-based servers. HTTP/URI servers do not receive inherited environment variables.
+- A server's own `env` values take precedence over inherited ones.
 - The preferred method is to set environment variables in the server configuration's `env` field for better reproducibility.
 - The `mbro` CLI tool also supports these same `--env-pass` and `--env-set` flags when connecting to servers.
 
@@ -996,7 +1001,7 @@ Magg organizes tools from multiple servers using a prefix system. Every tool nam
 
 Where:
 - `prefix` is the namespace for a server (configurable per server)
-- `separator` is configurable via `prefix_sep` field or `MAGG_PREFIX_SEP` env var (default: `_`)
+- `separator` is `_` (`MAGG_PREFIX_SEP` changes it for Magg's own tools only)
 - `tool_name` is the original tool name from the server
 
 Examples:
@@ -1019,7 +1024,7 @@ When adding a server, specify the `prefix` field:
 
 Special cases:
 - `"prefix": ""` or `"prefix": null` - No prefix, tools keep original names
-- Omitting prefix field - Server name is used as default prefix
+- Omitting the prefix field is the same as `null`
 - Multiple servers can share the same prefix (tools are merged)
 
 #### Configuring Magg's Prefix
diff --git a/docs/kits.md b/docs/kits.md
index 37a26fc..a270161 100644
--- a/docs/kits.md
+++ b/docs/kits.md
@@ -13,9 +13,10 @@ When you load a kit into Magg, all its servers are added to your configuration.
 
 ## Kit Discovery
 
-Magg looks for kits in these locations:
-1. `$MAGG_KITD_PATH` (defaults to `~/.magg/kit.d`)
-2. `.magg/kit.d` in the same directory as your `config.json`
+Magg looks for kits in these locations (the first kit found with a given name wins):
+1. `kit.d` in the same directory as your `config.json`
+2. `kit.d` in each directory on `MAGG_PATH` (colon-separated, default: `./.magg`, then `~/.magg`)
+3. Example kits bundled with Magg
 
 Kit files must have a `.json` extension and follow the kit schema.
 
@@ -37,7 +38,7 @@ Kit files must have a `.json` extension and follow the kit schema.
       "source": "https://github.com/example/mcp-calc-server",
       "command": "python",
       "args": ["-m", "mcp_calc_server"],
-      "notes": "Basic calculator server",
+      "notes": "Basic calculator server"
     }
   }
 }
diff --git a/magg/kit.py b/magg/kit.py
index 32cebae..8a0a655 100644
--- a/magg/kit.py
+++ b/magg/kit.py
@@ -69,6 +69,10 @@ def __init__(self, config_manager: ConfigManager, kitd_paths: list[Path] | None
         else:
             config = MaggConfig()
             self.kitd_paths = config.get_kitd_paths()
+            # MAGG_CONFIG_PATH or --config can put the config outside the search path
+            config_kitd = config_manager.config_path.parent / "kit.d"
+            if config_kitd.is_dir() and all(config_kitd.resolve() != p.resolve() for p in self.kitd_paths):
+                self.kitd_paths.insert(0, config_kitd)
         self._kits: dict[str, KitConfig] = {}
 
     def discover_kits(self) -> dict[str, Path]:
diff --git a/magg/reload.py b/magg/reload.py
index 4a12e23..22a5385 100644
--- a/magg/reload.py
+++ b/magg/reload.py
@@ -120,11 +120,12 @@ def __init__(self, config_path: Path, reload_callback: ReloadCallback, config_ma
         self._observer: Observer | None = None
         self._watchdog_handler: WatchdogHandler | None = None
 
-    async def start_watching(self, poll_interval: float = 1.0) -> None:
+    async def start_watching(self, poll_interval: float = 1.0, use_watchdog: bool | None = None) -> None:
         """Start watching the config file for changes.
 
         Args:
             poll_interval: How often to check for changes (seconds) - only used for polling mode
+            use_watchdog: Use file system notifications (True), polling (False), or auto-detect (None)
         """
         if self._watch_task and not self._watch_task.done():
             logger.warning("Config watcher already running")
@@ -133,17 +134,20 @@ async def start_watching(self, poll_interval: float = 1.0) -> None:
         self._shutdown_event.clear()
         self._reload_event.clear()
 
-        # Try to start watchdog observer
-        try:
-            self._observer = Observer()
-            self._watchdog_handler = WatchdogHandler(self.config_path, self._reload_event)
-            self._observer.schedule(self._watchdog_handler, str(self.config_path.parent), recursive=False)
-            self._observer.start()
-            logger.debug("Started config file watcher using file system notifications (watchdog)")
-        except Exception as e:
-            logger.warning("Failed to start watchdog observer: %s. Falling back to polling mode.", e)
-            self._observer = None
-            self._watchdog_handler = None
+        # Try to start watchdog observer, unless polling was requested
+        if use_watchdog is not False:
+            try:
+                self._observer = Observer()
+                self._watchdog_handler = WatchdogHandler(self.config_path, self._reload_event)
+                self._observer.schedule(self._watchdog_handler, str(self.config_path.parent), recursive=False)
+                self._observer.start()
+                logger.debug("Started config file watcher using file system notifications (watchdog)")
+            except Exception as e:
+                logger.warning("Failed to start watchdog observer: %s. Falling back to polling mode.", e)
+                self._observer = None
+                self._watchdog_handler = None
+
+        if not self._observer:
             logger.debug("Using polling mode (interval: %.1fs)", poll_interval)
 
         # Start the main watch loop
@@ -412,7 +416,9 @@ async def setup(self, reload_callback: ReloadCallback) -> None:
                     reload_callback=reload_callback,
                     config_manager=self.config_manager,
                 )
-                await self._config_reloader.start_watching(poll_interval=config.reload_poll_interval)
+                await self._config_reloader.start_watching(
+                    poll_interval=config.reload_poll_interval, use_watchdog=config.reload_use_watchdog
+                )
 
     async def stop(self) -> None:
         """Stop config file watching."""
diff --git a/magg/settings.py b/magg/settings.py
index 7cb23a4..0bfd536 100644
--- a/magg/settings.py
+++ b/magg/settings.py
@@ -221,6 +221,11 @@ class MaggConfig(BaseSettings):
     reload_poll_interval: float = Field(
         default=1.0, description="Config file poll interval in seconds (env: MAGG_RELOAD_POLL_INTERVAL)"
     )
+    reload_use_watchdog: bool | None = Field(
+        default=None,
+        description="Use file system notifications (true) or polling (false) to detect config changes, "
+        "default auto-detect (env: MAGG_RELOAD_USE_WATCHDOG)",
+    )
     stderr_show: bool = Field(
         default=False, description="Show stderr output from subprocess MCP servers (env: MAGG_STDERR_SHOW)"
     )
diff --git a/readme.md b/readme.md
index 154ec4c..2fd7cb6 100644
--- a/readme.md
+++ b/readme.md
@@ -330,7 +330,8 @@ Magg supports several environment variables for configuration:
 - `MAGG_RELOAD_POLL_INTERVAL` - Config polling interval in seconds (default: 1.0)
 - `MAGG_READ_ONLY=true` - Run in read-only mode
 - `MAGG_SELF_PREFIX` - Prefix for Magg tools (default: "magg"). Tools will be named as `{prefix}{sep}{tool}` (e.g., `magg_list_servers`)
-- `MAGG_PREFIX_SEP` - Separator between prefix and tool name (default: "_")
+- `MAGG_PREFIX_SEP` - Separator between Magg's prefix and its tool names (default: "_"). Backend server tools always use `_`
+- `MAGG_BACKEND_INIT_TIMEOUT` - Seconds to wait for a backend server to start before giving up on it (default: 30, `0` waits forever)
 
 Example configuration:
 ```json
diff --git a/test/magg/test_config_safety.py b/test/magg/test_config_safety.py
index a7e0cff..a9cd30b 100644
--- a/test/magg/test_config_safety.py
+++ b/test/magg/test_config_safety.py
@@ -8,6 +8,7 @@
 import pytest
 from watchdog.events import FileMovedEvent
 
+from magg.kit import KitManager
 from magg.reload import ConfigReloader, WatchdogHandler
 from magg.settings import BearerAuthConfig, ConfigFileError, ConfigManager, MaggConfig, ServerConfig
 
@@ -240,3 +241,31 @@ async def callback(change):
         config_path.write_text('{"servers": {')
         assert await reloader.reload_config() is None
         assert changes == []
+
+    @pytest.mark.asyncio
+    async def test_watchdog_can_be_disabled(self, config_path):
+        write_config(config_path, {"servers": {}})
+
+        async def callback(change):
+            pass
+
+        reloader = ConfigReloader(config_path, callback)
+        await reloader.start_watching(poll_interval=0.1, use_watchdog=False)
+        try:
+            assert reloader._observer is None
+        finally:
+            await reloader.stop_watching()
+
+
+class TestKitDiscovery:
+    """Kits next to an explicitly configured config file are found."""
+
+    def test_kitd_next_to_config(self, tmp_path, monkeypatch):
+        monkeypatch.setenv("MAGG_PATH", str(tmp_path / "elsewhere"))
+        config_dir = tmp_path / "project"
+        (config_dir / "kit.d").mkdir(parents=True)
+        (config_dir / "kit.d" / "local.json").write_text(json.dumps({"name": "local", "servers": {}}))
+
+        kit_manager = KitManager(ConfigManager(str(config_dir / "config.json")))
+
+        assert "local" in kit_manager.discover_kits()

From 6d1a0d7d70704daffe00f305f192e94b76dc988c Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Fri, 25 Sep 2026 20:07:57 +0000
Subject: [PATCH 05/14] Report invalid MAGG_* settings without a traceback

A bad value such as MAGG_AUTO_RELOAD=maybe or MAGG_SELF_PREFIX=my_prefix
used to print a pydantic traceback on every command, including
--version. The CLI now prints one line per invalid field naming the
environment variable to fix, and exits 1. python -m magg goes through the
same path.
---
 magg/__main__.py            |  4 ----
 magg/cli.py                 | 26 +++++++++++++++++++++++---
 test/magg/test_cli_fixes.py | 16 ++++++++++++++++
 3 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/magg/__main__.py b/magg/__main__.py
index d78936a..4f9e80a 100644
--- a/magg/__main__.py
+++ b/magg/__main__.py
@@ -1,7 +1,3 @@
-from magg import process
-
-process.setup()
-
 from magg.cli import main
 
 main()
diff --git a/magg/cli.py b/magg/cli.py
index 95d5690..2022951 100644
--- a/magg/cli.py
+++ b/magg/cli.py
@@ -8,6 +8,7 @@
 from pathlib import Path
 
 from cryptography.hazmat.primitives import serialization
+from pydantic import ValidationError
 
 from . import __version__, process
 from .auth import BearerAuthManager
@@ -27,7 +28,10 @@
     print_warning,
 )
 
-process.setup(source=__name__)
+try:
+    process.setup(source=__name__)
+except ValidationError:
+    pass  # Reported by main() without a traceback
 
 logger: logging.Logger = logging.getLogger(__name__)
 
@@ -1060,10 +1064,26 @@ async def run():
         exit(1)
 
 
+def format_validation_error(error: ValidationError) -> str:
+    """Describe a settings validation error in a line per field, pointing at the env var for MaggConfig."""
+    lines = []
+    for err in error.errors():
+        field = ".".join(str(part) for part in err["loc"])
+        line = f"Invalid setting {field!r}: {err['msg']} (got {err['input']!r})"
+        if error.title == "MaggConfig" and len(err["loc"]) == 1:
+            line += f". Check MAGG_{field.upper()} in your environment or .env file."
+        lines.append(line)
+    return "\n".join(lines)
+
+
 def main():
     """Run the CLI."""
-    process.setup()
-    asyncio.run(run())
+    try:
+        process.setup()
+        asyncio.run(run())
+    except ValidationError as e:
+        print_error(format_validation_error(e))
+        exit(1)
 
 
 if __name__ == "__main__":
diff --git a/test/magg/test_cli_fixes.py b/test/magg/test_cli_fixes.py
index 2b6e28f..df06b42 100644
--- a/test/magg/test_cli_fixes.py
+++ b/test/magg/test_cli_fixes.py
@@ -4,6 +4,7 @@
 import json
 import os
 import subprocess
+import sys
 import time
 
 import pytest
@@ -125,3 +126,18 @@ def _no_real_private_key(monkeypatch):
     # A MAGG_PRIVATE_KEY in the developer's environment would change auth behavior
     if "MAGG_PRIVATE_KEY" in os.environ:
         monkeypatch.delenv("MAGG_PRIVATE_KEY")
+
+
+class TestInvalidSettings:
+    """Bad MAGG_* values produce a readable error, not a traceback."""
+
+    def test_invalid_env_value(self, tmp_path):
+        env = {**os.environ, "MAGG_AUTO_RELOAD": "maybe", "MAGG_CONFIG_PATH": str(tmp_path / "config.json")}
+
+        result = subprocess.run(
+            [sys.executable, "-m", "magg", "server", "list"], capture_output=True, text=True, env=env, timeout=60
+        )
+
+        assert result.returncode == 1
+        assert "MAGG_AUTO_RELOAD" in result.stderr
+        assert "Traceback" not in result.stderr

From 32ff7fca0ad2d13090ea85d1c9561682787b9b60 Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Fri, 25 Sep 2026 20:14:08 +0000
Subject: [PATCH 06/14] Fix mbro argument parsing, error handling, and
 connection bugs

- call/prompt keep a JSON object argument as raw text (quoted or not),
  and scripts join JSON objects that span lines until braces balance
- key=value values are decoded as JSON with a raw-string fallback,
  keeping string-typed schema params as strings (no more 007 -> 7 or
  ValueError on 10-20); prompt accepts key=value
- Non-interactive runs (-n, -X, -) exit 1 when any command fails;
  per-command errors are contained and later commands keep running,
  with --fail-fast to stop at the first failure; quit stops scripts
- status takes args and checks for a connection; None descriptions no
  longer crash tools/prompts/search filtering or JSON search output
- script run/dump with a missing script report an error instead of
  exiting; resource/prompt tracebacks only print with -v
- Connection errors carry the real reason (including duplicate names);
  connects time out after --timeout seconds (default 30)
- Only bare URLs get /mcp/ appended; /mcp and /sse paths are kept
- mbro - runs non-interactively; --repl without arepl returns cleanly
- help lists status and script instead of the nonexistent refresh;
  unknown tools get "Did you mean" suggestions
- Remove unused JsonArgParser
---
 magg/mbro/cli.py                    | 147 ++++++++----
 magg/mbro/client.py                 |  69 ++++--
 magg/mbro/command.py                | 149 +++++++------
 magg/mbro/formatter.py              |  32 ++-
 magg/mbro/parser.py                 | 157 ++++---------
 magg/mbro/scripts.py                |  19 +-
 test/magg/test_mbro_cli_overhaul.py |  12 +-
 test/magg/test_mbro_commands.py     | 331 ++++++++++++++++++++++++++++
 8 files changed, 644 insertions(+), 272 deletions(-)
 create mode 100644 test/magg/test_mbro_commands.py

diff --git a/magg/mbro/cli.py b/magg/mbro/cli.py
index 2bcc46d..3568266 100644
--- a/magg/mbro/cli.py
+++ b/magg/mbro/cli.py
@@ -2,6 +2,8 @@
 
 import argparse
 import asyncio
+import json
+import shlex
 import sys
 from asyncio import CancelledError
 from functools import cached_property
@@ -22,12 +24,12 @@
     arepl = None
 
 from .. import __version__, process
-from .client import BrowserClient
+from .client import DEFAULT_CONNECT_TIMEOUT, BrowserClient
 from .command import Command
 from .completers import create_improved_completer
 from .formatter import OutputFormatter
 from .multiline import MultilineInputHandler
-from .parser import CommandParser, JsonArgParser
+from .parser import CommandParser
 from .validator import InputValidator
 
 
@@ -72,8 +74,10 @@ def __init__(
         env_pass: bool = False,
         env_vars: dict[str, str] | None = None,
         status_bar: bool = False,
+        fail_fast: bool = False,
+        timeout: float | None = DEFAULT_CONNECT_TIMEOUT,
     ):
-        self.browser = BrowserClient(env_pass=env_pass, env_vars=env_vars)
+        self.browser = BrowserClient(env_pass=env_pass, env_vars=env_vars, timeout=timeout)
         self.running = True
         self.formatter = OutputFormatter(json_only=json_only, use_rich=use_rich, indent=indent)
         self.verbose = verbose
@@ -81,10 +85,11 @@ def __init__(
         self.env_pass = env_pass
         self.env_vars = env_vars
         self.status_bar = status_bar
+        self.fail_fast = fail_fast
+        self.failed = False  # Set when a command run by run_commands fails
         self.command = Command(self)
 
         if not json_only:
-            self.json_parser = JsonArgParser()
             self.multiline_handler = MultilineInputHandler(self.formatter)
             self._multiline_buffer = []
 
@@ -112,6 +117,7 @@ def _completer(self):
                     "status",
                     "search",
                     "info",
+                    "script",
                 ],
                 meta_dict={
                     "help": "Show this help message",
@@ -131,6 +137,7 @@ def _completer(self):
                     "prompt": "Get a prompt by name with optional arguments",
                     "search": "Search tools, resources, and prompts by term",
                     "info": "Show detailed info about a tool/resource/prompt",
+                    "script": "Run or manage .mbro scripts",
                 },
             )
 
@@ -295,15 +302,21 @@ def _create_input_validator(self):
         return InputValidator(self)
 
     @classmethod
-    def parse_shell_args(cls, args: list[str]) -> dict:
+    def parse_shell_args(cls, args: list[str], schema: dict | None = None, *, strings: bool = False) -> dict:
         """Parse shell-style key=value arguments.
 
+        Values are decoded as JSON where possible, otherwise kept as strings. Values for
+        parameters the input schema declares as strings are kept as-is, as are all values
+        when `strings` is set.
+
         Examples:
             name="test" -> {"name": "test"}
             count=42 -> {"count": 42}
             enabled=true -> {"enabled": true}
             name="my server" count=5 -> {"name": "my server", "count": 5}
+            ids=[1,2] code=007 range=10-20 -> {"ids": [1, 2], "code": "007", "range": "10-20"}
         """
+        properties = (schema or {}).get("properties", {})
         result = {}
 
         for arg in args:
@@ -316,23 +329,33 @@ def parse_shell_args(cls, args: list[str]) -> dict:
             if not key:
                 continue
 
-            if value.lower() == "true":
-                result[key] = True
-            elif value.lower() == "false":
-                result[key] = False
-            elif value.replace(".", "", 1).replace("-", "", 1).isdigit():
-                if "." in value:
-                    result[key] = float(value)
-                else:
-                    result[key] = int(value)
+            if (value.startswith('"') and value.endswith('"')) or (value.startswith("'") and value.endswith("'")):
+                result[key] = value[1:-1]
+            elif strings or cls._is_string_param(properties.get(key)):
+                result[key] = value
+            elif value.lower() in ("true", "false"):
+                result[key] = value.lower() == "true"
             else:
-                if (value.startswith('"') and value.endswith('"')) or (value.startswith("'") and value.endswith("'")):
-                    result[key] = value[1:-1]
-                else:
+                try:
+                    result[key] = json.loads(value)
+                except json.JSONDecodeError:
                     result[key] = value
 
         return result
 
+    @staticmethod
+    def _is_string_param(prop: dict | None) -> bool:
+        """Check whether a JSON schema property only accepts strings (or null)."""
+        if not prop:
+            return False
+
+        types = set()
+        for option in prop.get("anyOf") or [prop]:
+            option_type = option.get("type", "any")
+            types.update(option_type if isinstance(option_type, list) else [option_type])
+
+        return types - {"null"} == {"string"}
+
     async def refresh_completer_cache(self):
         """Refresh the completer cache after connection changes."""
         if not self.formatter.json_only and hasattr(self, "_completer"):
@@ -350,6 +373,7 @@ async def start(self, repl: bool = False):
         if repl:
             if arepl is None:
                 self.formatter.print("REPL mode is only available with Python 3.13+")
+                return
 
             self.formatter.print(
                 "Entering REPL mode. `await self.handle_command(command)` to execute commands.", file=sys.stderr
@@ -419,20 +443,6 @@ async def handle_command(self, command: str):
         cmd = parts[0].lower()
         args = parts[1:]
 
-        if args and any("{" in arg for arg in args):
-            json_start_idx = None
-            for i, arg in enumerate(args):
-                if "{" in arg:
-                    json_start_idx = i
-                    break
-
-            if json_start_idx is not None:
-                json_part = " ".join(args[json_start_idx:])
-
-                if json_part.count("{") == json_part.count("}"):
-                    if cmd in ["call", "prompt", "get-prompt"]:
-                        args = args[:json_start_idx] + [json_part]
-
         cmd = self.ALIASES.get(cmd, cmd)
 
         if cmd not in self.COMMANDS:
@@ -440,17 +450,49 @@ async def handle_command(self, command: str):
             self.formatter.format_info("Type 'help' for available commands")
             return
 
-        match cmd:
-            case "help":
-                self.show_help()
-            case "quit":
-                self.running = False
-            case _:
-                await getattr(self.command, cmd)(args)
+        try:
+            match cmd:
+                case "help":
+                    self.show_help()
+                case "quit":
+                    self.running = False
+                case _:
+                    await getattr(self.command, cmd)(args)
+
+        except Exception as e:
+            self.formatter.format_error("Unexpected error in command handling", e)
 
         if not self.formatter.json_only:
             self.formatter.print()
 
+    async def run_commands(self, commands: list[str]) -> bool:
+        """Run commands in order, stopping at `quit` (or the first failure with fail_fast).
+
+        A command fails if it reports an error.
+
+        Returns:
+            True if every command that ran succeeded
+        """
+        ok = True
+
+        for command in commands:
+            if not self.running:
+                break
+
+            if self.verbose:
+                self.formatter.format_info(f"> {command}")
+
+            errors = self.formatter.error_count
+            await self.handle_command(command)
+
+            if self.formatter.error_count != errors:
+                ok = False
+                self.failed = True
+                if self.fail_fast:
+                    break
+
+        return ok
+
     def show_help(self):
         """Show help text."""
         self.formatter.format_help(enhanced=not self.formatter.json_only)
@@ -474,10 +516,7 @@ async def handle_commands(cli: MCPBrowserCLI, args) -> bool:
     if not commands_to_run:
         return False
 
-    for command in commands_to_run:
-        if command.strip():
-            await cli.handle_command(command)
-
+    await cli.run_commands(commands_to_run)
     return True
 
 
@@ -534,6 +573,16 @@ async def main_async():
         help="Execute script in non-interactive mode",
     )
     parser.add_argument("--status-bar", action="store_true", help="Show status bar with keyboard shortcuts")
+    parser.add_argument(
+        "--fail-fast", action="store_true", help="Stop running commands and scripts at the first failure"
+    )
+    parser.add_argument(
+        "--timeout",
+        type=float,
+        default=DEFAULT_CONNECT_TIMEOUT,
+        metavar="SECONDS",
+        help=f"Server initialization timeout (0 to disable, default: {DEFAULT_CONNECT_TIMEOUT:g})",
+    )
 
     parser.add_argument(
         "commands",
@@ -555,21 +604,29 @@ async def main_async():
         env_pass=args.env_pass,
         env_vars=dict(args.env_set) if args.env_set else None,
         status_bar=args.status_bar,
+        fail_fast=args.fail_fast,
+        timeout=args.timeout,
     )
 
+    # Commands piped through stdin run non-interactively
+    if args.commands and args.commands[0] == "-":
+        args.no_interactive = True
+
     try:
         if hasattr(args, "script_order") and args.script_order:
             has_non_interactive_script = any(is_non_interactive for _, is_non_interactive in args.script_order)
             if has_non_interactive_script:
                 args.no_interactive = True
 
-            for script_path, _ in args.script_order:
-                await cli.handle_command(f"script run {script_path}")
+            await cli.run_commands([f"script run {shlex.quote(path)}" for path, _ in args.script_order])
 
         commands_executed = False
-        if args.commands:
+        if args.commands and not (cli.failed and args.fail_fast):
             commands_executed = await handle_commands(cli, args)
 
+        if cli.failed and (args.no_interactive or args.fail_fast):
+            sys.exit(1)
+
         if not args.no_interactive:
             await cli.start(repl=args.repl)
         elif not commands_executed and not hasattr(args, "script_order"):
diff --git a/magg/mbro/client.py b/magg/mbro/client.py
index 154fb9e..62b7e3b 100644
--- a/magg/mbro/client.py
+++ b/magg/mbro/client.py
@@ -3,6 +3,7 @@
 import logging
 import os
 from typing import Any
+from urllib.parse import urlparse
 
 from fastmcp import Client
 from fastmcp.client import BearerAuth
@@ -23,6 +24,8 @@
 
 logger = logging.getLogger(__name__)
 
+DEFAULT_CONNECT_TIMEOUT = 30.0
+
 
 class BrowserConnection:
     """Represents a connection to an MCP server."""
@@ -80,23 +83,33 @@ async def get_prompts(self) -> list[dict[str, Any]]:
 
         return prompts_data
 
-    async def connect(self, env_pass: bool = False, env_vars: dict[str, str] | None = None) -> bool:
-        """Connect to the MCP server using FastMCP Client."""
+    @staticmethod
+    def normalize_url(url: str) -> str:
+        """Point a bare server URL (no path) at the default /mcp/ endpoint."""
+        parsed = urlparse(url)
+        if parsed.path in ("", "/"):
+            return parsed._replace(path="/mcp/").geturl()
+        return url
+
+    async def connect(
+        self, env_pass: bool = False, env_vars: dict[str, str] | None = None, timeout: float | None = None
+    ) -> None:
+        """Connect to the MCP server using FastMCP Client.
+
+        Raises the underlying error if the server can't be reached or doesn't initialize within `timeout`.
+        """
         jwt = os.getenv("MAGG_JWT", os.getenv("MBRO_JWT", os.getenv("MCP_JWT", None)))
         auth = BearerAuth(jwt) if jwt else None
 
         if is_connection_string_url(self.connection_string):
-            url = self.connection_string
-            if not url.endswith("/mcp/"):
-                url = url.rstrip("/") + "/mcp/"
-            client = Client(url, auth=auth)
+            client = Client(self.normalize_url(self.connection_string), auth=auth, init_timeout=timeout)
         else:
             from ..util.system import get_subprocess_environment
 
             env = get_subprocess_environment(inherit=env_pass, provided=env_vars)
 
             transport = get_transport_for_command_string(self.connection_string, env=env)
-            client = Client(transport, auth=auth)
+            client = Client(transport, auth=auth, init_timeout=timeout)
 
         try:
             async with client as conn:
@@ -106,12 +119,16 @@ async def connect(self, env_pass: bool = False, env_vars: dict[str, str] | None
                     logger.warning("Connected to %r but ping failed", client)
 
         except Exception as e:
-            logger.error("Failed to connect to MCP server: %s", e)
-            return False
+            # fastmcp's error doesn't say that initialization timed out
+            cause = e.__cause__
+            while cause is not None and not isinstance(cause, TimeoutError):
+                cause = cause.__cause__
+            if timeout and cause is not None:
+                raise TimeoutError(f"server did not initialize within {timeout:g}s") from e
+            raise
 
         self.client = client
         self.connected = True
-        return True
 
     async def call_tool(
         self, tool_name: str, arguments: dict[str, Any] = None
@@ -221,17 +238,27 @@ class BrowserClient:
     current_connection: str | None
     env_pass: bool
     env_vars: dict[str, str] | None
-
-    def __init__(self, env_pass: bool = False, env_vars: dict[str, str] | None = None):
+    timeout: float | None
+
+    def __init__(
+        self,
+        env_pass: bool = False,
+        env_vars: dict[str, str] | None = None,
+        timeout: float | None = DEFAULT_CONNECT_TIMEOUT,
+    ):
         self.connections: dict[str, BrowserConnection] = {}
         self.current_connection: str | None = None
         self.env_pass = env_pass
         self.env_vars = env_vars
+        self.timeout = timeout
+
+    async def add_connection(self, name: str, connection_string: str) -> None:
+        """Add a new MCP connection using FastMCP Client connection string.
 
-    async def add_connection(self, name: str, connection_string: str) -> bool:
-        """Add a new MCP connection using FastMCP Client connection string."""
+        Raises ValueError if the name is taken, or the connection error if connecting fails.
+        """
         if name in self.connections:
-            return False
+            raise ValueError(f"connection name {name!r} already exists")
 
         if is_connection_string_url(connection_string):
             connection_type = "http"
@@ -239,15 +266,11 @@ async def add_connection(self, name: str, connection_string: str) -> bool:
             connection_type = "command"
 
         connection = BrowserConnection(name, connection_type, connection_string)
-        success = await connection.connect(env_pass=self.env_pass, env_vars=self.env_vars)
+        await connection.connect(env_pass=self.env_pass, env_vars=self.env_vars, timeout=self.timeout)
 
-        if success:
-            self.connections[name] = connection
-            if not self.current_connection:
-                self.current_connection = name
-            return True
-        else:
-            return False
+        self.connections[name] = connection
+        if not self.current_connection:
+            self.current_connection = name
 
     async def switch_connection(self, name: str) -> bool:
         """Switch to a different connection."""
diff --git a/magg/mbro/command.py b/magg/mbro/command.py
index f483142..ca4693b 100644
--- a/magg/mbro/command.py
+++ b/magg/mbro/command.py
@@ -1,5 +1,6 @@
 """Command handlers for mbro CLI."""
 
+import difflib
 import json
 from typing import TYPE_CHECKING
 
@@ -62,20 +63,23 @@ async def connect(self, args: list):
         if connection_string.startswith('"') and connection_string.endswith('"'):
             connection_string = connection_string[1:-1]
 
-        success = await self.browser.add_connection(name, connection_string)
-        if success:
-            conn = self.browser.connections[name]
-            tools = await conn.get_tools()
-            resources = await conn.get_resources()
-            prompts = await conn.get_prompts()
-            if not self.cli.quiet:
-                self.formatter.format_success(
-                    f"Connected to '{name}' (Tools: {len(tools)}, Resources: {len(resources)}, Prompts: {len(prompts)})"
-                )
+        try:
+            await self.browser.add_connection(name, connection_string)
+        except Exception as e:
+            error_args = (e,) if self.cli.verbose else ()
+            self.formatter.format_error(f"Failed to connect to {name!r}: {e}", *error_args)
+            return
 
-            await self.cli.refresh_completer_cache()
-        else:
-            self.formatter.format_error(f"Failed to connect to {name!r}")
+        conn = self.browser.connections[name]
+        tools = await conn.get_tools()
+        resources = await conn.get_resources()
+        prompts = await conn.get_prompts()
+        if not self.cli.quiet:
+            self.formatter.format_success(
+                f"Connected to '{name}' (Tools: {len(tools)}, Resources: {len(resources)}, Prompts: {len(prompts)})"
+            )
+
+        await self.cli.refresh_completer_cache()
 
     async def switch(self, args: list):
         """Switch to a different connection."""
@@ -101,15 +105,21 @@ async def disconnect(self, args: list):
         if not success:
             self.formatter.format_error(f"Connection {name!r} not found")
 
-    async def status(self):
+    async def status(self, args: list):
         """Get status of the current connection.
         This includes counts of tools, resources, and prompts.
         """
         conn = self.browser.get_current_connection()
+        if not conn:
+            self.formatter.format_error("No active connection.")
+            return
+
         tools = await conn.get_tools()
         resources = await conn.get_resources()
         prompts = await conn.get_prompts()
-        self.formatter.format_json({"tools": len(tools), "resources": len(resources), "prompts": len(prompts)})
+        self.formatter.format_json(
+            {"connection": conn.name, "tools": len(tools), "resources": len(resources), "prompts": len(prompts)}
+        )
 
     async def tools(self, args: list):
         """List available tools."""
@@ -122,7 +132,11 @@ async def tools(self, args: list):
 
         tools = await conn.get_tools()
         if filter_term:
-            tools = [t for t in tools if filter_term in t["name"].lower() or filter_term in t["description"].lower()]
+            tools = [
+                t
+                for t in tools
+                if filter_term in t["name"].lower() or filter_term in (t.get("description") or "").lower()
+            ]
 
         if not tools:
             self.formatter.format_info(
@@ -169,7 +183,9 @@ async def prompts(self, args: list):
         prompts = await conn.get_prompts()
         if filter_term:
             prompts = [
-                p for p in prompts if filter_term in p["name"].lower() or filter_term in p["description"].lower()
+                p
+                for p in prompts
+                if filter_term in p["name"].lower() or filter_term in (p.get("description") or "").lower()
             ]
 
         if not prompts:
@@ -201,44 +217,16 @@ async def call(self, args: list):
             return
 
         tool_name = args[0]
-        arguments = {}
-
-        if len(args) > 1:
-            args_str = " ".join(args[1:])
-
-            if args_str.strip().startswith("{"):
-                try:
-                    arguments = json.loads(args_str)
-                except json.JSONDecodeError as e:
-                    self.formatter.format_error(f"Invalid JSON arguments: {e}")
-                    if not self.formatter.json_only:
-                        self.formatter.format_info(
-                            "\nJSON formatting tips:\n"
-                            '  - Use double quotes for strings: {"key": "value"}\n'
-                            '  - Numbers don\'t need quotes: {"count": 42}\n'
-                            '  - Booleans: {"enabled": true}\n'
-                            "  - Don't quote the entire JSON object\n"
-                            '  - Example: call tool {"param": "value"}'
-                        )
-                    return
-            else:
-                has_positional = False
-                for arg in args[1:]:
-                    if "=" not in arg and not arg.startswith("{"):
-                        has_positional = True
-                        break
-
-                if has_positional:
-                    self.formatter.format_error("Positional arguments are not supported. Use key=value syntax.")
-                    self.formatter.format_info(f"Example: call {tool_name} a=1 b=2")
-                    return
-
-                arguments = self.cli.parse_shell_args(args[1:])
 
         tools = await conn.get_tools()
         tool = next((t for t in tools if t["name"] == tool_name), None)
+        schema = tool.get("inputSchema", {}) if tool else {}
+
+        arguments = self._parse_arguments(f"call {tool_name}", args[1:], schema)
+        if arguments is None:
+            return
+
         if tool:
-            schema = tool.get("inputSchema", {})
             required = schema.get("required", [])
             if required:
                 missing = [param for param in required if param not in arguments]
@@ -271,6 +259,8 @@ async def call(self, args: list):
         except Exception as e:
             error_args = (e,) if self.cli.verbose else ()
             self.formatter.format_error(str(e), *error_args)
+            if not tool and (matches := difflib.get_close_matches(tool_name, [t["name"] for t in tools])):
+                self.formatter.format_info(f"Did you mean: {', '.join(matches)}?")
 
     async def resource(self, args: list):
         """Get a resource."""
@@ -295,12 +285,13 @@ async def resource(self, args: list):
                     self.formatter.format_resource(result[0])
 
         except Exception as e:
-            self.formatter.format_error(f"Error getting resource: {e}", e)
+            error_args = (e,) if self.cli.verbose else ()
+            self.formatter.format_error(f"Error getting resource: {e}", *error_args)
 
     async def prompt(self, args: list):
         """Get a prompt."""
         if not args:
-            self.formatter.format_error("Usage: prompt <name> [json_arguments]")
+            self.formatter.format_error("Usage: prompt <name> [arguments]")
             return
 
         conn = self.browser.get_current_connection()
@@ -309,14 +300,11 @@ async def prompt(self, args: list):
             return
 
         name = args[0]
-        arguments = {}
 
-        if len(args) > 1:
-            try:
-                arguments = json.loads(" ".join(args[1:]))
-            except json.JSONDecodeError as e:
-                self.formatter.format_error(f"Invalid JSON arguments: {e}")
-                return
+        # Prompt arguments are strings in MCP, so key=value values are passed through as-is
+        arguments = self._parse_arguments(f"prompt {name}", args[1:], strings=True)
+        if arguments is None:
+            return
 
         try:
             result = await conn.get_prompt(name, arguments)
@@ -324,7 +312,8 @@ async def prompt(self, args: list):
             self.formatter.format_prompt_result(result)
 
         except Exception as e:
-            self.formatter.format_error(f"Error getting prompt: {e}", e)
+            error_args = (e,) if self.cli.verbose else ()
+            self.formatter.format_error(f"Error getting prompt: {e}", *error_args)
 
     async def search(self, args: list):
         """Search tools, resources, and prompts."""
@@ -345,8 +334,8 @@ async def search(self, args: list):
 
         def matches_item(item, search_term):
             """Check if item matches search term with word splitting."""
-            name = item.get("name", "").lower()
-            desc = item.get("description", "").lower()
+            name = (item.get("name") or "").lower()
+            desc = (item.get("description") or "").lower()
 
             # Direct substring match
             if search_term in name or search_term in desc:
@@ -417,6 +406,40 @@ async def script(self, args: list):
         """Handle script commands."""
         await self.script_manager.handle_script_command(args)
 
+    def _parse_arguments(
+        self, usage: str, args: list, schema: dict | None = None, *, strings: bool = False
+    ) -> dict | None:
+        """Parse call/prompt arguments given as a JSON object or key=value pairs.
+
+        Returns None if the arguments are invalid (the error has already been reported).
+        """
+        if not args:
+            return {}
+
+        args_str = " ".join(args)
+
+        if args_str.strip().startswith("{"):
+            try:
+                return json.loads(args_str)
+            except json.JSONDecodeError as e:
+                self.formatter.format_error(f"Invalid JSON arguments: {e}")
+                if not self.formatter.json_only:
+                    self.formatter.format_info(
+                        "\nJSON formatting tips:\n"
+                        '  - Use double quotes for strings: {"key": "value"}\n'
+                        '  - Numbers don\'t need quotes: {"count": 42}\n'
+                        '  - Booleans: {"enabled": true}\n'
+                        f'  - Example: {usage} {{"param": "value"}}'
+                    )
+                return None
+
+        if any("=" not in arg for arg in args):
+            self.formatter.format_error("Positional arguments are not supported. Use key=value syntax.")
+            self.formatter.format_info(f"Example: {usage} a=1 b=2")
+            return None
+
+        return self.cli.parse_shell_args(args, schema, strings=strings)
+
     async def _handle_proxy_query_result(self, tool_name: str, result: list) -> bool:
         """
         Handle the ProxyMCP tool's [list, info] actions on [tool, resource, prompt].
diff --git a/magg/mbro/formatter.py b/magg/mbro/formatter.py
index 3c6afb8..99c33e8 100644
--- a/magg/mbro/formatter.py
+++ b/magg/mbro/formatter.py
@@ -20,6 +20,8 @@ def __init__(self, json_only: bool = False, use_rich: bool | None = True, indent
         self.console_stdout = Console(file=sys.stdout) if self.use_rich else None
         self.json_only = json_only
         self.indent = indent if indent > 0 else None
+        # Lets callers tell whether a command reported a failure
+        self.error_count = 0
 
     def format_json(self, data: Any) -> None:
         """Format and print JSON data."""
@@ -32,6 +34,8 @@ def format_json(self, data: Any) -> None:
 
     def format_error(self, message: str, exception: Exception | None = None) -> None:
         """Format and print an error message."""
+        self.error_count += 1
+
         if self.json_only:
             error_data = {"error": message}
             if exception:
@@ -455,12 +459,16 @@ def format_search_results(self, term: str, tools: list, resources: list, prompts
             results = {
                 "query": term,
                 "total_matches": total_matches,
-                "tools": [{"name": t["name"], "description": t["description"]} for t in tools],
+                "tools": [{"name": t["name"], "description": t.get("description")} for t in tools],
                 "resources": [
-                    {"name": r["name"], "uri": r.get("uri", r.get("uriTemplate", "")), "description": r["description"]}
+                    {
+                        "name": r["name"],
+                        "uri": r.get("uri", r.get("uriTemplate", "")),
+                        "description": r.get("description"),
+                    }
                     for r in resources
                 ],
-                "prompts": [{"name": p["name"], "description": p["description"]} for p in prompts],
+                "prompts": [{"name": p["name"], "description": p.get("description")} for p in prompts],
             }
             self.format_json(results)
             return
@@ -525,7 +533,7 @@ def format_help(self, *, enhanced: bool = False) -> None:
                         {"command": "connections, conns", "description": "List all connections"},
                         {"command": "switch <name>", "description": "Switch to a different connection"},
                         {"command": "disconnect <name>", "description": "Disconnect from a server"},
-                        {"command": "status", "description": "Show current status"},
+                        {"command": "status", "description": "Show current connection status"},
                     ],
                     "server_exploration": [
                         {"command": "tools [filter]", "description": "List available tools"},
@@ -539,6 +547,14 @@ def format_help(self, *, enhanced: bool = False) -> None:
                         {"command": "resource <uri>", "description": "Get a resource"},
                         {"command": "prompt <name> [args]", "description": "Get a prompt"},
                     ],
+                    "other": [
+                        {
+                            "command": "script <run|list|search|dump> ...",
+                            "description": "Run or manage .mbro scripts",
+                        },
+                        {"command": "help", "description": "Show this help"},
+                        {"command": "quit, exit", "description": "Exit mbro"},
+                    ],
                 }
             }
             self.format_json(help_data)
@@ -552,7 +568,7 @@ def format_help(self, *, enhanced: bool = False) -> None:
   connections, conns                  - List all connections
   switch <name>                       - Switch to a different connection
   disconnect <name>                   - Disconnect from a server
-  refresh                             - Refresh capabilities for current connection
+  status                              - Show current connection status
 
 Server Exploration:
   tools [filter]                      - List available tools
@@ -566,6 +582,11 @@ def format_help(self, *, enhanced: bool = False) -> None:
   resource <uri>                      - Get a resource
   prompt <name> [args]                - Get a prompt
 
+Other:
+  script <run|list|search|dump> ...   - Run or manage .mbro scripts
+  help                                - Show this help
+  quit, exit                          - Exit mbro
+
 Examples:
   connect magg "http://localhost:8080"
   connect calc "npx @wrtnlabs/calculator-mcp"
@@ -573,6 +594,7 @@ def format_help(self, *, enhanced: bool = False) -> None:
   call magg_status
   call magg_search_tools {"query": "calculator", "limit": 3}
   call add a=5 b=3
+  prompt greet name=Bob
   search calculator
   info tool magg_status
 
diff --git a/magg/mbro/parser.py b/magg/mbro/parser.py
index 688b4ec..a8d33ef 100644
--- a/magg/mbro/parser.py
+++ b/magg/mbro/parser.py
@@ -1,121 +1,10 @@
 """Command line and argument parsing for mbro."""
 
-import json
 import re
 import shlex
-from typing import Any
 
-
-class JsonArgParser:
-    """Parse and validate JSON and shell-style arguments."""
-
-    @staticmethod
-    def parse_command(text: str) -> tuple[str, list[str]]:
-        """Parse a command string into command and arguments."""
-        parts = []
-        current = []
-        in_json = False
-        brace_count = 0
-
-        for char in text:
-            if char == "{" and not in_json:
-                if current:
-                    parts.extend("".join(current).split())
-                    current = []
-                in_json = True
-                brace_count = 1
-                current.append(char)
-            elif in_json:
-                current.append(char)
-                if char == "{":
-                    brace_count += 1
-                elif char == "}":
-                    brace_count -= 1
-                    if brace_count == 0:
-                        parts.append("".join(current))
-                        current = []
-                        in_json = False
-            else:
-                current.append(char)
-
-        if current:
-            if in_json:
-                parts.append("".join(current))
-            else:
-                parts.extend("".join(current).split())
-
-        if not parts:
-            return "", []
-
-        command = parts[0].lower()
-        args = parts[1:]
-
-        args = JsonArgParser._process_arguments(command, args)
-
-        return command, args
-
-    @staticmethod
-    def _process_arguments(command: str, args: list[str]) -> list[str]:
-        """Process and enhance arguments based on command context."""
-        if not args:
-            return args
-
-        if command == "call" and len(args) > 1:
-            tool_name = args[0]
-            remaining = " ".join(args[1:])
-
-            if not remaining.strip().startswith("{"):
-                converted = JsonArgParser._convert_to_json(remaining)
-                if converted:
-                    return [tool_name, converted]
-
-        return args
-
-    @staticmethod
-    def _convert_to_json(arg_string: str) -> str | None:
-        """Try to convert various argument formats to JSON."""
-        if arg_string.strip().startswith("{"):
-            return arg_string
-
-        if "=" in arg_string:
-            args_dict = {}
-            pairs = re.split(r"\s+", arg_string)
-
-            for pair in pairs:
-                if "=" in pair:
-                    key, value = pair.split("=", 1)
-                    args_dict[key] = JsonArgParser._infer_type(value)
-
-            if args_dict:
-                return json.dumps(args_dict)
-
-        return None
-
-    @staticmethod
-    def _infer_type(value: str) -> Any:
-        """Infer the type of a string value."""
-        value = value.strip()
-
-        if (value.startswith('"') and value.endswith('"')) or (value.startswith("'") and value.endswith("'")):
-            return value[1:-1]
-
-        if value.lower() in ("true", "false"):
-            return value.lower() == "true"
-
-        if value.isdigit() or (value.startswith("-") and value[1:].isdigit()):
-            return int(value)
-
-        try:
-            if "." in value:
-                return float(value)
-        except ValueError:
-            pass
-
-        if "," in value:
-            items = [JsonArgParser._infer_type(item.strip()) for item in value.split(",")]
-            return items
-
-        return value
+# call/prompt followed by a JSON object: the object is kept as raw text rather than shell-split
+JSON_ARGS_PATTERN = re.compile(r"(call|prompt)\s+(\S+)\s+(\{.*)", re.DOTALL | re.IGNORECASE)
 
 
 class CommandParser:
@@ -137,6 +26,10 @@ def parse_command_line(line: str) -> list[str]:
         if not cleaned:
             return []
 
+        # shlex would strip the double quotes inside a JSON object
+        if match := JSON_ARGS_PATTERN.fullmatch(cleaned.strip()):
+            return list(match.groups())
+
         try:
             return shlex.split(cleaned)
         except ValueError:
@@ -195,16 +88,54 @@ def split_commands(text: str) -> list[str]:
             i += 1
 
         commands = []
+        pending = ""
         for line in merged_lines:
             line = CommandParser._remove_comments(line)
+            if pending:
+                line = pending + "\n" + line
+                pending = ""
+
             if not line.strip():
                 continue
 
+            # A JSON object spanning lines: keep reading until its braces balance
+            if CommandParser._brace_depth(line) > 0:
+                pending = line
+                continue
+
             parts = CommandParser._split_by_semicolon(line)
             commands.extend(parts)
 
+        if pending:
+            commands.extend(CommandParser._split_by_semicolon(pending))
+
         return [cmd.strip() for cmd in commands if cmd.strip()]
 
+    @staticmethod
+    def _brace_depth(text: str) -> int:
+        """Count braces opened but not closed, ignoring those in quotes."""
+        depth = 0
+        in_single_quote = False
+        in_double_quote = False
+        escaped = False
+
+        for char in text:
+            if escaped:
+                escaped = False
+            elif char == "\\":
+                escaped = True
+            elif char == '"' and not in_single_quote:
+                in_double_quote = not in_double_quote
+            elif char == "'" and not in_double_quote:
+                in_single_quote = not in_single_quote
+            elif not in_single_quote and not in_double_quote:
+                if char == "{":
+                    depth += 1
+                elif char == "}":
+                    depth -= 1
+
+        return depth
+
     @staticmethod
     def _split_by_semicolon(text: str) -> list[str]:
         """Split text by semicolons that aren't in quotes."""
diff --git a/magg/mbro/scripts.py b/magg/mbro/scripts.py
index c66d076..376aff2 100644
--- a/magg/mbro/scripts.py
+++ b/magg/mbro/scripts.py
@@ -102,7 +102,7 @@ async def run_script(self, args: List[str]):
         script_path = self.find_script(script_ref)
         if not script_path:
             self.formatter.format_error(f"Script not found: {script_ref!r}")
-            exit(1)
+            return
 
         try:
             script_content = script_path.read_text()
@@ -110,13 +110,7 @@ async def run_script(self, args: List[str]):
             self.formatter.format_error(f"Failed to read script: {e}")
             return
 
-        commands = CommandParser.split_commands(script_content)
-
-        for command in commands:
-            if command.strip():
-                if self.cli.verbose:
-                    self.formatter.format_info(f"> {command}")
-                await self.cli.handle_command(command)
+        await self.cli.run_commands(CommandParser.split_commands(script_content))
 
     async def list_scripts(self, args: List[str]):
         """List available scripts with optional filter."""
@@ -203,7 +197,7 @@ async def dump_script(self, args: List[str]):
         script_path = self.find_script(script_ref)
         if not script_path:
             self.formatter.format_error(f"Script not found: {script_ref!r}")
-            exit(1)
+            return
 
         try:
             script_content = script_path.read_text()
@@ -219,12 +213,7 @@ async def dump_script(self, args: List[str]):
             )
 
             if edited is not None:
-                commands = CommandParser.split_commands(edited)
-                for command in commands:
-                    if command.strip():
-                        if self.cli.verbose:
-                            self.formatter.format_info(f"> {command}")
-                        await self.cli.handle_command(command)
+                await self.cli.run_commands(CommandParser.split_commands(edited))
         else:
             self.formatter.format_info(f"Script: {script_path.name}")
             self.formatter.print(script_content)
diff --git a/test/magg/test_mbro_cli_overhaul.py b/test/magg/test_mbro_cli_overhaul.py
index 3587a5b..a77d6bf 100644
--- a/test/magg/test_mbro_cli_overhaul.py
+++ b/test/magg/test_mbro_cli_overhaul.py
@@ -77,7 +77,7 @@ class TestMBroCLIOverhaul:
     async def test_handle_commands_from_args(self):
         """Test handling commands from command line args."""
         cli = MagicMock()
-        cli.handle_command = AsyncMock()
+        cli.run_commands = AsyncMock()
 
         args = MagicMock()
         args.commands = ["connect test python server.py", ";", "tools"]
@@ -85,15 +85,13 @@ async def test_handle_commands_from_args(self):
         executed = await handle_commands(cli, args)
 
         assert executed is True
-        assert cli.handle_command.call_count == 2
-        cli.handle_command.assert_any_call("connect test python server.py")
-        cli.handle_command.assert_any_call("tools")
+        cli.run_commands.assert_awaited_once_with(["connect test python server.py", "tools"])
 
     @pytest.mark.asyncio
     async def test_handle_commands_from_stdin(self):
         """Test handling commands from stdin."""
         cli = MagicMock()
-        cli.handle_command = AsyncMock()
+        cli.run_commands = AsyncMock()
 
         args = MagicMock()
         args.commands = ["-"]
@@ -104,9 +102,7 @@ async def test_handle_commands_from_stdin(self):
             executed = await handle_commands(cli, args)
 
         assert executed is True
-        assert cli.handle_command.call_count == 2
-        cli.handle_command.assert_any_call("connect test python server.py")
-        cli.handle_command.assert_any_call("tools")
+        cli.run_commands.assert_awaited_once_with(["connect test python server.py", "tools"])
 
     def test_cli_command_parsing_with_comments(self):
         """Test that CLI properly handles comments."""
diff --git a/test/magg/test_mbro_commands.py b/test/magg/test_mbro_commands.py
new file mode 100644
index 0000000..9993368
--- /dev/null
+++ b/test/magg/test_mbro_commands.py
@@ -0,0 +1,331 @@
+"""Tests for mbro command handling: argument parsing, errors, exit codes."""
+
+import shutil
+import sys
+from unittest.mock import AsyncMock, patch
+
+import pytest
+from fastmcp import Client, FastMCP
+
+from magg.mbro import cli as cli_module
+from magg.mbro.cli import MCPBrowserCLI, main_async
+from magg.mbro.client import BrowserConnection
+from magg.mbro.parser import CommandParser
+
+
+def make_server() -> FastMCP:
+    mcp = FastMCP("test")
+
+    @mcp.tool(description="Echo a string value")
+    def echo(value: str) -> str:
+        return f"echo:{value!r}"
+
+    @mcp.tool
+    def nodesc(n: int) -> str:
+        return f"n:{n!r}"
+
+    @mcp.tool
+    def anyval(value: object = None) -> str:
+        return f"any:{value!r}"
+
+    @mcp.prompt
+    def greet(name: str) -> str:
+        return f"Hello {name}"
+
+    @mcp.resource("res://nodesc")
+    def nodesc_res() -> str:
+        return "resource-content"
+
+    return mcp
+
+
+@pytest.fixture
+def cli(tmp_path, monkeypatch):
+    monkeypatch.setenv("MAGG_PATH", str(tmp_path))
+    monkeypatch.setenv("MAGG_CONFIG_PATH", str(tmp_path / "config.json"))
+    return MCPBrowserCLI(use_rich=False)
+
+
+async def run(cli: MCPBrowserCLI, *commands: str) -> bool:
+    """Run commands, returning whether all of them succeeded."""
+    return await cli.run_commands(list(commands))
+
+
+def attach(cli: MCPBrowserCLI, name: str = "t") -> None:
+    """Attach an in-memory connection to the test server."""
+    conn = BrowserConnection(name, "memory", "memory")
+    conn.client = Client(make_server())
+    conn.connected = True
+    cli.browser.connections[name] = conn
+    cli.browser.current_connection = name
+
+
+class TestJsonArguments:
+    """JSON arguments for call/prompt keep their inner quotes."""
+
+    def test_unquoted_json(self):
+        parts = CommandParser.parse_command_line('call echo {"value": "hello world"}')
+        assert parts == ["call", "echo", '{"value": "hello world"}']
+
+    def test_quoted_json(self):
+        parts = CommandParser.parse_command_line("""call echo '{"value": "hello world"}'""")
+        assert parts == ["call", "echo", '{"value": "hello world"}']
+
+    def test_prompt_json(self):
+        parts = CommandParser.parse_command_line('prompt greet {"name": "Bob"}')
+        assert parts == ["prompt", "greet", '{"name": "Bob"}']
+
+    def test_other_commands_still_shell_split(self):
+        assert CommandParser.parse_command_line('search "a b"') == ["search", "a b"]
+
+    def test_multiline_json_in_script(self):
+        script = (
+            "connect t python server.py\n"
+            "call echo {\n"
+            '  "value": "a # b; c",  # trailing comment\n'
+            "\n"
+            "  \"other\": 'x'\n"
+            "}\n"
+            "tools\n"
+        )
+        assert CommandParser.split_commands(script) == [
+            "connect t python server.py",
+            'call echo {\n  "value": "a # b; c",\n\n  "other": \'x\'\n}',
+            "tools",
+        ]
+
+    def test_braces_in_quotes_do_not_join_lines(self):
+        commands = CommandParser.split_commands('call echo value="{"\ntools')
+        assert commands == ['call echo value="{"', "tools"]
+
+    @pytest.mark.asyncio
+    async def test_call_forms(self, cli, capsys):
+        attach(cli)
+        commands = [
+            "call echo value=kv",
+            'call echo {"value": "unquoted"}',
+            """call echo '{"value": "quoted"}'""",
+            *CommandParser.split_commands('call echo {\n  "value": "multi"\n}'),
+        ]
+        assert await cli.run_commands(commands)
+
+        out = capsys.readouterr().err
+        for value in ("kv", "unquoted", "quoted", "multi"):
+            assert f"echo:'{value}'" in out
+
+    @pytest.mark.asyncio
+    async def test_invalid_json_fails(self, cli, capsys):
+        attach(cli)
+        assert not await run(cli, "call echo {value: 1}")
+        assert "Invalid JSON arguments" in capsys.readouterr().err
+
+
+class TestValueParsing:
+    """key=value parsing for call/prompt."""
+
+    def test_without_schema(self):
+        args = ["a=007", "b=10-20", "c=5-", "d=null", "e=[1,2]", "f=1e5", "g=42", "h=-3", "i=True", "j=my server"]
+        assert MCPBrowserCLI.parse_shell_args(args) == {
+            "a": "007",
+            "b": "10-20",
+            "c": "5-",
+            "d": None,
+            "e": [1, 2],
+            "f": 100000.0,
+            "g": 42,
+            "h": -3,
+            "i": True,
+            "j": "my server",
+        }
+
+    def test_string_params_stay_strings(self):
+        schema = {
+            "properties": {
+                "s": {"type": "string"},
+                "opt": {"anyOf": [{"type": "string"}, {"type": "null"}]},
+                "n": {"type": "integer"},
+                "u": {"anyOf": [{"type": "string"}, {"type": "integer"}]},
+            }
+        }
+        args = ["s=123", "opt=true", "n=5", "u=5"]
+        assert MCPBrowserCLI.parse_shell_args(args, schema) == {"s": "123", "opt": "true", "n": 5, "u": 5}
+
+    def test_strings_flag(self):
+        assert MCPBrowserCLI.parse_shell_args(["a=5", "b=null"], strings=True) == {"a": "5", "b": "null"}
+
+    @pytest.mark.asyncio
+    async def test_call_uses_schema(self, cli, capsys):
+        attach(cli)
+        assert await cli.run_commands(["call echo value=10-20", "call echo value=007", "call nodesc n=5"])
+
+        out = capsys.readouterr().err
+        assert "echo:'10-20'" in out
+        assert "echo:'007'" in out
+        assert "n:5" in out
+
+    @pytest.mark.asyncio
+    async def test_prompt_key_value(self, cli, capsys):
+        attach(cli)
+        assert await run(cli, "prompt greet name=Bob")
+        assert "Hello Bob" in capsys.readouterr().err
+
+    @pytest.mark.asyncio
+    async def test_positional_rejected(self, cli, capsys):
+        attach(cli)
+        assert not await run(cli, "prompt greet Bob")
+        assert "Positional arguments are not supported" in capsys.readouterr().err
+
+
+class TestCommands:
+    """Individual command behavior."""
+
+    @pytest.mark.asyncio
+    async def test_status_without_connection(self, cli, capsys):
+        assert not await run(cli, "status")
+        assert "No active connection" in capsys.readouterr().err
+
+    @pytest.mark.asyncio
+    async def test_status(self, cli, capsys):
+        attach(cli)
+        assert await run(cli, "status")
+        assert '"tools": 3' in capsys.readouterr().out
+
+    @pytest.mark.asyncio
+    @pytest.mark.parametrize("command", ["tools zzz", "prompts zzz", "search zzz", "search nodesc"])
+    async def test_none_descriptions(self, cli, command):
+        attach(cli)
+        assert await run(cli, command)
+
+    @pytest.mark.asyncio
+    async def test_search_json_resource_without_description(self, tmp_path, monkeypatch, capsys):
+        monkeypatch.setenv("MAGG_PATH", str(tmp_path))
+        cli = MCPBrowserCLI(json_only=True, use_rich=False)
+        attach(cli)
+        assert await run(cli, "search nodesc")
+        out = capsys.readouterr().out
+        assert '"nodesc_res"' in out
+
+    @pytest.mark.asyncio
+    async def test_unknown_tool_suggestion(self, cli, capsys):
+        attach(cli)
+        assert not await run(cli, "call ech value=x")
+        assert "Did you mean: echo?" in capsys.readouterr().err
+
+    @pytest.mark.asyncio
+    async def test_duplicate_connection_name(self, cli, capsys):
+        attach(cli)
+        assert not await run(cli, "connect t python server.py")
+        assert "connection name 't' already exists" in capsys.readouterr().err
+
+    @pytest.mark.asyncio
+    async def test_connect_failure_reason(self, cli, capfd):
+        # capfd rather than capsys: the stdio transport needs a real stderr fd
+        assert not await run(cli, "connect nx this-command-does-not-exist-mbro")
+        assert "No such file or directory" in capfd.readouterr().err
+
+    @pytest.mark.asyncio
+    @pytest.mark.skipif(not shutil.which("sleep"), reason="needs sleep")
+    async def test_connect_timeout(self):
+        conn = BrowserConnection("s", "command", "sleep 100")
+        with pytest.raises(TimeoutError, match="0.5s"):
+            await conn.connect(timeout=0.5)
+
+    @pytest.mark.asyncio
+    async def test_missing_script_does_not_exit(self, cli, capsys):
+        assert not await run(cli, "script run nope")
+        assert "Script not found" in capsys.readouterr().err
+        assert cli.running
+
+    @pytest.mark.asyncio
+    async def test_repl_without_arepl(self, cli, capsys):
+        with patch.object(cli_module, "arepl", None):
+            await cli.start(repl=True)
+        assert "only available" in capsys.readouterr().err
+
+
+class TestUrlNormalization:
+    @pytest.mark.parametrize(
+        "url, expected",
+        [
+            ("http://h:1", "http://h:1/mcp/"),
+            ("http://h:1/", "http://h:1/mcp/"),
+            ("http://h:1?x=1", "http://h:1/mcp/?x=1"),
+            ("http://h:1/mcp", "http://h:1/mcp"),
+            ("http://h:1/mcp/", "http://h:1/mcp/"),
+            ("http://h:1/sse", "http://h:1/sse"),
+            ("https://h/api/mcp", "https://h/api/mcp"),
+        ],
+    )
+    def test_normalize_url(self, url, expected):
+        assert BrowserConnection.normalize_url(url) == expected
+
+
+class TestBatchExecution:
+    """Error handling and exit codes when running command batches."""
+
+    @pytest.mark.asyncio
+    async def test_continues_after_failures(self, cli, capsys):
+        attach(cli)
+        assert not await cli.run_commands(["bogus", "call echo value=after"])
+        assert cli.failed
+        assert "echo:'after'" in capsys.readouterr().err
+
+    @pytest.mark.asyncio
+    async def test_unexpected_exception_is_contained(self, cli, capsys):
+        attach(cli)
+        with patch.object(cli.command, "tools", AsyncMock(side_effect=RuntimeError("boom"))):
+            assert not await cli.run_commands(["tools", "call echo value=after"])
+        assert "echo:'after'" in capsys.readouterr().err
+
+    @pytest.mark.asyncio
+    async def test_fail_fast(self, tmp_path, monkeypatch, capsys):
+        monkeypatch.setenv("MAGG_PATH", str(tmp_path))
+        cli = MCPBrowserCLI(use_rich=False, fail_fast=True)
+        attach(cli)
+        assert not await cli.run_commands(["bogus", "call echo value=after"])
+        assert "echo:'after'" not in capsys.readouterr().err
+
+    @pytest.mark.asyncio
+    async def test_quit_stops_script(self, cli, tmp_path, capsys):
+        attach(cli)
+        script = tmp_path / "s.mbro"
+        script.write_text("call echo value=before\nquit\ncall echo value=after\n")
+        assert await cli.run_commands([f"script run {script}", "call echo value=next"])
+
+        out = capsys.readouterr().err
+        assert "echo:'before'" in out
+        assert "after" not in out
+        assert "next" not in out
+
+    @staticmethod
+    async def run_main(monkeypatch, *argv: str) -> int:
+        monkeypatch.setattr(sys, "argv", ["mbro", "--no-rich", *argv])
+        try:
+            await main_async()
+        except SystemExit as e:
+            return e.code
+        return 0
+
+    @pytest.mark.asyncio
+    @pytest.mark.parametrize(
+        "command, code",
+        [("help", 0), ("bogus", 1), ("status", 1), ("tools", 1), ('call echo {"value": 1}', 1)],
+    )
+    async def test_exit_codes(self, command, code, tmp_path, monkeypatch):
+        monkeypatch.setenv("MAGG_PATH", str(tmp_path))
+        assert await self.run_main(monkeypatch, "-n", command) == code
+
+    @pytest.mark.asyncio
+    async def test_script_exit_code(self, tmp_path, monkeypatch):
+        monkeypatch.setenv("MAGG_PATH", str(tmp_path))
+        assert await self.run_main(monkeypatch, "-X", str(tmp_path / "missing.mbro")) == 1
+
+    @pytest.mark.asyncio
+    async def test_stdin_is_non_interactive(self, tmp_path, monkeypatch):
+        monkeypatch.setenv("MAGG_PATH", str(tmp_path))
+        with (
+            patch("sys.stdin.read", return_value="help\n"),
+            patch.object(MCPBrowserCLI, "start", AsyncMock()) as start,
+        ):
+            assert await self.run_main(monkeypatch, "-") == 0
+        start.assert_not_called()

From 44feea74ec5c359b178f89a1b5513104c6351fe6 Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Fri, 25 Sep 2026 20:14:13 +0000
Subject: [PATCH 07/14] Align mbro docs with actual commands and options

Replace nonexistent --connect/--call-tool flags and `mbro scripts`,
document `disconnect <name>`, script subcommands, script discovery
paths, JSON/key=value value handling, exit status, --fail-fast,
--timeout, URL defaults, and fix the REPL example.
---
 docs/mbro.md | 79 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 27 deletions(-)

diff --git a/docs/mbro.md b/docs/mbro.md
index 103d2b9..adaa06d 100644
--- a/docs/mbro.md
+++ b/docs/mbro.md
@@ -61,13 +61,18 @@ mbro connect calc npx -y @modelcontextprotocol/server-calculator \; tools
 # Multiple commands separated by semicolons
 mbro connect magg http://localhost:8080 \; call magg_status
 
-# Read commands from stdin
+# Run commands and exit instead of dropping into interactive mode
+mbro -n connect calc npx -y @modelcontextprotocol/server-calculator \; call add a=5 b=3
+
+# Read commands from stdin (runs non-interactively)
 echo "connect calc npx calculator; tools" | mbro -
 
 # Execute a script file
 mbro -x setup.mbro
 ```
 
+In non-interactive mode (`-n`, `-X`, or `-`), mbro exits with status 1 if any command failed (unknown command, failed connection, no active connection, tool error, invalid arguments, ...). Commands keep running after a failure unless `--fail-fast` is given.
+
 ### Authentication
 
 When connecting to HTTP servers that require bearer token authentication, mbro automatically checks for JWT tokens in these environment variables (in order):
@@ -123,7 +128,8 @@ mbro:memory> prompt summarize topic=recent_memories max_length=500
 | `connections` | List all connections | `connections` |
 | `connect <name> <connection>` | Connect to a server | `connect calc npx -y @modelcontextprotocol/server-calculator` |
 | `switch <name>` | Switch to another connection | `switch calc` |
-| `disconnect` | Disconnect from current server | `disconnect` |
+| `disconnect <name>` | Disconnect from a server | `disconnect calc` |
+| `status` | Show the current connection's tool, resource, and prompt counts | `status` |
 
 ### Tool Operations
 
@@ -152,13 +158,17 @@ mbro:memory> prompt summarize topic=recent_memories max_length=500
 |---------|-------------|---------|
 | `search <query>` | Search tools, resources, and prompts | `search add` |
 | `info <type> <name>` | Get detailed info | `info tool add` |
-| `status` | Show connection status | `status` |
 
 ### Utility Commands
 
 | Command | Description | Example |
 |---------|-------------|---------|
-| `exit` / `quit` | Exit mbro | `exit` |
+| `script run <name>` | Run a script by name or path | `script run setup` |
+| `script list [filter]` | List discovered scripts | `script list` |
+| `script search <regex>` | Search script names and contents | `script search calc` |
+| `script dump <name>` | Show (or edit and run) a script | `script dump setup` |
+| `help` | Show available commands | `help` |
+| `exit` / `quit` | Exit mbro (also stops a running script) | `exit` |
 
 ## Advanced Features
 
@@ -205,20 +215,21 @@ mbro -x setup.mbro -n
 - **Comments**: Lines starting with `#` are ignored
 - **Empty lines**: Ignored for readability
 - **Line continuation**: Use backslash `\` at end of line
+- **Multi-line JSON**: A JSON object argument can span lines until its braces balance
 - **All commands supported**: Any interactive command works in scripts
-- **Error handling**: Scripts continue on errors unless fatal
+- **Error handling**: Scripts continue after a failed command (or stop with `--fail-fast`); `quit` stops the script
 
 #### Script Discovery
 
 mbro can discover scripts in configured paths:
 ```bash
 # List available scripts
-mbro scripts
+mbro -n script list
 
-# Scripts are searched in:
-# - Current directory: ./.mbro/
-# - Home directory: ~/.mbro/
-# - MAGG_PATH directories
+# Scripts (*.mbro, searched recursively) are found in the Magg path:
+# - Project directory: ./.magg/
+# - Home directory: ~/.magg/
+# - Or the directories in MAGG_PATH (colon-separated)
 ```
 
 #### Example Scripts
@@ -247,7 +258,7 @@ call get_test_results format=json
 
 # Cleanup
 call cleanup_test_data
-disconnect
+disconnect test
 ```
 
 ### Modern CLI Features
@@ -304,12 +315,12 @@ Tab completion provides rich parameter information:
 - **ESC to cancel**: Exit completion without selecting
 - **Smart parsing**: Detects existing parameters to avoid duplicates
 
-#### 💡 Intelligent Error Suggestions
-When errors occur, get helpful suggestions:
+#### 💡 Tool Name Suggestions
+Calling an unknown tool suggests close matches:
 ```bash
 mbro> call wether {"city": "London"}
-Error: Tool 'wether' not found.
-Did you mean: weather, whether_tool?
+Error: Failed to call tool 'wether': Unknown tool: 'wether'
+Did you mean: weather?
 ```
 
 #### ⚡ Search
@@ -335,10 +346,11 @@ mbro supports multiple ways to provide tool arguments:
 
 #### JSON Format (Traditional)
 ```bash
-# In interactive mode, use JSON directly (no surrounding quotes)
+# In interactive mode and scripts, use JSON directly (surrounding single quotes are optional)
 call search {"query": "python tutorials", "limit": 10}
+call search '{"query": "python tutorials", "limit": 10}'
 
-# Complex nested structures
+# Complex nested structures (can span multiple lines)
 call create_task {
   "title": "Review PR",
   "details": {
@@ -348,10 +360,12 @@ call create_task {
   "tags": ["urgent", "review"]
 }
 
-# From command line, quotes may be needed for shell parsing
-mbro --call-tool search '{"query": "python tutorials"}'
+# From the command line, quote the JSON so the shell keeps its double quotes
+mbro -n connect s python server.py \; call search '{"query": "python tutorials"}'
 ```
 
+Prompts accept the same formats: `prompt greet name=Bob` or `prompt greet {"name": "Bob"}`.
+
 #### Shell-Style Key=Value Format (New!)
 ```bash
 # Simple key=value pairs (no quotes needed for single words)
@@ -367,6 +381,8 @@ call configure debug=true port=8080 timeout=30.5
 call create_task title="Review PR" details='{"repo": "myproject", "pr_number": 123}'
 ```
 
+Values are decoded as JSON (numbers, `true`/`false`, `null`, arrays, objects) and anything that isn't valid JSON stays a string, so `code=007` and `range=10-20` are passed as strings. If the tool's input schema declares a parameter as a string, its value is always passed as-is (`id=12345` stays `"12345"`). Prompt arguments are always strings.
+
 #### Multiline Support
 ```bash
 # Press Enter to continue on next line
@@ -400,6 +416,10 @@ mbro connect myserver python -m mypackage.mcp_server \; tools
 mbro connect api http://localhost:3000 \; status
 ```
 
+A URL without a path (like `http://localhost:3000`) connects to the default `/mcp/` endpoint; URLs with a path (`/mcp`, `/sse`, ...) are used as given.
+
+Connecting fails with the underlying reason (for example, a missing command or a refused connection). If the server doesn't finish initializing within 30 seconds, the connection attempt times out; use `--timeout SECONDS` to change this (`0` disables it).
+
 ### Running Magg in stdio Mode
 
 A unique feature of mbro is the ability to run Magg directly in stdio mode for quick inspection:
@@ -446,16 +466,17 @@ mbro --repl
 ```
 
 In the Python REPL, you have access to:
-- `current_connection`: The active MCP connection object
+- `current_connection`: The connection that was active when the REPL started (e.g. from a `-x` script), or `None`
 - `self`: The MCPBrowserCLI instance for executing commands
 - All standard Python functionality with async/await support
 
 Example REPL session:
 ```python
 >>> # Direct access to the current connection
->>> tools = await current_connection.list_tools()
->>> print(tools[0].name)
-'add'
+>>> conn = self.browser.get_current_connection()
+>>> tools = await conn.get_tools()
+>>> print(tools[0]["name"])
+add
 
 >>> # Execute mbro commands via self
 >>> await self.handle_command('tools')
@@ -464,7 +485,7 @@ Available tools:
   ...
 
 >>> # Call a tool directly
->>> result = await current_connection.call_tool('add', {'a': 5, 'b': 3})
+>>> result = await conn.call_tool('add', {'a': 5, 'b': 3})
 >>> print(result)
 [TextContent(type='text', text='8')]
 ```
@@ -483,7 +504,7 @@ When used with Magg, mbro can browse the aggregated server:
 magg serve --http --port 8000
 
 # In another terminal, connect mbro to Magg
-mbro --connect magg http://localhost:8000
+mbro connect magg http://localhost:8000/mcp
 ```
 
 This allows you to use mbro to explore all tools from all servers managed by Magg through a single interface.
@@ -592,7 +613,7 @@ $ cat > weather_check.mbro << 'EOF'
 # Weather check script
 connect weather npx -y @modelcontextprotocol/server-weather
 call get_forecast location="San Francisco" days=3
-disconnect
+disconnect weather
 quit
 EOF
 
@@ -680,10 +701,14 @@ mbro -q connect calc npx -y @modelcontextprotocol/server-calculator \; call add
    - `-n` / `--no-interactive` - Don't drop into interactive mode after commands
    - `-x SCRIPT` / `--execute-script SCRIPT` - Execute .mbro script file (can be used multiple times)
    - `-X SCRIPT` / `--execute-script-n SCRIPT` - Execute script in non-interactive mode (equivalent to -n -x)
+   - `--status-bar` - Show status bar with keyboard shortcuts
+   - `--fail-fast` - Stop running commands and scripts at the first failure
+   - `--timeout SECONDS` - Server initialization timeout when connecting (0 to disable, default: 30)
    - `--help` - Show help message
 
 Special command line usage:
-   - Use `-` as command to read from stdin
+   - Use `-` as command to read commands from stdin (non-interactive)
+   - In non-interactive mode, the exit status is 1 if any command failed
    - Use `\;` to separate multiple commands (escape semicolon)
    - Scripts provide reusable command sequences
    - Minimal quoting: only quote values with spaces

From 4a8819276ef5c8a47431a215dafd4f08fae52d19 Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Fri, 25 Sep 2026 20:17:19 +0000
Subject: [PATCH 08/14] Fix server robustness bugs: unmount, hung backends,
 signals, read-only, auth

- Unmount on fastmcp 3: track the provider each mount adds and remove it
  from mcp.providers, so disable/remove/update actually drop or replace a
  backend's tools (previously a no-op that kept stale providers).
- Hung backends: pass backend_init_timeout to backend clients and resolve
  Magg's own tools/resources/prompts locally first, so one unresponsive
  backend no longer blocks list_tools or Magg's tools.
- Suppress backend stderr via the stdio transport's log_file; drop the
  stdio_client monkeypatch that fastmcp bypassed.
- Per-server env now overrides --env-pass/--env-set.
- magg_add_server: accept uri (AnyUrl -> str), require command or uri,
  report the mount failure cause, roll back the mount if saving fails.
- magg_check: use MountedServer.client (it crashed with any mount) and
  iterate over a snapshot of mounted servers.
- Read-only mode is checked up front in add/remove/enable/disable, kit
  load/unload, and non-report check actions.
- Refuse to serve HTTP/hybrid when a configured private key can't be
  loaded instead of silently running without auth.
- Runner: use loop signal handlers so SIGTERM wakes an idle stdio loop,
  bound the wait on the cancelled server task, exit via the signal once
  cleanup is done, force exit on a second signal, and restore SIG_DFL.
- Proxy tool: path is required for 'info' (fix description).
---
 magg/proxy/mixin.py                 |   2 +-
 magg/proxy/server.py                |  33 ++-
 magg/server/defaults.py             |   2 +-
 magg/server/manager.py              | 117 ++++----
 magg/server/runner.py               |  93 +++++--
 magg/server/server.py               |  29 +-
 magg/util/stdio_patch.py            |  43 ---
 test/magg/test_config_reload.py     |   5 +-
 test/magg/test_kit_tools.py         |   1 +
 test/magg/test_server_robustness.py | 412 ++++++++++++++++++++++++++++
 10 files changed, 601 insertions(+), 136 deletions(-)
 delete mode 100644 magg/util/stdio_patch.py
 create mode 100644 test/magg/test_server_robustness.py

diff --git a/magg/proxy/mixin.py b/magg/proxy/mixin.py
index 792a907..f5c9a84 100644
--- a/magg/proxy/mixin.py
+++ b/magg/proxy/mixin.py
@@ -84,7 +84,7 @@ async def _proxy_tool(
             str | None,
             Field(
                 description="Name or URI of the specific tool/resource/prompt (with FastMCP prefixing).\n"
-                "Not allowed for 'list' and 'info' actions.",
+                "Required for 'info' and 'call' actions; not allowed for 'list'.",
                 # validation_alias=AliasChoices("name", "uri"),
             ),
         ] = None,
diff --git a/magg/proxy/server.py b/magg/proxy/server.py
index 5cb189c..60a1700 100644
--- a/magg/proxy/server.py
+++ b/magg/proxy/server.py
@@ -7,7 +7,10 @@
 from fastmcp import Client, FastMCP
 from fastmcp.client import FastMCPTransport
 from fastmcp.client.messages import MessageHandler
-from fastmcp.tools import FunctionTool
+from fastmcp.prompts import Prompt
+from fastmcp.resources import Resource, ResourceTemplate
+from fastmcp.tools import FunctionTool, Tool
+from fastmcp.utilities.versions import VersionSpec
 
 from ..messaging import MessageRouter, ServerMessageCoordinator
 from .mixin import ProxyMCP
@@ -67,6 +70,34 @@ def _proxy_backend_client(self) -> Client:
         transport = FastMCPTransport(self)
         return Client(transport)
 
+    # FastMCP resolves a component by querying every provider in parallel, so one slow or hung
+    # (unprefixed) backend would delay every lookup. Resolve our own components first; local
+    # components take precedence over providers anyway. Auth-guarded ones go the normal route.
+
+    async def _get_tool(self, name: str, version: VersionSpec | None = None) -> Tool | None:
+        tool = await self.local_provider.get_tool(name, version)
+        if tool is not None and tool.auth is None:
+            return tool
+        return await super()._get_tool(name, version)
+
+    async def _get_resource(self, uri: str, version: VersionSpec | None = None) -> Resource | None:
+        resource = await self.local_provider.get_resource(uri, version)
+        if resource is not None and resource.auth is None:
+            return resource
+        return await super()._get_resource(uri, version)
+
+    async def _get_resource_template(self, uri: str, version: VersionSpec | None = None) -> ResourceTemplate | None:
+        template = await self.local_provider.get_resource_template(uri, version)
+        if template is not None and template.auth is None:
+            return template
+        return await super()._get_resource_template(uri, version)
+
+    async def _get_prompt(self, name: str, version: VersionSpec | None = None) -> Prompt | None:
+        prompt = await self.local_provider.get_prompt(name, version)
+        if prompt is not None and prompt.auth is None:
+            return prompt
+        return await super()._get_prompt(name, version)
+
     def _register_proxy_tool(self):
         tool = FunctionTool.from_function(
             self._proxy_tool,
diff --git a/magg/server/defaults.py b/magg/server/defaults.py
index e880edd..162facd 100644
--- a/magg/server/defaults.py
+++ b/magg/server/defaults.py
@@ -124,7 +124,7 @@
     Can be provided as a dict or JSON string (automatically parsed).
   path (string | null) (optional)
     Name or URI of the specific tool/resource/prompt (with FastMCP prefixing).
-    Not allowed for 'list' and 'info' actions.
+    Required for 'info' and 'call' actions; not allowed for 'list'.
   limit (integer | null) (optional)
     Maximum number of items to return (for 'list' action only). Default: 100, Max: 1000
   offset (integer | null) (optional)
diff --git a/magg/server/manager.py b/magg/server/manager.py
index ccfb7cf..56b89e9 100644
--- a/magg/server/manager.py
+++ b/magg/server/manager.py
@@ -5,10 +5,14 @@
 import os
 from dataclasses import dataclass
 from functools import cached_property
+from pathlib import Path
 from typing import Annotated
 
 from fastmcp import Client, FastMCP
+from fastmcp.client.transports import StdioTransport
 from fastmcp.server import create_proxy
+from fastmcp.server.providers import FastMCPProvider, Provider
+from fastmcp.server.transforms import Namespace
 from pydantic import Field
 
 from ..auth import BearerAuthManager
@@ -16,7 +20,6 @@
 from ..proxy.server import BackendMessageHandler, ProxyFastMCP
 from ..reload import ConfigChange, ServerChange
 from ..settings import ConfigManager, MaggConfig, ServerConfig
-from ..util.stdio_patch import patch_stdio_transport_stderr
 from ..util.transport import get_transport_for_command, get_transport_for_uri
 from .defaults import MAGG_INSTRUCTIONS
 from .response import MaggResponse
@@ -30,6 +33,7 @@ class MountedServer:
 
     proxy: FastMCP
     client: Client
+    provider: Provider | None = None
 
 
 class ServerManager:
@@ -38,7 +42,9 @@ class ServerManager:
     config_manager: ConfigManager
     mcp: ProxyFastMCP
     mounted_servers: dict[str, MountedServer]
+    mount_errors: dict[str, str]
     subprocess_env: dict | None = None
+    auth_error: str | None = None
 
     def __init__(self, config_manager: ConfigManager, *, env: dict | None = None):
         self.config_manager = config_manager
@@ -52,8 +58,12 @@ def __init__(self, config_manager: ConfigManager, *, env: dict | None = None):
             try:
                 auth_provider = auth_manager.provider
                 logger.debug("Authentication enabled (bearer)")
-            except RuntimeError as e:
-                logger.warning("Authentication disabled: %s", e)
+            except RuntimeError:
+                bearer = auth_config.bearer
+                key_source = "MAGG_PRIVATE_KEY" if bearer.private_key_env else str(bearer.private_key_path)
+                self.auth_error = f"Private key from {key_source} could not be loaded (invalid or passphrase-protected)"
+                # Only fatal when serving HTTP (see require_http_auth); stdio does not use auth
+                logger.warning("%s; bearer authentication is unavailable", self.auth_error)
 
         self.mcp = ProxyFastMCP(
             name=self.self_prefix,
@@ -61,6 +71,12 @@ def __init__(self, config_manager: ConfigManager, *, env: dict | None = None):
             auth=auth_provider,
         )
         self.mounted_servers = {}
+        self.mount_errors = {}
+
+    def require_http_auth(self) -> None:
+        """Refuse to serve HTTP when a configured private key failed to load (fail closed)."""
+        if self.auth_error:
+            raise RuntimeError(f"{self.auth_error}; refusing to serve HTTP without authentication")
 
     @property
     def config(self) -> MaggConfig:
@@ -94,6 +110,7 @@ async def mount_server(self, server: ServerConfig) -> bool:
             return False
 
         try:
+            config = self.config
             message_handler = BackendMessageHandler(server_id=server.name, coordinator=self.mcp.message_coordinator)
 
             if server.command:
@@ -102,12 +119,13 @@ async def mount_server(self, server: ServerConfig) -> bool:
                 if server.env or self.subprocess_env:
                     env = {}
 
-                    if server.env:
-                        env.update(server.env)
-
+                    # Per-server env takes precedence over the inherited/provided environment
                     if self.subprocess_env:
                         env.update(self.subprocess_env)
 
+                    if server.env:
+                        env.update(server.env)
+
                 transport = get_transport_for_command(
                     command=server.command,
                     args=server.args or [],
@@ -116,91 +134,57 @@ async def mount_server(self, server: ServerConfig) -> bool:
                     transport_config=server.transport,
                 )
 
-                if not self.config.stderr_show:
-                    transport = patch_stdio_transport_stderr(transport)
-
-                client = Client(transport, message_handler=message_handler)
+                if not config.stderr_show and isinstance(transport, StdioTransport):
+                    transport.log_file = Path(os.devnull)
 
             elif server.uri:
                 transport = get_transport_for_uri(uri=server.uri, transport_config=server.transport)
 
-                client = Client(transport, message_handler=message_handler)
-
             else:
                 logger.error("No command or URI specified for %s", server.name)
+                self.mount_errors[server.name] = "No command or URI specified"
                 return False
 
+            # Bound connect/initialize so a hung backend can't block list operations indefinitely
+            init_timeout = config.backend_init_timeout if config.backend_init_timeout > 0 else None
+            client = Client(transport, message_handler=message_handler, init_timeout=init_timeout)
+
+            # Build the provider that FastMCP.mount() would add, so we can remove it on unmount
             proxy_server = create_proxy(client, name=server.name)
-            self.mcp.mount(server=proxy_server, namespace=server.prefix)
+            provider: Provider = FastMCPProvider(proxy_server)
+            if server.prefix:
+                provider = provider.wrap_transform(Namespace(server.prefix))
+            self.mcp.add_provider(provider)
 
-            self.mounted_servers[server.name] = MountedServer(proxy=proxy_server, client=client)
+            self.mounted_servers[server.name] = MountedServer(proxy=proxy_server, client=client, provider=provider)
+            self.mount_errors.pop(server.name, None)
 
             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)
+            self.mount_errors[server.name] = str(e)
             return False
 
-    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)
-                    found = True
-                    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)
-                    found = True
-                    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:
-            unmounted = self._unmount_from_fastmcp(name)
-            if unmounted:
-                logger.debug("Unmounted server %s from FastMCP", name)
+            server_info = self.mounted_servers.pop(name)
+
+            if server_info.provider is not None and server_info.provider in self.mcp.providers:
+                self.mcp.providers.remove(server_info.provider)
+                logger.debug("Removed provider for server %s from FastMCP", name)
             else:
-                logger.debug("Server %s was not found in FastMCP's mounted servers", name)
+                logger.debug("Server %s had no provider registered with FastMCP", name)
 
-            server_info = self.mounted_servers.get(name)
-            if server_info and server_info.client:
+            if server_info.client:
                 try:
                     await server_info.client.close()
                     logger.debug("Closed client for server %s", name)
                 except Exception as e:
                     logger.warning("Error closing client for server %s: %s", name, e)
 
-            del self.mounted_servers[name]
             logger.debug("Unmounted server %s", name)
             return True
 
@@ -429,12 +413,14 @@ async def run_stdio(self):
 
     async def run_http(self, host: str = "localhost", port: int = 8000, log_level: str | None = None):
         """Run Magg in HTTP mode."""
+        self.server_manager.require_http_auth()
         log_level = log_level or os.getenv("FASTMCP_LOG_LEVEL", "CRITICAL").upper() or "CRITICAL"
         await self.setup()
         await self.mcp.run_http_async(host=host, port=port, log_level=log_level, show_banner=False)
 
     async def run_hybrid(self, host: str = "localhost", port: int = 8000, log_level: str | None = None):
         """Run Magg in hybrid mode - both stdio and HTTP simultaneously."""
+        self.server_manager.require_http_auth()
         log_level = log_level or os.getenv("FASTMCP_LOG_LEVEL", "CRITICAL").upper() or "CRITICAL"
 
         await self.setup()
@@ -491,6 +477,10 @@ async def load_kit(
         """Load a kit and its servers into the configuration."""
         try:
             config = self.config
+
+            if config.read_only:
+                return MaggResponse.error("Cannot load kit in read-only mode")
+
             success, message = self.kit_manager.load_kit_to_config(name, config)
 
             if success:
@@ -516,6 +506,9 @@ async def unload_kit(
         try:
             config = self.config
 
+            if config.read_only:
+                return MaggResponse.error("Cannot unload kit in read-only mode")
+
             servers_before = set(config.servers.keys())
 
             success, message = self.kit_manager.unload_kit_from_config(name, config)
diff --git a/magg/server/runner.py b/magg/server/runner.py
index 72d52db..78a9594 100644
--- a/magg/server/runner.py
+++ b/magg/server/runner.py
@@ -19,12 +19,19 @@
 class MaggRunner:
     """Manages Magg server lifecycle with proper signal handling."""
 
+    # How long to wait for the server task to finish after cancelling it. The stdio transport
+    # reads stdin in a worker thread that can't be interrupted, so its task may never finish.
+    SHUTDOWN_TIMEOUT: float = 1.0
+
     _server: MaggServer
     _shutdown_event: asyncio.Event
     _reload_event: asyncio.Event
-    _original_sigint: Callable | None
-    _original_sigterm: Callable | None
-    _original_sighup: Callable | None
+    _original_sigint: Callable | int | None
+    _original_sigterm: Callable | int | None
+    _original_sighup: Callable | int | None
+    _signal_loop: asyncio.AbstractEventLoop | None = None
+    _shutdown_signal: int | None = None
+    _abandoned_task: bool = False
     _hook_signals: bool
     _hooked_signals: bool = False
 
@@ -63,18 +70,27 @@ async def __aexit__(self, exc_type, exc_value, traceback):
         await self._server.__aexit__(exc_type, exc_value, traceback)
 
     # noinspection PyUnusedLocal
-    def _handle_signal(self, signum, frame):
-        """Handle shutdown signals gracefully."""
+    def _handle_signal(self, signum, frame=None):
+        """Handle shutdown signals gracefully; a second signal forces exit."""
         signame = signal.Signals(signum).name
+
+        if self._shutdown_event.is_set():
+            logger.warning("Received %s again, forcing exit", signame)
+            self._exit_by_signal(signum)
+            return
+
         logger.debug("Received signal %s, shutting down gracefully...", signame)
+        self._shutdown_signal = signum
         self._shutdown_event.set()
 
-        # Prevent multiple signal handlers from firing
-        signal.signal(signal.SIGINT, signal.SIG_IGN)
-        signal.signal(signal.SIGTERM, signal.SIG_IGN)
+    @staticmethod
+    def _exit_by_signal(signum):
+        """Terminate the process with the signal's default action."""
+        signal.signal(signum, signal.SIG_DFL)
+        signal.raise_signal(signum)
 
     # noinspection PyUnusedLocal
-    def _handle_reload_signal(self, signum, frame):
+    def _handle_reload_signal(self, signum, frame=None):
         """Handle reload signal (SIGHUP)."""
         logger.debug("Received SIGHUP, triggering config reload...")
         self._reload_event.set()
@@ -82,22 +98,43 @@ def _handle_reload_signal(self, signum, frame):
     def _setup_signal_handlers(self):
         """Setup signal handlers for graceful shutdown and reload."""
         if self._hook_signals and self._hooked_signals is False:
-            self._original_sigint = signal.signal(signal.SIGINT, self._handle_signal)
-            self._original_sigterm = signal.signal(signal.SIGTERM, self._handle_signal)
+            handlers = {signal.SIGINT: self._handle_signal, signal.SIGTERM: self._handle_signal}
             # SIGHUP for config reload (Unix-like systems only)
             if hasattr(signal, "SIGHUP"):
-                self._original_sighup = signal.signal(signal.SIGHUP, self._handle_reload_signal)
+                handlers[signal.SIGHUP] = self._handle_reload_signal
+
+            self._original_sigint = signal.getsignal(signal.SIGINT)
+            self._original_sigterm = signal.getsignal(signal.SIGTERM)
+            if hasattr(signal, "SIGHUP"):
+                self._original_sighup = signal.getsignal(signal.SIGHUP)
+
+            loop = asyncio.get_running_loop()
+            try:
+                # Loop handlers run as loop callbacks, so they also wake an idle loop
+                for signum, handler in handlers.items():
+                    loop.add_signal_handler(signum, handler, signum)
+                self._signal_loop = loop
+            except (NotImplementedError, RuntimeError):
+                # No loop signal support (e.g. Windows): hand the signal to the loop thread-safely
+                for signum, handler in handlers.items():
+                    signal.signal(signum, lambda s, _f, h=handler: loop.call_soon_threadsafe(h, s))
             self._hooked_signals = True
 
     def _restore_signal_handlers(self):
         """Restore original signal handlers."""
         if self._hook_signals and self._hooked_signals:
-            if self._original_sigint:
-                signal.signal(signal.SIGINT, self._original_sigint)
-            if self._original_sigterm:
-                signal.signal(signal.SIGTERM, self._original_sigterm)
-            if hasattr(signal, "SIGHUP") and self._original_sighup:
-                signal.signal(signal.SIGHUP, self._original_sighup)
+            originals = {signal.SIGINT: self._original_sigint, signal.SIGTERM: self._original_sigterm}
+            if hasattr(signal, "SIGHUP"):
+                originals[signal.SIGHUP] = self._original_sighup
+
+            for signum, original in originals.items():
+                if self._signal_loop is not None:
+                    self._signal_loop.remove_signal_handler(signum)
+                # SIG_DFL is 0, so check against None rather than truthiness
+                if original is not None:
+                    signal.signal(signum, original)
+
+            self._signal_loop = None
             self._hooked_signals = False
 
     @asynccontextmanager
@@ -111,6 +148,12 @@ async def _server_context(self):
         finally:
             self._restore_signal_handlers()
 
+            # asyncio.run() would wait forever on an abandoned server task, so now that cleanup
+            # is done, finish the signal-requested shutdown with the signal's default action
+            if self._abandoned_task and self._shutdown_signal is not None:
+                logger.debug("Magg server stopped, exiting with abandoned server task")
+                self._exit_by_signal(self._shutdown_signal)
+
     async def _serve(self, coro: Coroutine):
         server_task = asyncio.create_task(coro)
         shutdown_task = asyncio.create_task(self._shutdown_event.wait())
@@ -127,10 +170,12 @@ async def _serve(self, coro: Coroutine):
 
         for task in pending:
             task.cancel()
-            try:
-                await task
-            except asyncio.CancelledError:
-                pass
+
+        if pending:
+            _, stuck = await asyncio.wait(pending, timeout=self.SHUTDOWN_TIMEOUT)
+            if stuck:
+                logger.debug("Server task did not stop within %ss, abandoning it", self.SHUTDOWN_TIMEOUT)
+                self._abandoned_task = True
 
         for task in done:
             if task is server_task and task.exception():
@@ -165,6 +210,8 @@ async def run_stdio(self):
 
     async def run_http(self, host: str = "localhost", port: int = 8000):
         """Run server in HTTP mode with proper signal handling."""
+        self._server.server_manager.require_http_auth()
+
         try:
             async with self._server_context() as server:
                 logger.debug("Starting Magg HTTP server on %s:%s", host, port)
@@ -175,6 +222,8 @@ async def run_http(self, host: str = "localhost", port: int = 8000):
 
     async def run_hybrid(self, host: str = "localhost", port: int = 8000):
         """Run server in hybrid mode (stdio + HTTP) with proper signal handling."""
+        self._server.server_manager.require_http_auth()
+
         try:
             async with self._server_context() as server:
                 logger.debug("Starting Magg hybrid server (stdio + HTTP on %s:%s)", host, port)
diff --git a/magg/server/server.py b/magg/server/server.py
index 83764dc..5871c66 100644
--- a/magg/server/server.py
+++ b/magg/server/server.py
@@ -273,9 +273,15 @@ async def add_server(
         try:
             config = self.config
 
+            if config.read_only:
+                return MaggResponse.error("Cannot add server in read-only mode")
+
             if name in config.servers:
                 return MaggResponse.error(f"Server '{name}' already exists")
 
+            if not command and not uri:
+                return MaggResponse.error(f"Server '{name}' needs a way to run: provide either command or uri")
+
             actual_command = command
             actual_args = None
             if command:
@@ -300,7 +306,7 @@ async def add_server(
                     prefix=prefix,
                     command=actual_command,
                     args=actual_args,
-                    uri=uri,
+                    uri=str(uri) if uri else None,
                     env=json_to_dict(env),
                     cwd=cwd,
                     notes=notes,
@@ -316,11 +322,14 @@ async def add_server(
                 mount_success = await self.server_manager.mount_server(server)
 
                 if not mount_success:
-                    return MaggResponse.error(f"Failed to mount server '{name}'")
+                    reason = self.server_manager.mount_errors.get(name)
+                    return MaggResponse.error(f"Failed to mount server '{name}'" + (f": {reason}" if reason else ""))
 
             config.add_server(server)
 
             if not self.save_config(config):
+                if mount_success:
+                    await self.server_manager.unmount_server(name)
                 return MaggResponse.error(f"Failed to save configuration for added server '{name}'")
 
             return MaggResponse.success(
@@ -355,6 +364,9 @@ async def remove_server(
         try:
             config = self.config
 
+            if config.read_only:
+                return MaggResponse.error("Cannot remove server in read-only mode")
+
             if name in config.servers:
                 config.remove_server(name)
 
@@ -412,6 +424,9 @@ async def enable_server(
         try:
             config = self.config
 
+            if config.read_only:
+                return MaggResponse.error("Cannot enable server in read-only mode")
+
             if name not in config.servers:
                 return MaggResponse.error(f"Server '{name}' not found")
 
@@ -440,6 +455,9 @@ async def disable_server(
         try:
             config = self.config
 
+            if config.read_only:
+                return MaggResponse.error("Cannot disable server in read-only mode")
+
             if name not in config.servers:
                 return MaggResponse.error(f"Server '{name}' not found")
 
@@ -786,12 +804,15 @@ async def check(
         timeout: Annotated[float, Field(description="Timeout in seconds for health check per server")] = 2.5,
     ) -> MaggResponse:
         """Check health of all mounted servers and handle unresponsive ones."""
+        if action != "report" and self.config.read_only:
+            return MaggResponse.error(f"Check action {action!r} is not allowed in read-only mode")
+
         try:
             results = {}
             unresponsive_servers = []
 
-            for server_name, server_info in self.server_manager.mounted_servers.items():
-                client = server_info.get("client")
+            for server_name, server_info in list(self.server_manager.mounted_servers.items()):
+                client = server_info.client
                 if not client:
                     results[server_name] = {"status": "error", "reason": "No client found"}
                     unresponsive_servers.append(server_name)
diff --git a/magg/util/stdio_patch.py b/magg/util/stdio_patch.py
deleted file mode 100644
index 81bc3f0..0000000
--- a/magg/util/stdio_patch.py
+++ /dev/null
@@ -1,43 +0,0 @@
-"""Utilities for patching stdio transports to control stderr behavior."""
-
-import os
-from contextlib import asynccontextmanager
-from pathlib import Path
-
-__all__ = ("patch_stdio_transport_stderr",)
-
-
-def patch_stdio_transport_stderr(transport):
-    """Patch a stdio transport to suppress stderr output.
-
-    This monkey-patches the transport's connect method to redirect stderr to /dev/null.
-    Only works for StdioTransport and its subclasses.
-    """
-    from fastmcp.client.transports import StdioTransport
-
-    if not isinstance(transport, StdioTransport):
-        return transport
-
-    original_connect = transport.connect
-
-    async def patched_connect(**session_kwargs):
-        from mcp.client import stdio
-
-        original_stdio_client = stdio.stdio_client
-
-        @asynccontextmanager
-        async def silent_stdio_client(server_params, errlog=None):
-            with Path(os.devnull).open("w") as devnull:
-                async with original_stdio_client(server_params, errlog=devnull) as result:
-                    yield result
-
-        stdio.stdio_client = silent_stdio_client
-
-        try:
-            return await original_connect(**session_kwargs)
-
-        finally:
-            stdio.stdio_client = original_stdio_client
-
-    transport.connect = patched_connect
-    return transport
diff --git a/test/magg/test_config_reload.py b/test/magg/test_config_reload.py
index 779d001..e6c35a4 100644
--- a/test/magg/test_config_reload.py
+++ b/test/magg/test_config_reload.py
@@ -9,6 +9,7 @@
 import pytest
 
 from magg.reload import ConfigChange, ConfigReloader, ServerChange
+from magg.server.manager import MountedServer
 from magg.server.runner import MaggRunner
 from magg.server.server import MaggServer
 from magg.settings import MaggConfig, ServerConfig
@@ -355,8 +356,8 @@ async def test_check_with_failed_server(self, temp_config_file):
         mock_bad_client.list_tools.side_effect = Exception("Server session was closed unexpectedly")
 
         server.server_manager.mounted_servers = {
-            "good-server": {"client": mock_good_client},
-            "bad-server": {"client": mock_bad_client},
+            "good-server": MountedServer(proxy=None, client=mock_good_client),
+            "bad-server": MountedServer(proxy=None, client=mock_bad_client),
         }
 
         # Run check with default 0.5s timeout
diff --git a/test/magg/test_kit_tools.py b/test/magg/test_kit_tools.py
index 2106c2e..598fea4 100644
--- a/test/magg/test_kit_tools.py
+++ b/test/magg/test_kit_tools.py
@@ -99,6 +99,7 @@ async def test_unload_kit_success(self, server):
         """Test successfully unloading a kit."""
         # Create a persistent config object
         mock_config = MagicMock()
+        mock_config.read_only = False
         from magg.settings import KitInfo
 
         mock_config.kits = {"test-kit": KitInfo(name="test-kit", source="file")}
diff --git a/test/magg/test_server_robustness.py b/test/magg/test_server_robustness.py
new file mode 100644
index 0000000..285cd43
--- /dev/null
+++ b/test/magg/test_server_robustness.py
@@ -0,0 +1,412 @@
+"""Robustness tests: unmounting, hung backends, read-only mode, auth, signals, and related fixes."""
+
+import asyncio
+import json
+import os
+import select
+import signal
+import subprocess
+import sys
+import time
+from pathlib import Path
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from fastmcp import Client
+from pydantic import AnyUrl
+
+from magg.reload import ConfigChange, ServerChange
+from magg.server.runner import MaggRunner
+from magg.server.server import MaggServer
+from magg.settings import ServerConfig
+
+BACKEND_CODE = '''
+import sys
+
+from fastmcp import FastMCP
+
+label = sys.argv[1] if len(sys.argv) > 1 else "default"
+mcp = FastMCP("backend")
+
+
+@mcp.tool
+def greet(name: str) -> str:
+    """Greet someone."""
+    return f"{label}: hello {name}"
+
+
+if __name__ == "__main__":
+    mcp.run(show_banner=False)
+'''
+
+
+@pytest.fixture
+def backend_script(tmp_path) -> Path:
+    path = tmp_path / "backend.py"
+    path.write_text(BACKEND_CODE)
+    return path
+
+
+def backend_config(name: str, script: Path, label: str, prefix: str | None = None, **kwds) -> ServerConfig:
+    return ServerConfig(
+        name=name, source="test", command=sys.executable, args=[str(script), label], prefix=prefix, **kwds
+    )
+
+
+def write_config(path: Path, servers: dict[str, ServerConfig]):
+    data = {
+        "servers": {
+            name: server.model_dump(mode="json", exclude_none=True, exclude={"name"})
+            for name, server in servers.items()
+        }
+    }
+    path.write_text(json.dumps(data))
+
+
+async def tool_names(client: Client) -> list[str]:
+    return [tool.name for tool in await client.list_tools()]
+
+
+def response_output(result) -> dict:
+    return json.loads(result.content[0].text)
+
+
+class TestUnmount:
+    """Unmounting removes a backend's provider from FastMCP."""
+
+    @pytest.mark.asyncio
+    async def test_disable_enable_remove(self, tmp_path, backend_script):
+        config_path = tmp_path / "config.json"
+        write_config(config_path, {"bk": backend_config("bk", backend_script, "one", prefix="bk")})
+        server = MaggServer(config_path, enable_config_reload=False)
+
+        async with server:
+            providers_before = len(server.mcp.providers)
+
+            async with Client(server.mcp) as client:
+                assert (await tool_names(client)).count("bk_greet") == 1
+
+                result = response_output(await client.call_tool("magg_disable_server", {"name": "bk"}))
+                assert result["errors"] is None
+                assert "bk_greet" not in await tool_names(client)
+                assert len(server.mcp.providers) == providers_before - 1
+
+                result = response_output(await client.call_tool("magg_enable_server", {"name": "bk"}))
+                assert result["output"]["mounted"] is True
+                assert (await tool_names(client)).count("bk_greet") == 1
+                assert len(server.mcp.providers) == providers_before
+
+                greeting = await client.call_tool("bk_greet", {"name": "x"})
+                assert greeting.content[0].text == "one: hello x"
+
+                result = response_output(await client.call_tool("magg_remove_server", {"name": "bk"}))
+                assert result["errors"] is None
+                assert "bk_greet" not in await tool_names(client)
+                assert len(server.mcp.providers) == providers_before - 1
+
+    @pytest.mark.asyncio
+    async def test_update_applies_new_args_and_prefix(self, tmp_path, backend_script):
+        config_path = tmp_path / "config.json"
+        old = backend_config("bk", backend_script, "old", prefix="old")
+        write_config(config_path, {"bk": old})
+        server = MaggServer(config_path, enable_config_reload=False)
+
+        async with server:
+            async with Client(server.mcp) as client:
+                assert (await client.call_tool("old_greet", {"name": "x"})).content[0].text == "old: hello x"
+
+                new = backend_config("bk", backend_script, "new", prefix="new")
+                config = server.config
+                change = ConfigChange(
+                    old_config=config,
+                    new_config=config,
+                    server_changes=[ServerChange(name="bk", action="update", old_config=old, new_config=new)],
+                )
+                await server.server_manager.handle_config_reload(change)
+
+                names = await tool_names(client)
+                assert "old_greet" not in names
+                assert names.count("new_greet") == 1
+                assert (await client.call_tool("new_greet", {"name": "x"})).content[0].text == "new: hello x"
+
+
+class TestCheck:
+    """magg_check works on mounted servers without breaking them."""
+
+    @pytest.mark.asyncio
+    async def test_check_mounted_server(self, tmp_path, backend_script):
+        config_path = tmp_path / "config.json"
+        write_config(config_path, {"bk": backend_config("bk", backend_script, "one", prefix="bk")})
+        server = MaggServer(config_path, enable_config_reload=False)
+
+        async with server:
+            async with Client(server.mcp) as client:
+                assert (await client.call_tool("bk_greet", {"name": "a"})).content[0].text == "one: hello a"
+
+                result = response_output(await client.call_tool("magg_check", {"timeout": 20}))
+                assert result["errors"] is None
+                assert result["output"]["results"]["bk"]["status"] == "healthy"
+                assert result["output"]["healthy"] == 1
+
+                # The mounted proxy still works after the check used the backend client
+                assert (await client.call_tool("bk_greet", {"name": "b"})).content[0].text == "one: hello b"
+
+
+class TestHungBackend:
+    """A backend that never initializes degrades only itself."""
+
+    @pytest.mark.asyncio
+    async def test_hung_backend(self, tmp_path, backend_script, monkeypatch):
+        monkeypatch.setenv("MAGG_BACKEND_INIT_TIMEOUT", "2")
+        config_path = tmp_path / "config.json"
+        hung = ServerConfig(
+            name="hung", source="test", command=sys.executable, args=["-c", "import time; time.sleep(1000)"]
+        )
+        write_config(config_path, {"hung": hung, "good": backend_config("good", backend_script, "g", prefix="good")})
+        server = MaggServer(config_path, enable_config_reload=False)
+
+        async with server:
+            assert server.server_manager.mounted_servers["hung"].client._init_timeout == 2.0
+
+            async with Client(server.mcp) as client:
+                start = time.monotonic()
+                names = await tool_names(client)
+                assert time.monotonic() - start < 10
+                assert "magg_list_servers" in names
+                assert "good_greet" in names
+
+                # Magg's own tools resolve without waiting on the hung backend
+                start = time.monotonic()
+                result = response_output(await client.call_tool("magg_list_servers", {}))
+                assert time.monotonic() - start < 1.5
+                assert {s["name"] for s in result["output"]} == {"hung", "good"}
+
+                assert (await client.call_tool("good_greet", {"name": "x"})).content[0].text == "g: hello x"
+
+    @pytest.mark.asyncio
+    async def test_init_timeout_zero_waits_forever(self, tmp_path, monkeypatch):
+        monkeypatch.setenv("MAGG_BACKEND_INIT_TIMEOUT", "0")
+        server = MaggServer(tmp_path / "config.json", enable_config_reload=False)
+        assert await server.server_manager.mount_server(ServerConfig(name="s", source="t", command="true"))
+        assert server.server_manager.mounted_servers["s"].client._init_timeout is None
+        await server.server_manager.unmount_server("s")
+
+
+class TestTransportSetup:
+    """Backend env precedence and stderr handling."""
+
+    @pytest.mark.asyncio
+    async def test_server_env_overrides_passed_env(self, tmp_path):
+        server = MaggServer(tmp_path / "config.json", enable_config_reload=False, env={"FOO": "outer", "BAR": "b"})
+        config = ServerConfig(name="s", source="t", command="true", env={"FOO": "server"})
+        assert await server.server_manager.mount_server(config)
+        env = server.server_manager.mounted_servers["s"].client.transport.env
+        assert env["FOO"] == "server"
+        assert env["BAR"] == "b"
+        await server.server_manager.unmount_server("s")
+
+    @pytest.mark.asyncio
+    @pytest.mark.parametrize("show", [False, True])
+    async def test_stderr_log_file(self, tmp_path, monkeypatch, show):
+        monkeypatch.setenv("MAGG_STDERR_SHOW", str(show).lower())
+        server = MaggServer(tmp_path / "config.json", enable_config_reload=False)
+        assert await server.server_manager.mount_server(ServerConfig(name="s", source="t", command="true"))
+        log_file = server.server_manager.mounted_servers["s"].client.transport.log_file
+        assert log_file == (None if show else Path(os.devnull))
+        await server.server_manager.unmount_server("s")
+
+
+class TestAddServer:
+    """add_server input handling and rollback."""
+
+    @pytest.mark.asyncio
+    async def test_add_with_uri(self, tmp_path):
+        server = MaggServer(tmp_path / "config.json", enable_config_reload=False)
+
+        async with Client(server.mcp) as client:
+            result = response_output(
+                await client.call_tool(
+                    "magg_add_server",
+                    {"name": "web", "source": "test", "uri": "http://localhost:9/mcp", "enable": False},
+                )
+            )
+        assert result["errors"] is None
+        assert server.config.servers["web"].uri == "http://localhost:9/mcp"
+
+        response = await server.add_server(name="web2", source="test", uri=AnyUrl("http://localhost:9/mcp"))
+        assert response.is_success
+        assert "web2" in server.server_manager.mounted_servers
+
+    @pytest.mark.asyncio
+    async def test_add_requires_command_or_uri(self, tmp_path):
+        server = MaggServer(tmp_path / "config.json", enable_config_reload=False)
+
+        for enable in (True, False):
+            response = await server.add_server(name="empty", source="test", enable=enable)
+            assert response.is_error
+            assert "provide either command or uri" in response.errors[0]
+        assert "empty" not in server.config.servers
+
+    @pytest.mark.asyncio
+    async def test_mount_failure_reason(self, tmp_path):
+        server = MaggServer(tmp_path / "config.json", enable_config_reload=False)
+
+        with patch("magg.server.manager.get_transport_for_command", side_effect=ValueError("no such tool")):
+            response = await server.add_server(name="bad", source="test", command="whatever")
+
+        assert response.is_error
+        assert "no such tool" in response.errors[0]
+        assert "bad" not in server.config.servers
+
+    @pytest.mark.asyncio
+    async def test_add_rolls_back_mount_when_save_fails(self, tmp_path):
+        server = MaggServer(tmp_path / "config.json", enable_config_reload=False)
+        providers_before = len(server.mcp.providers)
+
+        with patch.object(server, "save_config", return_value=False):
+            response = await server.add_server(name="s", source="test", command="true")
+
+        assert response.is_error
+        assert "s" not in server.server_manager.mounted_servers
+        assert len(server.mcp.providers) == providers_before
+
+
+class TestReadOnly:
+    """Mutating tools refuse up front in read-only mode."""
+
+    @pytest.fixture
+    def server(self, tmp_path, monkeypatch):
+        config_path = tmp_path / "config.json"
+        write_config(config_path, {"s": ServerConfig(name="s", source="t", command="true", enabled=False)})
+        monkeypatch.setenv("MAGG_READ_ONLY", "true")
+        return MaggServer(config_path, enable_config_reload=False)
+
+    @pytest.mark.asyncio
+    async def test_mutating_tools_refuse(self, server):
+        server.server_manager.mount_server = AsyncMock(return_value=True)
+        server.server_manager.unmount_server = AsyncMock(return_value=True)
+        server.kit_manager.load_kit_to_config = MagicMock()
+        server.kit_manager.unload_kit_from_config = MagicMock()
+
+        responses = [
+            await server.add_server(name="new", source="test", command="true"),
+            await server.remove_server(name="s"),
+            await server.enable_server(name="s"),
+            await server.disable_server(name="s"),
+            await server.load_kit(name="k"),
+            await server.unload_kit(name="k"),
+            await server.check(action="disable"),
+            await server.check(action="unmount"),
+        ]
+
+        for response in responses:
+            assert response.is_error
+            assert "read-only" in response.errors[0]
+
+        server.server_manager.mount_server.assert_not_called()
+        server.server_manager.unmount_server.assert_not_called()
+        server.kit_manager.load_kit_to_config.assert_not_called()
+        server.kit_manager.unload_kit_from_config.assert_not_called()
+        assert set(server.config.servers) == {"s"}
+
+    @pytest.mark.asyncio
+    async def test_check_report_allowed(self, server):
+        response = await server.check(action="report")
+        assert response.is_success
+
+
+class TestAuthFailClosed:
+    """An unloadable private key refuses HTTP instead of serving without auth."""
+
+    @pytest.mark.asyncio
+    async def test_bad_private_key(self, tmp_path, monkeypatch):
+        monkeypatch.setenv("MAGG_PRIVATE_KEY", "not a key")
+        runner = MaggRunner(tmp_path / "config.json")
+        server = runner.server
+
+        assert "MAGG_PRIVATE_KEY" in server.server_manager.auth_error
+        assert server.mcp.auth is None
+
+        with patch.object(server.server_manager, "mount_all_enabled", new_callable=AsyncMock) as mount_all:
+            for run in (runner.run_http, runner.run_hybrid, server.run_http, server.run_hybrid):
+                with pytest.raises(RuntimeError, match="MAGG_PRIVATE_KEY.*refusing to serve HTTP"):
+                    await run()
+            mount_all.assert_not_called()
+
+
+class TestRunnerSignals:
+    """Shutdown signals wake the loop, restore handlers, and escalate on repeat."""
+
+    @pytest.mark.skipif(not hasattr(signal, "SIGHUP"), reason="POSIX signals required")
+    @pytest.mark.asyncio
+    async def test_sigterm_wakes_idle_loop_and_restores_default(self, tmp_path):
+        runner = MaggRunner(tmp_path / "config.json")
+
+        async def idle():
+            await asyncio.Event().wait()
+
+        previous = signal.signal(signal.SIGTERM, signal.SIG_DFL)
+        try:
+            async with runner._server_context():
+                serve_task = asyncio.create_task(runner._serve(idle()))
+                await asyncio.sleep(0.1)
+                os.kill(os.getpid(), signal.SIGTERM)
+                await asyncio.wait_for(serve_task, 5)
+
+            # SIG_DFL is falsy, but must still be restored
+            assert signal.getsignal(signal.SIGTERM) == signal.SIG_DFL
+        finally:
+            signal.signal(signal.SIGTERM, previous)
+
+    def test_second_signal_forces_exit(self, tmp_path):
+        runner = MaggRunner(tmp_path / "config.json")
+
+        with patch.object(MaggRunner, "_exit_by_signal") as exit_by_signal:
+            runner._handle_signal(signal.SIGINT)
+            assert runner._shutdown_event.is_set()
+            exit_by_signal.assert_not_called()
+
+            runner._handle_signal(signal.SIGINT)
+            exit_by_signal.assert_called_once_with(signal.SIGINT)
+
+    @pytest.mark.skipif(sys.platform == "win32", reason="POSIX signals required")
+    def test_stdio_serve_exits_on_sigterm(self, tmp_path):
+        env = {**os.environ, "MAGG_CONFIG_PATH": str(tmp_path / "config.json"), "MAGG_LOG_LEVEL": "WARNING"}
+        proc = subprocess.Popen(
+            [sys.executable, "-m", "magg", "serve"],
+            stdin=subprocess.PIPE,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.DEVNULL,
+            env=env,
+        )
+        try:
+            init = {
+                "jsonrpc": "2.0",
+                "id": 1,
+                "method": "initialize",
+                "params": {
+                    "protocolVersion": "2025-06-18",
+                    "capabilities": {},
+                    "clientInfo": {"name": "t", "version": "1"},
+                },
+            }
+            proc.stdin.write((json.dumps(init) + "\n").encode())
+            proc.stdin.flush()
+            assert select.select([proc.stdout], [], [], 30)[0], "server did not respond"
+            assert b'"id":1' in proc.stdout.readline()  # Server is up; stdin stays open
+
+            proc.send_signal(signal.SIGTERM)
+            start = time.monotonic()
+            proc.wait(timeout=15)
+            assert time.monotonic() - start < 5
+        finally:
+            if proc.poll() is None:
+                proc.kill()
+                proc.wait()
+
+
+def test_proxy_path_description(tmp_path):
+    server = MaggServer(tmp_path / "config.json", enable_config_reload=False)
+    tool = asyncio.run(server.mcp.get_tool("proxy"))
+    description = tool.parameters["properties"]["path"]["description"]
+    assert "Required for 'info' and 'call'" in description

From 3c7036f5fb5e72674bc9bfbc36d6ec5dcd36fb78 Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Fri, 25 Sep 2026 20:23:53 +0000
Subject: [PATCH 09/14] Allow check remount/unmount in read-only mode; fix
 Python 3.12.3 CLI parsing

- magg_check's remount and unmount actions only affect the running
  process, so they stay available in read-only mode; disable still
  refuses since it writes the config.
- On Python 3.12.3 (Ubuntu 24.04's system Python), argparse treated
  `magg server update --env` as an ambiguous abbreviation of the global
  --env-pass/--env-set options. Disable abbreviations on the top-level
  parser.
- magg_add_server's prefix description matches actual behavior: no
  prefix by default.
---
 magg/cli.py                         | 3 +++
 magg/server/server.py               | 9 ++++++---
 test/magg/test_server_robustness.py | 1 -
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/magg/cli.py b/magg/cli.py
index 2022951..feb27f5 100644
--- a/magg/cli.py
+++ b/magg/cli.py
@@ -872,6 +872,9 @@ def create_parser() -> argparse.ArgumentParser:
         prog="magg",
         description="Magg - MCP Aggregator: Manage and aggregate MCP servers",
         epilog='Use "magg <command> --help" for more information about a command.',
+        # Some Python versions (e.g. 3.12.3) match subcommand options like "server update --env"
+        # against abbreviations of global options, which fails as ambiguous
+        allow_abbrev=False,
     )
 
     parser.add_argument(
diff --git a/magg/server/server.py b/magg/server/server.py
index 5871c66..041438b 100644
--- a/magg/server/server.py
+++ b/magg/server/server.py
@@ -250,7 +250,9 @@ async def add_server(
         self,
         name: Annotated[str, Field(description="Unique server name")],
         source: Annotated[str, Field(description="URL of the server package/repository")],
-        prefix: Annotated[str | None, Field(description="Tool prefix (defaults to conformed server name)")] = None,
+        prefix: Annotated[
+            str | None, Field(description="Tool prefix (default: none, tools keep their original names)")
+        ] = None,
         command: Annotated[
             str | None,
             Field(description="Full command to run (e.g., 'python server.py', 'npx @playwright/mcp@latest')"),
@@ -804,8 +806,9 @@ async def check(
         timeout: Annotated[float, Field(description="Timeout in seconds for health check per server")] = 2.5,
     ) -> MaggResponse:
         """Check health of all mounted servers and handle unresponsive ones."""
-        if action != "report" and self.config.read_only:
-            return MaggResponse.error(f"Check action {action!r} is not allowed in read-only mode")
+        # Remounting and unmounting only affect this process; disabling writes the config
+        if action == "disable" and self.config.read_only:
+            return MaggResponse.error("Check action 'disable' is not allowed in read-only mode")
 
         try:
             results = {}
diff --git a/test/magg/test_server_robustness.py b/test/magg/test_server_robustness.py
index 285cd43..bf23895 100644
--- a/test/magg/test_server_robustness.py
+++ b/test/magg/test_server_robustness.py
@@ -296,7 +296,6 @@ async def test_mutating_tools_refuse(self, server):
             await server.load_kit(name="k"),
             await server.unload_kit(name="k"),
             await server.check(action="disable"),
-            await server.check(action="unmount"),
         ]
 
         for response in responses:

From c80ba4c372dc1305d4946c726b94d017bef4aab5 Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Sat, 26 Sep 2026 15:18:29 +0000
Subject: [PATCH 10/14] Make new tests check behavior instead of internals

- Backend stderr: assert what actually reaches stderr (via capfd) with
  suppression on and off, instead of checking a transport attribute.
- Reload: rename a real file over the watched config and check the
  reload fires; check Magg's own atomic save doesn't trigger a reload
  while the next outside edit still does; check polling mode without
  watchdog by observing a detected change.
- Placeholder and unparseable-file reload tests go through
  reload_config() rather than private helpers.
- Hung backend test fails with a timeout instead of hanging on
  regression, and no longer inspects private client attributes.
- --key-path test checks that `auth token` works afterwards, which was
  the user-facing failure.
- Search concurrency is proven with a barrier instead of timing.
- Settings: a bad value no longer skips later valid settings.
- Remove tests that couldn't fail: a tool description string check, and
  a timeout-0 check that fastmcp already guarantees.
---
 test/magg/test_cli_fixes.py         |  49 +++++----
 test/magg/test_config_safety.py     | 158 ++++++++++++++++++++--------
 test/magg/test_server_robustness.py |  44 ++++----
 3 files changed, 157 insertions(+), 94 deletions(-)

diff --git a/test/magg/test_cli_fixes.py b/test/magg/test_cli_fixes.py
index df06b42..a0de3f9 100644
--- a/test/magg/test_cli_fixes.py
+++ b/test/magg/test_cli_fixes.py
@@ -5,7 +5,6 @@
 import os
 import subprocess
 import sys
-import time
 
 import pytest
 
@@ -51,13 +50,21 @@ async def test_init_twice_reports_error(self, auth_env, tmp_path, capsys):
         assert "already exists" in capsys.readouterr().err
 
     @pytest.mark.asyncio
-    async def test_init_saves_custom_key_path(self, auth_env, tmp_path):
+    async def test_custom_key_path_is_used_afterwards(self, auth_env, tmp_path, capsys):
+        # Previously the key was generated at --key-path, but later commands looked in the default place
         key_path = tmp_path / "keys"
 
         assert await self.run_auth(auth_env, "init", "--key-path", str(key_path)) == 0
+        capsys.readouterr()
+
+        assert await self.run_auth(auth_env, "token", "-q") == 0
+        token = capsys.readouterr().out.strip()
+        assert token.count(".") == 2
 
-        saved = json.loads((auth_env.parent / "auth.json").read_text())
-        assert saved["bearer"]["key_path"] == str(key_path)
+        manager = BearerAuthManager(BearerAuthConfig(key_path=key_path))
+        manager.load_keys()
+        assert manager.provider is not None
+        assert json.loads((auth_env.parent / "auth.json").read_text())["bearer"]["key_path"] == str(key_path)
 
     @pytest.mark.asyncio
     async def test_private_key_export_is_shell_safe(self, auth_env, tmp_path, capsys):
@@ -100,32 +107,28 @@ def test_npm_null_author_and_description(self):
 
     @pytest.mark.asyncio
     async def test_search_all_runs_sources_concurrently(self, monkeypatch):
-        async def slow(self, query, limit):
-            await asyncio.sleep(0.3)
-            return []
+        # Each source waits until all three have started, which can only happen if they run together
+        all_started = asyncio.Barrier(3)
+
+        def source(name):
+            async def search(self, query, limit):
+                await all_started.wait()
+                return [name]
+
+            return search
 
         async def broken(self, query, limit):
             raise RuntimeError("offline")
 
-        monkeypatch.setattr(ToolSearchEngine, "search_registry", slow)
-        monkeypatch.setattr(ToolSearchEngine, "search_glama", slow)
-        monkeypatch.setattr(ToolSearchEngine, "search_github", slow)
+        monkeypatch.setattr(ToolSearchEngine, "search_registry", source("registry"))
+        monkeypatch.setattr(ToolSearchEngine, "search_glama", source("glama"))
+        monkeypatch.setattr(ToolSearchEngine, "search_github", source("github"))
         monkeypatch.setattr(ToolSearchEngine, "search_npm", broken)
 
-        engine = ToolSearchEngine()
-        start = time.monotonic()
-        results = await engine.search_all("x")
-        elapsed = time.monotonic() - start
-
-        assert elapsed < 0.8
-        assert results == {"mcp-registry": [], "glama": [], "github": [], "npm": []}
-
+        results = await asyncio.wait_for(ToolSearchEngine().search_all("x"), 5)
 
-@pytest.fixture(autouse=True)
-def _no_real_private_key(monkeypatch):
-    # A MAGG_PRIVATE_KEY in the developer's environment would change auth behavior
-    if "MAGG_PRIVATE_KEY" in os.environ:
-        monkeypatch.delenv("MAGG_PRIVATE_KEY")
+        # One failing source doesn't affect the others
+        assert results == {"mcp-registry": ["registry"], "glama": ["glama"], "github": ["github"], "npm": []}
 
 
 class TestInvalidSettings:
diff --git a/test/magg/test_config_safety.py b/test/magg/test_config_safety.py
index a9cd30b..4201f8f 100644
--- a/test/magg/test_config_safety.py
+++ b/test/magg/test_config_safety.py
@@ -4,13 +4,15 @@
 import json
 import os
 import stat
+from pathlib import Path
+from unittest.mock import MagicMock
 
 import pytest
-from watchdog.events import FileMovedEvent
 
+from magg import reload as reload_module
 from magg.kit import KitManager
-from magg.reload import ConfigReloader, WatchdogHandler
-from magg.settings import BearerAuthConfig, ConfigFileError, ConfigManager, MaggConfig, ServerConfig
+from magg.reload import ConfigReloader
+from magg.settings import BearerAuthConfig, ConfigFileError, ConfigManager, ServerConfig
 
 
 @pytest.fixture
@@ -26,6 +28,27 @@ def read_config(path):
     return json.loads(path.read_text())
 
 
+def replace_config(path, data):
+    """Write the config the way many editors and tools do: a new file renamed over the old one."""
+    tmp_path = path.with_name(f".{path.name}.tmp")
+    tmp_path.write_text(json.dumps(data))
+    os.replace(tmp_path, path)
+
+
+async def wait_until(predicate, timeout=5.0):
+    """Poll until predicate() is true, returning whether it became true in time."""
+    deadline = asyncio.get_running_loop().time() + timeout
+    while not predicate():
+        if asyncio.get_running_loop().time() > deadline:
+            return False
+        await asyncio.sleep(0.05)
+    return True
+
+
+def server_names(change):
+    return {server_change.name for server_change in change.server_changes}
+
+
 class TestEnvironmentIsolation:
     """Nested config models must not read unrelated environment variables."""
 
@@ -50,7 +73,7 @@ def test_bearer_auth_config_ignores_env(self, monkeypatch, tmp_path):
         config = BearerAuthConfig()
 
         assert config.audience == "magg"
-        assert config.key_path != tmp_path
+        assert config.key_path == Path.home() / ".ssh" / "magg"
 
 
 class TestSaveConfig:
@@ -137,14 +160,16 @@ def test_top_level_settings_are_kept(self, config_path):
         assert data["self_prefix"] == "hub"
         assert data["custom"] == {"x": 1}
 
-    def test_invalid_setting_does_not_discard_servers(self, config_path):
+    def test_invalid_setting_does_not_discard_later_settings(self, config_path):
         write_config(
             config_path,
-            {"auto_reload": "maybe", "servers": {"a": {"source": "s", "command": "echo"}}},
+            {"auto_reload": "maybe", "self_prefix": "hub", "servers": {"a": {"source": "s", "command": "echo"}}},
         )
 
         config = ConfigManager(str(config_path)).load_config()
 
+        assert config.auto_reload is True  # Invalid value skipped, default kept
+        assert config.self_prefix == "hub"
         assert list(config.servers) == ["a"]
 
     def test_write_preserves_mode_and_symlink(self, tmp_path):
@@ -196,66 +221,109 @@ async def callback(change):
 
 
 class TestReload:
-    """Config reload handles files replaced by rename and placeholder servers."""
+    """Config reload picks up real file changes, including ones made by rename."""
 
-    @pytest.mark.asyncio
-    async def test_watchdog_handles_rename_over_config(self, config_path, tmp_path):
-        event = asyncio.Event()
-        handler = WatchdogHandler(config_path, event)
+    @pytest.fixture
+    def changes(self):
+        return []
 
-        handler.on_moved(FileMovedEvent(str(tmp_path / ".config.json.tmp"), str(config_path)))
-        await asyncio.sleep(0)
+    @pytest.fixture
+    def callback(self, changes):
+        async def callback(change):
+            changes.append(change)
 
-        assert event.is_set()
+        return callback
 
     @pytest.mark.asyncio
-    async def test_watchdog_ignores_other_files(self, config_path, tmp_path):
-        event = asyncio.Event()
-        handler = WatchdogHandler(config_path, event)
-
-        handler.on_moved(FileMovedEvent(str(config_path), str(tmp_path / "backup.json")))
-        await asyncio.sleep(0)
-
-        assert not event.is_set()
-
-    def test_disabled_placeholder_does_not_block_reload(self, config_path):
-        config = MaggConfig()
-        config.servers["placeholder"] = ServerConfig(name="placeholder", source="s", enabled=False)
-        config.servers["real"] = ServerConfig(name="real", source="s", command="echo")
+    async def test_rename_over_config_triggers_reload(self, config_path, changes, callback):
+        write_config(config_path, {"servers": {}})
+        reloader = ConfigReloader(config_path, callback)
+        await reloader.start_watching(poll_interval=0.1)
+        try:
+            await asyncio.sleep(0.2)  # Let the watcher load its baseline
 
-        reloader = ConfigReloader(config_path, lambda change: None)
+            replace_config(config_path, {"servers": {"a": {"source": "s", "command": "echo"}}})
 
-        assert reloader._validate_config(config)
+            assert await wait_until(lambda: changes), "rename over the config was not detected"
+            assert server_names(changes[0]) == {"a"}
+        finally:
+            await reloader.stop_watching()
 
     @pytest.mark.asyncio
-    async def test_unparseable_file_is_not_applied(self, config_path):
-        write_config(config_path, {"servers": {"a": {"source": "s", "command": "echo"}}})
-        changes = []
+    async def test_own_save_is_ignored_but_next_edit_is_not(self, config_path, changes, callback):
+        write_config(config_path, {"servers": {}})
+        manager = ConfigManager(str(config_path))
+        await manager.setup_config_reload(callback)
+        try:
+            await asyncio.sleep(0.2)
 
-        async def callback(change):
-            changes.append(change)
+            # Magg's own save replaces the file too, and must not echo back as a reload
+            config = manager.load_config()
+            config.add_server(ServerConfig(name="saved", source="s", command="echo"))
+            assert manager.save_config(config) is True
+            await asyncio.sleep(0.5)
+            assert changes == []
 
-        reloader = ConfigReloader(config_path, callback)
-        reloader._last_config = reloader._load_config()
+            # The ignore must be used up by that save, so the next outside edit still reloads
+            data = read_config(config_path)
+            data["servers"]["edited"] = {"source": "s", "command": "echo"}
+            replace_config(config_path, data)
 
-        config_path.write_text('{"servers": {')
-        assert await reloader.reload_config() is None
-        assert changes == []
+            assert await wait_until(lambda: changes), "edit after an internal save was not detected"
+            assert server_names(changes[0]) == {"edited"}
+        finally:
+            await manager.stop_config_reload()
 
     @pytest.mark.asyncio
-    async def test_watchdog_can_be_disabled(self, config_path):
+    async def test_polling_when_watchdog_disabled(self, config_path, changes, callback, monkeypatch):
+        observer = MagicMock()
+        monkeypatch.setattr(reload_module, "Observer", observer)
         write_config(config_path, {"servers": {}})
-
-        async def callback(change):
-            pass
-
         reloader = ConfigReloader(config_path, callback)
         await reloader.start_watching(poll_interval=0.1, use_watchdog=False)
         try:
-            assert reloader._observer is None
+            await asyncio.sleep(0.2)
+            write_config(config_path, {"servers": {"a": {"source": "s", "command": "echo"}}})
+            # Polling compares mtimes, so make sure it moves even on coarse-grained filesystems
+            os.utime(config_path, (config_path.stat().st_atime, config_path.stat().st_mtime + 1))
+
+            assert await wait_until(lambda: changes), "polling did not detect the change"
         finally:
             await reloader.stop_watching()
 
+        observer.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_disabled_placeholder_does_not_block_reload(self, config_path, changes, callback):
+        write_config(config_path, {"servers": {}})
+        reloader = ConfigReloader(config_path, callback)
+        await reloader.reload_config()  # Establish the baseline
+
+        write_config(
+            config_path,
+            {
+                "servers": {
+                    "placeholder": {"source": "s", "enabled": False},
+                    "real": {"source": "s", "command": "echo"},
+                }
+            },
+        )
+        assert await reloader.reload_config() is not None
+
+        assert server_names(changes[-1]) == {"placeholder", "real"}
+
+    @pytest.mark.asyncio
+    async def test_unparseable_file_is_not_applied(self, config_path, changes, callback):
+        write_config(config_path, {"servers": {"a": {"source": "s", "command": "echo"}}})
+        reloader = ConfigReloader(config_path, callback)
+        await reloader.reload_config()  # Establish the baseline
+        changes.clear()
+
+        # A half-written or mistyped file must not look like "every server was removed"
+        config_path.write_text('{"servers": {')
+        assert await reloader.reload_config() is None
+        assert changes == []
+
 
 class TestKitDiscovery:
     """Kits next to an explicitly configured config file are found."""
diff --git a/test/magg/test_server_robustness.py b/test/magg/test_server_robustness.py
index bf23895..09e811b 100644
--- a/test/magg/test_server_robustness.py
+++ b/test/magg/test_server_robustness.py
@@ -32,6 +32,7 @@
 @mcp.tool
 def greet(name: str) -> str:
     """Greet someone."""
+    print(f"backend stderr: greeting {name}", file=sys.stderr, flush=True)
     return f"{label}: hello {name}"
 
 
@@ -166,11 +167,10 @@ async def test_hung_backend(self, tmp_path, backend_script, monkeypatch):
         server = MaggServer(config_path, enable_config_reload=False)
 
         async with server:
-            assert server.server_manager.mounted_servers["hung"].client._init_timeout == 2.0
-
             async with Client(server.mcp) as client:
+                # Without the init timeout this waits forever, so bound it to fail instead of hang
                 start = time.monotonic()
-                names = await tool_names(client)
+                names = await asyncio.wait_for(tool_names(client), 15)
                 assert time.monotonic() - start < 10
                 assert "magg_list_servers" in names
                 assert "good_greet" in names
@@ -183,14 +183,6 @@ async def test_hung_backend(self, tmp_path, backend_script, monkeypatch):
 
                 assert (await client.call_tool("good_greet", {"name": "x"})).content[0].text == "g: hello x"
 
-    @pytest.mark.asyncio
-    async def test_init_timeout_zero_waits_forever(self, tmp_path, monkeypatch):
-        monkeypatch.setenv("MAGG_BACKEND_INIT_TIMEOUT", "0")
-        server = MaggServer(tmp_path / "config.json", enable_config_reload=False)
-        assert await server.server_manager.mount_server(ServerConfig(name="s", source="t", command="true"))
-        assert server.server_manager.mounted_servers["s"].client._init_timeout is None
-        await server.server_manager.unmount_server("s")
-
 
 class TestTransportSetup:
     """Backend env precedence and stderr handling."""
@@ -207,13 +199,18 @@ async def test_server_env_overrides_passed_env(self, tmp_path):
 
     @pytest.mark.asyncio
     @pytest.mark.parametrize("show", [False, True])
-    async def test_stderr_log_file(self, tmp_path, monkeypatch, show):
+    async def test_backend_stderr(self, tmp_path, backend_script, monkeypatch, capfd, show):
+        # capfd captures at the file descriptor level, which is where the backend process writes
         monkeypatch.setenv("MAGG_STDERR_SHOW", str(show).lower())
-        server = MaggServer(tmp_path / "config.json", enable_config_reload=False)
-        assert await server.server_manager.mount_server(ServerConfig(name="s", source="t", command="true"))
-        log_file = server.server_manager.mounted_servers["s"].client.transport.log_file
-        assert log_file == (None if show else Path(os.devnull))
-        await server.server_manager.unmount_server("s")
+        config_path = tmp_path / "config.json"
+        write_config(config_path, {"bk": backend_config("bk", backend_script, "one", prefix="bk")})
+        server = MaggServer(config_path, enable_config_reload=False)
+
+        async with server:
+            async with Client(server.mcp) as client:
+                assert (await client.call_tool("bk_greet", {"name": "x"})).content[0].text == "one: hello x"
+
+        assert ("backend stderr: greeting x" in capfd.readouterr().err) == show
 
 
 class TestAddServer:
@@ -309,8 +306,10 @@ async def test_mutating_tools_refuse(self, server):
         assert set(server.config.servers) == {"s"}
 
     @pytest.mark.asyncio
-    async def test_check_report_allowed(self, server):
-        response = await server.check(action="report")
+    @pytest.mark.parametrize("action", ["report", "remount", "unmount"])
+    async def test_check_actions_that_do_not_write_config(self, server, action):
+        # Only "disable" writes the config; the others just affect this process
+        response = await server.check(action=action)
         assert response.is_success
 
 
@@ -402,10 +401,3 @@ def test_stdio_serve_exits_on_sigterm(self, tmp_path):
             if proc.poll() is None:
                 proc.kill()
                 proc.wait()
-
-
-def test_proxy_path_description(tmp_path):
-    server = MaggServer(tmp_path / "config.json", enable_config_reload=False)
-    tool = asyncio.run(server.mcp.get_tool("proxy"))
-    description = tool.parameters["properties"]["path"]["description"]
-    assert "Required for 'info' and 'call'" in description

From a5c284975b0cf8227e67b812951aca202e9ebab0 Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Sat, 26 Sep 2026 15:21:07 +0000
Subject: [PATCH 11/14] Test that mbro --timeout 0 disables the connect timeout

fastmcp treats an init_timeout of 0 as disabled, so passing it through
already matches the documented behavior. The test fails if 0 ever turns
into an immediate timeout.
---
 test/magg/test_mbro_commands.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/test/magg/test_mbro_commands.py b/test/magg/test_mbro_commands.py
index 9993368..c726d70 100644
--- a/test/magg/test_mbro_commands.py
+++ b/test/magg/test_mbro_commands.py
@@ -1,7 +1,9 @@
 """Tests for mbro command handling: argument parsing, errors, exit codes."""
 
+import asyncio
 import shutil
 import sys
+import time
 from unittest.mock import AsyncMock, patch
 
 import pytest
@@ -230,6 +232,16 @@ async def test_connect_timeout(self):
         with pytest.raises(TimeoutError, match="0.5s"):
             await conn.connect(timeout=0.5)
 
+    @pytest.mark.asyncio
+    @pytest.mark.skipif(not shutil.which("sleep"), reason="needs sleep")
+    async def test_connect_timeout_zero_disables_it(self):
+        # fastmcp treats an init_timeout of 0 as disabled, so --timeout 0 keeps waiting
+        conn = BrowserConnection("s", "command", "sleep 100")
+        start = time.monotonic()
+        with pytest.raises(TimeoutError):
+            await asyncio.wait_for(conn.connect(timeout=0), 1)
+        assert time.monotonic() - start >= 1
+
     @pytest.mark.asyncio
     async def test_missing_script_does_not_exit(self, cli, capsys):
         assert not await run(cli, "script run nope")

From a42ac4a7b964c5475e65d972867ade75bc8aa3d0 Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Sat, 26 Sep 2026 15:36:32 +0000
Subject: [PATCH 12/14] Bump version to 1.3.0

---
 pyproject.toml | 2 +-
 server.json    | 4 ++--
 uv.lock        | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/pyproject.toml b/pyproject.toml
index eecdd86..2f437c6 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -1,6 +1,6 @@
 [project]
 name = "magg"
-version = "1.2.1"
+version = "1.3.0"
 requires-python = ">=3.12"
 description = "MCP Aggregator"
 authors = [{ name = "Phillip Sitbon", email = "phillip.sitbon@gmail.com"}]
diff --git a/server.json b/server.json
index ddd17b0..95b0763 100644
--- a/server.json
+++ b/server.json
@@ -7,14 +7,14 @@
     "url": "https://github.com/sitbon/magg",
     "source": "github"
   },
-  "version": "1.2.1",
+  "version": "1.3.0",
   "websiteUrl": "https://github.com/sitbon/magg",
   "packages": [
     {
       "registryType": "pypi",
       "registryBaseUrl": "https://pypi.org",
       "identifier": "magg",
-      "version": "1.2.1",
+      "version": "1.3.0",
       "runtimeHint": "uvx",
       "transport": {
         "type": "stdio"
diff --git a/uv.lock b/uv.lock
index 0478d7e..8017ff4 100644
--- a/uv.lock
+++ b/uv.lock
@@ -902,7 +902,7 @@ wheels = [
 
 [[package]]
 name = "magg"
-version = "1.2.1"
+version = "1.3.0"
 source = { editable = "." }
 dependencies = [
     { name = "aiohttp" },

From 3eeb617cbcbfb1d830437fcc166e923f766e7037 Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Sat, 26 Sep 2026 15:41:05 +0000
Subject: [PATCH 13/14] Add a version bump script and drop the server.json
 tests

scripts/bump_version.py bumps the version with `uv version` (which
updates pyproject.toml and re-locks uv.lock) and copies it into
server.json. With --check it only verifies that server.json matches
pyproject.toml, for CI to run instead of the tests.

The server.json tests in test_registry_search.py checked release
metadata rather than code behavior, so they're removed.
---
 contributing.md                   |  5 +-
 docs/mcp-registry.md              |  6 +--
 scripts/bump_version.py           | 77 +++++++++++++++++++++++++++++++
 test/magg/test_registry_search.py | 33 -------------
 4 files changed, 83 insertions(+), 38 deletions(-)
 create mode 100755 scripts/bump_version.py

diff --git a/contributing.md b/contributing.md
index 3b1e45f..64749a6 100644
--- a/contributing.md
+++ b/contributing.md
@@ -11,8 +11,9 @@ Please note we have a code of conduct, please follow it in all your interactions
    build.
 2. Update the readme.md with details of changes to the interface; this includes new environment 
    variables, exposed ports, useful file locations, and container parameters.
-3. Increase the version numbers in any example files and pyproject.toml to the new version that this
-   Pull Request would represent. The versioning scheme we use is [SemVer](http://semver.org/).
+3. Bump the version to the one this Pull Request would represent with `scripts/bump_version.py`
+   (`major`, `minor`, `patch`, or an explicit version). It updates pyproject.toml, uv.lock and
+   server.json together. The versioning scheme we use is [SemVer](http://semver.org/).
 4. You may merge the Pull Request in once you have the sign-off of two other developers, or if you 
    do not have permission to do that, you may request the second reviewer to merge it for you.
 
diff --git a/docs/mcp-registry.md b/docs/mcp-registry.md
index 7828129..41d7352 100644
--- a/docs/mcp-registry.md
+++ b/docs/mcp-registry.md
@@ -44,9 +44,9 @@ Note: building from source with `go install .../cmd/publisher@latest` produces a
 not on `PATH` by default — the prebuilt binary above is the simpler route.
 
 **Release checklist**: the `version` fields in `server.json` (both the top-level version
-and the pypi package version) must be bumped to match `pyproject.toml` before
-publishing, and the PyPI release must already exist — the registry verifies that the
-referenced package version is published.
+and the pypi package version) must match `pyproject.toml`. `scripts/bump_version.py` keeps
+them in sync when bumping, and CI fails if they drift. The PyPI release must already exist
+before publishing — the registry verifies that the referenced package version is published.
 
 **Ownership verification**: the registry proves control of the PyPI package by requiring
 the line `mcp-name: io.github.sitbon/magg` in the package README as published on PyPI
diff --git a/scripts/bump_version.py b/scripts/bump_version.py
new file mode 100755
index 0000000..1340b17
--- /dev/null
+++ b/scripts/bump_version.py
@@ -0,0 +1,77 @@
+#!/usr/bin/env python3
+"""Bump Magg's version, or check that the versions in the repo agree.
+
+    scripts/bump_version.py minor      # major, minor, patch, or an explicit version like 1.3.0
+    scripts/bump_version.py --check    # exit 1 if server.json doesn't match pyproject.toml
+
+Bumping runs `uv version`, which updates pyproject.toml and re-locks uv.lock, then copies
+the new version into server.json (its top-level version and each package version).
+"""
+
+import argparse
+import json
+import subprocess
+import sys
+import tomllib
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parent.parent
+PYPROJECT = ROOT / "pyproject.toml"
+SERVER_JSON = ROOT / "server.json"
+
+# Values `uv version --bump` accepts; anything else is passed as an explicit version
+BUMPS = ("major", "minor", "patch", "stable", "alpha", "beta", "rc", "post", "dev")
+
+
+def project_version() -> str:
+    with PYPROJECT.open("rb") as f:
+        return tomllib.load(f)["project"]["version"]
+
+
+def manifest_versions(manifest: dict) -> list[str]:
+    return [manifest["version"], *(package["version"] for package in manifest.get("packages", []))]
+
+
+def check() -> int:
+    version = project_version()
+    versions = manifest_versions(json.loads(SERVER_JSON.read_text()))
+
+    if any(v != version for v in versions):
+        print(
+            f"server.json versions {versions} don't match pyproject.toml version {version}. "
+            f"Run: scripts/bump_version.py {version}",
+            file=sys.stderr,
+        )
+        return 1
+
+    print(f"server.json matches pyproject.toml version {version}")
+    return 0
+
+
+def bump(value: str) -> int:
+    args = ["--bump", value] if value in BUMPS else [value]
+    subprocess.run(["uv", "version", "--no-sync", *args], cwd=ROOT, check=True)
+    version = project_version()
+
+    manifest = json.loads(SERVER_JSON.read_text())
+    manifest["version"] = version
+    for package in manifest.get("packages", []):
+        package["version"] = version
+    SERVER_JSON.write_text(json.dumps(manifest, indent=2) + "\n")
+
+    print(f"Updated server.json to {version}")
+    return 0
+
+
+def main() -> int:
+    parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
+    group = parser.add_mutually_exclusive_group(required=True)
+    group.add_argument("version", nargs="?", help=f"one of {', '.join(BUMPS)}, or an explicit version")
+    group.add_argument("--check", action="store_true", help="only check that server.json matches pyproject.toml")
+    args = parser.parse_args()
+
+    return check() if args.check else bump(args.version)
+
+
+if __name__ == "__main__":
+    sys.exit(main())
diff --git a/test/magg/test_registry_search.py b/test/magg/test_registry_search.py
index a8d2197..50f2053 100644
--- a/test/magg/test_registry_search.py
+++ b/test/magg/test_registry_search.py
@@ -1,11 +1,5 @@
 """Tests for the official MCP Registry search backend."""
 
-import json
-from pathlib import Path
-
-import pytest
-
-import magg
 from magg.discovery.search import ToolSearchEngine
 
 SAMPLE_RESPONSE = {
@@ -106,30 +100,3 @@ def test_install_command_types(self):
         )
         assert cmd([]) is None
         assert cmd([{"registryType": "npm"}]) is None
-
-
-class TestServerManifest:
-    """Test Magg's own server.json registry manifest."""
-
-    def test_server_json_consistency(self):
-        root = Path(__file__).parent.parent.parent
-        manifest = json.loads((root / "server.json").read_text())
-
-        assert manifest["name"] == "io.github.sitbon/magg"
-        assert manifest["repository"]["url"] == "https://github.com/sitbon/magg"
-
-        package = manifest["packages"][0]
-        assert package["registryType"] == "pypi"
-        assert package["identifier"] == "magg"
-        # Top-level and package versions must stay in sync for publishing
-        assert package["version"] == manifest["version"]
-        assert package["transport"]["type"] == "stdio"
-
-    def test_server_json_version_matches_package(self):
-        """Fail release PRs that bump pyproject.toml but forget server.json."""
-        if magg.__version__ == "unknown":
-            pytest.skip("magg package metadata not available")
-
-        root = Path(__file__).parent.parent.parent
-        manifest = json.loads((root / "server.json").read_text())
-        assert manifest["version"] == magg.__version__

From 8aad64d3407c6861955071a1ca70d2f29754f35c Mon Sep 17 00:00:00 2001
From: Phillip Sitbon <phillip.sitbon@gmail.com>
Date: Sat, 26 Sep 2026 09:21:52 -0700
Subject: [PATCH 14/14] Enhance CI with version consistency check job

Added version consistency check for server.json and pyproject.toml.
---
 .github/workflows/test.yml | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index aad76d7..c5801dc 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -8,6 +8,8 @@ on:
       - 'test/**'
       - 'pyproject.toml'
       - 'uv.lock'
+      - 'server.json'
+      - 'scripts/bump_version.py'
       - '.github/workflows/test.yml'
   pull_request:
     branches: ['*']
@@ -16,36 +18,48 @@ on:
       - 'test/**'
       - 'pyproject.toml'
       - 'uv.lock'
+      - 'server.json'
+      - 'scripts/bump_version.py'
       - '.github/workflows/test.yml'
 
 permissions:
   contents: read
 
 jobs:
+  versions:
+    name: Check version consistency
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v7
+
+      - name: Check server.json matches pyproject.toml
+        run: python3 scripts/bump_version.py --check
+
   test:
     runs-on: ubuntu-latest
     strategy:
       matrix:
         python-version: ['3.12', '3.13', '3.14']
       fail-fast: false  # Continue testing other versions if one fails
-    
+
     steps:
       - name: Checkout code
         uses: actions/checkout@v7
-      
+
       - name: Install uv
         uses: astral-sh/setup-uv@v8.2.0
         with:
           enable-cache: true
           cache-suffix: ${{ matrix.python-version }}
-      
+
       - name: Set up Python ${{ matrix.python-version }}
         run: uv python install ${{ matrix.python-version }}
-      
+
       - name: Install dependencies
         run: |
           uv sync --all-groups --locked --python ${{ matrix.python-version }}
 
       - name: Run tests
         run: |
-          uv run --python ${{ matrix.python-version }} pytest -v
\ No newline at end of file
+          uv run --python ${{ matrix.python-version }} pytest -v