fix(mcp): tolerate a malformed mcpServers or participants section (#433)

[pjdoland]

Summary

"mcpServers": null in mcp.json took out MCP setup for the whole session. update_mcp_servers defaulted with mcp_config.get(key, {}), which returns the default only when the key is absent, so an explicit JSON null yielded None and the loader then raised AttributeError: 'NoneType' object has no attribute 'keys' at every session start. The user lost every MCP server and got no error at the point they caused it; a later reader saw an unrelated traceback.

participants had the identical defect. It is read on the next line and iterated directly, so "participants": null failed the same way. That half was not in the original report and is fixed here too.

The shape validator (validate_mcp_config) already rejects both values, but it is wired only into the save path (extension.py:1059). A file already on disk never meets it: hand-edited, written by an older build, or produced by any tool other than the settings dialog. The read path runs at session start via ai_service_manager.py:257-258 and again from extension.py:1084 and :1103.

Solution

The two section reads go through a small helper that coerces a non-mapping value to empty and logs a warning naming the key.

Choosing to degrade rather than validate-and-refuse on read: no servers is a state the loader already handles, and every downstream use of both sections is inert against an empty mapping (servers_config at :892, :898, :907; participants_config at :885, :886, :908). Refusing to start would turn a bad optional section into a hard failure of the whole extension. The strict validator stays untouched on the save path, where rejecting bad input at the point of entry is still the right behavior.

The helper distinguishes an absent key from a present-but-unusable one: absent is silent, since configuring only one of the two sections is the ordinary shape, while a present bad value warns and names which key, because the loader reads two sections of identical shape and "invalid mcp.json" alone would not say which.

Testing

pytest 1771 passed, including 18 new cases in tests/test_mcp_null_config.py.

Unlike a helper-only suite, these drive the real MCPManager.update_mcp_servers: with no servers configured the loader connects nothing, so the manager is constructible in a unit test without stubbing the worker machinery. Covered: null mcpServers, null participants, both null, wrong-typed sections (string, list, int, bool), the ordinary empty config, a valid section still reaching create_servers with disabled honored, and the reload path (update_mcp_servers is called again on every config POST) leaving the manager empty rather than half-populated.

Verified by mutation that these pin the regression rather than the implementation. Reverting just the two call sites to mcp_config.get(key, {}) fails 8 of the 18; the 10 that still pass are the pure-helper cases plus the ordinary empty and valid paths, which is the expected split since those do not depend on the loader wiring. The original AttributeError also reproduces directly from the old expression.

tsc --noEmit clean, jest 423 passed, eslint, stylelint, prettier clean. No TypeScript changed.

Risks and follow-ups

• A malformed section is now silent to the user, visible only as a server-log warning and absent servers in the panel. That is strictly better than the traceback it replaces, but the panel still gives no in-UI reason. Surfacing "this server was ignored, and why" in the MCP panel is a separate piece of work and would also cover the admin-policy rejection path, which has the same no-UI-signal problem.
• The helper only guards the two sections the loader reads. It does not validate their contents; a server entry that is itself malformed still takes the existing create_servers error path.
• Review coverage was curtailed: the persona review I would normally run was cut short by a session rate limit, so this had my own diff pass plus the automated gate and the mutation check rather than independent reviewer eyes.

Closes #433