Merge PR #81: make the update checker visible — persistent banner + Help menu item (#78, #79)

Three conflicts, two of them semantic rather than textual:

- bcc.py QSS: this branch added the noticeBanner rules using the old
  module-level constants ({MUTED}, {ACCENT}); main had since moved the
  stylesheet onto palette slots ({p.muted}). Took main's form and
  translated the notice rules into it -- picking either side wholesale
  would have either dropped the banner styling or reintroduced globals
  that test_stylesheet_builder_has_no_hardcoded_colours now forbids.
- bcc.py methods: both sides appended to MainWindow (update-notice
  handlers vs theme handlers). Additive, kept both.
- tests/test_core.py: the usual EOF append. Kept both blocks.

_build_menu_bar auto-merged cleanly (View menu above, Help menu below);
verified both are present with their menu roles intact.

Verified: 272 test functions = 265 (main) + 7 (this branch), no
duplicates; 421 passed, ruff clean.

.github/workflows/ci.yml

@@ -95,12 +95,39 @@ jobs:
       - name: Install dependencies
         run: pip install cryptography
 
+      # 🔴 TRUST ANCHOR — issue #68 finding 4.
+      #
+      # This step used to do `import bcc_core as c` FROM THE CHECKED-OUT PR
+      # BRANCH and verify the catalog against c.CATALOG_PUBKEYS — i.e. it
+      # trusted the public key shipped in the very diff it was reviewing. A
+      # PR that changed data/catalog.json AND bcc_core.CATALOG_PUBKEYS (to
+      # an attacker key, with a matching signature produced by the attacker's
+      # matching private key) went green, because there was nothing outside
+      # the PR's own content to check the key against. The gate's whole
+      # point is catching a friendly-looking PR the maintainer merges
+      # without really reading it — and that hole made it a two-file diff.
+      #
+      # EXPECTED_CATALOG_PUBKEY_B64 below is hardcoded HERE, in the workflow
+      # file, independent of whatever bcc_core.py says on the PR branch. It
+      # is intentionally the only line in this step that matters for
+      # security review: changing it changes what this gate is willing to
+      # trust. THIS CONSTANT IS A TRUST ANCHOR. A PR that changes this line
+      # in the same diff as a catalog change is exactly the attack this gate
+      # exists to prevent — review a change to this line on its own,
+      # never bundled with a catalog update.
+      #
+      # NOTE for the next key rotation: update EXPECTED_CATALOG_PUBKEY_B64
+      # below to the new key's base64 form, as its own reviewed change.
       - name: Verify data/catalog.json.sig
+        env:
+          EXPECTED_CATALOG_PUBKEY_B64: "082NOwVB7uURkvfyS3+knJ+40Fk6C9unsF47+2uPKo4="
         run: |
           python - <<'PY'
-          import pathlib, sys
+          import base64, os, pathlib, sys
           import bcc_core as c
 
+          expected_pubkey_b64 = os.environ["EXPECTED_CATALOG_PUBKEY_B64"]
+
           raw = pathlib.Path("data/catalog.json").read_bytes()
           sig_path = pathlib.Path("data/catalog.json.sig")
 
@@ -111,6 +138,26 @@ jobs:
           if b"\x00" * 32 in c.CATALOG_PUBKEYS:
               sys.exit("FAIL: CATALOG_PUBKEYS still holds the placeholder key.")
 
+          # Trust anchor check FIRST, before verifying anything against
+          # bcc_core.CATALOG_PUBKEYS: a PR is not allowed to bring its own
+          # key. CATALOG_PUBKEYS on the checked-out branch must be EXACTLY
+          # the key(s) this workflow file itself expects -- no more, no
+          # fewer, no substitutions.
+          actual_pubkeys_b64 = [base64.b64encode(k).decode() for k in c.CATALOG_PUBKEYS]
+          if actual_pubkeys_b64 != [expected_pubkey_b64]:
+              sys.exit(
+                  "FAIL: bcc_core.CATALOG_PUBKEYS on this branch does not match the "
+                  "trust anchor hardcoded in .github/workflows/ci.yml.\n"
+                  f"  expected: {[expected_pubkey_b64]}\n"
+                  f"  actual:   {actual_pubkeys_b64}\n"
+                  "\n"
+                  "This PR is changing (or has changed) the catalog signing key. That "
+                  "change must be reviewed on its own, separately from any catalog "
+                  "content change, and the workflow's EXPECTED_CATALOG_PUBKEY_B64 "
+                  "updated deliberately -- not accepted because it happened to match "
+                  "whatever bcc_core.py says on this branch."
+              )
+
           if not c.verify_catalog_signature(raw, sig_path.read_bytes(), c.CATALOG_PUBKEYS):
               sys.exit(
                   "FAIL: data/catalog.json does NOT match its signature.\n"
@@ -124,5 +171,6 @@ jobs:
           if problems:
               sys.exit("FAIL: catalog failed validation:\n  " + "\n  ".join(problems))
 
-          print("OK: catalog signature verifies and the catalog validates clean.")
+          print("OK: catalog signature verifies, the pubkey matches the CI trust anchor, "
+                "and the catalog validates clean.")
           PY

bcc.py

@@ -10,6 +10,7 @@ Run:  python mcp_manager.py
 
 from __future__ import annotations
 
+import contextlib
 import sys
 import time
 from pathlib import Path
@@ -18,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,
@@ -25,6 +27,7 @@ from PySide6.QtGui import (
     QIcon,
     QKeySequence,
     QPainter,
+    QPalette,
     QPixmap,
 )
 from PySide6.QtWidgets import (
@@ -65,85 +68,122 @@ import bcc_core as core
 # thread during drag-and-drop import, so skip anything larger than this.
 MAX_DROP_IMPORT_BYTES = 5 * 1024 * 1024  # 5 MB
 
-# --- One-line rebrand: change this to recolor the whole app --------------- #
+# --- Theming (issue #75) -------------------------------------------------- #
-ACCENT = "#f97316"  # warm orange
+# The palette lives in bcc_core (testable without a Qt app); these module-level
-ACCENT_DIM = "#c2570b"
+# names are rebound by `apply_palette()` whenever the theme changes.
-BG = "#1b1d23"
+#
-PANEL = "#23262e"
+# Why globals rather than passing a palette around: ~20 inline
-PANEL_2 = "#2b2f39"
+# `setStyleSheet(f"color: {MUTED}")` calls are scattered through this file, and
-TEXT = "#e7e9ee"
+# an f-string resolves its names when it runs, not when it's compiled. Rebinding
-MUTED = "#9aa0ad"
+# the globals means every one of those call sites picks up the new colour on its
-BORDER = "#3a3f4b"
+# next render, with no change to the call sites themselves.
-GOOD = "#4ade80"
+PALETTE = core.DARK_PALETTE
-BAD = "#f87171"
+ACCENT = ACCENT_DIM = BG = PANEL = PANEL_2 = TEXT = MUTED = BORDER = ""
-WARN = "#fbbf24"
+GOOD = BAD = WARN = REMOTE = ON_ACCENT = DISABLED_BG = MONO_BG = SEL_TEXT = ""
+STATUS_COLORS: dict[str, str] = {}
+HEALTH_COLORS: dict[str, str] = {}
 
-STATUS_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": "#60a5fa", "unknown": WARN}
+STATUS_GLYPH = {
-STATUS_GLYPH = {"ok": "●", "missing": "●", "warn": "▲", "remote": "◆", "unknown": "○"}
+    "ok": "\u25cf",
+    "missing": "\u25cf",
+    "warn": "\u25b2",
+    "remote": "\u25c6",
+    "unknown": "\u25cb",
+}
 
 # Health dot (spawn-test outcome, see core.HealthStatus) shown per row in the
 # server tables' "Health" column -- distinct from the PATH-dependency Status
 # column above.
-HEALTH_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED}
+HEALTH_GLYPH = {"ok": "\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)
 # --------------------------------------------------------------------------- #
@@ -1493,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.
@@ -1550,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)
@@ -1585,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()
 
@@ -1610,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):
@@ -1939,9 +2116,21 @@ class MainWindow(QMainWindow):
                 return
             self.full_config = cfg
             repaired = True
+        # extract_servers tolerates malformed entries rather than raising (#72),
+        # but keep it inside the guard: a load failure must leave the previously
+        # loaded profile intact instead of half-swapping the window's state.
+        try:
+            servers = core.extract_servers(self.full_config)
+        except Exception as exc:  # pragma: no cover - defence in depth
+            QMessageBox.critical(
+                self,
+                "Could not read config",
+                f"{profile.path}\n\nThe server list couldn't be read: {exc}",
+            )
+            return
         self._loaded_stat = core.config_fingerprint(profile.path)
         self.current_profile = profile
-        self.servers = core.extract_servers(self.full_config)
+        self.servers = servers
         self.dirty = False
         self.restart_btn.hide()
         self._undo_stack.clear()
@@ -2264,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
@@ -2358,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))
@@ -2478,6 +2667,11 @@ class MainWindow(QMainWindow):
                 except Exception as e:
                     QMessageBox.critical(self, "Reload failed", str(e))
                     return
+                # The reload above is the on-disk truth for everything the user
+                # didn't touch -- but it also wipes BCC-authored keys the user
+                # changed in this session (named sets), which apply_servers
+                # doesn't write. Carry them over before saving (#73).
+                contested = core.carry_owned_keys(self.full_config, fresh)
                 core.apply_servers(fresh, self.servers)
                 try:
                     backup = core.write_config(self.current_profile.path, fresh)
@@ -2490,8 +2684,13 @@ class MainWindow(QMainWindow):
                 self.dirty = False
                 self.save_btn.setEnabled(False)
                 bnote = f"  ·  backup: {backup.name}" if backup else "  ·  (new file)"
+                cnote = (
+                    f"  ·  kept your {', '.join(contested)} (the file on disk had a different copy)"
+                    if contested
+                    else ""
+                )
                 self.status.setText(
-                    f"Merged & saved {self.current_profile.path}{bnote}"
+                    f"Merged & saved {self.current_profile.path}{bnote}{cnote}"
                     f"  ·  Restart {self.current_profile.label} to apply."
                 )
                 self._offer_restart_button()
@@ -2649,6 +2848,33 @@ class MainWindow(QMainWindow):
         e.accept()
 
 
+def system_is_dark(app: QApplication) -> bool:
+    """Whether the desktop is currently using a dark appearance.
+
+    Read from the style's own window colour rather than per-platform APIs --
+    Qt has already resolved the OS appearance by the time it builds the
+    default palette, so this works the same on all three platforms.
+    """
+    try:
+        return app.palette().color(QPalette.ColorRole.Window).lightness() < 128
+    except Exception:  # pragma: no cover - defensive; never block startup on theming
+        return True
+
+
+def stored_theme_setting() -> str:
+    """The user's theme choice, defaulting to following the system."""
+    value = QSettings("BCC", "BetterClaudeConfig").value("ui/theme", core.THEME_SYSTEM)
+    return value if value in core.THEME_CHOICES else core.THEME_SYSTEM
+
+
+def theme_stylesheet_for(app: QApplication, setting: str | None = None) -> str:
+    """Resolve setting + OS appearance into a palette, apply it, return the QSS."""
+    if setting is None:
+        setting = stored_theme_setting()
+    theme = core.resolve_theme(setting, system_is_dark(app))
+    return apply_palette(core.palette_for(theme))
+
+
 def main():
     if sys.platform == "win32":
         # Without an explicit AppUserModelID, Windows taskbar groups the app
@@ -2667,7 +2893,7 @@ def main():
     icon = _app_icon()
     if not icon.isNull():
         app.setWindowIcon(icon)
-    app.setStyleSheet(STYLESHEET)
+    app.setStyleSheet(theme_stylesheet_for(app))
     win = MainWindow()
     win.show()
     sys.exit(app.exec())

bcc_core.py

@@ -15,6 +15,7 @@ from __future__ import annotations
 
 import base64
 import contextlib
+import copy
 import difflib
 import functools
 import glob
@@ -49,6 +50,12 @@ DISABLED_KEY = "_disabledMcpServers"
 # parks the rest under DISABLED_KEY.
 SETS_KEY = "_bccServerSets"
 
+# Top-level keys BCC itself authors. They live in the client's config file, but
+# BCC is their owner, so on a stale-file merge the in-memory copy wins over the
+# on-disk one (see `carry_owned_keys`). Any future BCC-authored key belongs
+# here -- forgetting to add one is exactly how #73 happened.
+BCC_OWNED_KEYS = (SETS_KEY,)
+
 BACKUP_DIRNAME = ".bcc_backups"
 MAX_BACKUPS = 15
 
@@ -163,6 +170,39 @@ def fetch_latest_release(timeout: float = 4.0) -> dict | None:
     return {"version": tag, "url": payload.get("html_url") or RELEASES_URL}
 
 
+def update_notice(
+    current: str, release: dict | None, url_fallback: str = RELEASES_URL
+) -> dict | None:
+    """Decide whether to tell the user about a release, and what to say.
+
+    Returns {"version", "text", "url"} when `release` is newer than `current`,
+    or None when it isn't, when the check failed, or when the payload is
+    malformed. Kept here rather than in the GUI so the wording and the
+    should-we-notify decision are testable -- bcc.py can't be imported by the
+    test suite, which has no PySide6.
+
+    The text deliberately names no menu path. The old status-line notice read
+    "Help > About to view it", which is wrong on macOS: Qt relocates the About
+    action into the application menu (#79). A notice that carries its own
+    action can't drift out of sync with the platform.
+    """
+    if not isinstance(release, dict):
+        return None
+    version = release.get("version")
+    if not version or not isinstance(version, str):
+        return None
+    if not is_newer_version(current, version):
+        return None
+    # Tags carry a "v" prefix and __version__ doesn't; render both the same way
+    # so the notice doesn't read "Version v1.3.0 ... you're running 1.2.0".
+    shown = version.lstrip("vV")
+    return {
+        "version": version,
+        "text": f"Version {shown} is available. You're running {current.lstrip('vV')}.",
+        "url": release.get("url") or url_fallback,
+    }
+
+
 # --------------------------------------------------------------------------- #
 # Data model
 # --------------------------------------------------------------------------- #
@@ -178,16 +218,195 @@ class Profile:
         self.path = Path(self.path)
 
 
+class _NoRaw:
+    """Sentinel for ServerEntry.raw.
+
+    `None` can't do this job: `{"mcpServers": {"foo": null}}` is legal JSON and
+    a real malformed-entry case, so None has to mean "the config said null",
+    not "there was nothing here".
+    """
+
+    __slots__ = ()
+
+    def __repr__(self) -> str:  # keeps ServerEntry reprs readable in test output
+        return "<no raw>"
+
+
+NO_RAW = _NoRaw()
+
+
 @dataclass
 class ServerEntry:
+    """One server definition.
+
+    `data` is always a dict so every consumer can treat it as one. When the
+    config held something that wasn't a JSON object for this server (a string,
+    a number, a list -- all legal JSON, all wrong here), `data` is empty and
+    the original value is preserved verbatim in `raw` so Save round-trips it
+    instead of silently deleting the user's line. `lint_servers` surfaces it.
+    `raw` defaults to the NO_RAW sentinel rather than None, because a config
+    value of literal `null` is itself a malformed entry worth preserving.
+
+    Assigning `data` means the user replaced the definition through the editor,
+    which retires `raw` -- use `set_data` so that can't be forgotten.
+    """
+
     name: str
     data: dict
     enabled: bool = True
+    raw: object = NO_RAW
 
     @property
     def kind(self) -> str:
         return "remote" if "url" in self.data and "command" not in self.data else "stdio"
 
+    @property
+    def malformed(self) -> bool:
+        """True when the config value for this server wasn't a JSON object."""
+        return self.raw is not NO_RAW
+
+    def set_data(self, data: dict) -> None:
+        """Replace the definition from the editor, clearing any malformed original."""
+        self.data = data
+        self.raw = NO_RAW
+
+    def config_value(self):
+        """What to write back to the config: the edited dict, or the untouched
+        malformed original when the user never edited it."""
+        return self.data if self.raw is NO_RAW else self.raw
+
+
+# --------------------------------------------------------------------------- #
+# Theming (issue #75)
+# --------------------------------------------------------------------------- #
+THEME_SYSTEM = "system"
+THEME_LIGHT = "light"
+THEME_DARK = "dark"
+THEME_CHOICES = (THEME_SYSTEM, THEME_LIGHT, THEME_DARK)
+
+
+@dataclass(frozen=True)
+class Palette:
+    """Every colour the UI draws with.
+
+    Deliberately exhaustive: the stylesheet used to inline a handful of
+    near-black literals (`#1a1205` for text on the accent, `#202229` for the
+    disabled table, `#16181d` for the diagnostics pane), which is fine while
+    there's one theme and invisible breakage the moment there are two. Each
+    gets a slot here so a light palette can't silently inherit a dark value.
+    """
+
+    name: str
+    accent: str
+    accent_dim: str
+    bg: str
+    panel: str
+    panel_2: str
+    text: str
+    muted: str
+    border: str
+    good: str
+    bad: str
+    warn: str
+    remote: str
+    on_accent: str  # text drawn on top of an accent fill
+    disabled_bg: str  # the parked-servers table
+    mono_bg: str  # diagnostics / log panes
+    selection_text: str
+
+
+# The shipping theme through v1.3.0. These values are carried over verbatim --
+# adding a light theme must not restyle the dark one.
+DARK_PALETTE = Palette(
+    name="dark",
+    accent="#f97316",
+    accent_dim="#c2570b",
+    bg="#1b1d23",
+    panel="#23262e",
+    panel_2="#2b2f39",
+    text="#e7e9ee",
+    muted="#9aa0ad",
+    border="#3a3f4b",
+    good="#4ade80",
+    bad="#f87171",
+    warn="#fbbf24",
+    remote="#60a5fa",
+    on_accent="#1a1205",
+    disabled_bg="#202229",
+    mono_bg="#16181d",
+    selection_text="#ffffff",
+)
+
+# The semantic colours are NOT the dark ones lightened. #4ade80 / #fbbf24 sit
+# around 1.7:1 against white -- illegible. These are darkened to clear 4.5:1,
+# which `test_light_palette_meets_contrast` enforces so nobody "tidies" them
+# back toward the dark hues later.
+LIGHT_PALETTE = Palette(
+    name="light",
+    accent="#c2410c",
+    accent_dim="#9a3412",
+    bg="#f6f7f9",
+    panel="#ffffff",
+    panel_2="#eef0f4",
+    text="#1b1d23",
+    muted="#5c6270",
+    border="#d3d7de",
+    good="#15803d",
+    bad="#b91c1c",
+    warn="#a16207",
+    remote="#1d4ed8",
+    on_accent="#ffffff",
+    disabled_bg="#e9ebef",
+    mono_bg="#f0f2f5",
+    selection_text="#ffffff",
+)
+
+PALETTES = {DARK_PALETTE.name: DARK_PALETTE, LIGHT_PALETTE.name: LIGHT_PALETTE}
+
+
+def resolve_theme(setting: str, system_is_dark: bool) -> str:
+    """Map a stored theme setting + the OS appearance onto a concrete palette name.
+
+    Anything unrecognised (a hand-edited QSettings value, a setting written by
+    a future version) falls back to following the system rather than to a
+    fixed theme -- the user's desktop is the better guess.
+    """
+    if setting == THEME_DARK:
+        return THEME_DARK
+    if setting == THEME_LIGHT:
+        return THEME_LIGHT
+    return THEME_DARK if system_is_dark else THEME_LIGHT
+
+
+def palette_for(theme: str) -> Palette:
+    """Concrete palette by name; unknown names fall back to dark (the historical look)."""
+    return PALETTES.get(theme, DARK_PALETTE)
+
+
+def _hex_to_rgb(value: str) -> tuple[int, int, int]:
+    v = value.lstrip("#")
+    if len(v) == 3:
+        v = "".join(ch * 2 for ch in v)
+    return int(v[0:2], 16), int(v[2:4], 16), int(v[4:6], 16)
+
+
+def relative_luminance(color: str) -> float:
+    """WCAG relative luminance for a #rrggbb colour."""
+
+    def chan(c: int) -> float:
+        srgb = c / 255.0
+        return srgb / 12.92 if srgb <= 0.04045 else ((srgb + 0.055) / 1.055) ** 2.4
+
+    r, g, b = (chan(c) for c in _hex_to_rgb(color))
+    return 0.2126 * r + 0.7152 * g + 0.0722 * b
+
+
+def contrast_ratio(fg: str, bg: str) -> float:
+    """WCAG contrast ratio between two #rrggbb colours (1.0 to 21.0)."""
+    a, b = relative_luminance(fg), relative_luminance(bg)
+    lighter, darker = max(a, b), min(a, b)
+    return (lighter + 0.05) / (darker + 0.05)
+
 
 # --------------------------------------------------------------------------- #
 # Discovery
@@ -452,13 +671,30 @@ def repair_config_file(path: str | os.PathLike) -> tuple[dict, list[str], str]:
     return obj, notes, pretty
 
 
+def _server_entry(name: str, data, enabled: bool) -> ServerEntry:
+    """Build a ServerEntry, tolerating a value that isn't a JSON object.
+
+    A hand-edited config can legally hold `{"mcpServers": {"foo": "oops"}}` --
+    valid JSON, wrong shape. Calling dict() on that raises, which used to take
+    the whole load down before the linter ever got a look at it (#72). Keep the
+    original instead and let the linter report it.
+    """
+    if isinstance(data, dict):
+        return ServerEntry(name=name, data=dict(data), enabled=enabled)
+    return ServerEntry(name=name, data={}, enabled=enabled, raw=data)
+
+
 def extract_servers(cfg: dict) -> list[ServerEntry]:
-    """Pull enabled (`mcpServers`) and disabled (`_disabledMcpServers`) servers."""
+    """Pull enabled (`mcpServers`) and disabled (`_disabledMcpServers`) servers.
+
+    Never raises on a structurally-odd config -- malformed entries come back as
+    empty-data entries carrying their original value (see `_server_entry`).
+    """
     out: list[ServerEntry] = []
     for name, data in (cfg.get("mcpServers") or {}).items():
-        out.append(ServerEntry(name=name, data=dict(data), enabled=True))
+        out.append(_server_entry(name, data, True))
     for name, data in (cfg.get(DISABLED_KEY) or {}).items():
-        out.append(ServerEntry(name=name, data=dict(data), enabled=False))
+        out.append(_server_entry(name, data, False))
     return out
 
 
@@ -550,8 +786,8 @@ def apply_servers(cfg: dict, servers: list[ServerEntry]) -> dict:
     Write the server list back into `cfg` in place, preserving every other key
     and the position of `mcpServers`. Returns the same dict for convenience.
     """
-    enabled = {s.name: s.data for s in servers if s.enabled}
+    enabled = {s.name: s.config_value() for s in servers if s.enabled}
-    disabled = {s.name: s.data for s in servers if not s.enabled}
+    disabled = {s.name: s.config_value() for s in servers if not s.enabled}
 
     cfg["mcpServers"] = enabled  # replaces value if key existed; appends otherwise
     if disabled:
@@ -564,6 +800,34 @@ def apply_servers(cfg: dict, servers: list[ServerEntry]) -> dict:
 # --------------------------------------------------------------------------- #
 # Write (atomic, with rotating backups)
 # --------------------------------------------------------------------------- #
+def carry_owned_keys(local_cfg: dict, fresh_cfg: dict) -> list[str]:
+    """Carry BCC-authored top-level keys from `local_cfg` onto `fresh_cfg`.
+
+    Used by the stale-file "Merge & save" path, which reloads the file from
+    disk and re-applies the user's server edits. That reload used to drop
+    anything BCC owns but `apply_servers` doesn't write -- named server sets
+    vanished without a word (#73). BCC owns these keys, so the in-memory copy
+    wins; mutates `fresh_cfg` in place.
+
+    Returns the keys where the on-disk copy differed and was overwritten, so
+    the caller can tell the user something was actually contested rather than
+    merely carried across.
+
+    Deliberately one-directional: a key absent locally is left alone on disk.
+    We can't tell "user deleted their last set" from "user never had sets and
+    another machine just added some", and silently deleting someone else's
+    data is the worse of the two failures.
+    """
+    conflicts: list[str] = []
+    for key in BCC_OWNED_KEYS:
+        if key not in local_cfg:
+            continue
+        if key in fresh_cfg and fresh_cfg[key] != local_cfg[key]:
+            conflicts.append(key)
+        fresh_cfg[key] = copy.deepcopy(local_cfg[key])
+    return conflicts
+
+
 def _make_backup(path: Path) -> Path:
     bdir = path.parent / BACKUP_DIRNAME
     bdir.mkdir(exist_ok=True)
@@ -1380,9 +1644,21 @@ def lint_server(name: str, data: dict) -> list[str]:
 
 
 def lint_servers(servers: list[ServerEntry]) -> list[str]:
-    """Concatenate lint_server warnings across every entry, in order."""
+    """Concatenate lint_server warnings across every entry, in order.
+
+    Entries whose config value wasn't a JSON object at all are reported here
+    rather than in lint_server, which takes an already-dict `data` (#72).
+    """
     out: list[str] = []
     for s in servers:
+        if s.malformed:
+            nm = s.name.strip() or "(unnamed)"
+            out.append(
+                f"'{nm}': server definition is not an object "
+                f"(found {type(s.raw).__name__}) -- it is preserved as-is; "
+                f"edit it to replace it with a proper definition"
+            )
+            continue
         out.extend(lint_server(s.name, s.data))
     return out
 
@@ -2164,6 +2440,30 @@ def restart_claude_desktop() -> RestartResult:
 # is rejected by validate_catalog() regardless of how plausible it looks.
 CATALOG_ALLOWED_COMMANDS = frozenset({"npx", "uvx", "docker", "node", "python", "python3"})
 
+# Env var keys a catalog entry's config.env must never set. Every one of
+# these is a loader/interpreter override that lets a value walk straight
+# past CATALOG_ALLOWED_COMMANDS and the -e/--eval/-c deny-rule below: e.g.
+# NODE_OPTIONS="--require /tmp/x.js" turns an allowlisted `npx` entry into
+# arbitrary code execution without ever touching config.args, which is the
+# only field the allowlist/deny-rules/ASCII/secret checks used to cover.
+# Matched case-insensitively -- env keys are case-sensitive on POSIX, but a
+# `node_options` lookalike is exactly the kind of thing this must catch.
+CATALOG_DENIED_ENV_KEYS = frozenset(
+    {
+        "NODE_OPTIONS",
+        "PYTHONSTARTUP",
+        "PYTHONPATH",
+        "PYTHONHOME",
+        "LD_PRELOAD",
+        "LD_LIBRARY_PATH",
+        "DYLD_INSERT_LIBRARIES",
+        "DYLD_LIBRARY_PATH",
+        "BROWSER",
+        "PATH",
+        "NODE_REPL_EXTERNAL_MODULE",
+    }
+)
+
 # Ed25519 public keys allowed to sign a catalog, raw 32-byte form. A LIST
 # (not a single key) so keys can be rotated without bricking installs that
 # still trust an older key: verify_catalog_signature() accepts a match
@@ -2189,6 +2489,36 @@ _CATALOG_SECRET_ARG_RE = re.compile(r"(?i)--api[-_]?key=|--token=|--password=")
 # <PLACEHOLDER>-style tokens the GUI must have the user fill in before Save.
 _PLACEHOLDER_RE = re.compile(r"<[^<>\s]+>")
 
+# A catalog entry's id becomes an mcpServers JSON key AND is interpolated
+# into Qt.AutoText widgets (status bar, QMessageBox) -- an id like
+# "<b>Verified</b>" renders as markup there. Not RCE, but UI spoofing, so
+# ids are constrained to a plain lowercase slug.
+_CATALOG_ID_RE = re.compile(r"^[a-z0-9][a-z0-9._-]{0,63}$")
+
+# Docker flags that consume the next arg as a value (so that value must not
+# be mistaken for the image reference when locating it in config.args).
+_DOCKER_VALUE_FLAGS = frozenset(
+    {
+        "-e",
+        "--env",
+        "-v",
+        "--volume",
+        "-p",
+        "--publish",
+        "--name",
+        "-w",
+        "--workdir",
+        "-u",
+        "--user",
+        "--entrypoint",
+        "--network",
+        "--platform",
+        "--add-host",
+        "-l",
+        "--label",
+    }
+)
+
 # How many versions a single accepted catalog jump may leap in one go. Bounds
 # a "freeze" attack: a compromised/leaked signing key claiming an absurd
 # future version would otherwise permanently outrank every legitimate
@@ -2263,6 +2593,121 @@ def _docker_arg_violations(tag: str, args: list[str]) -> list[str]:
     return problems
 
 
+def _catalog_package_spec_version(spec: str) -> str | None:
+    """
+    Extract the version pin from an npm-style package spec, or None if the
+    spec carries no pin.
+
+    Handles unscoped "name@version" and scoped "@scope/name@version" --
+    scoped names have a leading "@" that is NOT the version separator, so a
+    naive split on the first/only "@" misparses "@scope/pkg" (no version)
+    as pinned to "scope/pkg". Splitting from the right side instead is safe
+    for both forms because a package name may contain "@" only as the
+    scope's leading character.
+    """
+    if spec.startswith("@"):
+        rest = spec[1:]
+        if "@" not in rest:
+            return None
+        _, _, version = rest.rpartition("@")
+        return version or None
+    if "@" not in spec:
+        return None
+    _, _, version = spec.rpartition("@")
+    return version or None
+
+
+def _catalog_package_spec_pinned(spec: str) -> bool:
+    """
+    True if `spec` carries an exact version pin. Covers npm's "name@version"
+    / "@scope/name@version" and uv's documented PyPI pin forms
+    "name@version" and "name==version".
+    """
+    if "==" in spec:
+        _, _, version = spec.partition("==")
+        return bool(version)
+    return bool(_catalog_package_spec_version(spec))
+
+
+def _first_catalog_package_spec(args: list[str]) -> str | None:
+    """
+    The first arg that could plausibly BE a package spec: skip flags
+    (leading "-") and <PLACEHOLDER> tokens (which can't be validated and
+    are filled in by the user later, never shipped by the catalog as the
+    package name itself). Everything after the first hit is ignored --
+    trailing flags, paths, and placeholders are not package specs.
+    """
+    for a in args:
+        if a.startswith("-"):
+            continue
+        if _PLACEHOLDER_RE.fullmatch(a):
+            continue
+        return a
+    return None
+
+
+def _catalog_pin_violations(tag: str, command: str, args: list[str]) -> list[str]:
+    """
+    Version-pinning enforcement (finding #3): a catalog PR can otherwise
+    ship `npx -y @scope/pkg` or `docker run img:latest` and the *next*
+    resolve of that package/image is whatever the registry serves that day
+    -- outside review, outside the signature's meaning. This is the only
+    place that enforces pinning at runtime; catalog_review.py's
+    risk_unpinned_package() is a maintainer-facing hint, not a gate.
+    """
+    if command in ("npx", "uvx"):
+        spec = _first_catalog_package_spec(args)
+        if spec is None:
+            return [f"{tag}: config.args must include a package spec to pin (e.g. name@1.2.3)."]
+        if not _catalog_package_spec_pinned(spec):
+            return [
+                f"{tag}: config.args package {spec!r} is not version-pinned; use "
+                "name@version, @scope/name@version, or name==version."
+            ]
+        return []
+
+    if command == "docker":
+        image = _docker_image_ref(args)
+        if image is None:
+            return [f"{tag}: config.args docker command has no image reference to pin."]
+        _, sep, image_tag = image.rpartition(":")
+        if not sep or "/" in image_tag:
+            return [
+                f"{tag}: config.args docker image {image!r} has no explicit tag; "
+                "pin an exact version (not 'latest', not untagged)."
+            ]
+        if image_tag == "latest":
+            return [
+                f"{tag}: config.args docker image {image!r} uses the 'latest' tag, "
+                "which is not allowed; pin an exact version."
+            ]
+        return []
+
+    return []
+
+
+def _docker_image_ref(args: list[str]) -> str | None:
+    """
+    Locate the image reference in a `docker run ...` args list: skip the
+    "run" subcommand and any flags, including ones that consume the next
+    token as a value (-e, -v, --name, ...) so that value isn't mistaken for
+    the image. The first remaining positional token is the image.
+    """
+    i = 0
+    if i < len(args) and args[i] == "run":
+        i += 1
+    while i < len(args):
+        a = args[i]
+        if a.startswith("-"):
+            if a in _DOCKER_VALUE_FLAGS and "=" not in a:
+                i += 2
+            else:
+                i += 1
+            continue
+        return a
+    return None
+
+
 def _validate_catalog_config(tag: str, config) -> list[str]:
     """Validate the `config` block of a basic-tier catalog entry."""
     if not isinstance(config, dict):
@@ -2283,10 +2728,11 @@ def _validate_catalog_config(tag: str, config) -> list[str]:
             f"({', '.join(sorted(CATALOG_ALLOWED_COMMANDS))})."
         )
 
-    args = config.get("args")
+    raw_args = config.get("args")
-    if not isinstance(args, list) or not all(isinstance(a, str) for a in args):
+    args_ok = isinstance(raw_args, list) and all(isinstance(a, str) for a in raw_args)
+    args = raw_args if args_ok else []
+    if not args_ok:
         problems.append(f"{tag}: config.args must be a list of strings.")
-        args = []
 
     for a in args:
         if not a.isascii():
@@ -2305,11 +2751,55 @@ def _validate_catalog_config(tag: str, config) -> list[str]:
     if command == "docker":
         problems.extend(_docker_arg_violations(tag, args))
 
+    # Version pinning (finding #3) -- only meaningful once command/args are
+    # actually well-formed; a malformed args list already got its own
+    # problem above and has nothing left to pin-check.
+    if args_ok and command in ("npx", "uvx", "docker"):
+        problems.extend(_catalog_pin_violations(tag, command, args))
+
     env = config.get("env")
-    if env is not None and (
+    if env is not None:
-        not isinstance(env, dict) or any(not isinstance(v, str) for v in env.values())
+        env_ok = isinstance(env, dict) and all(
-    ):
+            isinstance(k, str) and isinstance(v, str) for k, v in env.items()
-        problems.append(f"{tag}: config.env must be an object of string values.")
+        )
+        if not env_ok:
+            problems.append(f"{tag}: config.env must be an object of string values.")
+        else:
+            # config.env (finding #2): unlike args, env was previously
+            # type-checked ONLY -- no allowlist, no deny-rule, no ASCII
+            # check, no secret check. That made it the single easiest way
+            # to smuggle a payload past every other guard in this
+            # function: an allowlisted `command: npx` plus
+            # NODE_OPTIONS=--require /tmp/x.js in env walks straight past
+            # the command allowlist AND the -e/--eval/-c deny-rule above,
+            # because neither of those ever looks at env.
+            for key, value in env.items():
+                if not key.isascii():
+                    problems.append(
+                        f"{tag}: config.env key {key!r} must be ASCII "
+                        "(non-ASCII code points rejected)."
+                    )
+                if key.upper() in CATALOG_DENIED_ENV_KEYS:
+                    problems.append(
+                        f"{tag}: config.env key {key!r} is on the catalog deny-list "
+                        "(interpreter/loader override) and is not allowed."
+                    )
+                if not value.isascii():
+                    problems.append(
+                        f"{tag}: config.env value for {key!r} must be ASCII "
+                        "(non-ASCII code points rejected)."
+                    )
+                if _is_secret_value(value):
+                    problems.append(
+                        f"{tag}: config.env[{key!r}] looks like a real secret value; "
+                        "catalog entries must never ship secret values."
+                    )
+                if value != "" and not _PLACEHOLDER_RE.fullmatch(value):
+                    problems.append(
+                        f"{tag}: config.env[{key!r}] must be an empty string or a "
+                        "single <PLACEHOLDER> token -- the catalog declares which env "
+                        "vars a server needs, it never supplies their values."
+                    )
 
     return problems
 
@@ -2329,6 +2819,12 @@ def _validate_catalog_entry(idx: int, entry, seen_ids: set[str]) -> list[str]:
         tag = f"servers[{idx}] ({entry_id!r})"
         if not entry_id.isascii():
             problems.append(f"{tag}: 'id' must be ASCII (non-ASCII code points rejected).")
+        elif not _CATALOG_ID_RE.match(entry_id):
+            problems.append(
+                f"{tag}: 'id' must match ^[a-z0-9][a-z0-9._-]{{0,63}}$ "
+                "(it becomes an mcpServers JSON key and is interpolated into "
+                "Qt.AutoText widgets)."
+            )
         if entry_id in seen_ids:
             problems.append(f"{tag}: duplicate id.")
         seen_ids.add(entry_id)
@@ -2442,15 +2938,32 @@ def verify_catalog_signature(raw: bytes, sig: bytes, pubkeys: list[bytes]) -> bo
     return False
 
 
+def _verify_catalog_candidate(candidate: tuple[bytes, bytes] | None) -> tuple[dict | None, int]:
+    """Verify+load+validate one (raw, sig) candidate. Returns (None, -1) on any failure."""
+    if not candidate:
+        return None, -1
+    raw, sig = candidate
+    if not verify_catalog_signature(raw, sig, CATALOG_PUBKEYS):
+        return None, -1
+    try:
+        data = load_catalog(raw)
+    except (ValueError, TypeError):
+        return None, -1
+    if validate_catalog(data):
+        return None, -1
+    return data, catalog_version(data)
+
+
 def resolve_catalog(
     bundled: tuple[bytes, bytes] | None,
     cached: tuple[bytes, bytes] | None,
     remote: tuple[bytes, bytes] | None,
+    floor: int = 0,
 ) -> dict:
     """
-    Pick the highest-version catalog among bundled/cached/remote. Each
+    Pick the highest-version catalog among bundled/cached/remote. Each of
-    argument is either None (unavailable) or an (raw_bytes, signature_bytes)
+    bundled/cached/remote is either None (unavailable) or an (raw_bytes,
-    pair.
+    signature_bytes) pair.
 
     🔴 SECURITY: every candidate — including `bundled`, the copy frozen into
     this binary — is verified against CATALOG_PUBKEYS and re-validated from
@@ -2461,46 +2974,68 @@ def resolve_catalog(
     by virtue of being local. Signing (and checking the signature at
     runtime, every time) closes that.
 
-    Anti-rollback: a candidate's version is never accepted if it's lower
+    `floor` is a pure, caller-supplied lower bound (e.g. a persisted
-    than the best verified candidate already found in this same resolution
+    "last accepted version" the GUI can load from disk and pass in) — this
-    pass — an attacker replaying an old, since-superseded signed catalog
+    function does no storage of its own.
-    can't downgrade you.
 
-    Anti-freeze: a candidate whose version leaps more than
+    Anti-rollback / anti-freeze, and WHY they apply to every candidate
-    _CATALOG_MAX_VERSION_JUMP past the current best is also rejected. A
+    including the first one evaluated: the previous version of this
-    compromised/leaked signing key claiming an absurd future version would
+    function only ran these checks `if best_version >= 0`, i.e. once a
-    otherwise permanently outrank every legitimate release from then on,
+    candidate had already been accepted in this pass. That let the FIRST
-    since the resolver always prefers the highest verified version — this
+    verified candidate through unconditionally — a signed catalog claiming
-    caps how far a single accepted jump can go.
+    version=999999999 sailed straight past both guards if it happened to be
+    evaluated first, and rollback protection reset on every call anyway
+    (nothing persisted across restarts). Now both guards are anchored to
+    something that doesn't depend on iteration order:
+
+    - The anti-freeze cap is measured against the BUNDLED catalog's version
+      (verified independently, once), not against "whatever was accepted
+      so far in this loop." Bundled ships inside the binary, so it's the
+      one candidate that isn't attacker-supplied at resolve time — the
+      natural trust anchor. If bundled itself doesn't verify, `floor` is
+      the anchor instead.
+    - The anti-rollback floor is max(floor, bundled's version), so a
+      caller that persists `floor` across restarts gets real rollback
+      protection; a caller that doesn't still gets "never below bundled."
+
+    On a version TIE, the bundled candidate wins over cached/remote (it
+    previously lost ties to whichever candidate happened to be evaluated
+    last, silently preferring remote over bundled at equal version).
 
     Returns the winning catalog dict, or {} if nothing verified and
     validated.
     """
+    bundled_data, bundled_version = _verify_catalog_candidate(bundled)
+
+    anchor = bundled_version if bundled_version >= 0 else floor
+    min_accepted = max(floor, bundled_version if bundled_version >= 0 else 0)
+
+    candidates = (
+        ("bundled", bundled_data, bundled_version),
+        ("cached", *_verify_catalog_candidate(cached)),
+        ("remote", *_verify_catalog_candidate(remote)),
+    )
+
     best: dict = {}
     best_version = -1
+    best_is_bundled = False
 
-    for candidate in (bundled, cached, remote):
+    for source, data, version in candidates:
-        if not candidate:
+        if data is None:
-            continue
-        raw, sig = candidate
-        if not verify_catalog_signature(raw, sig, CATALOG_PUBKEYS):
-            continue
-        try:
-            data = load_catalog(raw)
-        except (ValueError, TypeError):
-            continue
-        if validate_catalog(data):
             continue
+        if version < min_accepted:
+            continue  # anti-rollback / below the persisted floor
+        if version > anchor + _CATALOG_MAX_VERSION_JUMP:
+            continue  # anti-freeze, capped against the bundled trust anchor
 
-        version = catalog_version(data)
+        is_bundled = source == "bundled"
-        if best_version >= 0:
+        better = version > best_version or (
-            if version < best_version:
+            version == best_version and is_bundled and not best_is_bundled
-                continue  # anti-rollback
+        )
-            if version > best_version + _CATALOG_MAX_VERSION_JUMP:
+        if better:
-                continue  # anti-freeze
+            best = data
-
+            best_version = version
-        best = data
+            best_is_bundled = is_bundled
-        best_version = version
 
     return best
 

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
@@ -1690,8 +1691,12 @@ def _minimal_catalog(version: int = 1) -> dict:
                 "official": True,
                 "setup": "basic",
                 "config": {
+                    # Pinned on purpose (issue #68 finding 3): an earlier
+                    # version of this fixture used an unpinned package and
+                    # asserted it validated clean, which enshrined the bug
+                    # instead of catching it.
                     "command": "npx",
-                    "args": ["-y", "widget-mcp"],
+                    "args": ["-y", "widget-mcp@1.0.0"],
                 },
                 "placeholders": {},
                 "env_required": {},
@@ -1952,6 +1957,213 @@ def test_validate_catalog_rejects_duplicate_ids():
     assert any("duplicate id" in p for p in problems)
 
 
+# --- validate_catalog: config.env (issue #68 finding 2) -------------------- #
+def test_validate_catalog_rejects_each_denied_env_key():
+    for key in sorted(c.CATALOG_DENIED_ENV_KEYS):
+        data = _catalog_with(
+            {"config": {"command": "npx", "args": ["-y", "widget-mcp@1.0.0"], "env": {key: ""}}}
+        )
+        problems = c.validate_catalog(data)
+        assert any("deny-list" in p for p in problems), (key, problems)
+
+
+def test_validate_catalog_rejects_denied_env_key_case_insensitively():
+    data = _catalog_with(
+        {
+            "config": {
+                "command": "npx",
+                "args": ["-y", "widget-mcp@1.0.0"],
+                "env": {"node_options": ""},
+            }
+        }
+    )
+    problems = c.validate_catalog(data)
+    assert any("deny-list" in p for p in problems)
+
+
+def test_validate_catalog_rejects_nonempty_nonplaceholder_env_value():
+    data = _catalog_with(
+        {
+            "config": {
+                "command": "npx",
+                "args": ["-y", "widget-mcp@1.0.0"],
+                "env": {"FOO_URL": "https://example.com"},
+            }
+        }
+    )
+    problems = c.validate_catalog(data)
+    assert any("empty string or a single <PLACEHOLDER>" in p for p in problems)
+
+
+def test_validate_catalog_accepts_empty_and_placeholder_env_values():
+    data = _catalog_with(
+        {
+            "config": {
+                "command": "npx",
+                "args": ["-y", "widget-mcp@1.0.0"],
+                "env": {"FOO": "", "BAR_URL": "<BAR_URL>"},
+            }
+        }
+    )
+    assert c.validate_catalog(data) == []
+
+
+def test_validate_catalog_rejects_non_ascii_env_key():
+    data = _catalog_with(
+        {
+            "config": {
+                "command": "npx",
+                "args": ["-y", "widget-mcp@1.0.0"],
+                "env": {"FÖO": ""},
+            }
+        }
+    )
+    problems = c.validate_catalog(data)
+    assert any("config.env key" in p and "ASCII" in p for p in problems)
+
+
+def test_validate_catalog_rejects_non_ascii_env_value():
+    data = _catalog_with(
+        {
+            "config": {
+                "command": "npx",
+                "args": ["-y", "widget-mcp@1.0.0"],
+                "env": {"FOO": "<Bäd>"},
+            }
+        }
+    )
+    problems = c.validate_catalog(data)
+    assert any("config.env value" in p and "ASCII" in p for p in problems)
+
+
+def test_validate_catalog_rejects_secret_looking_env_value():
+    data = _catalog_with(
+        {
+            "config": {
+                "command": "npx",
+                "args": ["-y", "widget-mcp@1.0.0"],
+                "env": {"SOME_TOKEN": "ghp_abcdef1234567890"},
+            }
+        }
+    )
+    problems = c.validate_catalog(data)
+    assert any("real secret value" in p for p in problems)
+
+
+def test_validate_catalog_rejects_node_options_env_walking_past_allowlist():
+    # The exact reproduction from issue #68 finding 2: an allowlisted
+    # `npx` command carrying NODE_OPTIONS in env, which previously passed
+    # validation and would have flowed straight into the executed
+    # subprocess via catalog_entry_to_paste_json().
+    data = _catalog_with(
+        {
+            "config": {
+                "command": "npx",
+                "args": ["-y", "widget-mcp@1.0.0"],
+                "env": {"NODE_OPTIONS": "--require /tmp/payload.js"},
+            }
+        }
+    )
+    problems = c.validate_catalog(data)
+    assert problems != []
+
+
+# --- validate_catalog: version pinning (issue #68 finding 3) --------------- #
+def test_validate_catalog_rejects_unpinned_npx_package():
+    data = _catalog_with({"config": {"command": "npx", "args": ["-y", "widget-mcp"]}})
+    problems = c.validate_catalog(data)
+    assert any("not version-pinned" in p for p in problems)
+
+
+def test_validate_catalog_rejects_unpinned_scoped_npx_package():
+    data = _catalog_with({"config": {"command": "npx", "args": ["-y", "@scope/pkg"]}})
+    problems = c.validate_catalog(data)
+    assert any("not version-pinned" in p for p in problems)
+
+
+def test_validate_catalog_accepts_pinned_scoped_npx_package():
+    data = _catalog_with({"config": {"command": "npx", "args": ["-y", "@scope/pkg@1.2.3"]}})
+    assert c.validate_catalog(data) == []
+
+
+def test_validate_catalog_rejects_unpinned_uvx_package():
+    data = _catalog_with({"config": {"command": "uvx", "args": ["some-tool"]}})
+    problems = c.validate_catalog(data)
+    assert any("not version-pinned" in p for p in problems)
+
+
+def test_validate_catalog_accepts_uvx_at_version_pin():
+    data = _catalog_with({"config": {"command": "uvx", "args": ["some-tool@1.0.0"]}})
+    assert c.validate_catalog(data) == []
+
+
+def test_validate_catalog_accepts_uvx_double_equals_pin():
+    data = _catalog_with({"config": {"command": "uvx", "args": ["some-tool==1.0.0"]}})
+    assert c.validate_catalog(data) == []
+
+
+def test_validate_catalog_rejects_docker_latest_tag():
+    data = _catalog_with({"config": {"command": "docker", "args": ["run", "some/image:latest"]}})
+    problems = c.validate_catalog(data)
+    assert any("'latest'" in p for p in problems)
+
+
+def test_validate_catalog_rejects_docker_untagged_image():
+    data = _catalog_with({"config": {"command": "docker", "args": ["run", "some/image"]}})
+    problems = c.validate_catalog(data)
+    assert any("no explicit tag" in p for p in problems)
+
+
+def test_validate_catalog_accepts_pinned_docker_image_with_flags():
+    data = _catalog_with(
+        {
+            "config": {
+                "command": "docker",
+                "args": ["run", "-i", "--rm", "-e", "SOME_TOKEN", "some/image:1.2.3"],
+            }
+        }
+    )
+    assert c.validate_catalog(data) == []
+
+
+def test_validate_catalog_does_not_pin_check_placeholders_flags_or_subcommand():
+    # A pinned uvx spec followed by flags and a <PLACEHOLDER> positional
+    # must not itself get mistaken for an unpinned package.
+    data = _catalog_with(
+        {
+            "config": {
+                "command": "uvx",
+                "args": ["mcp-server-git@2026.7.10", "--repository", "<REPO_PATH>"],
+            }
+        }
+    )
+    assert c.validate_catalog(data) == []
+
+
+# --- validate_catalog: id constraint (issue #68 finding 7) ----------------- #
+def test_validate_catalog_rejects_id_with_markup():
+    data = _catalog_with({"id": "<b>Verified</b>"})
+    problems = c.validate_catalog(data)
+    assert any("must match" in p for p in problems)
+
+
+def test_validate_catalog_rejects_id_with_uppercase():
+    data = _catalog_with({"id": "Widget"})
+    problems = c.validate_catalog(data)
+    assert any("must match" in p for p in problems)
+
+
+def test_validate_catalog_rejects_id_starting_with_dash():
+    data = _catalog_with({"id": "-widget"})
+    problems = c.validate_catalog(data)
+    assert any("must match" in p for p in problems)
+
+
+def test_validate_catalog_accepts_valid_slug_id():
+    data = _catalog_with({"id": "widget-2.thing-ok"})
+    assert c.validate_catalog(data) == []
+
+
 # --- resolve_catalog -------------------------------------------------------- #
 def test_resolve_catalog_nothing_available_returns_empty_dict():
     assert c.resolve_catalog(None, None, None) == {}
@@ -2003,13 +2215,93 @@ def test_resolve_catalog_rejects_absurd_version_jump(monkeypatch):
     pub = priv.public_key().public_bytes_raw()
     monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
 
-    cached = _signed(_minimal_catalog(version=5), priv)
+    # The freeze attempt goes FIRST (as `cached`), the legitimate catalog
+    # SECOND (as `remote`) -- on purpose. Putting the good catalog first
+    # (as an earlier version of this test did) never exercises the
+    # vulnerable path: the old implementation only guarded a candidate
+    # against "the best accepted so far," so whichever candidate was
+    # evaluated FIRST got in unconditionally, uncapped. Ordering the freeze
+    # attempt first is what actually proves the cap holds regardless of
+    # evaluation order.
     freeze_attempt = _signed(_minimal_catalog(version=999999), priv)
+    good = _signed(_minimal_catalog(version=5), priv)
 
-    result = c.resolve_catalog(None, cached, freeze_attempt)
+    result = c.resolve_catalog(None, freeze_attempt, good)
     assert c.catalog_version(result) == 5
 
 
+def test_resolve_catalog_caps_first_and_only_candidate(monkeypatch):
+    # issue #68 finding 6: with no bundled catalog to anchor against, a
+    # signed catalog claiming an absurd version must still be capped even
+    # when it is the ONLY candidate resolve_catalog() ever sees -- there is
+    # no "best so far" for it to be compared against, so the cap has to
+    # apply unconditionally, not "once something else has already landed."
+    priv = Ed25519PrivateKey.generate()
+    pub = priv.public_key().public_bytes_raw()
+    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
+
+    freeze_attempt = _signed(_minimal_catalog(version=999999999), priv)
+
+    result = c.resolve_catalog(None, None, freeze_attempt)
+    assert result == {}
+
+
+def test_resolve_catalog_anchors_cap_to_bundled_not_a_chained_best(monkeypatch):
+    # Anti-freeze must be measured against the BUNDLED version specifically,
+    # not against "whatever the best-so-far happens to be after each
+    # candidate is accepted" -- a chained anchor lets each accepted
+    # candidate ratchet the allowed ceiling upward, so a legitimate
+    # moderate bump (cached) plus a second, much larger jump (remote) can
+    # each individually look "within _CATALOG_MAX_VERSION_JUMP of the
+    # previous one" while remote is nowhere near bundled's version.
+    priv = Ed25519PrivateKey.generate()
+    pub = priv.public_key().public_bytes_raw()
+    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
+
+    bundled = _signed(_minimal_catalog(version=2), priv)
+    cached = _signed(_minimal_catalog(version=1000), priv)  # within 1000 of bundled
+    remote = _signed(_minimal_catalog(version=1900), priv)  # within 1000 of cached,
+    #                                                          NOT of bundled
+
+    result = c.resolve_catalog(bundled, cached, remote)
+    assert c.catalog_version(result) == 1000
+
+
+def test_resolve_catalog_prefers_bundled_on_version_tie(monkeypatch):
+    # issue #68 finding 6: on a tie the LAST candidate evaluated used to
+    # win, so remote silently beat bundled at equal version. Bundled --
+    # the copy frozen into the binary -- must win ties.
+    priv = Ed25519PrivateKey.generate()
+    pub = priv.public_key().public_bytes_raw()
+    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
+
+    bundled_data = _minimal_catalog(version=5)
+    bundled = _signed(bundled_data, priv)
+
+    remote_data = _minimal_catalog(version=5)
+    remote_data["servers"][0]["display"] = "Remote Impostor"
+    remote = _signed(remote_data, priv)
+
+    result = c.resolve_catalog(bundled, None, remote)
+    assert c.catalog_version(result) == 5
+    assert result["servers"][0]["display"] == "Widget"
+
+
+def test_resolve_catalog_floor_rejects_below_persisted_version(monkeypatch):
+    # `floor` is a pure parameter: the caller (eventually the GUI, from
+    # persisted storage) can pass a previously-accepted version, and
+    # nothing below it may be accepted even with no bundled catalog to
+    # anchor against.
+    priv = Ed25519PrivateKey.generate()
+    pub = priv.public_key().public_bytes_raw()
+    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
+
+    stale = _signed(_minimal_catalog(version=3), priv)
+
+    result = c.resolve_catalog(None, None, stale, floor=10)
+    assert result == {}
+
+
 def test_resolve_catalog_malformed_candidate_does_not_raise(monkeypatch):
     priv = Ed25519PrivateKey.generate()
     pub = priv.public_key().public_bytes_raw()
@@ -2039,15 +2331,32 @@ def test_resolve_catalog_invalid_but_signed_candidate_is_skipped(monkeypatch):
 def test_catalog_entry_to_paste_json_basic_shape():
     entry = _minimal_catalog()["servers"][0]
     result = c.catalog_entry_to_paste_json(entry)
-    assert result == {"widget": {"command": "npx", "args": ["-y", "widget-mcp"]}}
+    assert result == {"widget": {"command": "npx", "args": ["-y", "widget-mcp@1.0.0"]}}
 
 
 def test_catalog_entry_to_paste_json_includes_env_when_present():
+    # NOTE: catalog_entry_to_paste_json() is a pure shape-converter for an
+    # entry that has ALREADY passed validate_catalog() -- it is correct for
+    # it to carry env through verbatim. The bug (issue #68 finding 2) was
+    # never in this function; it was that validate_catalog() let entries
+    # with dangerous/non-placeholder env values reach this function in the
+    # first place. This test now proves that boundary explicitly: a
+    # validation-legal env value (a <PLACEHOLDER> token) survives the
+    # conversion, and a value validate_catalog() would have rejected is
+    # confirmed rejected before it ever gets here.
     entry = _minimal_catalog()["servers"][0]
     entry["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"}
     result = c.catalog_entry_to_paste_json(entry)
     assert result["widget"]["env"] == {"GRAFANA_URL": "<GRAFANA_URL>"}
 
+    catalog = _minimal_catalog()
+    catalog["servers"][0]["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"}
+    assert c.validate_catalog(catalog) == []
+
+    malicious = _minimal_catalog()
+    malicious["servers"][0]["config"]["env"] = {"NODE_OPTIONS": "--require /tmp/payload.js"}
+    assert c.validate_catalog(malicious) != []
+
 
 def test_config_has_unfilled_placeholders_true_for_token():
     cfg = {"command": "npx", "args": ["-y", "server", "<ALLOWED_DIR>"]}
@@ -2062,3 +2371,312 @@ def test_config_has_unfilled_placeholders_false_after_fill():
 def test_config_has_unfilled_placeholders_checks_env_too():
     cfg = {"command": "uvx", "args": ["mcp-grafana"], "env": {"GRAFANA_URL": "<GRAFANA_URL>"}}
     assert c.config_has_unfilled_placeholders(cfg) is True
+
+
+# --------------------------------------------------------------------------- #
+# #72 -- a server value that isn't a JSON object must not take the load down
+# --------------------------------------------------------------------------- #
+@pytest.mark.parametrize("bad", ["not-a-dict", 123, ["a", "b"], None, True, 1.5])
+def test_extract_servers_survives_non_dict_server_value(bad):
+    entries = c.extract_servers({"mcpServers": {"foo": bad}})
+    assert len(entries) == 1
+    assert entries[0].name == "foo"
+    assert entries[0].data == {}
+    assert entries[0].malformed is True
+    assert entries[0].raw == bad
+
+
+def test_extract_servers_marks_only_the_bad_entry():
+    cfg = {"mcpServers": {"good": {"command": "npx"}, "bad": "oops"}}
+    by_name = {e.name: e for e in c.extract_servers(cfg)}
+    assert by_name["good"].malformed is False
+    assert by_name["good"].data == {"command": "npx"}
+    assert by_name["bad"].malformed is True
+
+
+def test_extract_servers_handles_malformed_disabled_entry():
+    entries = c.extract_servers({c.DISABLED_KEY: {"parked": ["nope"]}})
+    assert entries[0].enabled is False
+    assert entries[0].malformed is True
+
+
+def test_malformed_entry_round_trips_through_save_unchanged():
+    """The cardinal rule: never silently delete what the user had on disk."""
+    cfg = {"mcpServers": {"good": {"command": "npx"}, "bad": "oops"}}
+    servers = c.extract_servers(cfg)
+    out = c.apply_servers(dict(cfg), servers)
+    assert out["mcpServers"]["bad"] == "oops"
+    assert out["mcpServers"]["good"] == {"command": "npx"}
+
+
+def test_editing_a_malformed_entry_retires_the_raw_value():
+    entry = c.extract_servers({"mcpServers": {"bad": "oops"}})[0]
+    entry.set_data({"command": "npx"})
+    assert entry.malformed is False
+    assert entry.config_value() == {"command": "npx"}
+    assert c.apply_servers({}, [entry])["mcpServers"]["bad"] == {"command": "npx"}
+
+
+def test_lint_reports_the_malformed_entry_by_name():
+    servers = c.extract_servers({"mcpServers": {"bad": "oops"}})
+    warnings = c.lint_servers(servers)
+    assert len(warnings) == 1
+    assert "'bad'" in warnings[0]
+    assert "not an object" in warnings[0]
+    assert "str" in warnings[0]
+
+
+def test_lint_still_reports_normal_warnings_alongside_malformed():
+    cfg = {"mcpServers": {"bad": "oops", "sloppy": {"command": "npx", "args": "one two"}}}
+    warnings = c.lint_servers(c.extract_servers(cfg))
+    assert any("not an object" in w for w in warnings)
+    assert any("'args' should be a list" in w for w in warnings)
+
+
+# --------------------------------------------------------------------------- #
+# #73 -- the stale-file merge must not discard BCC-authored keys
+# --------------------------------------------------------------------------- #
+def test_carry_owned_keys_moves_sets_onto_the_reloaded_config():
+    local = {"mcpServers": {}, c.SETS_KEY: {"work": ["a", "b"]}}
+    fresh = {"mcpServers": {"external": {"command": "npx"}}}
+    contested = c.carry_owned_keys(local, fresh)
+    assert contested == []
+    assert fresh[c.SETS_KEY] == {"work": ["a", "b"]}
+    assert fresh["mcpServers"] == {"external": {"command": "npx"}}
+
+
+def test_carry_owned_keys_reports_a_genuine_conflict():
+    local = {c.SETS_KEY: {"work": ["a"]}}
+    fresh = {c.SETS_KEY: {"work": ["a", "b"]}}
+    assert c.carry_owned_keys(local, fresh) == [c.SETS_KEY]
+    assert fresh[c.SETS_KEY] == {"work": ["a"]}  # local wins: BCC owns the key
+
+
+def test_carry_owned_keys_is_quiet_when_both_sides_agree():
+    local = {c.SETS_KEY: {"work": ["a"]}}
+    fresh = {c.SETS_KEY: {"work": ["a"]}}
+    assert c.carry_owned_keys(local, fresh) == []
+
+
+def test_carry_owned_keys_leaves_disk_alone_when_absent_locally():
+    """Can't distinguish 'deleted my last set' from 'never had sets'; keep theirs."""
+    fresh = {c.SETS_KEY: {"remote": ["a"]}}
+    assert c.carry_owned_keys({}, fresh) == []
+    assert fresh[c.SETS_KEY] == {"remote": ["a"]}
+
+
+def test_carry_owned_keys_deep_copies_so_later_edits_do_not_leak():
+    local = {c.SETS_KEY: {"work": ["a"]}}
+    fresh = {}
+    c.carry_owned_keys(local, fresh)
+    local[c.SETS_KEY]["work"].append("b")
+    assert fresh[c.SETS_KEY] == {"work": ["a"]}
+
+
+def test_merge_flow_preserves_sets_and_external_servers(tmp_path):
+    """End-to-end shape of the Merge & save path that lost sets in #73."""
+    path = tmp_path / "claude.json"
+    path.write_text(json.dumps({"mcpServers": {"old": {"command": "old"}}}))
+
+    # BCC loads, user saves a named set and edits servers in memory.
+    local = c.load_config(path)
+    servers = c.extract_servers(local)
+    c.save_server_set(local, "work", servers)
+
+    # Something else rewrites the file underneath us.
+    path.write_text(json.dumps({"mcpServers": {"external": {"command": "new"}}, "other": 1}))
+
+    # Merge & save: reload disk, carry BCC keys, re-apply the user's servers.
+    fresh = c.load_config(path)
+    c.carry_owned_keys(local, fresh)
+    c.apply_servers(fresh, servers)
+    c.write_config(path, fresh)
+
+    saved = c.load_config(path)
+    assert saved[c.SETS_KEY] == {"work": ["old"]}  # the set survived
+    assert saved["other"] == 1  # unrelated external key preserved
+    assert "old" in saved["mcpServers"]  # user's servers re-applied
+
+
+def test_null_server_value_is_malformed_not_mistaken_for_absent():
+    """`{"mcpServers": {"foo": null}}` is legal JSON and a real malformed case,
+    so None must not double as the 'nothing here' sentinel."""
+    entry = c.extract_servers({"mcpServers": {"foo": None}})[0]
+    assert entry.malformed is True
+    assert entry.raw is None
+    assert c.apply_servers({}, [entry])["mcpServers"]["foo"] is None
+
+
+def test_a_normal_entry_is_not_malformed():
+    entry = c.extract_servers({"mcpServers": {"foo": {"command": "npx"}}})[0]
+    assert entry.malformed is False
+    assert entry.raw is c.NO_RAW
+
+
+# --------------------------------------------------------------------------- #
+# #75 -- theming
+# --------------------------------------------------------------------------- #
+@pytest.mark.parametrize(
+    "setting,system_dark,expected",
+    [
+        (c.THEME_DARK, False, "dark"),
+        (c.THEME_DARK, True, "dark"),
+        (c.THEME_LIGHT, False, "light"),
+        (c.THEME_LIGHT, True, "light"),
+        (c.THEME_SYSTEM, True, "dark"),
+        (c.THEME_SYSTEM, False, "light"),
+    ],
+)
+def test_resolve_theme_covers_every_setting_and_appearance(setting, system_dark, expected):
+    assert c.resolve_theme(setting, system_dark) == expected
+
+
+@pytest.mark.parametrize("junk", ["", "solarized", None, "DARK", 3])
+def test_resolve_theme_falls_back_to_following_the_system(junk):
+    """A hand-edited or future QSettings value should follow the desktop,
+    not pin a fixed theme."""
+    assert c.resolve_theme(junk, True) == "dark"
+    assert c.resolve_theme(junk, False) == "light"
+
+
+def test_palette_for_known_names():
+    assert c.palette_for("dark") is c.DARK_PALETTE
+    assert c.palette_for("light") is c.LIGHT_PALETTE
+
+
+def test_palette_for_unknown_name_falls_back_to_dark():
+    assert c.palette_for("chartreuse") is c.DARK_PALETTE
+
+
+def test_dark_palette_is_unchanged_from_the_shipped_look():
+    """v1.3.0 shipped these exact colours; adding a light theme must not
+    quietly restyle the dark one."""
+    p = c.DARK_PALETTE
+    assert (p.accent, p.bg, p.panel, p.panel_2) == ("#f97316", "#1b1d23", "#23262e", "#2b2f39")
+    assert (p.text, p.muted, p.border) == ("#e7e9ee", "#9aa0ad", "#3a3f4b")
+    assert (p.good, p.bad, p.warn, p.remote) == ("#4ade80", "#f87171", "#fbbf24", "#60a5fa")
+    assert (p.on_accent, p.disabled_bg, p.mono_bg) == ("#1a1205", "#202229", "#16181d")
+
+
+def test_both_palettes_define_every_slot():
+    """A missing slot should fail here rather than render a broken window."""
+    for pal in (c.DARK_PALETTE, c.LIGHT_PALETTE):
+        for f in dataclasses.fields(c.Palette):
+            value = getattr(pal, f.name)
+            assert value, f"{pal.name}.{f.name} is empty"
+            if f.name != "name":
+                assert re.fullmatch(r"#[0-9a-fA-F]{6}", value), f"{pal.name}.{f.name}={value!r}"
+
+
+@pytest.mark.parametrize("pal_name", ["dark", "light"])
+@pytest.mark.parametrize("slot", ["text", "muted", "good", "bad", "warn", "remote", "accent"])
+def test_palette_meets_contrast_on_panel(pal_name, slot):
+    """Every colour drawn as text/glyph must clear WCAG AA (4.5:1) against the
+    surface it sits on. The light palette's semantic colours are NOT the dark
+    ones lightened -- #4ade80 sits near 1.7:1 on white -- so this guards
+    against someone 'harmonising' them back toward the dark hues."""
+    pal = c.palette_for(pal_name)
+    assert c.contrast_ratio(getattr(pal, slot), pal.panel) >= 4.5
+
+
+@pytest.mark.parametrize("pal_name", ["dark", "light"])
+def test_on_accent_is_legible_against_the_accent_fill(pal_name):
+    """Primary buttons and selected rows draw on_accent on top of accent."""
+    pal = c.palette_for(pal_name)
+    assert c.contrast_ratio(pal.on_accent, pal.accent) >= 4.5
+
+
+def test_contrast_ratio_endpoints():
+    assert c.contrast_ratio("#000000", "#ffffff") == pytest.approx(21.0, abs=0.01)
+    assert c.contrast_ratio("#123456", "#123456") == pytest.approx(1.0, abs=0.001)
+    assert c.contrast_ratio("#ffffff", "#000000") == pytest.approx(21.0, abs=0.01)
+
+
+def test_relative_luminance_extremes():
+    assert c.relative_luminance("#000000") == pytest.approx(0.0)
+    assert c.relative_luminance("#ffffff") == pytest.approx(1.0)
+
+
+def test_stylesheet_builder_has_no_hardcoded_colours():
+    """Every colour in the QSS must come from the palette.
+
+    Three near-black literals used to be inlined here (#1a1205, #202229,
+    #16181d). Harmless with one theme; with two, they silently render dark
+    chrome on a light window. Reads the source rather than importing bcc,
+    which needs PySide6.
+    """
+    src = (Path(__file__).resolve().parent.parent / "bcc.py").read_text(encoding="utf-8")
+    start = src.index("def build_stylesheet")
+    body = src[start : src.index("def apply_palette")]
+    assert re.findall(r"#[0-9a-fA-F]{6}", body) == []
+
+
+def test_every_palette_slot_is_consumed():
+    """A slot added to Palette but never wired up is dead weight.
+
+    Checks for `p.<slot>` anywhere in bcc.py, which covers both the QSS and
+    apply_palette's global bindings -- not every slot belongs in the
+    stylesheet (`good` and `remote` feed the inline status dots via
+    STATUS_COLORS/HEALTH_COLORS, never the QSS). This won't catch a slot bound
+    to a global that nothing then uses; it does catch the common mistake of
+    extending the dataclass and forgetting to plumb it through.
+    """
+    src = (Path(__file__).resolve().parent.parent / "bcc.py").read_text(encoding="utf-8")
+    for f in dataclasses.fields(c.Palette):
+        if f.name == "name":
+            continue
+        assert f"p.{f.name}" in src, f"palette slot {f.name!r} is never consumed"
+
+
+# --------------------------------------------------------------------------- #
+# #78/#79 -- update notice: when to show it, and what it says
+# --------------------------------------------------------------------------- #
+def test_update_notice_when_a_newer_release_exists():
+    n = c.update_notice("1.2.0", {"version": "v1.3.0", "url": "https://example.test/rel"})
+    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"]
+
+
+def test_update_notice_is_silent_when_current():
+    assert c.update_notice("1.3.0", {"version": "v1.3.0"}) is None
+    assert c.update_notice("1.4.0", {"version": "v1.3.0"}) is None
+
+
+@pytest.mark.parametrize("bad", [None, {}, {"version": ""}, {"version": None}, {"version": 3}, []])
+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_update_notice_falls_back_to_the_releases_page_without_a_url():
+    n = c.update_notice("1.0.0", {"version": "v2.0.0"})
+    assert n["url"] == c.RELEASES_URL
+
+
+def test_update_notice_names_no_menu_path():
+    """The old status-line text said 'Help > About to view it', which is wrong
+    on macOS -- Qt moves the About action into the application menu (#79). The
+    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_update_notice_handles_the_v_prefix_consistently():
+    assert c.update_notice("1.2.0", {"version": "1.3.0"}) is not None
+    assert c.update_notice("v1.2.0", {"version": "v1.3.0"}) is not None
+    assert c.update_notice("1.3.0", {"version": "v1.3.0"}) is None
+
+
+def test_update_notice_renders_both_versions_the_same_way():
+    """Tags carry a 'v' prefix, __version__ doesn't -- don't show both forms
+    in one sentence."""
+    n = c.update_notice("1.2.0", {"version": "v1.3.0"})
+    assert "v1.3.0" not in n["text"]
+    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"