MCP Management: toggling one server's stub restarts the whole broker and freezes every control on the page #4317

Existing issues

Searched -- #2780 collapsed the switches, #926 fixed the enable-at-runtime overlay capture, #2931/#4016 cover reaching an already-open chat. None of them covers the cost of the apply itself.

KiroCrew version

0.3.0

Release channel

Nightly (main HEAD)

How is it installed?

From source

Platform

Not platform-specific

What happened

On Developer -> MCP Management -> Servers, flipping one server's stub switch freezes the page for several seconds to tens of seconds. Every other switch on the page -- including the global sharing switch -- goes dead for the whole duration, so it reads as a hung UI.

Two separate things produce that:

1. The apply is a full broker restart, done synchronously inside the request.
POST /api/mcp-gateway/servers/stub persists the allowlist and then awaits _apply_mcp_stub(), which does:

• _stop_mcp_broker() -- SIGTERM and wait for the drain. The supervisor's grace is derived from the daemon's own budget, DRAIN_SECS 10 + POOL_SHUTDOWN_SECS 5 + SIGNAL_MARGIN_SECS 5 = 20s upper bound (mcp_gateway/shutdown_budget.py). The real cost scales with the live fleet, because every pooled backend and in-flight connection has to be torn down.
• _init_mcp_gateway() -- re-runs the rewriter across every agent spec, resolving each declared MCP command with which(), then respawns the daemon (socket wait 5s + ping 2s).
• sessions.refresh_defaults() -- rebuilds the provider factory and warm pool.

Then onSuccess invalidates both queries, and GET /api/mcp-gateway/servers re-reads every agent spec synchronously on the event loop.

On an install with ~70 agent specs and ~35 stubbed servers this is a multi-second to tens-of-seconds operation, and it blocks the dashboard event loop for parts of it.

2. The UI disables every control for the whole mutation.
McpManagement.tsx computes one page-wide busy = setStub.isPending || setSharing.isPending and feeds it to every row's switch and the global sharing switch. There is also no spinner or progress text, so a 20s operation is indistinguishable from a dead page.

Why the restart buys nothing. A session's MCP toolset is frozen at session/new, so existing sessions cannot pick up the new stub set no matter what the broker does; new sessions connect to the daemon fresh either way. The restart is paid for and discarded.

Why it cannot be fixed by making the apply async alone. Applies are serialised under _MCP_GATEWAY_APPLY_LOCK, so simply returning early would queue N full restarts for N clicks -- and an unblocked UI actively invites those clicks. The batch endpoint's own docstring already names this hazard for "toggle all":

The batch form exists because the UI's "toggle all" would otherwise issue one request per server: N config rewrites and N pool re-applies for a single user gesture, each one racing the others for the config lock.

Repeated single toggles take exactly the path that batching exists to avoid.

Steps to reproduce

1. Run a gateway with MCP sharing enabled and a non-trivial number of stubbed servers (the effect grows with the number of live pooled backends and agent specs).
2. Open the dashboard -> Developer -> MCP Management -> Servers.
3. Flip any eligible server's stub switch.
4. Observe: the clicked switch does not settle for several seconds to tens of seconds, and every other switch on the page is disabled for the whole window, with no progress indication.

Relevant log output

# each broker start re-runs the rewriter over every agent spec
WARNING kiro_crew.mcp_gateway.rewriter: rewriter: cannot resolve MCP command '<name>' for opted-in server '<name>' (agent '<agent>')
WARNING kiro_crew.mcp_gateway.rewriter: rewriter: opted-in server '<name>' (agent '<agent>') declares env (1 keys) of which some would not reach the shared backend
...one line per agent spec...

# and the daemon is torn down and respawned
WARNING kiro_crew.mcp_gateway.manager: mcp-gateway: daemon exited rc=0 - respawning in 1.0s
WARNING kiro_crew.mcp_gateway.manager: mcp-gateway ping connect failed: [Errno 2] No such file or directory
INFO  __main__ gatewayd listening socket=<...>/gateway.sock max_backends=64 idle_timeout=300s

Anything else

Root cause, stated once: the daemon resolves a stubbed server's real launch command from its own process environment (env_target_resolver reads KIROCREW_MCP_TARGET_<SERVER> out of os.environ, mcp_gateway/gatewayd.py). A live process's environment cannot be changed, so the only way to change the routing table is to respawn the daemon. _apply_mcp_stub's docstring says as much:

stubbed -> stubbed: RESTART, so the rewriter re-runs with the new set and the daemon re-spawns with updated MC_MCP_TARGET_* env.

Proposed fix -- make the target table a file the daemon re-reads, so there is nothing left to restart.

The resolution seam already exists and is already pluggable. gatewayd calls resolver(pool_key) per connection at backend-spawn time, and its own error text advertises the alternative:

no target mapping for server ...; set KIROCREW_MCP_TARGET_<SERVER> env var or pass a target_resolver

So env_target_resolver is a default implementation, not an architectural constraint. Concretely:

1. The rewriter already computes the whole mapping as target_env ({KIROCREW_MCP_TARGET_<SERVER>[__<hash>]: "cmd arg arg"}). Also write it, atomically, to a small file next to the gateway socket.
2. Add a file-backed resolver that reads that file (mtime-cached) using the identical lookup order and wire format as today -- args-disambiguated key, then bare server name, then the legacy MC_ prefix -- so only the data source changes.
3. _apply_mcp_stub becomes: rewrite agent specs + write the target file. No stop, no start.

Consequences:

• Live pooled backends keep serving -- their PoolKey is unchanged.
• A newly stubbed server resolves on its first connection.
• An unstubbed server simply stops receiving new connections and is reclaimed by the existing idle sweep. An immediate teardown, if wanted later, is a targeted "evict backends for server X" control frame, not a restart.
• The apply becomes a single atomic file write, so it does not need to leave the request path at all, and repeated clicks cannot queue expensive work. This is what makes the page-wide busy freeze go away without building a background-job + progress + coalescing layer that would otherwise be needed.

Prewarm is unaffected (checked, since it is the other thing captured at daemon spawn): prewarm_count is a scalar unrelated to the stub set, and prewarming is driven by observed hot keys from the hot-key store plus a periodic top-up pass, not by the stub list. A newly stubbed server has no hot-key history to warm from -- the same as today after a restart -- and enters the store naturally once used. An unstubbed server whose hot key is still in the store is already handled: prewarm_from_payloads documents "One payload failing (unknown target, spawn error, malformed key) is logged and skipped -- it must not abort the remaining prewarms", so an unresolvable target degrades to a logged skip.

Security requirement, not optional. Today the routing table can only be injected by whoever sets the daemon's spawn environment. Making it a file the daemon re-reads at runtime turns it into a runtime-writable input that decides which processes the daemon launches. So the file must be owner-only (0600), written atomically, and validated on every read (refuse a file the daemon does not own, or that is group/world-writable); parsing must stay shlex.split with no shell; and it must carry command lines only, never environment values -- backend env stays on its existing separate path, and that boundary must not move.

Open question for the maintainer. Whether the file should replace the spawn-env mapping or be a second source consulted on miss. Replacing is cleaner and removes the only reason the daemon has to be restarted; keeping both means a stale environment can shadow a newer file and the two can disagree. My inclination is to replace it, keeping only the legacy-key acceptance for compatibility with an already-running daemon, but that is a compatibility call worth making explicitly.

Also worth doing, but out of scope unless asked: the UI should disable only the row being applied rather than the whole page, and GET /api/mcp-gateway/servers should not read every agent spec synchronously on the event loop.