Robustness fixes: config data loss, unmounting, hung backends, CLI and mbro bugs

[sitbon]

A sweep of bugs found by exercising Magg, the magg CLI and mbro. Every fix was reproduced first and has a regression test. Tests go from 244 to 334 and pass on Python 3.12 and 3.13. About 1,150 of the added lines are tests.

Bumps the version to 1.3.0, so merging publishes a release.

The commits are split by area and can be reviewed one at a time.

Config data loss (0b961e4, d432c19)

Each of these silently deleted user config while reporting success:

• A JSON syntax error in config.json (e.g. a trailing comma), followed by any save, replaced the file with only the new entry. The save is now refused and the file is left alone. An empty file still counts as new.
• With auto-reload on (the default), the reloader cached a config parsed from servers only, so the first save from a running server erased kits. It now uses the same loader as ConfigManager.
• Server entries that fail validation, and top-level settings like self_prefix, were dropped on save. They are now written back unchanged.
• ServerConfig, KitConfig, AuthConfig and BearerAuthConfig were BaseSettings without an env prefix, so they read env vars like PREFIX, URI, ENV and KEY_PATH. Termux sets PREFIX=/usr, which made every server without an explicit prefix invalid, and the next save deleted them. They are plain models now.
• Config and auth.json writes go through a temp file and rename. The file mode is kept, and symlinks are followed.
• 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 later reload.
• A kit.d next to a config given with --config or MAGG_CONFIG_PATH is now searched, as the docs said.
• MAGG_RELOAD_USE_WATCHDOG was documented but ignored. It's implemented now; false forces polling.

Server core (4a88192, 3c7036f)

• Unmounting did nothing on fastmcp 3. The old code looked for _tool_manager._mounted_servers, which no longer exists. As a result:

  • After disable or remove, the server's tools stayed listed, and calling one restarted the subprocess.
  • Re-enabling added a duplicate provider.
  • An update applied through reload kept the old provider first, so the change never took effect.

  Mounting now keeps the provider it adds and removes it from mcp.providers on unmount.

• magg_check failed whenever any server was mounted, because it called server_info.get("client") on a dataclass. This is the same fix as Fix Docker build errors and MountedServer attribute access #2.

• magg_add_server with a uri always failed (an AnyUrl passed into a str field). It now also requires command or uri.

• One hung backend froze everything, including Magg's own tools.

  • Backend clients now get an init_timeout: MAGG_BACKEND_INIT_TIMEOUT, default 30s, 0 to disable.
  • ProxyFastMCP resolves Magg's own components before asking backends.

• Backend stderr suppression was never applied: fastmcp imports stdio_client directly, so the patch had no effect. It now uses the transport's log_file, and stdio_patch.py is deleted.

• Signal handling:

  • SIGTERM didn't stop stdio mode: signal.signal plus Event.set doesn't wake an idle loop. Handlers now go through loop.add_signal_handler.
  • A second signal now forces exit.
  • SIG_DFL (which is 0) is now restored correctly.

• Read-only mode is checked before mounting or changing anything. Previously add_server mounted first and only then failed to save.

• A server's own env now overrides --env-pass and --env-set; before, it was the other way around.

• HTTP and hybrid mode refuse to start if a configured private key can't be loaded. Previously Magg logged a warning and served without auth. Stdio only warns.

CLI (0609c98, 6d1a0d7, 3c7036f)

• auth init failed when ~/.ssh was missing, and printed a traceback when a key already existed.
• auth init --key-path wasn't saved.
• The private key file is created 0600 from the start.
• auth private-key --export output couldn't be eval'd.
• Server search:
  • A GitHub repo with no description, or an npm package with no author, discarded all results from that source.
  • Sources are now searched in parallel with a 15s timeout, instead of one after another with aiohttp's 300s default.
• An invalid MAGG_* value prints one line naming the env var instead of a traceback on every command.
• On Python 3.12.3 (Ubuntu 24.04's system Python), argparse rejected magg server update --env as an ambiguous abbreviation of --env-pass/--env-set. Abbreviations are now disabled on the top-level parser.

mbro (32ff7fc, 44feea7)

• status always crashed.
• JSON tool arguments failed when written the way the help text said, and multi-line JSON in scripts didn't work.
• key=value values are parsed as JSON where possible, and parameters the tool's schema declares as strings are left as strings. value=10-20 used to crash, and 007 became 7.
• Failures in -n, -x and stdin mode now exit 1, and one failing command no longer stops the rest. There's a new --fail-fast flag.
• http://host/mcp was rewritten to /mcp/mcp/.
• Connecting now has a timeout (--timeout, default 30s, 0 to disable).
• Connect errors include the actual cause.
• Help and docs now match the actual commands.

Tests

The new tests check behavior rather than internals where possible:

• Backend stderr is checked at the file descriptor level.
• Reload tests rename real files over the watched config.
• Search concurrency is proven with a barrier instead of timing.

Each regression test fails against the pre-fix code, except a few that guard against regressions from the refactor itself (mode and symlink preservation on save, unparseable files during reload).

Behavior changes

• HTTP and hybrid mode fail closed when a configured private key can't be loaded.
• magg_add_server requires command or uri.
• New config files are created 0600.
• A backend that takes more than 30s to initialize (e.g. a first npx download) now times out. Raise MAGG_BACKEND_INIT_TIMEOUT if that's a problem.
• Global CLI options no longer accept abbreviations (e.g. --conf).
• mbro failures exit 1.

Docs

• --env-pass/--env-set are global options, so they go before serve (magg --env-pass serve).
• Replaced the nonexistent MAGG_KITD_PATH with how discovery actually works through MAGG_PATH.
• An omitted prefix means no prefix, not the server name.
• MAGG_PREFIX_SEP only applies to Magg's own tools.
• The magg_check timeout default is 2.5.
• The kit example is now valid JSON.

Known gaps (not in this PR)

• Backend progress, log, sampling and elicitation messages aren't forwarded to clients. This needs fastmcp's ProxyClient.
• A hung backend with no prefix still delays lookups of other backends' tools by the init timeout.
• Concurrent CLI writes can lose an update, since nothing locks the file.

Supersedes #2.

[Copilot]

Copilot review overview

🟡 Changes recommended

mbro --timeout 0 is documented as disabling timeouts but currently passes init_timeout=0 through to fastmcp, which likely causes immediate timeouts instead of disabling them.

Review effort: Lite
Findings: 1

Open (1)

• Treat zero timeout as no init timeout · New

What changed in this PR

This PR is a broad robustness sweep across Magg’s config persistence/reload, server mounting lifecycle, CLI UX, and the mbro client, with substantial regression test additions to prevent data loss and hangs.

Changes:

• Harden config load/save/reload to avoid silent data loss (atomic writes, preserve invalid entries, watchdog rename/create handling, env isolation, kit discovery).
• Fix server lifecycle issues (provider-based unmounting, hung-backend init timeout, signal handling, fail-closed HTTP auth).
• Improve CLI/mbro ergonomics and resilience (better error reporting, concurrent search, safer auth init/export, improved mbro scripting/argument parsing and exit codes).

File	Description
test/​magg/​test_server_robustness.py	New end-to-end robustness tests for unmounting, hung backends, read-only mode, auth fail-closed, and signals.
test/​magg/​test_mbro_commands.py	New tests covering mbro argument parsing, error handling, batch execution, and exit codes.
test/​magg/​test_mbro_cli_overhaul.py	Adjust tests to validate the new run_commands() batching behavior.
test/​magg/​test_kit_tools.py	Update kit tool tests to account for read-only gating.
test/​magg/​test_config_safety.py	New tests ensuring config persistence never loses user-written data and reload edge cases behave safely.
test/​magg/​test_config_reload.py	Update tests to reflect MountedServer being a dataclass (not dict-like).
test/​magg/​test_cli_fixes.py	New CLI tests for auth init/export, search robustness, and invalid env var handling.
readme.md	Document new/clarified env vars and backend init timeout behavior.
magg/​util/​stdio_patch.py	Remove ineffective stderr monkeypatch approach.
magg/​settings.py	Implement atomic JSON writes, add config schema hardening, add new settings (reload_use_watchdog, backend_init_timeout), and prevent env leakage in nested models.
magg/​server/​server.py	Add read-only upfront checks, stricter add_server validation, improved mount failure messages, and safer URI handling.
magg/​server/​runner.py	Improve signal handling with loop-based handlers, escalation on repeat signals, and bounded shutdown behavior.
magg/​server/​manager.py	Rework mount/unmount to track and remove providers correctly; add init timeout and proper stderr suppression; enforce fail-closed HTTP auth.
magg/​server/​defaults.py	Update proxy tool parameter docs to match actual semantics.
magg/​reload.py	Use ConfigManager for consistent config loading, handle rename/create events, and honor MAGG_RELOAD_USE_WATCHDOG.
magg/​proxy/​server.py	Resolve local components before querying backend providers to avoid hung backend stalling lookups.
magg/​proxy/​mixin.py	Align parameter descriptions with updated proxy behavior.
magg/​mbro/​scripts.py	Run scripts via run_commands() and avoid exit(1) from deep command handlers.
magg/​mbro/​parser.py	Replace JSON arg parsing with brace-aware multiline/JSON-preserving parsing.
magg/​mbro/​formatter.py	Track error count for batch exit codes; harden formatting for missing descriptions; expand help text.
magg/​mbro/​command.py	Improve connection/status handling, schema-aware arg parsing, better null-safe filtering, and tool name suggestions.
magg/​mbro/​client.py	Add URL normalization, connect timeout support, and improve connect error reporting.
magg/​mbro/​cli.py	Add batch execution (run_commands), fail-fast mode, connect timeout option, and non-interactive exit code behavior.
magg/​kit.py	Prevent env leakage by switching models and improve kit discovery when config path is outside MAGG_PATH.
magg/​discovery/​search.py	Add bounded concurrent searching and null-safe parsing of upstream results.
magg/​discovery/​metadata.py	Add timeout to GitHub metadata fetch to avoid long stalls.
magg/​cli.py	Disable argparse abbreviations, improve ValidationError reporting, and harden auth init/export flows.
magg/​auth.py	Ensure key directory parents are created, enforce 0600-at-creation semantics, and improve failure messages.
magg/​__main__.py	Remove eager process.setup() at import-time; delegate to CLI main().
docs/​mbro.md	Update mbro docs to match new commands, scripting, JSON/key=value parsing, exit codes, and timeouts.
docs/​kits.md	Correct kit discovery documentation and fix JSON example validity.
docs/​index.md	Refresh docs for changed defaults/options, env var behavior, and prefix semantics.
docs/​config-reload.md	Clarify watchdog control and document safety behavior for invalid JSON / invalid server entries.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sitbon]

Re: the Copilot finding on --timeout 0: this doesn't reproduce. fastmcp treats an init_timeout of 0 as disabled: Client.__init__ comments "0 means disabled", and normalize_timeout_to_seconds(0) returns None. Normalizing 0 to None in mbro would actually be worse, since None falls back to fastmcp's FASTMCP_CLIENT_INIT_TIMEOUT setting. The error wrapping is already skipped for 0 (if timeout and ...).

a5c2849 adds a test that connects to sleep 100 with timeout=0 and checks it's still waiting after 1s. The test fails if 0 is turned into a short timeout.