diff --git a/bcc.py b/bcc.py index 46a2425..a7770e0 100644 --- a/bcc.py +++ b/bcc.py @@ -19,6 +19,7 @@ from typing import ClassVar from PySide6.QtCore import QRect, QSettings, QSize, Qt, QThread, QTimer, QUrl, Signal from PySide6.QtGui import ( QAction, + QActionGroup, QColor, QCursor, QDesktopServices, @@ -26,6 +27,7 @@ from PySide6.QtGui import ( QIcon, QKeySequence, QPainter, + QPalette, QPixmap, ) from PySide6.QtWidgets import ( @@ -66,89 +68,122 @@ import bcc_core as core # thread during drag-and-drop import, so skip anything larger than this. MAX_DROP_IMPORT_BYTES = 5 * 1024 * 1024 # 5 MB -# --- One-line rebrand: change this to recolor the whole app --------------- # -ACCENT = "#f97316" # warm orange -ACCENT_DIM = "#c2570b" -BG = "#1b1d23" -PANEL = "#23262e" -PANEL_2 = "#2b2f39" -TEXT = "#e7e9ee" -MUTED = "#9aa0ad" -BORDER = "#3a3f4b" -GOOD = "#4ade80" -BAD = "#f87171" -WARN = "#fbbf24" +# --- Theming (issue #75) -------------------------------------------------- # +# The palette lives in bcc_core (testable without a Qt app); these module-level +# names are rebound by `apply_palette()` whenever the theme changes. +# +# Why globals rather than passing a palette around: ~20 inline +# `setStyleSheet(f"color: {MUTED}")` calls are scattered through this file, and +# an f-string resolves its names when it runs, not when it's compiled. Rebinding +# the globals means every one of those call sites picks up the new colour on its +# next render, with no change to the call sites themselves. +PALETTE = core.DARK_PALETTE +ACCENT = ACCENT_DIM = BG = PANEL = PANEL_2 = TEXT = MUTED = BORDER = "" +GOOD = BAD = WARN = REMOTE = ON_ACCENT = DISABLED_BG = MONO_BG = SEL_TEXT = "" +STATUS_COLORS: dict[str, str] = {} +HEALTH_COLORS: dict[str, str] = {} -STATUS_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": "#60a5fa", "unknown": WARN} -STATUS_GLYPH = {"ok": "●", "missing": "●", "warn": "▲", "remote": "◆", "unknown": "○"} +STATUS_GLYPH = { + "ok": "\u25cf", + "missing": "\u25cf", + "warn": "\u25b2", + "remote": "\u25c6", + "unknown": "\u25cb", +} # Health dot (spawn-test outcome, see core.HealthStatus) shown per row in the # server tables' "Health" column -- distinct from the PATH-dependency Status # column above. -HEALTH_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED} -HEALTH_GLYPH = {"ok": "●", "failed": "●", "untested": "○"} +HEALTH_GLYPH = {"ok": "\u25cf", "failed": "\u25cf", "untested": "\u25cb"} -STYLESHEET = f""" + +def build_stylesheet(p: core.Palette) -> str: + """Render the global QSS for a palette.""" + return f""" /* No font-family here on purpose: Qt already uses the native system UI font on every platform (San Francisco / Segoe UI / desktop default). Naming web-CSS aliases like -apple-system forces a costly font-alias scan. */ -* {{ font-size: 13px; color: {TEXT}; }} -QMainWindow, QDialog {{ background: {BG}; }} +* {{ font-size: 13px; color: {p.text}; }} +QMainWindow, QDialog {{ background: {p.bg}; }} QLabel#h1 {{ font-size: 15px; font-weight: 600; }} -QLabel#muted {{ color: {MUTED}; }} -QFrame#card {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px; }} +QLabel#muted {{ color: {p.muted}; }} +QFrame#card {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px; }} QLineEdit, QPlainTextEdit, QComboBox {{ - background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px; - padding: 6px 8px; selection-background-color: {ACCENT}; selection-color: #1a1205; + background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px; + padding: 6px 8px; selection-background-color: {p.accent}; selection-color: {p.on_accent}; }} -QLineEdit:focus, QPlainTextEdit:focus, QComboBox:focus {{ border: 1px solid {ACCENT}; }} +QLineEdit:focus, QPlainTextEdit:focus, QComboBox:focus {{ border: 1px solid {p.accent}; }} QComboBox::drop-down {{ border: none; width: 22px; }} -QComboBox QAbstractItemView {{ background: {PANEL_2}; border: 1px solid {BORDER}; - selection-background-color: {ACCENT}; outline: none; }} -QPushButton {{ background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px; +QComboBox QAbstractItemView {{ background: {p.panel_2}; border: 1px solid {p.border}; + selection-background-color: {p.accent}; outline: none; }} +QPushButton {{ background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px; padding: 7px 13px; }} -QPushButton:hover {{ border: 1px solid {ACCENT}; }} -QPushButton:disabled {{ color: {MUTED}; background: {PANEL}; }} -QPushButton#primary {{ background: {ACCENT}; border: 1px solid {ACCENT}; color: #1a1205; font-weight: 600; }} -QPushButton#primary:hover {{ background: {ACCENT_DIM}; }} -QPushButton#primary:disabled {{ background: {PANEL}; color: {MUTED}; border: 1px solid {BORDER}; }} -QPushButton#danger:hover {{ border: 1px solid {BAD}; color: {BAD}; }} -QTableWidget {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px; +QPushButton:hover {{ border: 1px solid {p.accent}; }} +QPushButton:disabled {{ color: {p.muted}; background: {p.panel}; }} +QPushButton#primary {{ background: {p.accent}; border: 1px solid {p.accent}; color: {p.on_accent}; font-weight: 600; }} +QPushButton#primary:hover {{ background: {p.accent_dim}; }} +QPushButton#primary:disabled {{ background: {p.panel}; color: {p.muted}; border: 1px solid {p.border}; }} +QPushButton#danger:hover {{ border: 1px solid {p.bad}; color: {p.bad}; }} +QTableWidget {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px; gridline-color: transparent; outline: none; }} QTableWidget::item {{ padding: 6px 8px; border: none; }} -QTableWidget::item:selected {{ background: {ACCENT}; color: #1a1205; }} +QTableWidget::item:selected {{ background: {p.accent}; color: {p.on_accent}; }} /* Inline cell editors: the global QLineEdit padding/radius clips the text inside a table row, so give editors a compact, flat style instead. */ QTableWidget QLineEdit {{ - background: {PANEL_2}; color: {TEXT}; border: 1px solid {ACCENT}; + background: {p.panel_2}; color: {p.text}; border: 1px solid {p.accent}; border-radius: 3px; padding: 0px 4px; margin: 0px; - selection-background-color: {ACCENT_DIM}; selection-color: #ffffff; + selection-background-color: {p.accent_dim}; selection-color: {p.selection_text}; }} -QHeaderView::section {{ background: {PANEL}; color: {MUTED}; border: none; - border-bottom: 1px solid {BORDER}; padding: 8px; font-weight: 600; }} +QHeaderView::section {{ background: {p.panel}; color: {p.muted}; border: none; + border-bottom: 1px solid {p.border}; padding: 8px; font-weight: 600; }} QScrollBar:vertical {{ background: transparent; width: 10px; margin: 2px; }} -QScrollBar::handle:vertical {{ background: {BORDER}; border-radius: 5px; min-height: 24px; }} +QScrollBar::handle:vertical {{ background: {p.border}; border-radius: 5px; min-height: 24px; }} QScrollBar::add-line, QScrollBar::sub-line {{ height: 0; }} -QLabel#statusbar {{ color: {MUTED}; padding: 4px 2px; }} -QLabel#warnBanner {{ color: #1a1205; background: {WARN}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }} -QFrame#noticeBanner {{ background: {PANEL_2}; border: 1px solid {ACCENT}; border-radius: 8px; }} -QLabel#noticeText {{ color: {TEXT}; }} -QPushButton#noticeClose {{ background: transparent; border: none; color: {MUTED}; font-size: 14px; padding: 2px; }} -QPushButton#noticeClose:hover {{ color: {TEXT}; }} -QLabel#section {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }} -QLabel#sectionDisabled {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }} -QLabel#placeholder {{ color: {MUTED}; padding: 12px; background: {PANEL_2}; border: 1px dashed {BORDER}; border-radius: 8px; }} -QTableWidget#disabledTable {{ background: #202229; }} -QTableWidget#disabledTable::item:selected {{ background: {ACCENT}; color: #1a1205; }} +QLabel#statusbar {{ color: {p.muted}; padding: 4px 2px; }} +QLabel#warnBanner {{ color: {p.on_accent}; background: {p.warn}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }} +QFrame#noticeBanner {{ background: {p.panel_2}; border: 1px solid {p.accent}; border-radius: 8px; }} +QLabel#noticeText {{ color: {p.text}; }} +QPushButton#noticeClose {{ background: transparent; border: none; color: {p.muted}; font-size: 14px; padding: 2px; }} +QPushButton#noticeClose:hover {{ color: {p.text}; }} +QLabel#section {{ color: {p.muted}; font-weight: 600; font-size: 12px; padding: 2px 2px; }} +QLabel#sectionDisabled {{ color: {p.muted}; font-weight: 600; font-size: 12px; padding: 2px 2px; }} +QLabel#placeholder {{ color: {p.muted}; padding: 12px; background: {p.panel_2}; border: 1px dashed {p.border}; border-radius: 8px; }} +QTableWidget#disabledTable {{ background: {p.disabled_bg}; }} +QTableWidget#disabledTable::item:selected {{ background: {p.accent}; color: {p.on_accent}; }} QPlainTextEdit#diag {{ font-family: "Menlo", "Cascadia Code", "Consolas", "DejaVu Sans Mono", monospace; - font-size: 12px; background: #16181d; border: 1px solid {BORDER}; border-radius: 8px; }} + font-size: 12px; background: {p.mono_bg}; border: 1px solid {p.border}; border-radius: 8px; }} QFrame#diagCard {{ background: transparent; border: none; }} QSplitter::handle {{ background: transparent; }} -QSplitter::handle:hover {{ background: {BORDER}; border-radius: 4px; }} -QSplitter::handle:pressed {{ background: {ACCENT}; border-radius: 4px; }} +QSplitter::handle:hover {{ background: {p.border}; border-radius: 4px; }} +QSplitter::handle:pressed {{ background: {p.accent}; border-radius: 4px; }} """ +def apply_palette(p: core.Palette) -> str: + """Rebind the module-level colour names to `p` and return its stylesheet.""" + global PALETTE, ACCENT, ACCENT_DIM, BG, PANEL, PANEL_2, TEXT, MUTED, BORDER + global GOOD, BAD, WARN, REMOTE, ON_ACCENT, DISABLED_BG, MONO_BG, SEL_TEXT + global STATUS_COLORS, HEALTH_COLORS + PALETTE = p + ACCENT, ACCENT_DIM = p.accent, p.accent_dim + BG, PANEL, PANEL_2 = p.bg, p.panel, p.panel_2 + TEXT, MUTED, BORDER = p.text, p.muted, p.border + GOOD, BAD, WARN, REMOTE = p.good, p.bad, p.warn, p.remote + ON_ACCENT, DISABLED_BG, MONO_BG, SEL_TEXT = ( + p.on_accent, + p.disabled_bg, + p.mono_bg, + p.selection_text, + ) + STATUS_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": REMOTE, "unknown": WARN} + HEALTH_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED} + return build_stylesheet(p) + + +STYLESHEET = apply_palette(core.DARK_PALETTE) + + # --------------------------------------------------------------------------- # # Background reachability tester (keeps the UI responsive during the request) # --------------------------------------------------------------------------- # @@ -1643,6 +1678,22 @@ class MainWindow(QMainWindow): # --- menu bar ---------------------------------------------------------- # def _build_menu_bar(self): + view_menu = self.menuBar().addMenu("&View") + theme_menu = view_menu.addMenu("Theme") + self._theme_group = QActionGroup(self) + self._theme_group.setExclusive(True) + current = stored_theme_setting() + for setting, label in ( + (core.THEME_SYSTEM, "Match system"), + (core.THEME_LIGHT, "Light"), + (core.THEME_DARK, "Dark"), + ): + act = QAction(label, self, checkable=True) + act.setChecked(setting == current) + act.triggered.connect(lambda _checked=False, s=setting: self._set_theme(s)) + self._theme_group.addAction(act) + theme_menu.addAction(act) + help_menu = self.menuBar().addMenu("&Help") # "Check for updates" used to exist only as a button inside the About @@ -1697,6 +1748,26 @@ class MainWindow(QMainWindow): self.update_banner.hide() self.status.setText(f"You're up to date ({core.__version__}).") + def _set_theme(self, setting: str): + """Persist the theme choice and repaint the running window.""" + QSettings("BCC", "BetterClaudeConfig").setValue("ui/theme", setting) + app = QApplication.instance() + if app is None: # pragma: no cover - only in a headless test harness + return + app.setStyleSheet(theme_stylesheet_for(app, setting)) + # The global stylesheet covers most of the UI, but the inline + # setStyleSheet calls (status dots, warning labels, update banner) only + # pick up the new palette when their widget next renders -- so re-render + # them now rather than leaving dark-on-light text behind. + self._repaint_themed_widgets() + + def _repaint_themed_widgets(self): + """Re-run the inline-styled bits after a palette change.""" + self.status.setStyleSheet(f"color: {MUTED};") + idx = self._current_index() + self._refresh_tables(select_index=idx if idx >= 0 else -1) + self._update_status(saved=False) + def _show_about(self): AboutDialog(self).exec() @@ -2045,9 +2116,21 @@ class MainWindow(QMainWindow): return self.full_config = cfg repaired = True + # extract_servers tolerates malformed entries rather than raising (#72), + # but keep it inside the guard: a load failure must leave the previously + # loaded profile intact instead of half-swapping the window's state. + try: + servers = core.extract_servers(self.full_config) + except Exception as exc: # pragma: no cover - defence in depth + QMessageBox.critical( + self, + "Could not read config", + f"{profile.path}\n\nThe server list couldn't be read: {exc}", + ) + return self._loaded_stat = core.config_fingerprint(profile.path) self.current_profile = profile - self.servers = core.extract_servers(self.full_config) + self.servers = servers self.dirty = False self.restart_btn.hide() self._undo_stack.clear() @@ -2370,7 +2453,7 @@ class MainWindow(QMainWindow): entry = self.servers[idx] old_name = entry.name entry.name = self.editor.current_name() - entry.data = self.editor.dump_data() + entry.set_data(self.editor.dump_data()) # The server stays in its section (enable state unchanged), so update # its existing row in place rather than re-rendering. # An edit invalidates any cached "Test all" result -- the server that @@ -2464,7 +2547,7 @@ class MainWindow(QMainWindow): QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No, ) if ans == QMessageBox.StandardButton.Yes: - self.servers[existing[name]].data = data + self.servers[existing[name]].set_data(data) return False, True name = core.resolve_name_collision(name, {s.name for s in self.servers}) self.servers.append(core.ServerEntry(name, data, True)) @@ -2584,6 +2667,11 @@ class MainWindow(QMainWindow): except Exception as e: QMessageBox.critical(self, "Reload failed", str(e)) return + # The reload above is the on-disk truth for everything the user + # didn't touch -- but it also wipes BCC-authored keys the user + # changed in this session (named sets), which apply_servers + # doesn't write. Carry them over before saving (#73). + contested = core.carry_owned_keys(self.full_config, fresh) core.apply_servers(fresh, self.servers) try: backup = core.write_config(self.current_profile.path, fresh) @@ -2596,8 +2684,13 @@ class MainWindow(QMainWindow): self.dirty = False self.save_btn.setEnabled(False) bnote = f" · backup: {backup.name}" if backup else " · (new file)" + cnote = ( + f" · kept your {', '.join(contested)} (the file on disk had a different copy)" + if contested + else "" + ) self.status.setText( - f"Merged & saved {self.current_profile.path}{bnote}" + f"Merged & saved {self.current_profile.path}{bnote}{cnote}" f" · Restart {self.current_profile.label} to apply." ) self._offer_restart_button() @@ -2755,6 +2848,33 @@ class MainWindow(QMainWindow): e.accept() +def system_is_dark(app: QApplication) -> bool: + """Whether the desktop is currently using a dark appearance. + + Read from the style's own window colour rather than per-platform APIs -- + Qt has already resolved the OS appearance by the time it builds the + default palette, so this works the same on all three platforms. + """ + try: + return app.palette().color(QPalette.ColorRole.Window).lightness() < 128 + except Exception: # pragma: no cover - defensive; never block startup on theming + return True + + +def stored_theme_setting() -> str: + """The user's theme choice, defaulting to following the system.""" + value = QSettings("BCC", "BetterClaudeConfig").value("ui/theme", core.THEME_SYSTEM) + return value if value in core.THEME_CHOICES else core.THEME_SYSTEM + + +def theme_stylesheet_for(app: QApplication, setting: str | None = None) -> str: + """Resolve setting + OS appearance into a palette, apply it, return the QSS.""" + if setting is None: + setting = stored_theme_setting() + theme = core.resolve_theme(setting, system_is_dark(app)) + return apply_palette(core.palette_for(theme)) + + def main(): if sys.platform == "win32": # Without an explicit AppUserModelID, Windows taskbar groups the app @@ -2773,7 +2893,7 @@ def main(): icon = _app_icon() if not icon.isNull(): app.setWindowIcon(icon) - app.setStyleSheet(STYLESHEET) + app.setStyleSheet(theme_stylesheet_for(app)) win = MainWindow() win.show() sys.exit(app.exec()) diff --git a/bcc_core.py b/bcc_core.py index 71d6eec..63b0dc8 100644 --- a/bcc_core.py +++ b/bcc_core.py @@ -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 = _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 diff --git a/tests/test_core.py b/tests/test_core.py index 7be231d..4ccea2b 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -1,6 +1,7 @@ """Pytest port of the original test_core.py script (same 23 behaviours, now proper test functions with tmp_path/monkeypatch fixtures).""" +import dataclasses import json import os import re @@ -2372,6 +2373,261 @@ def test_config_has_unfilled_placeholders_checks_env_too(): assert c.config_has_unfilled_placeholders(cfg) is True +# --------------------------------------------------------------------------- # +# #72 -- a server value that isn't a JSON object must not take the load down +# --------------------------------------------------------------------------- # +@pytest.mark.parametrize("bad", ["not-a-dict", 123, ["a", "b"], None, True, 1.5]) +def test_extract_servers_survives_non_dict_server_value(bad): + entries = c.extract_servers({"mcpServers": {"foo": bad}}) + assert len(entries) == 1 + assert entries[0].name == "foo" + assert entries[0].data == {} + assert entries[0].malformed is True + assert entries[0].raw == bad + + +def test_extract_servers_marks_only_the_bad_entry(): + cfg = {"mcpServers": {"good": {"command": "npx"}, "bad": "oops"}} + by_name = {e.name: e for e in c.extract_servers(cfg)} + assert by_name["good"].malformed is False + assert by_name["good"].data == {"command": "npx"} + assert by_name["bad"].malformed is True + + +def test_extract_servers_handles_malformed_disabled_entry(): + entries = c.extract_servers({c.DISABLED_KEY: {"parked": ["nope"]}}) + assert entries[0].enabled is False + assert entries[0].malformed is True + + +def test_malformed_entry_round_trips_through_save_unchanged(): + """The cardinal rule: never silently delete what the user had on disk.""" + cfg = {"mcpServers": {"good": {"command": "npx"}, "bad": "oops"}} + servers = c.extract_servers(cfg) + out = c.apply_servers(dict(cfg), servers) + assert out["mcpServers"]["bad"] == "oops" + assert out["mcpServers"]["good"] == {"command": "npx"} + + +def test_editing_a_malformed_entry_retires_the_raw_value(): + entry = c.extract_servers({"mcpServers": {"bad": "oops"}})[0] + entry.set_data({"command": "npx"}) + assert entry.malformed is False + assert entry.config_value() == {"command": "npx"} + assert c.apply_servers({}, [entry])["mcpServers"]["bad"] == {"command": "npx"} + + +def test_lint_reports_the_malformed_entry_by_name(): + servers = c.extract_servers({"mcpServers": {"bad": "oops"}}) + warnings = c.lint_servers(servers) + assert len(warnings) == 1 + assert "'bad'" in warnings[0] + assert "not an object" in warnings[0] + assert "str" in warnings[0] + + +def test_lint_still_reports_normal_warnings_alongside_malformed(): + cfg = {"mcpServers": {"bad": "oops", "sloppy": {"command": "npx", "args": "one two"}}} + warnings = c.lint_servers(c.extract_servers(cfg)) + assert any("not an object" in w for w in warnings) + assert any("'args' should be a list" in w for w in warnings) + + +# --------------------------------------------------------------------------- # +# #73 -- the stale-file merge must not discard BCC-authored keys +# --------------------------------------------------------------------------- # +def test_carry_owned_keys_moves_sets_onto_the_reloaded_config(): + local = {"mcpServers": {}, c.SETS_KEY: {"work": ["a", "b"]}} + fresh = {"mcpServers": {"external": {"command": "npx"}}} + contested = c.carry_owned_keys(local, fresh) + assert contested == [] + assert fresh[c.SETS_KEY] == {"work": ["a", "b"]} + assert fresh["mcpServers"] == {"external": {"command": "npx"}} + + +def test_carry_owned_keys_reports_a_genuine_conflict(): + local = {c.SETS_KEY: {"work": ["a"]}} + fresh = {c.SETS_KEY: {"work": ["a", "b"]}} + assert c.carry_owned_keys(local, fresh) == [c.SETS_KEY] + assert fresh[c.SETS_KEY] == {"work": ["a"]} # local wins: BCC owns the key + + +def test_carry_owned_keys_is_quiet_when_both_sides_agree(): + local = {c.SETS_KEY: {"work": ["a"]}} + fresh = {c.SETS_KEY: {"work": ["a"]}} + assert c.carry_owned_keys(local, fresh) == [] + + +def test_carry_owned_keys_leaves_disk_alone_when_absent_locally(): + """Can't distinguish 'deleted my last set' from 'never had sets'; keep theirs.""" + fresh = {c.SETS_KEY: {"remote": ["a"]}} + assert c.carry_owned_keys({}, fresh) == [] + assert fresh[c.SETS_KEY] == {"remote": ["a"]} + + +def test_carry_owned_keys_deep_copies_so_later_edits_do_not_leak(): + local = {c.SETS_KEY: {"work": ["a"]}} + fresh = {} + c.carry_owned_keys(local, fresh) + local[c.SETS_KEY]["work"].append("b") + assert fresh[c.SETS_KEY] == {"work": ["a"]} + + +def test_merge_flow_preserves_sets_and_external_servers(tmp_path): + """End-to-end shape of the Merge & save path that lost sets in #73.""" + path = tmp_path / "claude.json" + path.write_text(json.dumps({"mcpServers": {"old": {"command": "old"}}})) + + # BCC loads, user saves a named set and edits servers in memory. + local = c.load_config(path) + servers = c.extract_servers(local) + c.save_server_set(local, "work", servers) + + # Something else rewrites the file underneath us. + path.write_text(json.dumps({"mcpServers": {"external": {"command": "new"}}, "other": 1})) + + # Merge & save: reload disk, carry BCC keys, re-apply the user's servers. + fresh = c.load_config(path) + c.carry_owned_keys(local, fresh) + c.apply_servers(fresh, servers) + c.write_config(path, fresh) + + saved = c.load_config(path) + assert saved[c.SETS_KEY] == {"work": ["old"]} # the set survived + assert saved["other"] == 1 # unrelated external key preserved + assert "old" in saved["mcpServers"] # user's servers re-applied + + +def test_null_server_value_is_malformed_not_mistaken_for_absent(): + """`{"mcpServers": {"foo": null}}` is legal JSON and a real malformed case, + so None must not double as the 'nothing here' sentinel.""" + entry = c.extract_servers({"mcpServers": {"foo": None}})[0] + assert entry.malformed is True + assert entry.raw is None + assert c.apply_servers({}, [entry])["mcpServers"]["foo"] is None + + +def test_a_normal_entry_is_not_malformed(): + entry = c.extract_servers({"mcpServers": {"foo": {"command": "npx"}}})[0] + assert entry.malformed is False + assert entry.raw is c.NO_RAW + + +# --------------------------------------------------------------------------- # +# #75 -- theming +# --------------------------------------------------------------------------- # +@pytest.mark.parametrize( + "setting,system_dark,expected", + [ + (c.THEME_DARK, False, "dark"), + (c.THEME_DARK, True, "dark"), + (c.THEME_LIGHT, False, "light"), + (c.THEME_LIGHT, True, "light"), + (c.THEME_SYSTEM, True, "dark"), + (c.THEME_SYSTEM, False, "light"), + ], +) +def test_resolve_theme_covers_every_setting_and_appearance(setting, system_dark, expected): + assert c.resolve_theme(setting, system_dark) == expected + + +@pytest.mark.parametrize("junk", ["", "solarized", None, "DARK", 3]) +def test_resolve_theme_falls_back_to_following_the_system(junk): + """A hand-edited or future QSettings value should follow the desktop, + not pin a fixed theme.""" + assert c.resolve_theme(junk, True) == "dark" + assert c.resolve_theme(junk, False) == "light" + + +def test_palette_for_known_names(): + assert c.palette_for("dark") is c.DARK_PALETTE + assert c.palette_for("light") is c.LIGHT_PALETTE + + +def test_palette_for_unknown_name_falls_back_to_dark(): + assert c.palette_for("chartreuse") is c.DARK_PALETTE + + +def test_dark_palette_is_unchanged_from_the_shipped_look(): + """v1.3.0 shipped these exact colours; adding a light theme must not + quietly restyle the dark one.""" + p = c.DARK_PALETTE + assert (p.accent, p.bg, p.panel, p.panel_2) == ("#f97316", "#1b1d23", "#23262e", "#2b2f39") + assert (p.text, p.muted, p.border) == ("#e7e9ee", "#9aa0ad", "#3a3f4b") + assert (p.good, p.bad, p.warn, p.remote) == ("#4ade80", "#f87171", "#fbbf24", "#60a5fa") + assert (p.on_accent, p.disabled_bg, p.mono_bg) == ("#1a1205", "#202229", "#16181d") + + +def test_both_palettes_define_every_slot(): + """A missing slot should fail here rather than render a broken window.""" + for pal in (c.DARK_PALETTE, c.LIGHT_PALETTE): + for f in dataclasses.fields(c.Palette): + value = getattr(pal, f.name) + assert value, f"{pal.name}.{f.name} is empty" + if f.name != "name": + assert re.fullmatch(r"#[0-9a-fA-F]{6}", value), f"{pal.name}.{f.name}={value!r}" + + +@pytest.mark.parametrize("pal_name", ["dark", "light"]) +@pytest.mark.parametrize("slot", ["text", "muted", "good", "bad", "warn", "remote", "accent"]) +def test_palette_meets_contrast_on_panel(pal_name, slot): + """Every colour drawn as text/glyph must clear WCAG AA (4.5:1) against the + surface it sits on. The light palette's semantic colours are NOT the dark + ones lightened -- #4ade80 sits near 1.7:1 on white -- so this guards + against someone 'harmonising' them back toward the dark hues.""" + pal = c.palette_for(pal_name) + assert c.contrast_ratio(getattr(pal, slot), pal.panel) >= 4.5 + + +@pytest.mark.parametrize("pal_name", ["dark", "light"]) +def test_on_accent_is_legible_against_the_accent_fill(pal_name): + """Primary buttons and selected rows draw on_accent on top of accent.""" + pal = c.palette_for(pal_name) + assert c.contrast_ratio(pal.on_accent, pal.accent) >= 4.5 + + +def test_contrast_ratio_endpoints(): + assert c.contrast_ratio("#000000", "#ffffff") == pytest.approx(21.0, abs=0.01) + assert c.contrast_ratio("#123456", "#123456") == pytest.approx(1.0, abs=0.001) + assert c.contrast_ratio("#ffffff", "#000000") == pytest.approx(21.0, abs=0.01) + + +def test_relative_luminance_extremes(): + assert c.relative_luminance("#000000") == pytest.approx(0.0) + assert c.relative_luminance("#ffffff") == pytest.approx(1.0) + + +def test_stylesheet_builder_has_no_hardcoded_colours(): + """Every colour in the QSS must come from the palette. + + Three near-black literals used to be inlined here (#1a1205, #202229, + #16181d). Harmless with one theme; with two, they silently render dark + chrome on a light window. Reads the source rather than importing bcc, + which needs PySide6. + """ + src = (Path(__file__).resolve().parent.parent / "bcc.py").read_text(encoding="utf-8") + start = src.index("def build_stylesheet") + body = src[start : src.index("def apply_palette")] + assert re.findall(r"#[0-9a-fA-F]{6}", body) == [] + + +def test_every_palette_slot_is_consumed(): + """A slot added to Palette but never wired up is dead weight. + + Checks for `p.` anywhere in bcc.py, which covers both the QSS and + apply_palette's global bindings -- not every slot belongs in the + stylesheet (`good` and `remote` feed the inline status dots via + STATUS_COLORS/HEALTH_COLORS, never the QSS). This won't catch a slot bound + to a global that nothing then uses; it does catch the common mistake of + extending the dataclass and forgetting to plumb it through. + """ + src = (Path(__file__).resolve().parent.parent / "bcc.py").read_text(encoding="utf-8") + for f in dataclasses.fields(c.Palette): + if f.name == "name": + continue + assert f"p.{f.name}" in src, f"palette slot {f.name!r} is never consumed" + + # --------------------------------------------------------------------------- # # #78/#79 -- update notice: when to show it, and what it says # --------------------------------------------------------------------------- #