Merge branch 'main' into fix/78-79-update-visibility

Three conflicts, two of them semantic rather than textual: - bcc.py QSS: this branch added the noticeBanner rules using the old module-level constants ({MUTED}, {ACCENT}); main had since moved the stylesheet onto palette slots ({p.muted}). Took main's form and translated the notice rules into it -- picking either side wholesale would have either dropped the banner styling or reintroduced globals that test_stylesheet_builder_has_no_hardcoded_colours now forbids. - bcc.py methods: both sides appended to MainWindow (update-notice handlers vs theme handlers). Additive, kept both. - tests/test_core.py: the usual EOF append. Kept both blocks. _build_menu_bar auto-merged cleanly (View menu above, Help menu below); verified both are present with their menu roles intact. Verified: 272 test functions = 265 (main) + 7 (this branch), no duplicates; 421 passed, ruff clean.
2026-07-20 12:51:55 -04:00
parent 3068e74e5c 9a0433225e
commit 7517e16b15
3 changed files with 684 additions and 65 deletions
@@ -15,6 +15,7 @@ from __future__ import annotations

 import base64
 import contextlib
+import copy
 import difflib
 import functools
 import glob
@@ -49,6 +50,12 @@ DISABLED_KEY = "_disabledMcpServers"
 # parks the rest under DISABLED_KEY.
 SETS_KEY = "_bccServerSets"

+# Top-level keys BCC itself authors. They live in the client's config file, but
+# BCC is their owner, so on a stale-file merge the in-memory copy wins over the
+# on-disk one (see `carry_owned_keys`). Any future BCC-authored key belongs
+# here -- forgetting to add one is exactly how #73 happened.
+BCC_OWNED_KEYS = (SETS_KEY,)
+
 BACKUP_DIRNAME = ".bcc_backups"
 MAX_BACKUPS = 15

@@ -211,16 +218,195 @@ class Profile:
        self.path = Path(self.path)


+class _NoRaw:
+    """Sentinel for ServerEntry.raw.
+
+    `None` can't do this job: `{"mcpServers": {"foo": null}}` is legal JSON and
+    a real malformed-entry case, so None has to mean "the config said null",
+    not "there was nothing here".
+    """
+
+    __slots__ = ()
+
+    def __repr__(self) -> str:  # keeps ServerEntry reprs readable in test output
+        return "<no raw>"
+
+
+NO_RAW = _NoRaw()
+
+
@dataclass
 class ServerEntry:
+    """One server definition.
+
+    `data` is always a dict so every consumer can treat it as one. When the
+    config held something that wasn't a JSON object for this server (a string,
+    a number, a list -- all legal JSON, all wrong here), `data` is empty and
+    the original value is preserved verbatim in `raw` so Save round-trips it
+    instead of silently deleting the user's line. `lint_servers` surfaces it.
+    `raw` defaults to the NO_RAW sentinel rather than None, because a config
+    value of literal `null` is itself a malformed entry worth preserving.
+
+    Assigning `data` means the user replaced the definition through the editor,
+    which retires `raw` -- use `set_data` so that can't be forgotten.
+    """
+
    name: str
    data: dict
    enabled: bool = True
+    raw: object = NO_RAW

    @property
    def kind(self) -> str:
        return "remote" if "url" in self.data and "command" not in self.data else "stdio"

+    @property
+    def malformed(self) -> bool:
+        """True when the config value for this server wasn't a JSON object."""
+        return self.raw is not NO_RAW
+
+    def set_data(self, data: dict) -> None:
+        """Replace the definition from the editor, clearing any malformed original."""
+        self.data = data
+        self.raw = NO_RAW
+
+    def config_value(self):
+        """What to write back to the config: the edited dict, or the untouched
+        malformed original when the user never edited it."""
+        return self.data if self.raw is NO_RAW else self.raw
+
+
+# --------------------------------------------------------------------------- #
+# Theming (issue #75)
+# --------------------------------------------------------------------------- #
+THEME_SYSTEM = "system"
+THEME_LIGHT = "light"
+THEME_DARK = "dark"
+THEME_CHOICES = (THEME_SYSTEM, THEME_LIGHT, THEME_DARK)
+
+
+@dataclass(frozen=True)
+class Palette:
+    """Every colour the UI draws with.
+
+    Deliberately exhaustive: the stylesheet used to inline a handful of
+    near-black literals (`#1a1205` for text on the accent, `#202229` for the
+    disabled table, `#16181d` for the diagnostics pane), which is fine while
+    there's one theme and invisible breakage the moment there are two. Each
+    gets a slot here so a light palette can't silently inherit a dark value.
+    """
+
+    name: str
+    accent: str
+    accent_dim: str
+    bg: str
+    panel: str
+    panel_2: str
+    text: str
+    muted: str
+    border: str
+    good: str
+    bad: str
+    warn: str
+    remote: str
+    on_accent: str  # text drawn on top of an accent fill
+    disabled_bg: str  # the parked-servers table
+    mono_bg: str  # diagnostics / log panes
+    selection_text: str
+
+
+# The shipping theme through v1.3.0. These values are carried over verbatim --
+# adding a light theme must not restyle the dark one.
+DARK_PALETTE = Palette(
+    name="dark",
+    accent="#f97316",
+    accent_dim="#c2570b",
+    bg="#1b1d23",
+    panel="#23262e",
+    panel_2="#2b2f39",
+    text="#e7e9ee",
+    muted="#9aa0ad",
+    border="#3a3f4b",
+    good="#4ade80",
+    bad="#f87171",
+    warn="#fbbf24",
+    remote="#60a5fa",
+    on_accent="#1a1205",
+    disabled_bg="#202229",
+    mono_bg="#16181d",
+    selection_text="#ffffff",
+)
+
+# The semantic colours are NOT the dark ones lightened. #4ade80 / #fbbf24 sit
+# around 1.7:1 against white -- illegible. These are darkened to clear 4.5:1,
+# which `test_light_palette_meets_contrast` enforces so nobody "tidies" them
+# back toward the dark hues later.
+LIGHT_PALETTE = Palette(
+    name="light",
+    accent="#c2410c",
+    accent_dim="#9a3412",
+    bg="#f6f7f9",
+    panel="#ffffff",
+    panel_2="#eef0f4",
+    text="#1b1d23",
+    muted="#5c6270",
+    border="#d3d7de",
+    good="#15803d",
+    bad="#b91c1c",
+    warn="#a16207",
+    remote="#1d4ed8",
+    on_accent="#ffffff",
+    disabled_bg="#e9ebef",
+    mono_bg="#f0f2f5",
+    selection_text="#ffffff",
+)
+
+PALETTES = {DARK_PALETTE.name: DARK_PALETTE, LIGHT_PALETTE.name: LIGHT_PALETTE}
+
+
+def resolve_theme(setting: str, system_is_dark: bool) -> str:
+    """Map a stored theme setting + the OS appearance onto a concrete palette name.
+
+    Anything unrecognised (a hand-edited QSettings value, a setting written by
+    a future version) falls back to following the system rather than to a
+    fixed theme -- the user's desktop is the better guess.
+    """
+    if setting == THEME_DARK:
+        return THEME_DARK
+    if setting == THEME_LIGHT:
+        return THEME_LIGHT
+    return THEME_DARK if system_is_dark else THEME_LIGHT
+
+
+def palette_for(theme: str) -> Palette:
+    """Concrete palette by name; unknown names fall back to dark (the historical look)."""
+    return PALETTES.get(theme, DARK_PALETTE)
+
+
+def _hex_to_rgb(value: str) -> tuple[int, int, int]:
+    v = value.lstrip("#")
+    if len(v) == 3:
+        v = "".join(ch * 2 for ch in v)
+    return int(v[0:2], 16), int(v[2:4], 16), int(v[4:6], 16)
+
+
+def relative_luminance(color: str) -> float:
+    """WCAG relative luminance for a #rrggbb colour."""
+
+    def chan(c: int) -> float:
+        srgb = c / 255.0
+        return srgb / 12.92 if srgb <= 0.04045 else ((srgb + 0.055) / 1.055) ** 2.4
+
+    r, g, b = (chan(c) for c in _hex_to_rgb(color))
+    return 0.2126 * r + 0.7152 * g + 0.0722 * b
+
+
+def contrast_ratio(fg: str, bg: str) -> float:
+    """WCAG contrast ratio between two #rrggbb colours (1.0 to 21.0)."""
+    a, b = relative_luminance(fg), relative_luminance(bg)
+    lighter, darker = max(a, b), min(a, b)
+    return (lighter + 0.05) / (darker + 0.05)
+

 # --------------------------------------------------------------------------- #
 # Discovery
@@ -485,13 +671,30 @@ def repair_config_file(path: str | os.PathLike) -> tuple[dict, list[str], str]:
    return obj, notes, pretty


+def _server_entry(name: str, data, enabled: bool) -> ServerEntry:
+    """Build a ServerEntry, tolerating a value that isn't a JSON object.
+
+    A hand-edited config can legally hold `{"mcpServers": {"foo": "oops"}}` --
+    valid JSON, wrong shape. Calling dict() on that raises, which used to take
+    the whole load down before the linter ever got a look at it (#72). Keep the
+    original instead and let the linter report it.
+    """
+    if isinstance(data, dict):
+        return ServerEntry(name=name, data=dict(data), enabled=enabled)
+    return ServerEntry(name=name, data={}, enabled=enabled, raw=data)
+
+
 def extract_servers(cfg: dict) -> list[ServerEntry]:
-    """Pull enabled (`mcpServers`) and disabled (`_disabledMcpServers`) servers."""
+    """Pull enabled (`mcpServers`) and disabled (`_disabledMcpServers`) servers.
+
+    Never raises on a structurally-odd config -- malformed entries come back as
+    empty-data entries carrying their original value (see `_server_entry`).
+    """
    out: list[ServerEntry] = []
    for name, data in (cfg.get("mcpServers") or {}).items():
-        out.append(ServerEntry(name=name, data=dict(data), enabled=True))
+        out.append(_server_entry(name, data, True))
    for name, data in (cfg.get(DISABLED_KEY) or {}).items():
-        out.append(ServerEntry(name=name, data=dict(data), enabled=False))
+        out.append(_server_entry(name, data, False))
    return out


@@ -583,8 +786,8 @@ def apply_servers(cfg: dict, servers: list[ServerEntry]) -> dict:
    Write the server list back into `cfg` in place, preserving every other key
    and the position of `mcpServers`. Returns the same dict for convenience.
    """
-    enabled = {s.name: s.data for s in servers if s.enabled}
-    disabled = {s.name: s.data for s in servers if not s.enabled}
+    enabled = {s.name: s.config_value() for s in servers if s.enabled}
+    disabled = {s.name: s.config_value() for s in servers if not s.enabled}

    cfg["mcpServers"] = enabled  # replaces value if key existed; appends otherwise
    if disabled:
@@ -597,6 +800,34 @@ def apply_servers(cfg: dict, servers: list[ServerEntry]) -> dict:
 # --------------------------------------------------------------------------- #
 # Write (atomic, with rotating backups)
 # --------------------------------------------------------------------------- #
+def carry_owned_keys(local_cfg: dict, fresh_cfg: dict) -> list[str]:
+    """Carry BCC-authored top-level keys from `local_cfg` onto `fresh_cfg`.
+
+    Used by the stale-file "Merge & save" path, which reloads the file from
+    disk and re-applies the user's server edits. That reload used to drop
+    anything BCC owns but `apply_servers` doesn't write -- named server sets
+    vanished without a word (#73). BCC owns these keys, so the in-memory copy
+    wins; mutates `fresh_cfg` in place.
+
+    Returns the keys where the on-disk copy differed and was overwritten, so
+    the caller can tell the user something was actually contested rather than
+    merely carried across.
+
+    Deliberately one-directional: a key absent locally is left alone on disk.
+    We can't tell "user deleted their last set" from "user never had sets and
+    another machine just added some", and silently deleting someone else's
+    data is the worse of the two failures.
+    """
+    conflicts: list[str] = []
+    for key in BCC_OWNED_KEYS:
+        if key not in local_cfg:
+            continue
+        if key in fresh_cfg and fresh_cfg[key] != local_cfg[key]:
+            conflicts.append(key)
+        fresh_cfg[key] = copy.deepcopy(local_cfg[key])
+    return conflicts
+
+
 def _make_backup(path: Path) -> Path:
    bdir = path.parent / BACKUP_DIRNAME
    bdir.mkdir(exist_ok=True)
@@ -1413,9 +1644,21 @@ def lint_server(name: str, data: dict) -> list[str]:


 def lint_servers(servers: list[ServerEntry]) -> list[str]:
-    """Concatenate lint_server warnings across every entry, in order."""
+    """Concatenate lint_server warnings across every entry, in order.
+
+    Entries whose config value wasn't a JSON object at all are reported here
+    rather than in lint_server, which takes an already-dict `data` (#72).
+    """
    out: list[str] = []
    for s in servers:
+        if s.malformed:
+            nm = s.name.strip() or "(unnamed)"
+            out.append(
+                f"'{nm}': server definition is not an object "
+                f"(found {type(s.raw).__name__}) -- it is preserved as-is; "
+                f"edit it to replace it with a proper definition"
+            )
+            continue
        out.extend(lint_server(s.name, s.data))
    return out