Merge PR #85: ClientSpec adapter refactor — cross-client phase 1 (#5)

Phase 1 of cross-client support: introduce the ClientSpec adapter and route Claude Desktop + Claude Code through it with no behavior change. Parameterizes servers_key/disabled_key/discovery and the entry translation seam; extract_servers/apply_servers/_server_sections/external_change_summary default to Claude's layout so no-spec calls are byte-identical. Verified: 458 tests + a synthetic non-mcpServers client proving the seam generalizes, CI green on all platforms incl. Windows, and the live GUI confirmed loading/saving identically. requirements.txt now declares cryptography as the runtime dep it always was.

Refs #5

CLAUDE.md

@@ -31,6 +31,8 @@ The codebase is split into two layers:
 
 **`bcc_core.py`** — All logic with no GUI imports. Contains:
 - `Profile` / `ServerEntry` dataclasses (the data model)
+- `ClientSpec` (issue #5, cross-client) — one adapter object per MCP host capturing everything client-specific: the top-level `servers_key` (Claude uses `mcpServers`; VS Code will use `servers`), the parking `disabled_key`, config `config_filename`, the capability flags (`expands_env_refs`, `supports_restart`), and a per-server `entry_to_internal`/`entry_from_internal` translation pair (identity for Claude; the seam a differently-shaped client overrides). `CLAUDE_DESKTOP` and `CLAUDE_CODE` are the two shipped specs; `resolve_client(path)` picks one by filename, and each `Profile` carries its resolved `client`. The read/write/diff functions take an optional `spec` and default to Claude's layout, so a call with no spec is unchanged.
+
 - `discover_profiles()` — scans the platform's app-support directory for `Claude*` folders (Claude Desktop) **and** always adds `~/.claude.json` (Claude Code user scope — what `claude mcp add` writes). `~/.claude/settings.json` is NOT a server config (it rejects `mcpServers` with a schema error) and is only surfaced, labelled legacy, if servers are found parked in it. Project-scope `.mcp.json` files can be opened via Add config…
 - `load_config` / `extract_servers` / `apply_servers` / `write_config` — the read/write pipeline; writes are atomic with rotating timestamped backups in `.bcc_backups/`
 - `parse_pasted_json()` / `parse_pasted_json_verbose()` — accepts three JSON shapes (full config, inner map, or bare server object). Input does not have to be valid JSON: `repair_json_text()` auto-fixes markdown fences, surrounding prose, `//` `/* */` `#` comments, trailing/missing commas, smart quotes, single quotes, unquoted keys, Python/JS literals, and unclosed braces. The verbose variant also returns human-readable notes describing every repair applied (shown live in the paste dialog)
@@ -43,7 +45,7 @@ The codebase is split into two layers:
 - `KeyValueTable` — reusable widget for env vars and headers
 - `ConnTester(QThread)` — background thread for remote reachability tests
 
-**The cardinal rule**: `apply_servers()` only ever writes to `mcpServers` and `_disabledMcpServers`. All other keys in the user's config are preserved verbatim and in their original order.
+**The cardinal rule**: `apply_servers()` only ever writes the two keys the target client's servers live under — by default `mcpServers` and `_disabledMcpServers`, or whatever the profile's `ClientSpec` declares (`servers_key` + `disabled_key`). All other keys in the user's config are preserved verbatim and in their original order. The rule generalises across clients precisely because it is parameterised by the spec rather than hard-coded.
 
 Disabled servers are parked under `_disabledMcpServers` (which Claude Desktop ignores) so they can be re-enabled without losing their definition.
 

bcc.py

@@ -10,7 +10,7 @@ Run:  python mcp_manager.py
 
 from __future__ import annotations
 
-import html
+import contextlib
 import sys
 import time
 from pathlib import Path
@@ -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,13 +27,12 @@ from PySide6.QtGui import (
     QIcon,
     QKeySequence,
     QPainter,
+    QPalette,
     QPixmap,
-    QTextCursor,
 )
 from PySide6.QtWidgets import (
     QAbstractItemView,
     QApplication,
-    QButtonGroup,
     QCheckBox,
     QComboBox,
     QDialog,
@@ -68,105 +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
 
+# --- 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] = {}
 
-def plain_label(text: object) -> QLabel:
+STATUS_GLYPH = {
-    """A QLabel guaranteed to render `text` as plain text, never HTML.
+    "ok": "\u25cf",
-
+    "missing": "\u25cf",
-    Qt's QLabel auto-interprets HTML by default (Qt.AutoText). Every catalog
+    "warn": "\u25b2",
-    entry field (description, notes, display name, urls -- and especially
+    "remote": "\u25c6",
-    args) is attacker-influenceable: catalog.json accepts community PRs, and
+    "unknown": "\u25cb",
-    only a valid Ed25519 signature stands between a PR and what a user sees
+}
-    here. A `<b>` or `<img onerror=...>` in a description must render as
-    visible text, not markup -- exactly the same reasoning catalog_console.py
-    documents for its own plain_label(). Every catalog-derived string shown
-    by the Browse dialog MUST go through this helper (or an inherently
-    plain-text widget like QPlainTextEdit) rather than a bare QLabel(...).
-    """
-    label = QLabel(html.escape(str(text)))
-    label.setTextFormat(Qt.TextFormat.PlainText)
-    label.setWordWrap(True)
-    return label
-
-
-# --- 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"
-
-STATUS_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": "#60a5fa", "unknown": WARN}
-STATUS_GLYPH = {"ok": "●", "missing": "●", "warn": "▲", "remote": "◆", "unknown": "○"}
 
 # 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": "\u25cf", "failed": "\u25cf", "untested": "\u25cb"}
-HEALTH_GLYPH = {"ok": "●", "failed": "●", "untested": "○"}
 
-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}; }}
+* {{ font-size: 13px; color: {p.text}; }}
-QMainWindow, QDialog {{ background: {BG}; }}
+QMainWindow, QDialog {{ background: {p.bg}; }}
 QLabel#h1 {{ font-size: 15px; font-weight: 600; }}
-QLabel#muted {{ color: {MUTED}; }}
+QLabel#muted {{ color: {p.muted}; }}
-QFrame#card {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px; }}
+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;
+    background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px;
-    padding: 6px 8px; selection-background-color: {ACCENT}; selection-color: #1a1205;
+    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};
+QComboBox QAbstractItemView {{ background: {p.panel_2}; border: 1px solid {p.border};
-    selection-background-color: {ACCENT}; outline: none; }}
+    selection-background-color: {p.accent}; outline: none; }}
-QPushButton {{ background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px;
+QPushButton {{ background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px;
     padding: 7px 13px; }}
-QPushButton:hover {{ border: 1px solid {ACCENT}; }}
+QPushButton:hover {{ border: 1px solid {p.accent}; }}
-QPushButton:disabled {{ color: {MUTED}; background: {PANEL}; }}
+QPushButton:disabled {{ color: {p.muted}; background: {p.panel}; }}
-QPushButton#primary {{ background: {ACCENT}; border: 1px solid {ACCENT}; color: #1a1205; font-weight: 600; }}
+QPushButton#primary {{ background: {p.accent}; border: 1px solid {p.accent}; color: {p.on_accent}; font-weight: 600; }}
-QPushButton#primary:hover {{ background: {ACCENT_DIM}; }}
+QPushButton#primary:hover {{ background: {p.accent_dim}; }}
-QPushButton#primary:disabled {{ background: {PANEL}; color: {MUTED}; border: 1px solid {BORDER}; }}
+QPushButton#primary:disabled {{ background: {p.panel}; color: {p.muted}; border: 1px solid {p.border}; }}
-QPushButton#danger:hover {{ border: 1px solid {BAD}; color: {BAD}; }}
+QPushButton#danger:hover {{ border: 1px solid {p.bad}; color: {p.bad}; }}
-QTableWidget {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px;
+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;
+QHeaderView::section {{ background: {p.panel}; color: {p.muted}; border: none;
-    border-bottom: 1px solid {BORDER}; padding: 8px; font-weight: 600; }}
+    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#statusbar {{ color: {p.muted}; padding: 4px 2px; }}
-QLabel#warnBanner {{ color: #1a1205; background: {WARN}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }}
+QLabel#warnBanner {{ color: {p.on_accent}; background: {p.warn}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }}
-QLabel#section {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
+QFrame#noticeBanner {{ background: {p.panel_2}; border: 1px solid {p.accent}; border-radius: 8px; }}
-QLabel#sectionDisabled {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
+QLabel#noticeText {{ color: {p.text}; }}
-QLabel#placeholder {{ color: {MUTED}; padding: 12px; background: {PANEL_2}; border: 1px dashed {BORDER}; border-radius: 8px; }}
+QPushButton#noticeClose {{ background: transparent; border: none; color: {p.muted}; font-size: 14px; padding: 2px; }}
-QTableWidget#disabledTable {{ background: #202229; }}
+QPushButton#noticeClose:hover {{ color: {p.text}; }}
-QTableWidget#disabledTable::item:selected {{ background: {ACCENT}; color: #1a1205; }}
+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:hover {{ background: {p.border}; border-radius: 4px; }}
-QSplitter::handle:pressed {{ background: {ACCENT}; 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)
 # --------------------------------------------------------------------------- #
@@ -251,7 +268,7 @@ class _SecretMaskDelegate(QStyledItemDelegate):
         if self.revealed or not option.text:
             return
         key_item = self._table.item(index.row(), 0)
-        if key_item and core.is_secret_key(key_item.text()):
+        if key_item and core.should_mask_value(key_item.text(), option.text):
             option.text = core.MASK
 
 
@@ -699,39 +716,6 @@ class ServerEditor(QFrame):
     def current_name(self) -> str:
         return self.name.text().strip()
 
-    def focus_target(self, target: tuple[str, int | str] | None):
-        """
-        Focus the field a catalog Add left unfilled -- `target` is whatever
-        core.first_unfilled_focus_target() returned: ("args", line_index),
-        ("env", var_name), or None (nothing to fill, so do nothing).
-
-        Only meaningful on the stdio page, which is the only page a catalog
-        entry ever populates (link-only entries never reach dump_data()).
-        """
-        if not target or self.type.currentIndex() != 0:
-            return
-        kind, value = target
-        if kind == "args":
-            self.args.setFocus()
-            cursor = self.args.textCursor()
-            cursor.movePosition(QTextCursor.MoveOperation.Start)
-            cursor.movePosition(
-                QTextCursor.MoveOperation.Down, QTextCursor.MoveMode.MoveAnchor, int(value)
-            )
-            cursor.movePosition(
-                QTextCursor.MoveOperation.EndOfLine, QTextCursor.MoveMode.KeepAnchor
-            )
-            self.args.setTextCursor(cursor)
-        elif kind == "env":
-            for r in range(self.env.table.rowCount()):
-                key_item = self.env.table.item(r, 0)
-                if key_item and key_item.text() == value:
-                    self.env.table.setCurrentCell(r, 1)
-                    val_item = self.env.table.item(r, 1)
-                    if val_item:
-                        self.env.table.editItem(val_item)
-                    break
-
     def _type_switched(self):
         self.stack.setCurrentIndex(self.type.currentIndex())
         self._emit()
@@ -1260,262 +1244,6 @@ class PasteDialog(QDialog):
             self.err.setText(str(e))
 
 
-# --------------------------------------------------------------------------- #
-# Browse catalog dialog (issue #10 phase 2): search/filter the signed,
-# bundled server catalog and add a "basic" entry through the existing
-# paste/import path, or send a "link-only" entry to its setup docs.
-#
-# Every widget here that shows catalog-derived text uses plain_label() or an
-# inherently-plain widget (QPlainTextEdit) -- see plain_label()'s docstring.
-# The dialog itself does no signature/schema work: MainWindow hands it an
-# already-verified `entries` list (bcc_core.load_bundled_catalog_entries()),
-# and an empty list here means "show the empty state", never "fall back to
-# something less trusted".
-# --------------------------------------------------------------------------- #
-class BrowseCatalogDialog(QDialog):
-    def __init__(self, parent, entries: list[dict]):
-        super().__init__(parent)
-        self.setWindowTitle("Browse catalog")
-        self.resize(880, 560)
-        self.entries = entries or []
-        self.result_entry: dict | None = None
-        self._current_entry: dict | None = None
-        self._current_homepage: str | None = None
-        self._current_docs_url: str | None = None
-        self._current_group = "All"
-
-        outer = QVBoxLayout(self)
-
-        if not self.entries:
-            # Signature verification failed, or nothing was bundled -- never
-            # show a half-trusted list, and never explain WHY beyond this;
-            # a stale/tampered catalog isn't the user's problem to diagnose.
-            msg = plain_label("Catalog unavailable.")
-            msg.setObjectName("placeholder")
-            msg.setAlignment(Qt.AlignmentFlag.AlignCenter)
-            outer.addWidget(msg, 1)
-            btns = QDialogButtonBox(QDialogButtonBox.StandardButton.Close)
-            btns.rejected.connect(self.reject)
-            outer.addWidget(btns)
-            return
-
-        search_row = QHBoxLayout()
-        self.search_box = QLineEdit()
-        self.search_box.setPlaceholderText("Search by name, description, or category…")
-        self.search_box.setClearButtonEnabled(True)
-        self.search_box.textChanged.connect(self._refresh_list)
-        search_row.addWidget(self.search_box, 1)
-        outer.addLayout(search_row)
-
-        chip_row = QHBoxLayout()
-        self._chip_group = QButtonGroup(self)
-        self._chip_group.setExclusive(True)
-        for label in core.CATALOG_CATEGORY_CHIPS:
-            btn = QPushButton(label)
-            btn.setCheckable(True)
-            btn.setChecked(label == "All")
-            btn.clicked.connect(lambda _checked=False, g=label: self._set_group(g))
-            self._chip_group.addButton(btn)
-            chip_row.addWidget(btn)
-        chip_row.addStretch()
-        outer.addLayout(chip_row)
-
-        splitter = QSplitter(Qt.Orientation.Horizontal)
-
-        left = QWidget()
-        lv = QVBoxLayout(left)
-        lv.setContentsMargins(0, 0, 0, 0)
-        self.list = QListWidget()
-        self.list.currentItemChanged.connect(self._on_selected)
-        lv.addWidget(self.list, 1)
-        splitter.addWidget(left)
-
-        right = QFrame()
-        right.setObjectName("card")
-        rv = QVBoxLayout(right)
-        self.detail_title = plain_label("")
-        self.detail_title.setObjectName("h1")
-        rv.addWidget(self.detail_title)
-        self.detail_meta = plain_label("")
-        self.detail_meta.setObjectName("muted")
-        rv.addWidget(self.detail_meta)
-        self.detail_freshness = plain_label("")
-        self.detail_freshness.setObjectName("muted")
-        rv.addWidget(self.detail_freshness)
-        self.detail_desc = plain_label("")
-        rv.addWidget(self.detail_desc)
-        self.detail_notes = plain_label("")
-        self.detail_notes.setObjectName("muted")
-        rv.addWidget(self.detail_notes)
-        self.detail_homepage_btn = QPushButton("Open homepage")
-        self.detail_homepage_btn.clicked.connect(self._open_homepage)
-        rv.addWidget(self.detail_homepage_btn)
-        rv.addWidget(plain_label("Exact command this will add:"))
-        self.detail_command_preview = QPlainTextEdit()
-        self.detail_command_preview.setObjectName("diag")
-        self.detail_command_preview.setReadOnly(True)
-        # Read-only QPlainTextEdit never interprets HTML, regardless of what
-        # a compromised/careless catalog entry's command/args contain -- this
-        # is the field that renders "the exact bytes that will be written".
-        rv.addWidget(self.detail_command_preview, 1)
-        self.detail_action_btn = QPushButton("")
-        self.detail_action_btn.setObjectName("primary")
-        self.detail_action_btn.clicked.connect(self._on_action)
-        rv.addWidget(self.detail_action_btn)
-        splitter.addWidget(right)
-
-        splitter.setSizes([360, 480])
-        outer.addWidget(splitter, 1)
-
-        btns = QDialogButtonBox(QDialogButtonBox.StandardButton.Close)
-        btns.rejected.connect(self.reject)
-        outer.addWidget(btns)
-
-        self._refresh_list()
-
-    # --- list / filtering -------------------------------------------------- #
-    def _set_group(self, group: str):
-        self._current_group = group
-        self._refresh_list()
-
-    def _visible_entries(self) -> list[dict]:
-        filtered = core.filter_catalog_entries(self.entries, self.search_box.text())
-        return core.catalog_entries_in_group(filtered, self._current_group)
-
-    def _format_row(self, entry: dict) -> str:
-        display = str(entry.get("display") or entry.get("id") or "")
-        official = "✓ " if entry.get("official") else ""
-        stars = entry.get("stars")
-        star_txt = f"   ★ {stars:,}" if isinstance(stars, int) else ""
-        group = core.catalog_category_group(entry.get("category", ""))
-        desc = str(entry.get("description") or "")
-        if len(desc) > 88:
-            desc = desc[:87] + "…"
-        # QListWidgetItem text is always rendered literally by Qt (no HTML
-        # interpretation), so no escaping is needed here -- unlike QLabel.
-        return f"{official}{display}{star_txt}\n{desc}   ·   {group}"
-
-    def _refresh_list(self):
-        self.list.blockSignals(True)
-        self.list.clear()
-        for entry in self._visible_entries():
-            item = QListWidgetItem(self._format_row(entry))
-            item.setData(Qt.ItemDataRole.UserRole, entry)
-            # Tooltips DO auto-detect rich text in Qt, so escape defensively
-            # even though descriptions are already shown, unescaped-but-safe,
-            # in the QListWidgetItem text above.
-            item.setToolTip(html.escape(str(entry.get("description", ""))))
-            self.list.addItem(item)
-        self.list.blockSignals(False)
-        if self.list.count():
-            self.list.setCurrentRow(0)
-        else:
-            self._on_selected(None, None)
-
-    # --- detail pane -------------------------------------------------------- #
-    def _on_selected(self, current, _previous=None):
-        if current is None:
-            self._current_entry = None
-            self._current_homepage = None
-            self._current_docs_url = None
-            self.detail_title.setText("")
-            self.detail_meta.setText("")
-            self.detail_freshness.setText("")
-            self.detail_desc.setText("No matching servers." if self.entries else "")
-            self.detail_notes.setText("")
-            self.detail_homepage_btn.setVisible(False)
-            self.detail_command_preview.setPlainText("")
-            self.detail_action_btn.setEnabled(False)
-            self.detail_action_btn.setText("Add")
-            return
-
-        entry = current.data(Qt.ItemDataRole.UserRole)
-        self._current_entry = entry
-        self.detail_title.setText(str(entry.get("display") or entry.get("id") or ""))
-
-        official = "✓ Official" if entry.get("official") else ""
-        stars = entry.get("stars")
-        star_txt = f"★ {stars:,}" if isinstance(stars, int) else ""
-        group = core.catalog_category_group(entry.get("category", ""))
-        meta_bits = [b for b in (official, star_txt, group) if b]
-        self.detail_meta.setText("  ·  ".join(meta_bits))
-
-        freshness = core.format_freshness_hint(entry.get("last_release"))
-        self.detail_freshness.setText(freshness)
-        self.detail_freshness.setVisible(bool(freshness))
-
-        self.detail_desc.setText(str(entry.get("description") or ""))
-
-        notes = entry.get("notes") or ""
-        self.detail_notes.setText(notes)
-        self.detail_notes.setVisible(bool(notes))
-
-        homepage = entry.get("homepage")
-        self._current_homepage = homepage if isinstance(homepage, str) else None
-        self.detail_homepage_btn.setVisible(bool(self._current_homepage))
-
-        self._current_docs_url = (
-            entry.get("docs_url") if isinstance(entry.get("docs_url"), str) else None
-        )
-
-        self.detail_command_preview.setPlainText(self._render_command_preview(entry))
-
-        if entry.get("setup") == "basic":
-            self.detail_action_btn.setText("Add")
-            self.detail_action_btn.setEnabled(True)
-        else:
-            self.detail_action_btn.setText("Open setup docs")
-            self.detail_action_btn.setEnabled(bool(self._current_docs_url))
-
-    def _render_command_preview(self, entry: dict) -> str:
-        """
-        The exact command that will be written, rendered verbatim. Every
-        value here comes straight from the (signature-verified) catalog
-        entry with no interpretation beyond str() -- this must never be the
-        place a markup-laced description sneaks back in as "helpful"
-        formatting.
-        """
-        if entry.get("setup") != "basic":
-            docs = entry.get("docs_url") or "(none provided)"
-            return (
-                "This is a hosted/managed integration -- there is no local "
-                "command to add.\n\nSetup docs:\n  " + str(docs)
-            )
-
-        config = entry.get("config") or {}
-        lines = [f"command: {config.get('command', '')}"]
-        args = config.get("args") or []
-        if args:
-            lines.append("args:")
-            lines.extend(f"  {a}" for a in args)
-        env = config.get("env") or {}
-        env_required = entry.get("env_required") or {}
-        env_keys = list(env.keys()) + [k for k in env_required if k not in env]
-        if env_keys:
-            lines.append("env (names only -- you provide the values):")
-            lines.extend(f"  {k}" for k in env_keys)
-        return "\n".join(lines)
-
-    def _open_homepage(self):
-        url = self._current_homepage
-        if url and url.startswith("https://"):
-            QDesktopServices.openUrl(QUrl(url))
-
-    def _on_action(self):
-        entry = self._current_entry
-        if not entry:
-            return
-        if entry.get("setup") == "basic":
-            self.result_entry = entry
-            self.accept()
-        else:
-            url = self._current_docs_url
-            if url and url.startswith("https://"):
-                QDesktopServices.openUrl(QUrl(url))
-            # link-only never auto-adds and never closes the dialog -- the
-            # user can keep browsing after opening the docs in their browser.
-
-
 # --------------------------------------------------------------------------- #
 # Log viewer dialog (issue #6): a read-only, auto-tailing view of a single
 # server's MCP log file. Polls on a QTimer instead of watching the filesystem
@@ -1805,6 +1533,54 @@ class AboutDialog(QDialog):
         QDesktopServices.openUrl(QUrl(self._release_url or core.RELEASES_URL))
 
 
+class NoticeBanner(QFrame):
+    """A persistent, dismissible notice with an optional action button.
+
+    The status bar is the wrong home for anything the user needs to act on --
+    21 call sites rewrite it, so a message posted there is gone by the next
+    click. That wiped the MSIX warning (#35) and then the update notice (#78).
+    This is the shared mechanism so it doesn't happen a third time.
+    """
+
+    def __init__(self, parent=None):
+        super().__init__(parent)
+        self.setObjectName("noticeBanner")
+        row = QHBoxLayout(self)
+        row.setContentsMargins(10, 8, 8, 8)
+        row.setSpacing(8)
+        self._label = QLabel("")
+        self._label.setObjectName("noticeText")
+        self._label.setWordWrap(True)
+        row.addWidget(self._label, 1)
+        self._action_btn = QPushButton("")
+        self._action_btn.setCursor(Qt.CursorShape.PointingHandCursor)
+        self._action_btn.hide()
+        row.addWidget(self._action_btn)
+        self._close_btn = QPushButton("\u2715")
+        self._close_btn.setObjectName("noticeClose")
+        self._close_btn.setCursor(Qt.CursorShape.PointingHandCursor)
+        self._close_btn.setFixedWidth(26)
+        self._close_btn.setToolTip("Dismiss")
+        self._close_btn.clicked.connect(self.hide)
+        row.addWidget(self._close_btn)
+        self.hide()
+
+    def show_notice(self, text: str, action_label: str = "", on_action=None):
+        self._label.setText(text)
+        self._label.setToolTip(text)
+        # Reconnect cleanly: a banner reused for a second notice would
+        # otherwise fire the previous notice's action too.
+        with contextlib.suppress(RuntimeError, TypeError):
+            self._action_btn.clicked.disconnect()
+        if action_label and on_action is not None:
+            self._action_btn.setText(action_label)
+            self._action_btn.clicked.connect(lambda _=False: on_action())
+            self._action_btn.show()
+        else:
+            self._action_btn.hide()
+        self.show()
+
+
 # --------------------------------------------------------------------------- #
 # Restart worker: core.restart_claude_desktop() blocks up to ~5 s on macOS
 # waiting for the old instance to exit, so it must run off the UI thread.
@@ -1862,6 +1638,11 @@ class MainWindow(QMainWindow):
         self.warn_banner.hide()
         root.addWidget(self.warn_banner)
 
+        # Update availability gets its own persistent banner rather than a
+        # status-line write, which the next UI action overwrites (#78).
+        self.update_banner = NoticeBanner(self)
+        root.addWidget(self.update_banner)
+
         # User-draggable divider between the server list and the editor.
         split = QSplitter(Qt.Orientation.Horizontal)
         split.setChildrenCollapsible(False)
@@ -1897,11 +1678,96 @@ 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
+        # dialog, which is not somewhere anyone looks for it (#79).
+        update_action = QAction("Check for updates…", self)
+        # Explicit role: macOS relocates actions it recognises by text, and
+        # some Qt versions treat "update" as application-menu material. Pin it
+        # so the item stays where the menu says it is on every platform.
+        update_action.setMenuRole(QAction.MenuRole.ApplicationSpecificRole)
+        update_action.triggered.connect(self.check_for_updates)
+        help_menu.addAction(update_action)
+        help_menu.addSeparator()
+
         about_action = QAction("About Better Claude Config…", self)
+        # Qt auto-assigns AboutRole to actions whose text starts with "About",
+        # which moves this into the application menu on macOS. That is the
+        # right home there -- state it explicitly rather than inheriting it by
+        # accident, since the behaviour is invisible from this call site.
+        about_action.setMenuRole(QAction.MenuRole.AboutRole)
         about_action.triggered.connect(self._show_about)
         help_menu.addAction(about_action)
 
+    def _show_update_notice(self, notice: dict):
+        """Surface an available update where it survives the next click."""
+        url = notice["url"]
+        self.update_banner.show_notice(
+            notice["text"],
+            action_label="Open releases page",
+            on_action=lambda: QDesktopServices.openUrl(QUrl(url)),
+        )
+
+    def check_for_updates(self):
+        """Menu-driven check. Unlike the startup check this is never throttled
+        and always reports back -- the user asked, so silence would read as a
+        broken button."""
+        self.status.setText("Checking for updates…")
+        self._menu_update_worker = UpdateCheckWorker()
+        self._menu_update_worker.done.connect(self._on_menu_update_checked)
+        self._menu_update_worker.start()
+
+    def _on_menu_update_checked(self, release: dict | None):
+        self._menu_update_worker = None
+        if release is None:
+            self.status.setText("Couldn't check for updates (offline?).")
+            return
+        QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
+        notice = core.update_notice(core.__version__, release)
+        if notice:
+            self._show_update_notice(notice)
+            self.status.setText(f"Update available: {notice['version']}")
+        else:
+            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()
 
@@ -1922,10 +1788,9 @@ class MainWindow(QMainWindow):
         if release is None:
             return  # offline/failed check: don't advance lastCheck, allow retry
         QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
-        if core.is_newer_version(core.__version__, release["version"]):
+        notice = core.update_notice(core.__version__, release)
-            self.status.setText(
+        if notice:
-                f"Update available: {release['version']}  ·  Help ▸ About to view it."
+            self._show_update_notice(notice)
-            )
 
     # --- layout persistence ---------------------------------------------- #
     def _restore_layout(self):
@@ -2092,10 +1957,6 @@ class MainWindow(QMainWindow):
         self.del_btn = QPushButton("Delete")
         self.del_btn.setObjectName("danger")
         self.paste_btn = QPushButton("Paste JSON...")
-        self.browse_catalog_btn = QPushButton("Browse catalog…")
-        self.browse_catalog_btn.setToolTip(
-            "Add a popular MCP server from the curated, signed catalog"
-        )
         self.copy_btn = QPushButton("Copy to ▸")
         self.undo_btn = QPushButton("Undo")
         self.undo_btn.setEnabled(False)
@@ -2108,7 +1969,6 @@ class MainWindow(QMainWindow):
         self.dup_btn.clicked.connect(self.duplicate_server)
         self.del_btn.clicked.connect(self.delete_server)
         self.paste_btn.clicked.connect(self.paste_json)
-        self.browse_catalog_btn.clicked.connect(self.browse_catalog)
         self.copy_btn.clicked.connect(self.copy_to_menu)
         self.undo_btn.clicked.connect(self._undo)
         self.test_all_btn.clicked.connect(self._test_all_servers)
@@ -2117,7 +1977,6 @@ class MainWindow(QMainWindow):
             self.dup_btn,
             self.del_btn,
             self.paste_btn,
-            self.browse_catalog_btn,
             self.copy_btn,
             self.undo_btn,
             self.test_all_btn,
@@ -2257,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, profile.client)
+        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()
@@ -2582,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
@@ -2676,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))
@@ -2696,41 +2567,6 @@ class MainWindow(QMainWindow):
         self._mark_dirty()
         self.status.setText(f"Imported {added} added, {replaced} replaced. Review and Save.")
 
-    def browse_catalog(self):
-        """
-        Open the Browse-catalog dialog (issue #10 phase 2). The catalog is
-        loaded and signature-verified fresh every time the dialog opens --
-        never cached across app runs at this phase (remote fetch/cache is
-        #61, not yet built) -- so a bundled-catalog swap only takes effect
-        on next dialog open, never mid-session in a stale way.
-        """
-        entries = core.load_bundled_catalog_entries(
-            _asset_dir() / "data" / "catalog.json", _asset_dir() / "data" / "catalog.json.sig"
-        )
-        dlg = BrowseCatalogDialog(self, entries)
-        if dlg.exec() != QDialog.DialogCode.Accepted or not dlg.result_entry:
-            return
-        entry = dlg.result_entry
-        paste = core.catalog_entry_to_paste_json(entry)
-        name, data = next(iter(paste.items()))
-
-        existing_before = {s.name: i for i, s in enumerate(self.servers)}
-        self._push_undo()
-        _added, replaced = self._import_server(name, data)
-        idx = (
-            existing_before.get(name, len(self.servers) - 1) if replaced else len(self.servers) - 1
-        )
-        self._refresh_tables(select_index=idx)
-        self._mark_dirty()
-
-        target = core.first_unfilled_focus_target(data)
-        self.editor.focus_target(target)
-
-        verb = "Replaced" if replaced else "Added"
-        self.status.setText(
-            f"{verb} “{name}” from the catalog. Fill in the highlighted field and Save."
-        )
-
     def copy_to_menu(self):
         idx = self._current_index()
         if not (0 <= idx < len(self.servers)):
@@ -2754,7 +2590,7 @@ class MainWindow(QMainWindow):
         except Exception as e:
             QMessageBox.critical(self, "Copy failed", f"Couldn't read {dest.label}:\n{e}")
             return
-        existing = core.extract_servers(dest_cfg)
+        existing = core.extract_servers(dest_cfg, dest.client)
         names = {s.name for s in existing}
         if src.name in names:
             ans = QMessageBox.question(
@@ -2766,7 +2602,7 @@ class MainWindow(QMainWindow):
                 return
             existing = [s for s in existing if s.name != src.name]
         existing.append(core.ServerEntry(src.name, dict(src.data), True))
-        core.apply_servers(dest_cfg, existing)
+        core.apply_servers(dest_cfg, existing, dest.client)
         try:
             backup = core.write_config(dest.path, dest_cfg)
         except Exception as e:
@@ -2786,6 +2622,12 @@ class MainWindow(QMainWindow):
             self.save_btn.setEnabled(False)
             return False
         lint_warnings = core.lint_servers(self.servers)
+        # ${VAR} references are only meaningful if the target client expands
+        # them -- Claude Desktop doesn't, so the same config is fine in one
+        # profile and broken in another (#76). Report against the loaded one.
+        for entry in self.servers:
+            for warning in core.env_ref_warnings(entry.data, self.current_profile):
+                lint_warnings.append(f"'{entry.name}': {warning}")
         if lint_warnings:
             self.validation_lbl.setText(f"⚠ {lint_warnings[0]}")
             self.validation_lbl.setStyleSheet(f"color: {WARN};")
@@ -2809,29 +2651,6 @@ class MainWindow(QMainWindow):
             QMessageBox.warning(self, "Can't save yet", "Fix the highlighted problem first.")
             return
 
-        # Placeholder guard (issue #10): a catalog Add can leave a
-        # <PLACEHOLDER>-style token in args/env until the user fills it in.
-        # This warns, it does not block -- the user may be deliberately
-        # saving a stub to finish later -- but it must never save silently,
-        # since a server launched with a literal "<PLACEHOLDER>" argument
-        # just fails in a confusing way at spawn time.
-        placeholder_names = [
-            s.name for s in self.servers if core.config_has_unfilled_placeholders(s.data)
-        ]
-        if placeholder_names:
-            names = ", ".join(f"“{n}”" for n in placeholder_names)
-            ans = QMessageBox.warning(
-                self,
-                "Unfilled placeholder",
-                f"{names} still has a <PLACEHOLDER> value that hasn't been "
-                "replaced with a real value. Claude won't be able to use "
-                "it as-is.\n\nSave anyway?",
-                QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
-                QMessageBox.StandardButton.No,
-            )
-            if ans != QMessageBox.StandardButton.Yes:
-                return
-
         # Stale-file check: if the file changed on disk since we loaded it, prompt.
         # Compare mtime AND size (not mtime alone) so a concurrent external write
         # that lands within the mtime resolution window, or that restores the
@@ -2843,7 +2662,7 @@ class MainWindow(QMainWindow):
             and disk_stat != self._loaded_stat
         ):
             changed_keys, server_diff = core.external_change_summary(
-                self.full_config, self.current_profile.path
+                self.full_config, self.current_profile.path, self.current_profile.client
             )
             dlg = StaleDialog(self, str(self.current_profile.path), changed_keys, server_diff)
             if not dlg.exec():
@@ -2854,7 +2673,12 @@ class MainWindow(QMainWindow):
                 except Exception as e:
                     QMessageBox.critical(self, "Reload failed", str(e))
                     return
-                core.apply_servers(fresh, self.servers)
+                # 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, self.current_profile.client)
                 try:
                     backup = core.write_config(self.current_profile.path, fresh)
                 except Exception as e:
@@ -2866,15 +2690,20 @@ 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()
                 return
             # else OVERWRITE: fall through to normal write
 
-        core.apply_servers(self.full_config, self.servers)
+        core.apply_servers(self.full_config, self.servers, self.current_profile.client)
         try:
             backup = core.write_config(self.current_profile.path, self.full_config)
         except Exception as e:
@@ -3025,6 +2854,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
@@ -3043,7 +2899,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())

bcc.spec

@@ -31,11 +31,7 @@ a = Analysis(
     ["bcc.py"],
     pathex=[],
     binaries=[],
-    datas=[
+    datas=[("icons", "icons"), ("data/catalog.json", "data")],
-        ("icons", "icons"),
-        ("data/catalog.json", "data"),
-        ("data/catalog.json.sig", "data"),
-    ],
     hiddenimports=[],
     hookspath=[],
     hooksconfig={},

requirements-dev.txt

@@ -1,5 +1,6 @@
 # Runtime (also in requirements.txt)
 PySide6>=6.6
+cryptography>=42.0   # catalog signature verification (bcc_core) + release checksum signing
 
 # Build / packaging
 pyinstaller>=6.0
@@ -8,4 +9,3 @@ pillow>=10.0        # generates icons/app.ico during CI (Windows build)
 # Test / lint
 pytest>=8.0
 ruff>=0.6
-cryptography>=42.0   # release checksum signing (scripts/sign_checksums.py)

requirements.txt

@@ -1 +1,2 @@
 PySide6>=6.6
+cryptography>=42.0   # bcc_core imports it at load (catalog signature verification)

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
@@ -2357,43 +2358,6 @@ def test_catalog_entry_to_paste_json_includes_env_when_present():
     assert c.validate_catalog(malicious) != []
 
 
-def test_catalog_entry_to_paste_json_seeds_env_required_keys():
-    # Regression: env_required is where the seed data actually keeps its
-    # secret VAR NAMES (postgres/github/notion/etc. all declare their secret
-    # here with config.env left empty) -- catalog_entry_to_paste_json must
-    # surface those names as blank env rows, not silently drop them.
-    entry = _minimal_catalog()["servers"][0]
-    entry["env_required"] = {"DATABASE_URI": ""}
-    result = c.catalog_entry_to_paste_json(entry)
-    assert result["widget"]["env"] == {"DATABASE_URI": ""}
-
-
-def test_catalog_entry_to_paste_json_config_env_wins_over_env_required_default():
-    entry = _minimal_catalog()["servers"][0]
-    entry["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"}
-    entry["env_required"] = {"GRAFANA_URL": "", "GRAFANA_SERVICE_ACCOUNT_TOKEN": ""}
-    result = c.catalog_entry_to_paste_json(entry)
-    assert result["widget"]["env"] == {
-        "GRAFANA_URL": "<GRAFANA_URL>",
-        "GRAFANA_SERVICE_ACCOUNT_TOKEN": "",
-    }
-
-
-def test_catalog_entry_to_paste_json_real_postgres_entry_seeds_database_uri():
-    """End-to-end regression against the actual shipped postgres entry,
-    which needs DATABASE_URI via env_required and has no config.env at
-    all -- this is exactly the shape that was silently dropping the env
-    field before catalog_entry_to_paste_json accounted for env_required."""
-    root = Path(__file__).resolve().parent.parent
-    raw = (root / "data" / "catalog.json").read_bytes()
-    data = c.load_catalog(raw)
-    entry = next(s for s in data["servers"] if s["id"] == "postgres")
-    result = c.catalog_entry_to_paste_json(entry)
-    assert result["postgres"]["env"] == {"DATABASE_URI": ""}
-    # And the focus-target helper now has something to point the user at.
-    assert c.first_unfilled_focus_target(result["postgres"]) == ("env", "DATABASE_URI")
-
-
 def test_config_has_unfilled_placeholders_true_for_token():
     cfg = {"command": "npx", "args": ["-y", "server", "<ALLOWED_DIR>"]}
     assert c.config_has_unfilled_placeholders(cfg) is True
@@ -2410,307 +2374,620 @@ def test_config_has_unfilled_placeholders_checks_env_too():
 
 
 # --------------------------------------------------------------------------- #
-# Browse-catalog dialog helpers (issue #10 phase 2)
+# #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
 
 
-# --- catalog_category_group / CATALOG_CATEGORY_GROUPS --------------------- #
+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(
-    "category,expected_group",
+    "setting,system_dark,expected",
     [
-        ("files", "Files & Dev"),
+        (c.THEME_DARK, False, "dark"),
-        ("dev", "Files & Dev"),
+        (c.THEME_DARK, True, "dark"),
-        ("code-hosting", "Files & Dev"),
+        (c.THEME_LIGHT, False, "light"),
-        ("browser", "Files & Dev"),
+        (c.THEME_LIGHT, True, "light"),
-        ("database", "Data"),
+        (c.THEME_SYSTEM, True, "dark"),
-        ("data", "Data"),
+        (c.THEME_SYSTEM, False, "light"),
-        ("search", "Search & AI"),
-        ("ai", "Search & AI"),
-        ("cloud", "Cloud & Infra"),
-        ("infra", "Cloud & Infra"),
-        ("observability", "Cloud & Infra"),
-        ("productivity", "Work"),
-        ("communication", "Work"),
-        ("crm", "Work"),
-        ("finance", "Work"),
-        ("design", "Work"),
-        ("media", "Home & Personal"),
-        ("smart-home", "Home & Personal"),
-        ("personal", "Home & Personal"),
     ],
 )
-def test_catalog_category_group_maps_every_taxonomy_value(category, expected_group):
+def test_resolve_theme_covers_every_setting_and_appearance(setting, system_dark, expected):
-    assert c.catalog_category_group(category) == expected_group
+    assert c.resolve_theme(setting, system_dark) == expected
 
 
-def test_catalog_category_group_unknown_falls_back_to_other():
+@pytest.mark.parametrize("junk", ["", "solarized", None, "DARK", 3])
-    assert c.catalog_category_group("some-future-category-nobody-has-seen-yet") == "Other"
+def test_resolve_theme_falls_back_to_following_the_system(junk):
-    assert c.catalog_category_group("") == "Other"
+    """A hand-edited or future QSettings value should follow the desktop,
-    assert c.catalog_category_group(None) == "Other"
+    not pin a fixed theme."""
+    assert c.resolve_theme(junk, True) == "dark"
+    assert c.resolve_theme(junk, False) == "light"
 
 
-def test_catalog_category_group_is_case_insensitive():
+def test_palette_for_known_names():
-    assert c.catalog_category_group("Files") == "Files & Dev"
+    assert c.palette_for("dark") is c.DARK_PALETTE
-    assert c.catalog_category_group("DATABASE") == "Data"
+    assert c.palette_for("light") is c.LIGHT_PALETTE
 
 
-def test_shipped_catalog_categories_all_have_a_known_group():
+def test_palette_for_unknown_name_falls_back_to_dark():
-    """Regression: every category actually used in data/catalog.json must
+    assert c.palette_for("chartreuse") is c.DARK_PALETTE
-    collapse to one of the 7 chips, never silently drop an entry."""
-    root = Path(__file__).resolve().parent.parent
-    raw = (root / "data" / "catalog.json").read_bytes()
-    data = c.load_catalog(raw)
-    for entry in data["servers"]:
-        group = c.catalog_category_group(entry["category"])
-        assert group in c.CATALOG_CATEGORY_CHIPS
 
 
-# --- catalog_entry_matches_query / filter_catalog_entries ------------------ #
+def test_dark_palette_is_unchanged_from_the_shipped_look():
-def _catalog_entries():
+    """v1.3.0 shipped these exact colours; adding a light theme must not
-    return [
+    quietly restyle the dark one."""
-        {
+    p = c.DARK_PALETTE
-            "id": "filesystem",
+    assert (p.accent, p.bg, p.panel, p.panel_2) == ("#f97316", "#1b1d23", "#23262e", "#2b2f39")
-            "display": "Filesystem",
+    assert (p.text, p.muted, p.border) == ("#e7e9ee", "#9aa0ad", "#3a3f4b")
-            "description": "Read/write access to local directories you choose.",
+    assert (p.good, p.bad, p.warn, p.remote) == ("#4ade80", "#f87171", "#fbbf24", "#60a5fa")
-            "category": "files",
+    assert (p.on_accent, p.disabled_bg, p.mono_bg) == ("#1a1205", "#202229", "#16181d")
-        },
-        {
-            "id": "postgres",
-            "display": "Postgres MCP Pro",
-            "description": "Query and inspect a PostgreSQL database.",
-            "category": "database",
-        },
-        {
-            "id": "slack",
-            "display": "Slack",
-            "description": "Search messages and send messages from your assistant.",
-            "category": "communication",
-        },
-    ]
 
 
-def test_catalog_entry_matches_query_empty_matches_everything():
+def test_both_palettes_define_every_slot():
-    entries = _catalog_entries()
+    """A missing slot should fail here rather than render a broken window."""
-    assert c.filter_catalog_entries(entries, "") == entries
+    for pal in (c.DARK_PALETTE, c.LIGHT_PALETTE):
-    assert c.filter_catalog_entries(entries, "   ") == entries
+        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}"
 
 
-def test_catalog_entry_matches_query_matches_id():
+@pytest.mark.parametrize("pal_name", ["dark", "light"])
-    result = c.filter_catalog_entries(_catalog_entries(), "postgres")
+@pytest.mark.parametrize("slot", ["text", "muted", "good", "bad", "warn", "remote", "accent"])
-    assert [e["id"] for e in result] == ["postgres"]
+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
 
 
-def test_catalog_entry_matches_query_matches_display_case_insensitive():
+@pytest.mark.parametrize("pal_name", ["dark", "light"])
-    result = c.filter_catalog_entries(_catalog_entries(), "SLACK")
+def test_on_accent_is_legible_against_the_accent_fill(pal_name):
-    assert [e["id"] for e in result] == ["slack"]
+    """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_catalog_entry_matches_query_matches_description():
+def test_contrast_ratio_endpoints():
-    result = c.filter_catalog_entries(_catalog_entries(), "PostgreSQL database")
+    assert c.contrast_ratio("#000000", "#ffffff") == pytest.approx(21.0, abs=0.01)
-    assert [e["id"] for e in result] == ["postgres"]
+    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_catalog_entry_matches_query_matches_category():
+def test_relative_luminance_extremes():
-    result = c.filter_catalog_entries(_catalog_entries(), "database")
+    assert c.relative_luminance("#000000") == pytest.approx(0.0)
-    assert [e["id"] for e in result] == ["postgres"]
+    assert c.relative_luminance("#ffffff") == pytest.approx(1.0)
 
 
-def test_catalog_entry_matches_query_no_match_returns_empty():
+def test_stylesheet_builder_has_no_hardcoded_colours():
-    assert c.filter_catalog_entries(_catalog_entries(), "kubernetes") == []
+    """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_catalog_entries_in_group_all_returns_everything():
+def test_every_palette_slot_is_consumed():
-    entries = _catalog_entries()
+    """A slot added to Palette but never wired up is dead weight.
-    assert c.catalog_entries_in_group(entries, "All") == entries
+
-    assert c.catalog_entries_in_group(entries, "") == entries
+    Checks for `p.<slot>` anywhere in bcc.py, which covers both the QSS and
-    assert c.catalog_entries_in_group(entries, None) == entries
+    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"
 
 
-def test_catalog_entries_in_group_filters_by_collapsed_category():
+# --------------------------------------------------------------------------- #
-    result = c.catalog_entries_in_group(_catalog_entries(), "Data")
+# #78/#79 -- update notice: when to show it, and what it says
-    assert [e["id"] for e in result] == ["postgres"]
+# --------------------------------------------------------------------------- #
-
+def test_update_notice_when_a_newer_release_exists():
-    result = c.catalog_entries_in_group(_catalog_entries(), "Work")
+    n = c.update_notice("1.2.0", {"version": "v1.3.0", "url": "https://example.test/rel"})
-    assert [e["id"] for e in result] == ["slack"]
+    assert n is not None
+    assert n["version"] == "v1.3.0"
+    assert n["url"] == "https://example.test/rel"
+    assert "1.3.0" in n["text"] and "1.2.0" in n["text"]
 
 
-# --- format_freshness_hint -------------------------------------------------- #
+def test_update_notice_is_silent_when_current():
-def test_format_freshness_hint_none_returns_empty_string():
+    assert c.update_notice("1.3.0", {"version": "v1.3.0"}) is None
-    assert c.format_freshness_hint(None) == ""
+    assert c.update_notice("1.4.0", {"version": "v1.3.0"}) is None
-    assert c.format_freshness_hint("") == ""
 
 
-def test_format_freshness_hint_unparseable_returns_empty_string():
+@pytest.mark.parametrize("bad", [None, {}, {"version": ""}, {"version": None}, {"version": 3}, []])
-    assert c.format_freshness_hint("not-a-date") == ""
+def test_update_notice_is_silent_on_a_failed_or_malformed_check(bad):
+    """fetch_latest_release returns None on any failure; a half-formed payload
+    must not produce a notice pointing at nothing."""
+    assert c.update_notice("1.0.0", bad) is None
 
 
-def test_format_freshness_hint_this_month():
+def test_update_notice_falls_back_to_the_releases_page_without_a_url():
-    assert (
+    n = c.update_notice("1.0.0", {"version": "v2.0.0"})
-        c.format_freshness_hint("2026-07-01", today=c.date(2026, 7, 12))
+    assert n["url"] == c.RELEASES_URL
-        == "Last updated this month"
-    )
 
 
-def test_format_freshness_hint_one_month_singular():
+def test_update_notice_names_no_menu_path():
-    assert (
+    """The old status-line text said 'Help > About to view it', which is wrong
-        c.format_freshness_hint("2026-06-01", today=c.date(2026, 7, 12))
+    on macOS -- Qt moves the About action into the application menu (#79). The
-        == "Last updated 1 month ago"
+    notice carries its own action, so it must not describe a menu path."""
-    )
+    n = c.update_notice("1.0.0", {"version": "v2.0.0"})
+    lowered = n["text"].lower()
+    for phrase in ("help", "about", "menu", "▸", ">"):
+        assert phrase not in lowered, f"notice text should not reference {phrase!r}"
 
 
-def test_format_freshness_hint_months_ago():
+def test_update_notice_handles_the_v_prefix_consistently():
-    # Exactly 14 full months elapsed, no day-of-month remainder to round off.
+    assert c.update_notice("1.2.0", {"version": "1.3.0"}) is not None
-    assert (
+    assert c.update_notice("v1.2.0", {"version": "v1.3.0"}) is not None
-        c.format_freshness_hint("2025-01-15", today=c.date(2026, 3, 15))
+    assert c.update_notice("1.3.0", {"version": "v1.3.0"}) is None
-        == "Last updated 14 months ago"
-    )
 
 
-def test_format_freshness_hint_rounds_down_partial_month():
+def test_update_notice_renders_both_versions_the_same_way():
-    # 2025-05-16 -> 2026-07-12 is 13 full months, not 14: the 14th month
+    """Tags carry a 'v' prefix, __version__ doesn't -- don't show both forms
-    # would only complete on 2026-07-16.
+    in one sentence."""
-    assert (
+    n = c.update_notice("1.2.0", {"version": "v1.3.0"})
-        c.format_freshness_hint("2025-05-16", today=c.date(2026, 7, 12))
+    assert "v1.3.0" not in n["text"]
-        == "Last updated 13 months ago"
+    assert "1.3.0" in n["text"] and "1.2.0" in n["text"]
-    )
+    # the machine-readable field keeps the real tag
+    assert n["version"] == "v1.3.0"
 
 
-def test_format_freshness_hint_years_ago():
+# --------------------------------------------------------------------------- #
-    assert (
+# #76 -- ${VAR} references. Semantics mirror Claude Code's documented
-        c.format_freshness_hint("2024-01-01", today=c.date(2026, 7, 12))
+# behaviour: ${VAR} and ${VAR:-default}, expanded in command/args/env/url/
-        == "Last updated 2 years ago"
+# headers, and an unset variable with no default left as literal text.
-    )
+# --------------------------------------------------------------------------- #
+def test_find_env_refs_plain_and_defaulted():
+    refs = c.find_env_refs("${A} and ${B:-fallback}")
+    assert [(r.name, r.default) for r in refs] == [("A", None), ("B", "fallback")]
 
 
-def test_format_freshness_hint_23_months_stays_in_months_not_years():
+@pytest.mark.parametrize("text", ["${}", "${1BAD}", "$NOTBRACED", "{NOPE}", "plain", "$${X"])
-    # The switch to "N years ago" happens at 24 full months, not 12 -- the
+def test_find_env_refs_ignores_non_references(text):
-    # whole point of this hint is the granular "14 months ago" phrasing the
+    assert c.find_env_refs(text) == []
-    # design comment on #10 asked for, so 13-23 months must stay in months.
-    assert (
-        c.format_freshness_hint("2024-08-12", today=c.date(2026, 7, 12))
-        == "Last updated 23 months ago"
-    )
 
 
-def test_format_freshness_hint_future_date_returns_empty_string():
+def test_find_env_refs_allows_an_empty_default():
-    # A last_release "in the future" relative to `today` is nonsensical --
+    """`${VAR:-}` is a documented way to say 'blank if unset'."""
-    # show nothing rather than a misleading negative offset.
+    refs = c.find_env_refs("${A:-}")
-    assert c.format_freshness_hint("2027-01-01", today=c.date(2026, 7, 12)) == ""
+    assert refs[0].default == ""
+    assert refs[0].has_default is True
 
 
-# --- first_unfilled_focus_target -------------------------------------------- #
+def test_server_env_refs_covers_all_five_documented_fields():
-def test_first_unfilled_focus_target_prefers_placeholder_arg():
     data = {
-        "command": "npx",
+        "command": "${BIN}",
-        "args": ["-y", "server", "<ALLOWED_DIR>"],
+        "args": ["--x", "${ARG}"],
-        "env": {"API_KEY": ""},
+        "env": {"K": "${ENVV}"},
+        "url": "${URL}/mcp",
+        "headers": {"Authorization": "Bearer ${HDR}"},
+    }
+    found = {(r.name, r.field) for r in c.server_env_refs(data)}
+    assert found == {
+        ("BIN", "command"),
+        ("ARG", "args"),
+        ("ENVV", "env"),
+        ("URL", "url"),
+        ("HDR", "headers"),
     }
-    assert c.first_unfilled_focus_target(data) == ("args", 2)
 
 
-def test_first_unfilled_focus_target_falls_back_to_first_blank_env():
+def test_server_env_refs_ignores_unexpanded_fields():
-    data = {"command": "uvx", "args": ["mcp-grafana"], "env": {"GRAFANA_URL": ""}}
+    """Claude Code expands five fields; a ${VAR} elsewhere isn't a reference."""
-    assert c.first_unfilled_focus_target(data) == ("env", "GRAFANA_URL")
+    assert c.server_env_refs({"description": "${NOPE}", "timeout": "${ALSO_NO}"}) == []
 
 
-def test_first_unfilled_focus_target_none_when_fully_filled():
+def test_expand_env_refs_matches_documented_semantics():
-    data = {"command": "npx", "args": ["-y", "server"], "env": {"API_KEY": "sk-real-value"}}
+    env = {"SET": "value"}
-    assert c.first_unfilled_focus_target(data) is None
+    assert c.expand_env_refs("${SET}", env) == "value"
+    assert c.expand_env_refs("${MISSING:-dflt}", env) == "dflt"
+    assert c.expand_env_refs("${SET:-dflt}", env) == "value"
+    # unset with no default: left as literal text, exactly as Claude Code does
+    assert c.expand_env_refs("${MISSING}", env) == "${MISSING}"
 
 
-def test_first_unfilled_focus_target_none_for_config_with_no_env_or_args():
+def test_expand_env_refs_handles_several_in_one_string():
-    assert c.first_unfilled_focus_target({"command": "npx", "args": []}) is None
+    assert c.expand_env_refs("${A}/${B:-two}/${C}", {"A": "one"}) == "one/two/${C}"
 
 
-# --- load_bundled_catalog_entries ------------------------------------------- #
+def test_unresolved_env_refs_only_flags_unset_without_default():
-def test_load_bundled_catalog_entries_valid_signature(tmp_path, monkeypatch):
+    data = {"env": {"A": "${SET}", "B": "${UNSET}", "C": "${OTHER:-has_default}"}}
-    priv = Ed25519PrivateKey.generate()
+    assert [r.name for r in c.unresolved_env_refs(data, {"SET": "x"})] == ["UNSET"]
-    pub = priv.public_key().public_bytes_raw()
-    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
-
-    raw, sig = _signed(_minimal_catalog(version=1), priv)
-    catalog_path = tmp_path / "catalog.json"
-    sig_path = tmp_path / "catalog.json.sig"
-    catalog_path.write_bytes(raw)
-    sig_path.write_bytes(sig)
-
-    entries = c.load_bundled_catalog_entries(catalog_path, sig_path)
-    assert len(entries) == 1
-    assert entries[0]["id"] == "widget"
 
 
-def test_load_bundled_catalog_entries_tampered_payload_returns_empty_list(tmp_path, monkeypatch):
+# --- the two interactions that were backwards for this feature ------------
-    priv = Ed25519PrivateKey.generate()
+def test_placeholder_under_a_secret_key_is_not_masked():
-    pub = priv.public_key().public_bytes_raw()
+    """A ${VAR} names a secret rather than being one. Masking it would make a
-    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
+    reference indistinguishable from a stored credential."""
-
+    assert c.should_mask_value("API_KEY", "${API_KEY}") is False
-    raw, sig = _signed(_minimal_catalog(version=1), priv)
+    assert c.should_mask_value("API_KEY", "ghp_realsecret") is True
-    tampered = bytearray(raw)
+    assert c.should_mask_value("NOT_SECRET", "${API_KEY}") is False
-    tampered[-2] ^= 0xFF  # flip a byte inside the trailing bytes, still valid-ish JSON shape
-    catalog_path = tmp_path / "catalog.json"
-    sig_path = tmp_path / "catalog.json.sig"
-    catalog_path.write_bytes(bytes(tampered))
-    sig_path.write_bytes(sig)
-
-    assert c.load_bundled_catalog_entries(catalog_path, sig_path) == []
 
 
-def test_load_bundled_catalog_entries_wrong_key_returns_empty_list(tmp_path, monkeypatch):
+def test_redacted_display_keeps_placeholders_but_masks_real_secrets():
-    priv = Ed25519PrivateKey.generate()
+    out = c._redact_server_data({"env": {"API_KEY": "${API_KEY}", "TOKEN": "ghp_real"}})
-    other_priv = Ed25519PrivateKey.generate()
+    assert out["env"]["API_KEY"] == "${API_KEY}"
-    other_pub = other_priv.public_key().public_bytes_raw()
+    assert out["env"]["TOKEN"] == c.MASK
-    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [other_pub])
-
-    raw, sig = _signed(_minimal_catalog(version=1), priv)  # signed by the WRONG key
-    catalog_path = tmp_path / "catalog.json"
-    sig_path = tmp_path / "catalog.json.sig"
-    catalog_path.write_bytes(raw)
-    sig_path.write_bytes(sig)
-
-    assert c.load_bundled_catalog_entries(catalog_path, sig_path) == []
 
 
-def test_load_bundled_catalog_entries_missing_files_returns_empty_list(tmp_path):
+def test_redact_args_keeps_placeholders_visible():
-    assert c.load_bundled_catalog_entries(tmp_path / "nope.json", tmp_path / "nope.json.sig") == []
+    assert c.redact_args(["--token", "${GH_TOKEN}"]) == ["--token", "${GH_TOKEN}"]
+    assert c.redact_args(["--api-key=${K}"]) == ["--api-key=${K}"]
+    # real secrets still masked
+    assert c.redact_args(["--token", "ghp_real"]) == ["--token", c.MASK]
+    assert c.redact_args(["--api-key=sk-real"]) == [f"--api-key={c.MASK}"]
 
 
-def test_load_bundled_catalog_entries_missing_sig_returns_empty_list(tmp_path, monkeypatch):
+def test_args_secret_warning_is_silenced_by_a_placeholder():
-    priv = Ed25519PrivateKey.generate()
+    """Moving a token into ${VAR} is the recommended fix for this warning --
-    pub = priv.public_key().public_bytes_raw()
+    still warning afterwards would punish the fix."""
-    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
+    assert c.args_secret_warning({"args": ["--token", "ghp_real"]}) is not None
-
+    assert c.args_secret_warning({"args": ["--token", "${GH_TOKEN}"]}) is None
-    raw, _sig = _signed(_minimal_catalog(version=1), priv)
-    catalog_path = tmp_path / "catalog.json"
-    catalog_path.write_bytes(raw)
-    missing_sig_path = tmp_path / "catalog.json.sig"  # never written
-
-    assert c.load_bundled_catalog_entries(catalog_path, missing_sig_path) == []
 
 
-def test_load_bundled_catalog_entries_invalid_but_signed_returns_empty_list(tmp_path, monkeypatch):
+def test_args_secret_warning_still_fires_on_the_arg_after_a_placeholder():
-    """A payload that verifies but fails validate_catalog() (disallowed
+    """A placeholder must clear the pending-flag state, not blanket-suppress."""
-    command) must still come back empty -- signing is necessary, not
+    assert c.args_secret_warning({"args": ["${SAFE}", "--token", "ghp_real"]}) is not None
-    sufficient."""
-    priv = Ed25519PrivateKey.generate()
-    pub = priv.public_key().public_bytes_raw()
-    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
-
-    raw, sig = _signed(_catalog_with({"config": {"command": "bash", "args": []}}), priv)
-    catalog_path = tmp_path / "catalog.json"
-    sig_path = tmp_path / "catalog.json.sig"
-    catalog_path.write_bytes(raw)
-    sig_path.write_bytes(sig)
-
-    assert c.load_bundled_catalog_entries(catalog_path, sig_path) == []
 
 
-def test_load_bundled_catalog_entries_real_shipped_catalog():
+# --- per-client gating ----------------------------------------------------
-    """End-to-end regression against the actual bundled data/catalog.json +
+def _profile(path):
-    .sig, using the real CATALOG_PUBKEYS (no monkeypatch) -- this is what
+    return c.Profile(label="p", path=Path(path), config_exists=True)
-    the Browse dialog actually calls on startup."""
+
-    root = Path(__file__).resolve().parent.parent
+
-    entries = c.load_bundled_catalog_entries(
+def test_claude_code_profiles_expand_references():
-        root / "data" / "catalog.json", root / "data" / "catalog.json.sig"
+    assert c.client_expands_env_refs(_profile(Path.home() / ".claude.json")) is True
+    assert c.client_expands_env_refs(_profile("/repo/.mcp.json")) is True
+
+
+def test_claude_desktop_profile_does_not_expand_references():
+    desktop = _profile(c.app_support_base() / "Claude" / c.CONFIG_FILENAME)
+    assert c.client_expands_env_refs(desktop) is False
+
+
+def test_desktop_profile_warns_that_references_are_literal():
+    data = {"env": {"API_KEY": "${API_KEY}"}}
+    desktop = _profile(c.app_support_base() / "Claude" / c.CONFIG_FILENAME)
+    warnings = c.env_ref_warnings(data, desktop, {"API_KEY": "set"})
+    assert len(warnings) == 1
+    assert "NOT be expanded" in warnings[0]
+    assert "${API_KEY}" in warnings[0]
+
+
+def test_claude_code_profile_warns_only_about_unset_variables():
+    code = _profile(Path.home() / ".claude.json")
+    data = {"env": {"A": "${UNSET_ONE}"}}
+    assert c.env_ref_warnings(data, code, {}) != []
+    assert c.env_ref_warnings(data, code, {"UNSET_ONE": "x"}) == []
+    # a default means it always resolves
+    assert c.env_ref_warnings({"env": {"A": "${X:-d}"}}, code, {}) == []
+
+
+def test_no_references_means_no_warnings():
+    assert c.env_ref_warnings({"command": "npx", "args": ["-y", "pkg"]}, None) == []
+
+
+# --------------------------------------------------------------------------- #
+# Client adapters (issue #5 — cross-client support, phase 1)
+#
+# The refactor's promise is twofold: (1) the two Claude clients behave exactly
+# as before, and (2) the ClientSpec seam is real — a client with a different
+# servers key and a different per-server shape flows through the same pipeline.
+# A synthetic "VS Code-like" spec stands in for the phase-2 client so the
+# abstraction is proven now, before anything depends on it.
+# --------------------------------------------------------------------------- #
+def test_claude_specs_are_registered_and_mcpservers_shaped():
+    assert c.CLAUDE_DESKTOP.servers_key == "mcpServers"
+    assert c.CLAUDE_CODE.servers_key == "mcpServers"
+    assert c.CLAUDE_DESKTOP.disabled_key == c.DISABLED_KEY
+    assert c.CLAUDE_CODE.disabled_key == c.DISABLED_KEY
+    # capabilities the old inline filename checks used to compute
+    assert c.CLAUDE_DESKTOP.expands_env_refs is False
+    assert c.CLAUDE_CODE.expands_env_refs is True
+    assert c.CLAUDE_DESKTOP.supports_restart is True
+    assert c.CLAUDE_CODE.supports_restart is False
+    assert set(c.CLIENT_SPECS) == {c.CLAUDE_DESKTOP, c.CLAUDE_CODE}
+    assert c.DEFAULT_CLIENT is c.CLAUDE_DESKTOP
+
+
+def test_client_by_key_round_trips_and_misses():
+    assert c.client_by_key("claude_desktop") is c.CLAUDE_DESKTOP
+    assert c.client_by_key("claude_code") is c.CLAUDE_CODE
+    assert c.client_by_key("nope") is None
+
+
+def test_resolve_client_matches_the_old_filename_rule():
+    assert c.resolve_client("/x/Claude/claude_desktop_config.json") is c.CLAUDE_DESKTOP
+    assert c.resolve_client(Path.home() / ".claude.json") is c.CLAUDE_CODE
+    assert c.resolve_client("/repo/.mcp.json") is c.CLAUDE_CODE
+    assert c.resolve_client(Path.home() / ".claude" / "settings.json") is c.CLAUDE_CODE
+
+
+def test_profile_auto_resolves_client_from_path():
+    desktop = c.Profile(
+        label="Claude", path="/x/Claude/claude_desktop_config.json", config_exists=True
     )
-    assert len(entries) == 19
+    code = c.Profile(label="Claude Code", path="/home/me/.claude.json", config_exists=True)
-    assert {e["id"] for e in entries} >= {"filesystem", "github", "slack", "postgres"}
+    assert desktop.client is c.CLAUDE_DESKTOP
+    assert code.client is c.CLAUDE_CODE
+
+
+def test_profile_honours_an_explicit_client():
+    # An explicit spec is not overridden by the path-based resolver.
+    p = c.Profile(
+        label="odd",
+        path="/somewhere/claude_desktop_config.json",
+        config_exists=True,
+        client=c.CLAUDE_CODE,
+    )
+    assert p.client is c.CLAUDE_CODE
+
+
+def test_desktop_gating_and_env_expansion_read_off_the_spec():
+    desktop = c.Profile(label="d", path="/x/Claude/claude_desktop_config.json", config_exists=True)
+    code = c.Profile(label="c", path=Path.home() / ".claude.json", config_exists=True)
+    assert c.profile_targets_claude_desktop(desktop) is True
+    assert c.profile_targets_claude_desktop(code) is False
+    assert c.client_expands_env_refs(desktop) is False
+    assert c.client_expands_env_refs(code) is True
+
+
+def test_extract_and_apply_default_spec_is_unchanged():
+    # No spec argument must behave byte-for-byte like the pre-refactor code.
+    cfg = {"mcpServers": {"a": {"command": "x"}}, "_disabledMcpServers": {"b": {"command": "y"}}}
+    servers = c.extract_servers(cfg)
+    assert {(s.name, s.enabled) for s in servers} == {("a", True), ("b", False)}
+    out = c.apply_servers({}, servers)
+    assert out == {
+        "mcpServers": {"a": {"command": "x"}},
+        "_disabledMcpServers": {"b": {"command": "y"}},
+    }
+
+
+# A stand-in for the phase-2 VS Code adapter: different top-level key
+# ("servers"), a different disabled key, and a per-server shape that carries a
+# `type` field the internal model doesn't. entry_to/from_internal are the only
+# things it overrides — proving that's the whole extension point.
+class _FakeVSCode(c.ClientSpec):
+    def entry_to_internal(self, value):
+        if not isinstance(value, dict):
+            return value
+        return {k: v for k, v in value.items() if k != "type"}
+
+    def entry_from_internal(self, data):
+        if not isinstance(data, dict):
+            return data
+        return {"type": "stdio", **data}
+
+
+_VSCODE = _FakeVSCode(
+    key="vscode_fake",
+    label="VS Code (test)",
+    servers_key="servers",
+    disabled_key="_bccDisabledServers",
+)
+
+
+def test_extract_reads_a_custom_servers_key_and_translates_shape():
+    cfg = {"servers": {"a": {"type": "stdio", "command": "x", "args": ["-y"]}}}
+    servers = c.extract_servers(cfg, _VSCODE)
+    assert len(servers) == 1
+    # the `type` field was translated out of the internal model
+    assert servers[0].data == {"command": "x", "args": ["-y"]}
+
+
+def test_apply_writes_a_custom_key_translates_back_and_keeps_other_keys():
+    original = {"servers": {"old": {"type": "stdio", "command": "z"}}, "keepMe": {"x": 1}}
+    servers = c.extract_servers(original, _VSCODE)
+    out = c.apply_servers(original, servers, _VSCODE)
+    # round-trips through the custom key with the shape restored
+    assert out["servers"] == {"old": {"type": "stdio", "command": "z"}}
+    # the cardinal rule generalises: mcpServers is never introduced, and every
+    # unrelated key survives verbatim
+    assert "mcpServers" not in out
+    assert out["keepMe"] == {"x": 1}
+
+
+def test_apply_uses_the_custom_disabled_key():
+    servers = [
+        c.ServerEntry("on", {"command": "a"}, True),
+        c.ServerEntry("off", {"command": "b"}, False),
+    ]
+    out = c.apply_servers({}, servers, _VSCODE)
+    assert out["servers"] == {"on": {"type": "stdio", "command": "a"}}
+    assert out["_bccDisabledServers"] == {"off": {"type": "stdio", "command": "b"}}
+    assert c.DISABLED_KEY not in out
+
+
+def test_spec_with_no_disabled_key_drops_disabled_and_never_parks():
+    no_park = c.ClientSpec(
+        key="nopark", label="No Park", servers_key="mcpServers", disabled_key=None
+    )
+    servers = [
+        c.ServerEntry("on", {"command": "a"}, True),
+        c.ServerEntry("off", {"command": "b"}, False),
+    ]
+    out = c.apply_servers({}, servers, no_park)
+    assert out == {"mcpServers": {"on": {"command": "a"}}}
+    assert c.DISABLED_KEY not in out
+    assert no_park.section_keys() == ("mcpServers",)
+
+
+def test_section_keys_reports_both_when_a_disabled_key_exists():
+    assert c.CLAUDE_DESKTOP.section_keys() == ("mcpServers", c.DISABLED_KEY)
+    assert _VSCODE.section_keys() == ("servers", "_bccDisabledServers")
+
+
+def test_malformed_entry_round_trips_through_the_default_spec():
+    # #72's non-object server value must still be preserved verbatim on save.
+    cfg = {"mcpServers": {"bad": "oops", "good": {"command": "x"}}}
+    servers = c.extract_servers(cfg)
+    assert any(s.malformed and s.name == "bad" for s in servers)
+    out = c.apply_servers({}, servers)
+    assert out["mcpServers"]["bad"] == "oops"
+
+
+def test_server_sections_and_change_summary_follow_a_custom_key(tmp_path):
+    loaded = {"servers": {"a": {"type": "stdio", "command": "x", "env": {"API_KEY": "sekret"}}}}
+    sections = c._server_sections(loaded, _VSCODE)
+    assert "servers" in sections
+    assert "mcpServers" not in sections
+    # secret masking still applies through the custom key
+    assert sections["servers"]["a"]["env"]["API_KEY"] == c.MASK
+
+    disk = {"servers": {"a": {"type": "stdio", "command": "CHANGED"}}}
+    p = tmp_path / "vscode.json"
+    p.write_text(json.dumps(disk), encoding="utf-8")
+    changed_keys, diff = c.external_change_summary(loaded, p, _VSCODE)
+    assert "servers" in changed_keys
+    assert diff  # a server-section change under the custom key is diffed