extract_servers raises on a non-dict server value — unhandled crash on load, and it makes the schema lint unreachable #72

New Issue

2026-07-20T12:06:58-04:00

the_og commented

2026-07-20 12:06:58 -04:00

What

extract_servers() does dict(data) on every server value without checking the shape:

for name, data in (cfg.get("mcpServers") or {}).items():
    out.append(ServerEntry(name=name, data=dict(data), enabled=True))

A config that is perfectly valid JSON but has a server whose value isn't an object blows up:

'not-a-dict'  -> ValueError: dictionary update sequence element #0 has length 1; 2 is required
123           -> TypeError: 'int' object is not iterable
['a', 'b']    -> ValueError: dictionary update sequence element #0 has length 1; 2 is required
None          -> TypeError: 'NoneType' object is not iterable

(Reproduced against cd2ac2f.)

Why it's a crash, not a handled error

In MainWindow._load_profile, the try/except only wraps the parse. Strict parse succeeds here — the JSON is well-formed — so the repair path is never entered, and core.extract_servers(self.full_config) (bcc.py ~L1944) runs outside any handler. Result: unhandled traceback, and self.current_profile has already been assigned while self.servers has not — the window is left in a half-loaded state.

Why it also defeats #54

lint_server() already has the right warnings for malformed shapes, but it can never fire on this class of problem: the load crashes before lint ever sees the entry. The structural-lint feature is unreachable for the single most-likely hand-edit mistake.

This also cuts against the project's core premise — a file that is valid JSON but structurally off should be reported and repaired, never a hard failure.

Suggested fix

Guard in extract_servers: coerce non-dict values to {} (or keep the raw value on the entry) rather than calling dict() on them, and let lint_server surface the warning.
Add a lint_server branch for "server definition is not an object" so the user gets told which entry is malformed.
Move extract_servers inside the load try/except as defence in depth.
Round-trip test: a malformed entry must not be silently dropped on Save (preserve-as-is is fine, silent deletion is not).

Tests

{"mcpServers": {"foo": "not-a-dict"}} and the int/list/None variants load without raising and produce a lint warning naming foo.

## What `extract_servers()` does `dict(data)` on every server value without checking the shape: ```python for name, data in (cfg.get("mcpServers") or {}).items(): out.append(ServerEntry(name=name, data=dict(data), enabled=True)) ``` A config that is **perfectly valid JSON** but has a server whose value isn't an object blows up: ``` 'not-a-dict' -> ValueError: dictionary update sequence element #0 has length 1; 2 is required 123 -> TypeError: 'int' object is not iterable ['a', 'b'] -> ValueError: dictionary update sequence element #0 has length 1; 2 is required None -> TypeError: 'NoneType' object is not iterable ``` (Reproduced against `cd2ac2f`.) ## Why it's a crash, not a handled error In `MainWindow._load_profile`, the `try/except` only wraps the parse. Strict parse **succeeds** here — the JSON is well-formed — so the repair path is never entered, and `core.extract_servers(self.full_config)` (bcc.py ~L1944) runs outside any handler. Result: unhandled traceback, and `self.current_profile` has already been assigned while `self.servers` has not — the window is left in a half-loaded state. ## Why it also defeats #54 `lint_server()` already has the right warnings for malformed shapes, but it can never fire on this class of problem: the load crashes before lint ever sees the entry. The structural-lint feature is unreachable for the single most-likely hand-edit mistake. This also cuts against the project's core premise — a file that is valid JSON but structurally off should be *reported and repaired*, never a hard failure. ## Suggested fix - Guard in `extract_servers`: coerce non-dict values to `{}` (or keep the raw value on the entry) rather than calling `dict()` on them, and let `lint_server` surface the warning. - Add a `lint_server` branch for "server definition is not an object" so the user gets told which entry is malformed. - Move `extract_servers` inside the load try/except as defence in depth. - Round-trip test: a malformed entry must not be silently dropped on Save (preserve-as-is is fine, silent deletion is not). ## Tests `{"mcpServers": {"foo": "not-a-dict"}}` and the int/list/None variants load without raising and produce a lint warning naming `foo`.

the_og added the P1 label 2026-07-20 12:06:58 -04:00

the_og referenced this issue from a commit

2026-07-20 12:11:58 -04:00

fix: tolerate non-object server values on load; keep named sets across a merge

the_og referenced a pull request that will close this issue

2026-07-20 12:12:19 -04:00

fix: tolerate non-object server values on load; keep named sets across a merge (#72, #73) #77

the_og closed this issue

2026-07-20 12:49:23 -04:00

the_og referenced this issue from a commit

2026-07-20 12:49:24 -04:00

Merge PR #77: tolerate non-object server values; keep named sets across a merge (#72, #73)