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/release.yml

@@ -101,9 +101,20 @@ jobs:
   # signing step — which means a wrong/missing RELEASE_SIGNING_KEY secret
   # would only be discovered at the worst possible moment: during a real
   # release. This job signs a throwaway manifest with the secret and verifies
-  # the result against the PUBLIC key already compiled into bcc_core.
+  # the result against scripts/sign_checksums.RELEASE_PUBKEYS.
   #
-  # It proves the two halves of the keypair actually match, without
+  # IMPORTANT (issue #68 finding 5): this must verify against the RELEASE
+  # public key, never bcc_core.CATALOG_PUBKEYS. The catalog key is the
+  # offline, maintainer-held root of trust for what BCC executes; it must
+  # NEVER be compared against a value that lives in a CI secret, because
+  # that comparison is itself a way to smuggle a catalog-trusted key through
+  # CI review ("does this repo secret match the catalog key" is a question
+  # this workflow must never even ask). The release key is a SEPARATE
+  # keypair, generated via `catalog_console.py keygen --release`, that only
+  # ever signs release SHA256SUMS manifests -- a CI/secret compromise burns
+  # this key, not the catalog key.
+  #
+  # It proves the two halves of the RELEASE keypair actually match, without
   # publishing anything. Run it from the Actions tab after setting or
   # rotating the secret.
   signing-smoke-test:
@@ -120,42 +131,54 @@ jobs:
       - name: Install dependencies
         run: pip install cryptography
 
-      - name: Sign a throwaway manifest and verify against the shipped pubkey
+      - name: Sign a throwaway manifest and verify against the RELEASE pubkey
         env:
           RELEASE_SIGNING_KEY: ${{ secrets.RELEASE_SIGNING_KEY }}
         run: |
           if [ -z "$RELEASE_SIGNING_KEY" ]; then
             echo "FAIL: RELEASE_SIGNING_KEY secret is not set."
-            echo "Generate it with: python catalog_console.py show-seed-b64"
-            echo "then add it under Settings -> Actions -> Secrets."
+            echo "Generate the RELEASE key (NOT the catalog key) with:"
+            echo "    python catalog_console.py keygen --release"
+            echo "then add its seed under Settings -> Actions -> Secrets, via:"
+            echo "    python catalog_console.py show-seed-b64 --release"
             exit 1
           fi
           mkdir -p smoke && echo "smoke test payload" > smoke/hello.txt
           python3 scripts/sign_checksums.py generate smoke --out smoke/SHA256SUMS
           python3 scripts/sign_checksums.py sign --sums smoke/SHA256SUMS --out smoke/SHA256SUMS.sig
           python - <<'PY'
-          import base64, pathlib, sys
-          import bcc_core as c
-          from scripts.sign_checksums import verify_checksums
+          import pathlib, sys
+          from scripts.sign_checksums import RELEASE_PUBKEYS, verify_checksums_against_any
 
-          # The public half that ships inside the binary. If the secret is a
-          # DIFFERENT key than the one users' copies trust, this fails here --
-          # which is the entire point of the job.
-          pub_b64 = base64.b64encode(c.CATALOG_PUBKEYS[0]).decode()
+          # Deliberately does NOT import bcc_core / CATALOG_PUBKEYS at all --
+          # this smoke test must never be able to compare the CI secret
+          # against the catalog's root of trust (issue #68 finding 5). Only
+          # RELEASE_PUBKEYS (scripts/sign_checksums.py) is a legitimate
+          # target for a CI-resident key.
+          if not RELEASE_PUBKEYS:
+              sys.exit(
+                  "FAIL: scripts/sign_checksums.RELEASE_PUBKEYS is empty.\n"
+                  "\n"
+                  "Generate the release keypair with:\n"
+                  "    python catalog_console.py keygen --release\n"
+                  "then paste the printed public key into RELEASE_PUBKEYS in\n"
+                  "scripts/sign_checksums.py and commit that change."
+              )
 
           sums = pathlib.Path("smoke/SHA256SUMS").read_text()
           sig = pathlib.Path("smoke/SHA256SUMS.sig").read_bytes()
 
-          if not verify_checksums(pub_b64, sums, sig):
+          if not verify_checksums_against_any(RELEASE_PUBKEYS, sums, sig):
               sys.exit(
                   "FAIL: the signature produced by RELEASE_SIGNING_KEY does NOT verify\n"
-                  "against the public key in bcc_core.CATALOG_PUBKEYS.\n"
+                  "against any key in scripts/sign_checksums.RELEASE_PUBKEYS.\n"
                   "\n"
-                  "The secret and the shipped public key are different keypairs. Users\n"
-                  "would reject every signature this CI produces. Re-copy the seed from\n"
-                  "`catalog_console.py show-seed-b64`, or update CATALOG_PUBKEYS."
+                  "The secret and the shipped release public key are different keypairs.\n"
+                  "Downloaders would reject every signature this CI produces. Re-copy the\n"
+                  "seed from `catalog_console.py show-seed-b64 --release`, or update\n"
+                  "RELEASE_PUBKEYS with the matching public key."
               )
-          print("OK: RELEASE_SIGNING_KEY matches the public key shipped in bcc_core.")
+          print("OK: RELEASE_SIGNING_KEY matches a key in RELEASE_PUBKEYS.")
           PY
 
   # ── Create GitHub Release with all three artifacts ──────────────────────
@@ -206,9 +229,13 @@ jobs:
       # checks. It does NOT remove Gatekeeper/SmartScreen warnings.
       #
       # The private key is a repo secret (RELEASE_SIGNING_KEY, base64 raw
-      # Ed25519 seed) generated via the Catalog Console (#62). If it's not
-      # set, we still publish the release — just without a .sig — rather
-      # than fail the release outright.
+      # Ed25519 seed) for the RELEASE key -- a SEPARATE keypair from the
+      # catalog key, generated via `python catalog_console.py keygen
+      # --release` (issue #68 finding 5; #62). This key is intentionally
+      # CI-resident and signs ONLY this checksum manifest; it is never
+      # trusted to sign data/catalog.json. If it's not set, we still
+      # publish the release — just without a .sig — rather than fail the
+      # release outright.
       - name: Check for signing key
         id: signing
         run: |
@@ -234,7 +261,7 @@ jobs:
       - name: Warn — release will be unsigned
         if: steps.signing.outputs.has_key != 'true'
         run: |
-          echo "::warning::RELEASE_SIGNING_KEY secret is not set — this release is being published WITHOUT a signed SHA256SUMS.sig. Add the secret (base64 raw Ed25519 seed, generated via the Catalog Console, #62) before the next tag."
+          echo "::warning::RELEASE_SIGNING_KEY secret is not set — this release is being published WITHOUT a signed SHA256SUMS.sig. Generate the RELEASE key (python catalog_console.py keygen --release) and add its seed (python catalog_console.py show-seed-b64 --release) as this secret before the next tag."
 
       - name: Create GitHub Release
         uses: softprops/action-gh-release@v2

README.md

@@ -36,11 +36,10 @@ are only suppressed by a paid OS-vendor certificate, which this project
 doesn't have. Verifying checksums is about detecting tampering in transit or
 on a mirror, not about vouching for the software.
 
-**Release signing public key** (Ed25519, base64, raw 32 bytes):
-
-```
-<PLACEHOLDER — AJ: paste the public key from the Catalog Console (#62) here>
-```
+This manifest is signed with BCC's **release key**, which is a different
+key from the one that signs the MCP server catalog — see
+[Signing keys](#signing-keys) below for why, and for the public key value
+to use with `--pubkey-b64` below.
 
 ### macOS / Linux
 
@@ -59,7 +58,7 @@ To also verify the manifest's signature (optional, requires Python +
 ```bash
 python3 scripts/sign_checksums.py verify \
   --sums SHA256SUMS --sig SHA256SUMS.sig \
-  --pubkey-b64 "<the public key above>"
+  --pubkey-b64 "<the release public key from Signing keys, below>"
 ```
 
 ### Windows (PowerShell)
@@ -78,6 +77,45 @@ release is missing the `.sig` file, the checksums themselves are still
 valid and safe to check against — the release workflow only skips signing,
 never checksum generation.
 
+## Signing keys
+
+BCC uses **two separate Ed25519 keypairs**, deliberately never the same
+key, because they protect different things and live in different places:
+
+| | Catalog key | Release key |
+|---|---|---|
+| Signs | `data/catalog.json` (the MCP server catalog every user's app trusts) | `SHA256SUMS` (the checksum manifest for release binaries) |
+| Verified by | `bcc_core.CATALOG_PUBKEYS` | `scripts/sign_checksums.RELEASE_PUBKEYS` |
+| Lives | Offline, passphrase-encrypted, maintainer's machine only (OS keychain or an encrypted file outside the repo — see the [Catalog Console](#files), issue #62) | A Gitea Actions repo secret, `RELEASE_SIGNING_KEY` — **intentionally CI-resident** |
+| Generated with | `python catalog_console.py keygen` | `python catalog_console.py keygen --release` |
+| Exported for CI with | *(never — there is no supported way to export this key)* | `python catalog_console.py show-seed-b64 --release` |
+
+**Why two keys:** the catalog key is the root of trust for what BCC
+actually *executes* on a user's machine — every `command`/`args` pair in
+the shipped catalog is only there because this key signed it. If that key
+and the release-checksum key were the same (as they briefly were — see
+[issue #68](../../issues/68)), then anything that can exfiltrate a Gitea
+Actions secret (a malicious workflow-file PR, a compromised runner, a leaky
+log) could sign a catalog every user's copy of BCC would trust, not just a
+checksum manifest. Splitting them means **a CI/secret compromise burns the
+release key, never the catalog key** — checksums for a future release could
+be forged, which is bad, but no attacker gains the ability to make BCC run
+arbitrary commands on installs that trust the catalog. That asymmetry is
+the entire point of having two keys instead of one.
+
+The catalog key is **never** meant to leave the maintainer's machine: it's
+generated, stored, unlocked, and used to sign entirely inside the Catalog
+Console (`catalog_console.py`), and `catalog_console.py show-seed-b64`
+refuses to run without `--release` specifically so the catalog seed can't
+be exported by habit or muscle memory.
+
+**Release signing public key** (Ed25519, base64, raw 32 bytes) — this is
+the RELEASE key, not the catalog key:
+
+```
+<PLACEHOLDER — AJ: paste the release public key from `catalog_console.py keygen --release` here>
+```
+
 ## Run from source
 
 ```bash
@@ -152,6 +190,7 @@ file is also listed, marked *legacy*, so you can copy them over.
 - `bcc.spec` — PyInstaller build spec (cross-platform).
 - `scripts/build_icons.py` — regenerates `icons/app.icns` and `icons/app.ico` from source PNGs.
 - `scripts/sign_checksums.py` — generates and Ed25519-signs the release `SHA256SUMS` manifest (see [Verifying your download](#verifying-your-download)).
+- `catalog_console.py` / `catalog_review.py` — **maintainer-only**, never shipped to users (excluded from `bcc.spec`; see `tests/test_catalog_console_packaging.py`). The Catalog Console: review + sign `data/catalog.json`, and generate/manage both signing keys (`keygen`, `keygen --release`) — see [Signing keys](#signing-keys).
 
 ## Building from source
 

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 --------------- #
-ACCENT = "#f97316"  # warm orange
-ACCENT_DIM = "#c2570b"
-BG = "#1b1d23"
-PANEL = "#23262e"
-PANEL_2 = "#2b2f39"
-TEXT = "#e7e9ee"
-MUTED = "#9aa0ad"
-BORDER = "#3a3f4b"
-GOOD = "#4ade80"
-BAD = "#f87171"
-WARN = "#fbbf24"
+# --- Theming (issue #75) -------------------------------------------------- #
+# The palette lives in bcc_core (testable without a Qt app); these module-level
+# names are rebound by `apply_palette()` whenever the theme changes.
+#
+# Why globals rather than passing a palette around: ~20 inline
+# `setStyleSheet(f"color: {MUTED}")` calls are scattered through this file, and
+# an f-string resolves its names when it runs, not when it's compiled. Rebinding
+# the globals means every one of those call sites picks up the new colour on its
+# next render, with no change to the call sites themselves.
+PALETTE = core.DARK_PALETTE
+ACCENT = ACCENT_DIM = BG = PANEL = PANEL_2 = TEXT = MUTED = BORDER = ""
+GOOD = BAD = WARN = REMOTE = ON_ACCENT = DISABLED_BG = MONO_BG = SEL_TEXT = ""
+STATUS_COLORS: dict[str, str] = {}
+HEALTH_COLORS: dict[str, str] = {}
 
-STATUS_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": "#60a5fa", "unknown": WARN}
-STATUS_GLYPH = {"ok": "●", "missing": "●", "warn": "▲", "remote": "◆", "unknown": "○"}
+STATUS_GLYPH = {
+    "ok": "\u25cf",
+    "missing": "\u25cf",
+    "warn": "\u25b2",
+    "remote": "\u25c6",
+    "unknown": "\u25cb",
+}
 
 # Health dot (spawn-test outcome, see core.HealthStatus) shown per row in the
 # server tables' "Health" column -- distinct from the PATH-dependency Status
 # column above.
-HEALTH_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED}
-HEALTH_GLYPH = {"ok": "●", "failed": "●", "untested": "○"}
+HEALTH_GLYPH = {"ok": "\u25cf", "failed": "\u25cf", "untested": "\u25cb"}
 
-STYLESHEET = f"""
+
+def build_stylesheet(p: core.Palette) -> str:
+    """Render the global QSS for a palette."""
+    return f"""
 /* No font-family here on purpose: Qt already uses the native system UI font
    on every platform (San Francisco / Segoe UI / desktop default). Naming
    web-CSS aliases like -apple-system forces a costly font-alias scan. */
-* {{ font-size: 13px; color: {TEXT}; }}
-QMainWindow, QDialog {{ background: {BG}; }}
+* {{ font-size: 13px; color: {p.text}; }}
+QMainWindow, QDialog {{ background: {p.bg}; }}
 QLabel#h1 {{ font-size: 15px; font-weight: 600; }}
-QLabel#muted {{ color: {MUTED}; }}
-QFrame#card {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px; }}
+QLabel#muted {{ color: {p.muted}; }}
+QFrame#card {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px; }}
 QLineEdit, QPlainTextEdit, QComboBox {{
-    background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px;
-    padding: 6px 8px; selection-background-color: {ACCENT}; selection-color: #1a1205;
+    background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px;
+    padding: 6px 8px; selection-background-color: {p.accent}; selection-color: {p.on_accent};
 }}
-QLineEdit:focus, QPlainTextEdit:focus, QComboBox:focus {{ border: 1px solid {ACCENT}; }}
+QLineEdit:focus, QPlainTextEdit:focus, QComboBox:focus {{ border: 1px solid {p.accent}; }}
 QComboBox::drop-down {{ border: none; width: 22px; }}
-QComboBox QAbstractItemView {{ background: {PANEL_2}; border: 1px solid {BORDER};
-    selection-background-color: {ACCENT}; outline: none; }}
-QPushButton {{ background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px;
+QComboBox QAbstractItemView {{ background: {p.panel_2}; border: 1px solid {p.border};
+    selection-background-color: {p.accent}; outline: none; }}
+QPushButton {{ background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px;
     padding: 7px 13px; }}
-QPushButton:hover {{ border: 1px solid {ACCENT}; }}
-QPushButton:disabled {{ color: {MUTED}; background: {PANEL}; }}
-QPushButton#primary {{ background: {ACCENT}; border: 1px solid {ACCENT}; color: #1a1205; font-weight: 600; }}
-QPushButton#primary:hover {{ background: {ACCENT_DIM}; }}
-QPushButton#primary:disabled {{ background: {PANEL}; color: {MUTED}; border: 1px solid {BORDER}; }}
-QPushButton#danger:hover {{ border: 1px solid {BAD}; color: {BAD}; }}
-QTableWidget {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px;
+QPushButton:hover {{ border: 1px solid {p.accent}; }}
+QPushButton:disabled {{ color: {p.muted}; background: {p.panel}; }}
+QPushButton#primary {{ background: {p.accent}; border: 1px solid {p.accent}; color: {p.on_accent}; font-weight: 600; }}
+QPushButton#primary:hover {{ background: {p.accent_dim}; }}
+QPushButton#primary:disabled {{ background: {p.panel}; color: {p.muted}; border: 1px solid {p.border}; }}
+QPushButton#danger:hover {{ border: 1px solid {p.bad}; color: {p.bad}; }}
+QTableWidget {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px;
     gridline-color: transparent; outline: none; }}
 QTableWidget::item {{ padding: 6px 8px; border: none; }}
-QTableWidget::item:selected {{ background: {ACCENT}; color: #1a1205; }}
+QTableWidget::item:selected {{ background: {p.accent}; color: {p.on_accent}; }}
 /* Inline cell editors: the global QLineEdit padding/radius clips the text
    inside a table row, so give editors a compact, flat style instead. */
 QTableWidget QLineEdit {{
-    background: {PANEL_2}; color: {TEXT}; border: 1px solid {ACCENT};
+    background: {p.panel_2}; color: {p.text}; border: 1px solid {p.accent};
     border-radius: 3px; padding: 0px 4px; margin: 0px;
-    selection-background-color: {ACCENT_DIM}; selection-color: #ffffff;
+    selection-background-color: {p.accent_dim}; selection-color: {p.selection_text};
 }}
-QHeaderView::section {{ background: {PANEL}; color: {MUTED}; border: none;
-    border-bottom: 1px solid {BORDER}; padding: 8px; font-weight: 600; }}
+QHeaderView::section {{ background: {p.panel}; color: {p.muted}; border: none;
+    border-bottom: 1px solid {p.border}; padding: 8px; font-weight: 600; }}
 QScrollBar:vertical {{ background: transparent; width: 10px; margin: 2px; }}
-QScrollBar::handle:vertical {{ background: {BORDER}; border-radius: 5px; min-height: 24px; }}
+QScrollBar::handle:vertical {{ background: {p.border}; border-radius: 5px; min-height: 24px; }}
 QScrollBar::add-line, QScrollBar::sub-line {{ height: 0; }}
-QLabel#statusbar {{ color: {MUTED}; padding: 4px 2px; }}
-QLabel#warnBanner {{ color: #1a1205; background: {WARN}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }}
-QLabel#section {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
-QLabel#sectionDisabled {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
-QLabel#placeholder {{ color: {MUTED}; padding: 12px; background: {PANEL_2}; border: 1px dashed {BORDER}; border-radius: 8px; }}
-QTableWidget#disabledTable {{ background: #202229; }}
-QTableWidget#disabledTable::item:selected {{ background: {ACCENT}; color: #1a1205; }}
+QLabel#statusbar {{ color: {p.muted}; padding: 4px 2px; }}
+QLabel#warnBanner {{ color: {p.on_accent}; background: {p.warn}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }}
+QFrame#noticeBanner {{ background: {p.panel_2}; border: 1px solid {p.accent}; border-radius: 8px; }}
+QLabel#noticeText {{ color: {p.text}; }}
+QPushButton#noticeClose {{ background: transparent; border: none; color: {p.muted}; font-size: 14px; padding: 2px; }}
+QPushButton#noticeClose:hover {{ color: {p.text}; }}
+QLabel#section {{ color: {p.muted}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
+QLabel#sectionDisabled {{ color: {p.muted}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
+QLabel#placeholder {{ color: {p.muted}; padding: 12px; background: {p.panel_2}; border: 1px dashed {p.border}; border-radius: 8px; }}
+QTableWidget#disabledTable {{ background: {p.disabled_bg}; }}
+QTableWidget#disabledTable::item:selected {{ background: {p.accent}; color: {p.on_accent}; }}
 QPlainTextEdit#diag {{ font-family: "Menlo", "Cascadia Code", "Consolas", "DejaVu Sans Mono", monospace;
-    font-size: 12px; background: #16181d; border: 1px solid {BORDER}; border-radius: 8px; }}
+    font-size: 12px; background: {p.mono_bg}; border: 1px solid {p.border}; border-radius: 8px; }}
 QFrame#diagCard {{ background: transparent; border: none; }}
 QSplitter::handle {{ background: transparent; }}
-QSplitter::handle:hover {{ background: {BORDER}; border-radius: 4px; }}
-QSplitter::handle:pressed {{ background: {ACCENT}; border-radius: 4px; }}
+QSplitter::handle:hover {{ background: {p.border}; border-radius: 4px; }}
+QSplitter::handle:pressed {{ background: {p.accent}; border-radius: 4px; }}
 """
 
 
+def apply_palette(p: core.Palette) -> str:
+    """Rebind the module-level colour names to `p` and return its stylesheet."""
+    global PALETTE, ACCENT, ACCENT_DIM, BG, PANEL, PANEL_2, TEXT, MUTED, BORDER
+    global GOOD, BAD, WARN, REMOTE, ON_ACCENT, DISABLED_BG, MONO_BG, SEL_TEXT
+    global STATUS_COLORS, HEALTH_COLORS
+    PALETTE = p
+    ACCENT, ACCENT_DIM = p.accent, p.accent_dim
+    BG, PANEL, PANEL_2 = p.bg, p.panel, p.panel_2
+    TEXT, MUTED, BORDER = p.text, p.muted, p.border
+    GOOD, BAD, WARN, REMOTE = p.good, p.bad, p.warn, p.remote
+    ON_ACCENT, DISABLED_BG, MONO_BG, SEL_TEXT = (
+        p.on_accent,
+        p.disabled_bg,
+        p.mono_bg,
+        p.selection_text,
+    )
+    STATUS_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": REMOTE, "unknown": WARN}
+    HEALTH_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED}
+    return build_stylesheet(p)
+
+
+STYLESHEET = apply_palette(core.DARK_PALETTE)
+
+
 # --------------------------------------------------------------------------- #
 # Background reachability tester (keeps the UI responsive during the request)
 # --------------------------------------------------------------------------- #
@@ -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"]):
-            self.status.setText(
-                f"Update available: {release['version']}  ·  Help ▸ About to view it."
-            )
+        notice = core.update_notice(core.__version__, release)
+        if notice:
+            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}
-    disabled = {s.name: s.data for s in servers if not s.enabled}
+    enabled = {s.name: s.config_value() for s in servers if s.enabled}
+    disabled = {s.name: s.config_value() for s in servers if not s.enabled}
 
     cfg["mcpServers"] = enabled  # replaces value if key existed; appends otherwise
     if disabled:
@@ -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
 

catalog_console.py

@@ -12,7 +12,12 @@ Flow: Load -> Review -> Sign.
   1. Load    -- pick a source: an open Gitea PR touching data/catalog.json,
                 or the current tip of `main`. The Console fetches the exact
                 git blob (via a local clone's git plumbing) and PINS its
-                blob SHA for the rest of this review pass.
+                blob SHA *and the ref it came from* for the rest of this
+                review pass. For a PR, the diff is against `main`; for
+                `main`, the diff is against the last catalog a maintainer
+                actually SIGNED (the bytes covered by the current
+                data/catalog.json.sig), never against itself -- an empty
+                diff must mean "nothing to sign", never "sign unlocked".
   2. Review  -- a semantic diff (catalog_review.diff_catalogs), one card per
                 changed entry, with risk annotations
                 (catalog_review.entry_risk_findings). A registry lookup for
@@ -25,20 +30,34 @@ Flow: Load -> Review -> Sign.
                 changed entry must be individually acknowledged (its
                 checkbox ticked) before Sign unlocks. There is no
                 "acknowledge all" -- see catalog_review.py.
-  3. Sign    -- re-fetches the current blob SHA and refuses to sign unless
-                it still matches the pinned SHA from step 1 (TOCTOU fix:
-                catalog_review.can_sign). On success, writes
+  3. Sign    -- re-resolves the current blob SHA from the SAME ref that was
+                reviewed (never a hardcoded "main") and refuses to sign
+                unless it still matches the pinned SHA from step 1, the diff
+                is non-empty, and no changed entry has an outstanding
+                blocking risk finding (catalog_review.sign_precondition /
+                can_sign -- the TOCTOU fix). On success, writes
                 data/catalog.json + data/catalog.json.sig and commits BOTH
                 in a single commit, then pushes -- so main is never red
-                between a catalog merge and its signature.
+                between a catalog merge and its signature. Signing uses the
+                CATALOG key ONLY -- see "Two signing keys" below.
 
 The signature must be the artefact of an actual review, not a step that
 follows one. Signing IS the approval act.
+
+Two signing keys (issue #68 finding 5): the CATALOG key (offline,
+Console-only, `keygen` / `keygen --release` picks which) is the root of
+trust for what BCC executes and must never touch CI. The RELEASE key is
+CI-resident and signs ONLY the release SHA256SUMS manifest
+(scripts/sign_checksums.py) -- `show-seed-b64 --release` is the only
+supported way to get a seed out of this tool, and it refuses to run without
+`--release` so the catalog seed can never be exported by habit. See the
+README's "Signing keys" section.
 """
 
 from __future__ import annotations
 
 import argparse
+import base64
 import contextlib
 import getpass
 import html
@@ -69,8 +88,28 @@ SIG_PATH = "data/catalog.json.sig"
 # maintainer-only tool, so a dotfile under $HOME is an acceptable fallback
 # when the OS keychain isn't available -- the blob stored there is always
 # passphrase-encrypted (see catalog_review.encrypt_private_key), never raw.
+#
+# Two SEPARATE keys are stored here, never conflated (issue #68 finding 5):
+#   "catalog" -- offline, Console-only. Verified by bcc_core.CATALOG_PUBKEYS.
+#                Roots of trust for every catalog entry BCC ships. Must
+#                NEVER leave this machine, never touch CI, never become an
+#                env var or a repo secret.
+#   "release" -- CI-resident. Verified by scripts.sign_checksums.
+#                RELEASE_PUBKEYS. Signs ONLY the release SHA256SUMS
+#                manifest. Its private seed is deliberately meant to be
+#                pasted into the RELEASE_SIGNING_KEY Gitea Actions secret
+#                (via `show-seed-b64 --release`) -- that is its normal,
+#                intended flow. A CI compromise burns this key, not the
+#                catalog key: that asymmetry is the whole point of having
+#                two keys instead of one.
 KEY_STORAGE_DIR = Path.home() / ".bcc-catalog-console"
-KEY_STORAGE_FILE = KEY_STORAGE_DIR / "signing_key.enc"
+_KEY_KINDS = ("catalog", "release")
+
+
+def _key_storage_file(kind: str) -> Path:
+    assert kind in _KEY_KINDS, f"unknown key kind {kind!r}, expected one of {_KEY_KINDS}"
+    return KEY_STORAGE_DIR / f"signing_key_{kind}.enc"
+
 
 HTTP_TIMEOUT = 6.0
 
@@ -97,51 +136,64 @@ def _keyring_module():
 
 
 _KEYRING_SERVICE = "bcc-catalog-console"
-_KEYRING_USERNAME = "signing-key"
 
 
-def store_encrypted_key(blob: bytes) -> str:
+def _keyring_username(kind: str) -> str:
+    assert kind in _KEY_KINDS, f"unknown key kind {kind!r}, expected one of {_KEY_KINDS}"
+    return f"signing-key-{kind}"
+
+
+def store_encrypted_key(blob: bytes, kind: str = "catalog") -> str:
     """Persist an already-encrypted key blob (see
-    catalog_review.encrypt_private_key). Prefers the OS keychain; falls back
-    to a file under KEY_STORAGE_DIR (outside the repo) with restrictive
+    catalog_review.encrypt_private_key) under the given `kind`
+    ("catalog" or "release" -- see the KEY_STORAGE_DIR comment above; the
+    two are stored under different keychain entries / filenames so they can
+    never be loaded interchangeably). Prefers the OS keychain; falls back to
+    a file under KEY_STORAGE_DIR (outside the repo) with restrictive
     permissions. Returns a human-readable description of where it went."""
     keyring = _keyring_module()
     if keyring is not None:
         try:
-            keyring.set_password(_KEYRING_SERVICE, _KEYRING_USERNAME, blob.hex())
+            keyring.set_password(_KEYRING_SERVICE, _keyring_username(kind), blob.hex())
             return "OS keychain (via the `keyring` package)"
         except Exception:
             pass  # fall through to the file-based path
     KEY_STORAGE_DIR.mkdir(parents=True, exist_ok=True)
-    KEY_STORAGE_FILE.write_bytes(blob)
+    key_file = _key_storage_file(kind)
+    key_file.write_bytes(blob)
     with contextlib.suppress(OSError):  # best-effort on platforms without POSIX perm bits
-        KEY_STORAGE_FILE.chmod(0o600)
-    return f"encrypted file at {KEY_STORAGE_FILE}"
+        key_file.chmod(0o600)
+    return f"encrypted file at {key_file}"
 
 
-def load_encrypted_key() -> bytes:
-    """Load the encrypted key blob from wherever store_encrypted_key() put
-    it. Raises FileNotFoundError if no key has been generated yet."""
+def load_encrypted_key(kind: str = "catalog") -> bytes:
+    """Load the encrypted key blob of the given `kind` from wherever
+    store_encrypted_key() put it. Raises FileNotFoundError if no key of that
+    kind has been generated yet."""
     keyring = _keyring_module()
     if keyring is not None:
         try:
-            hex_blob = keyring.get_password(_KEYRING_SERVICE, _KEYRING_USERNAME)
+            hex_blob = keyring.get_password(_KEYRING_SERVICE, _keyring_username(kind))
             if hex_blob:
                 return bytes.fromhex(hex_blob)
         except Exception:
             pass
-    if not KEY_STORAGE_FILE.exists():
+    key_file = _key_storage_file(kind)
+    if not key_file.exists():
+        flag = " --release" if kind == "release" else ""
         raise FileNotFoundError(
-            f"No signing key found (checked the OS keychain and {KEY_STORAGE_FILE}). "
-            "Run `python catalog_console.py keygen` first."
+            f"No {kind} signing key found (checked the OS keychain and {key_file}). "
+            f"Run `python catalog_console.py keygen{flag}` first."
         )
-    return KEY_STORAGE_FILE.read_bytes()
+    return key_file.read_bytes()
 
 
-def unlock_signing_key(passphrase: str) -> bytes:
-    """Load + decrypt the signing key seed. Raises ValueError on a wrong
-    passphrase, FileNotFoundError if no key exists yet."""
-    blob = load_encrypted_key()
+def unlock_signing_key(passphrase: str, kind: str = "catalog") -> bytes:
+    """Load + decrypt the signing key seed of the given `kind`. Raises
+    ValueError on a wrong passphrase, FileNotFoundError if no key of that
+    kind exists yet. Defaults to "catalog" because that's the key
+    ReviewWindow._on_sign uses -- the GUI never touches the release key."""
+    blob = load_encrypted_key(kind)
     return review.decrypt_private_key(blob, passphrase)
 
 
@@ -188,6 +240,49 @@ def read_catalog_at_commit(repo_dir: Path, commit: str) -> tuple[bytes, str]:
     return blob_bytes(repo_dir, sha), sha
 
 
+def catalog_blob_history(repo_dir: Path, ref: str, limit: int = 200) -> list[str]:
+    """Blob SHAs of CATALOG_PATH at each commit that touched it, walking
+    back from `ref`, most-recent-first. Used by last_signed_catalog_raw() to
+    find "the version of the catalog the current signature actually covers"
+    without assuming it's the tip commit."""
+    log_out = _git(repo_dir, "log", f"--max-count={limit}", "--format=%H", ref, "--", CATALOG_PATH)
+    commits = [line for line in log_out.splitlines() if line]
+    shas: list[str] = []
+    for commit in commits:
+        try:
+            shas.append(blob_sha_at(repo_dir, commit, CATALOG_PATH))
+        except GitError:
+            continue
+    return shas
+
+
+def last_signed_catalog_raw(repo_dir: Path, commit: str) -> bytes | None:
+    """The raw data/catalog.json bytes that verify against
+    data/catalog.json.sig as of `commit` -- i.e. "the last catalog a
+    maintainer actually signed", found by walking the catalog's git history
+    on that ref until a version verifies against the *current* signature.
+
+    This is what source="main (current tip)" diffs against (issue #68
+    finding 1): if main's tip catalog.json already matches its .sig, this
+    returns that same content and the diff is correctly empty (nothing new
+    to sign). If someone merged a catalog change to main without running it
+    through the Console -- the exact bypass that produced commit b08cf21 --
+    the signature still covers the OLDER content, so this returns that older
+    version and the diff surfaces exactly what was never actually reviewed.
+
+    Returns None if there's no signature yet, or none of the recent history
+    verifies against it (caller should treat this as "diff against nothing
+    ever signed", i.e. every current entry shows as newly added).
+    """
+    try:
+        sig_sha = blob_sha_at(repo_dir, commit, SIG_PATH)
+    except GitError:
+        return None
+    sig_bytes = blob_bytes(repo_dir, sig_sha)
+    candidates = [blob_bytes(repo_dir, sha) for sha in catalog_blob_history(repo_dir, commit)]
+    return review.find_last_signed_catalog_raw(candidates, sig_bytes, core.CATALOG_PUBKEYS)
+
+
 def commit_and_push_signed_catalog(
     repo_dir: Path, raw_bytes: bytes, signature: bytes, *, branch: str = "main"
 ) -> str:
@@ -644,28 +739,50 @@ class ReviewWindow(QMainWindow):
         row = self.source_list.currentRow()
         try:
             if row <= 0:
-                commit = fetch_ref(self.repo_dir, "main")
-                old_commit = None  # main vs itself has no "old" -- nothing to diff without a base
+                # source = main: diff against the last catalog a maintainer
+                # actually SIGNED (the bytes covered by the current
+                # data/catalog.json.sig), never against itself. Diffing
+                # main-vs-main is what made an empty diff -> instantly
+                # "signable" in the first place (issue #68 finding 1) --
+                # this is the only path that reaches sign_catalog_bytes(),
+                # so if it can't be trusted nothing can.
+                loaded_ref = "main"
+                commit = fetch_ref(self.repo_dir, loaded_ref)
+                new_raw, new_blob_sha = read_catalog_at_commit(self.repo_dir, commit)
+                new_catalog = core.load_catalog(new_raw)
+                old_raw = last_signed_catalog_raw(self.repo_dir, commit)
+                old_catalog = (
+                    core.load_catalog(old_raw)
+                    if old_raw is not None
+                    else {
+                        "schema": 1,
+                        "version": 0,
+                        "servers": [],
+                    }
+                )
             else:
                 pr = self._prs[row - 1]
-                commit = fetch_ref(self.repo_dir, pr.head_ref)
+                loaded_ref = pr.head_ref
+                commit = fetch_ref(self.repo_dir, loaded_ref)
                 old_commit = fetch_ref(self.repo_dir, "main")
 
-            new_raw, new_blob_sha = read_catalog_at_commit(self.repo_dir, commit)
-            new_catalog = core.load_catalog(new_raw)
-
-            if old_commit:
+                new_raw, new_blob_sha = read_catalog_at_commit(self.repo_dir, commit)
+                new_catalog = core.load_catalog(new_raw)
                 old_raw, _old_sha = read_catalog_at_commit(self.repo_dir, old_commit)
                 old_catalog = core.load_catalog(old_raw)
-            else:
-                old_catalog = new_catalog
 
         except (GitError, ValueError) as e:
             QMessageBox.critical(self, "Load failed", html.escape(str(e)))
             return
 
         self._new_raw = new_raw
-        self.session = review.start_review(new_blob_sha, old_catalog, new_catalog)
+        # `loaded_ref` is pinned into the session (not just this method's
+        # local variable) so _on_sign can re-resolve the TOCTOU blob SHA
+        # from the SAME ref that was reviewed, instead of a hardcoded
+        # "main" -- see review.sign_precondition and issue #68 finding 1.
+        self.session = review.start_review(
+            new_blob_sha, old_catalog, new_catalog, loaded_ref=loaded_ref
+        )
         self._render_cards()
 
     def _render_cards(self):
@@ -706,20 +823,43 @@ class ReviewWindow(QMainWindow):
 
     def _on_sign(self):
         assert self.session is not None
+
+        def _resolve_blob_sha(ref: str) -> str:
+            # Re-fetch fresh, immediately before signing, from the SAME ref
+            # that was reviewed (session.loaded_ref) -- NEVER hardcode
+            # "main" here. Hardcoding "main" is the bug that made the PR
+            # review path unable to sign at all: _on_load() pins the PR
+            # head's blob SHA, so comparing against main's SHA differs by
+            # definition for any PR that actually changes the catalog, and
+            # this refused every PR review permanently (see issue #68
+            # finding 1 and review.sign_precondition's docstring).
+            return blob_sha_at(self.repo_dir, fetch_ref(self.repo_dir, ref), CATALOG_PATH)
+
         try:
-            current_sha = blob_sha_at(self.repo_dir, fetch_ref(self.repo_dir, "main"), CATALOG_PATH)
+            decision = review.sign_precondition(self.session, _resolve_blob_sha)
         except GitError as e:
             QMessageBox.critical(self, "Sign failed", html.escape(str(e)))
             return
 
-        decision = review.can_sign(self.session, current_sha)
         if not decision.ok:
             QMessageBox.warning(self, "Cannot sign", html.escape(decision.reason or ""))
-            if decision.reason and "changed" in decision.reason.lower():
-                self._on_load()  # force a re-review against the new bytes
+            # Only the TOCTOU blob-SHA-mismatch reason should trigger a
+            # reload -- "mismatch" appears ONLY in that reason (deliberately
+            # checked instead of the broader "changed", which also matches
+            # "Not every CHANGED entry has been acknowledged yet" and would
+            # wrongly force a reload -- and with it a fresh review.py
+            # ReviewSession -- every time a reviewer pauses partway through
+            # ticking checkboxes).
+            if decision.reason and "mismatch" in decision.reason.lower():
+                # Force a genuine re-review against the new bytes -- this
+                # must call _on_load() (which re-fetches and re-diffs), NOT
+                # re-invoke the same stale resolver, or this becomes the
+                # infinite loop described in issue #68 finding 1: re-pinning
+                # the same wrong SHA forever instead of ever converging.
+                self._on_load()
             return
 
-        dialog = PassphraseDialog("Enter signing key passphrase:", self)
+        dialog = PassphraseDialog("Enter CATALOG signing key passphrase:", self)
         if dialog.exec() != QDialog.DialogCode.Accepted:
             return
         try:
@@ -744,9 +884,17 @@ class ReviewWindow(QMainWindow):
 # --------------------------------------------------------------------------- #
 
 
-def cmd_keygen(_args: argparse.Namespace) -> int:
+def cmd_keygen(args: argparse.Namespace) -> int:
+    # Two SEPARATE keypairs, never conflated (issue #68 finding 5): the
+    # catalog key is the offline root of trust for what BCC executes and
+    # must never touch CI; the release key is CI-resident and signs ONLY
+    # the release SHA256SUMS manifest. Which one this run generates is
+    # explicit via --release, and the printed instructions differ sharply
+    # so it's obvious which key is safe to paste into a CI secret (release)
+    # and which one never is (catalog).
+    kind = "release" if getattr(args, "release", False) else "catalog"
     seed, pubkey = review.generate_keypair()
-    passphrase = getpass.getpass("Choose a passphrase to encrypt the new signing key: ")
+    passphrase = getpass.getpass(f"Choose a passphrase to encrypt the new {kind} signing key: ")
     confirm = getpass.getpass("Confirm passphrase: ")
     if passphrase != confirm:
         print("error: passphrases did not match", file=sys.stderr)
@@ -756,31 +904,67 @@ def cmd_keygen(_args: argparse.Namespace) -> int:
         return 1
 
     blob = review.encrypt_private_key(seed, passphrase)
-    where = store_encrypted_key(blob)
-    pubkey_b64 = __import__("base64").b64encode(pubkey).decode("ascii")
+    where = store_encrypted_key(blob, kind=kind)
+    pubkey_b64 = base64.b64encode(pubkey).decode("ascii")
 
-    print(f"Private key encrypted and stored in: {where}")
+    print(f"{kind.capitalize()} private key encrypted and stored in: {where}")
     print()
-    print("Public key (base64, paste into bcc_core.CATALOG_PUBKEYS):")
-    print(f"    {pubkey_b64}")
-    print()
-    print(
-        "Also add it as the Gitea repo secret RELEASE_SIGNING_KEY (base64 of the "
-        "32-byte private seed) used by release.yml -- get that value with:"
-    )
-    print("    python catalog_console.py show-seed-b64   # careful: prints the raw key")
+    if kind == "catalog":
+        print(
+            "This is the CATALOG key. It is the root of trust for every catalog "
+            "entry BCC ships -- it must stay offline and Console-only. NEVER paste "
+            "it, its seed, or `show-seed-b64` output into CI, an env var, or a repo "
+            "secret. (If the key currently in bcc_core.CATALOG_PUBKEYS has ever been "
+            "pasted into a CI secret, treat it as burned for catalog use -- generate "
+            "a fresh one with this command and rotate.)"
+        )
+        print()
+        print(
+            "Public key (base64) -- hand this to whoever maintains bcc_core.py so "
+            "they can add it to CATALOG_PUBKEYS (this tool does not edit that file):"
+        )
+        print(f"    {pubkey_b64}")
+    else:
+        print(
+            "This is the RELEASE key. It signs ONLY the release SHA256SUMS "
+            "manifest in CI -- it is intentionally CI-resident and is NOT trusted "
+            "to sign the catalog (bcc_core.CATALOG_PUBKEYS does not and must not "
+            "contain it)."
+        )
+        print()
+        print("1. Public key (base64) -- paste into scripts/sign_checksums.py RELEASE_PUBKEYS:")
+        print(f"    {pubkey_b64}")
+        print()
+        print(
+            "2. Private key -- add it as the Gitea repo secret RELEASE_SIGNING_KEY "
+            "(base64 of the 32-byte private seed). Get that value with:"
+        )
+        print("    python catalog_console.py show-seed-b64 --release   # prints the raw key")
     return 0
 
 
-def cmd_show_seed_b64(_args: argparse.Namespace) -> int:
-    passphrase = getpass.getpass("Signing key passphrase: ")
+def cmd_show_seed_b64(args: argparse.Namespace) -> int:
+    # Deliberately requires --release: this command's whole purpose is to
+    # produce a value that gets pasted into a CI secret, and the catalog key
+    # must NEVER be pasted into CI (issue #68 finding 5 -- that is exactly
+    # how the catalog key ended up burned in the first place). Refusing to
+    # run without --release makes "export the catalog seed for CI" a
+    # structurally different, more deliberate action than a typo away.
+    if not getattr(args, "release", False):
+        print(
+            "error: show-seed-b64 only ever exports the RELEASE key -- it is the "
+            "only key allowed to leave this machine, for the RELEASE_SIGNING_KEY CI "
+            "secret. Re-run as `show-seed-b64 --release`. The catalog key must never "
+            "be exported this way; see issue #68 finding 5.",
+            file=sys.stderr,
+        )
+        return 1
+    passphrase = getpass.getpass("Release signing key passphrase: ")
     try:
-        seed = unlock_signing_key(passphrase)
+        seed = unlock_signing_key(passphrase, kind="release")
     except (FileNotFoundError, ValueError) as e:
         print(f"error: {e}", file=sys.stderr)
         return 1
-    import base64
-
     print(base64.b64encode(seed).decode("ascii"))
     return 0
 
@@ -810,11 +994,28 @@ def build_parser() -> argparse.ArgumentParser:
     p_gui.add_argument("--repo", default=".", help="path to a BCC git checkout (default: cwd)")
     p_gui.set_defaults(func=cmd_gui)
 
-    p_keygen = sub.add_parser("keygen", help="generate a new Ed25519 signing keypair")
+    p_keygen = sub.add_parser(
+        "keygen", help="generate a new Ed25519 signing keypair (catalog key by default)"
+    )
+    p_keygen.add_argument(
+        "--release",
+        action="store_true",
+        help=(
+            "generate the RELEASE key (CI-resident, signs SHA256SUMS only) instead "
+            "of the CATALOG key (offline, Console-only, signs data/catalog.json -- "
+            "see issue #68 finding 5)"
+        ),
+    )
     p_keygen.set_defaults(func=cmd_keygen)
 
     p_seed = sub.add_parser(
-        "show-seed-b64", help="print the base64 private seed (for the RELEASE_SIGNING_KEY secret)"
+        "show-seed-b64",
+        help="print a base64 private seed for a CI secret -- RELEASE key only",
+    )
+    p_seed.add_argument(
+        "--release",
+        action="store_true",
+        help="required: only the release key may ever be exported this way",
     )
     p_seed.set_defaults(func=cmd_show_seed_b64)
 

catalog_review.py

@@ -24,6 +24,7 @@ from urllib.parse import urlsplit
 
 from bcc_core import _CATALOG_SIG_DOMAIN as CATALOG_SIG_DOMAIN
 from bcc_core import CATALOG_ALLOWED_COMMANDS
+from bcc_core import verify_catalog_signature as _verify_catalog_signature
 
 # --------------------------------------------------------------------------- #
 # Semantic diff
@@ -432,11 +433,21 @@ def has_blocking_risk(change: EntryChange) -> bool:
 class ReviewSession:
     """State for one review pass. `pinned_blob_sha` is the git blob SHA of
     data/catalog.json as it existed the moment review began -- see
-    can_sign()."""
+    can_sign()/sign_precondition().
+
+    `loaded_ref` is the exact ref this review was loaded from ("main", or a
+    PR's `refs/pull/<n>/head`) -- see issue #68 finding 1. It exists so the
+    Sign path can re-resolve the TOCTOU blob SHA from *the ref that was
+    actually reviewed*, instead of a hardcoded "main" that silently diverges
+    from the reviewed ref on every PR review (the bug that made the PR path
+    unable to sign at all, and forced everyone onto the vacuous
+    main-vs-itself path instead).
+    """
 
     pinned_blob_sha: str
     old_catalog: dict
     new_catalog: dict
+    loaded_ref: str = "main"
     changes: list[EntryChange] = field(default_factory=list)
     acknowledged: set[str] = field(default_factory=set)
 
@@ -445,12 +456,39 @@ class ReviewSession:
             self.changes = diff_catalogs(self.old_catalog, self.new_catalog)
 
 
-def start_review(pinned_blob_sha: str, old_catalog: dict, new_catalog: dict) -> ReviewSession:
+def start_review(
+    pinned_blob_sha: str,
+    old_catalog: dict,
+    new_catalog: dict,
+    loaded_ref: str = "main",
+) -> ReviewSession:
     return ReviewSession(
-        pinned_blob_sha=pinned_blob_sha, old_catalog=old_catalog, new_catalog=new_catalog
+        pinned_blob_sha=pinned_blob_sha,
+        old_catalog=old_catalog,
+        new_catalog=new_catalog,
+        loaded_ref=loaded_ref,
     )
 
 
+def find_last_signed_catalog_raw(
+    candidates: list[bytes], sig: bytes, pubkeys: list[bytes]
+) -> bytes | None:
+    """Given `candidates` (candidate raw catalog.json byte-strings -- e.g.
+    successive historical versions from git log, most-recent-first),
+    return the first one whose signature verifies against `sig`/`pubkeys`,
+    or None if none do.
+
+    This is how source="main" review diffs against "the last catalog a
+    maintainer actually signed" instead of against itself (issue #68
+    finding 1): `catalog_console.last_signed_catalog_raw` walks
+    data/catalog.json's git history on main and hands the candidates here.
+    """
+    for raw in candidates:
+        if _verify_catalog_signature(raw, sig, pubkeys):
+            return raw
+    return None
+
+
 def acknowledge_entry(session: ReviewSession, entry_id: str) -> None:
     ids = {c.entry_id for c in session.changes}
     if entry_id not in ids:
@@ -483,22 +521,53 @@ class SignDecision:
 def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
     """Whether the Sign button may fire right now.
 
-    Two independent gates, both required:
-    1. TOCTOU: `current_blob_sha` (fetched fresh, immediately before signing)
+    Four independent gates, all required, checked in this order:
+
+    0. The diff must be non-empty. An empty diff historically meant "Sign
+       unlocks instantly" (`set() <= set()` is vacuously True), which is
+       exactly backwards: a vacuously-satisfied gate is worse than no gate
+       at all, because it *manufactures confidence* -- the signature looks
+       identical to one produced by a real review. "Nothing changed" must
+       mean "nothing to sign", never "sign unlocked". (Issue #68 finding 1;
+       this is what let commit b08cf21 sign all 19 entries with zero of them
+       ever reviewed.)
+    1. TOCTOU: `current_blob_sha` (fetched fresh, immediately before signing,
+       from the ref that was actually reviewed -- see sign_precondition())
        must match the blob SHA pinned when review began. If the bytes on the
        remote changed since -- a new commit pushed to the same PR, a
        force-push, another PR merged in between -- signing is refused and a
        re-review is forced. This is what makes "signing is the approval act"
        true rather than aspirational: the signature is bound to the exact
        reviewed bytes, not to "whatever the file happens to be now".
-    2. Every changed entry in the diff must be individually acknowledged.
+    2. No blocking risk finding may be outstanding on ANY changed entry, full
+       stop -- checked here, not just in the GUI. The GUI additionally
+       disables the acknowledge checkbox for a blocking entry, but that is a
+       UI nicety, not the enforcement point: if this pure gate didn't also
+       check it, a blocking risk would only be stopped by the GUI happening
+       to have wired the checkbox correctly, and nothing would catch a
+       regression in that wiring. The GUI must not be the only thing
+       standing between a blocking risk and a signature.
+    3. Every changed entry in the diff must be individually acknowledged.
     """
+    if not session.changes:
+        return SignDecision(
+            False,
+            "Nothing to sign: this review's diff is empty. If you expected "
+            "changes here, you may be diffing the wrong source/ref.",
+        )
     if current_blob_sha != session.pinned_blob_sha:
         return SignDecision(
             False,
             "The reviewed bytes changed since this review began (blob SHA "
             "mismatch) -- re-review required before signing.",
         )
+    blocking_ids = sorted({c.entry_id for c in session.changes if has_blocking_risk(c)})
+    if blocking_ids:
+        return SignDecision(
+            False,
+            "Blocking risk finding(s) outstanding on: "
+            f"{', '.join(blocking_ids)} -- fix the underlying change, do not sign around it.",
+        )
     if not all_entries_acknowledged(session):
         pending = sorted({c.entry_id for c in session.changes} - session.acknowledged)
         return SignDecision(
@@ -507,6 +576,28 @@ def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
     return SignDecision(True, None)
 
 
+def sign_precondition(
+    session: ReviewSession, resolve_blob_sha: Callable[[str], str]
+) -> SignDecision:
+    """The real Sign-button gate: resolves the current TOCTOU blob SHA from
+    *the ref this session was actually loaded from* (`session.loaded_ref`),
+    never a hardcoded "main", then delegates to can_sign().
+
+    `resolve_blob_sha` is injected so this stays testable without git/Qt --
+    catalog_console.ReviewWindow._on_sign passes a real resolver
+    (fetch_ref + blob_sha_at against self.repo_dir); tests pass a fake
+    dict-backed lookup. This is the fix for issue #68 finding 1's first bug:
+    `_on_sign` used to hardcode `fetch_ref(self.repo_dir, "main")` as the
+    comparison ref, so for any PR review (where `loaded_ref` is the PR's
+    head, not main) the SHAs differed by definition and Sign could never
+    fire -- and the retry path re-called the same hardcoded resolver, so it
+    re-pinned the same wrong value and looped forever instead of forcing a
+    genuine re-review.
+    """
+    current_blob_sha = resolve_blob_sha(session.loaded_ref)
+    return can_sign(session, current_blob_sha)
+
+
 def catalog_signing_message(raw_bytes: bytes) -> bytes:
     """The exact bytes that get signed: bcc_core's domain-separation prefix
     (imported, never retyped) + the raw catalog bytes. Using this function

scripts/sign_checksums.py

@@ -42,6 +42,25 @@ from pathlib import Path
 # message signed by the same key.
 DOMAIN_PREFIX = b"bcc-release-v1|"
 
+# Public half of the RELEASE signing key(s) -- a SEPARATE keypair from
+# bcc_core.CATALOG_PUBKEYS (issue #68 finding 5). The catalog key is the
+# offline, Console-only root of trust for what BCC executes; this key is
+# CI-resident and signs ONLY the release SHA256SUMS manifest, never the
+# catalog. Keeping them apart means a CI/repo-secret compromise burns the
+# release key -- annoying, but it never lets an attacker sign a catalog a
+# user's binary would trust. A LIST (not a single key), mirroring
+# CATALOG_PUBKEYS, so the release key can be rotated without invalidating
+# the signature on every past release: verification accepts a match against
+# ANY key here.
+#
+# Empty until the maintainer generates the release keypair (separately from
+# the catalog keypair) and pastes the public half in:
+#     python catalog_console.py keygen --release
+# This is intentionally NOT pre-populated with a placeholder that looks
+# like a real key -- release.yml's signing-smoke-test fails closed (loudly)
+# on an empty list rather than silently verifying against nothing.
+RELEASE_PUBKEYS: list[bytes] = []
+
 CHUNK_SIZE = 1024 * 1024
 
 
@@ -135,6 +154,19 @@ def public_key_b64_from_seed(seed_b64: str) -> str:
     return base64.b64encode(raw).decode("ascii")
 
 
+def verify_checksums_against_any(pubkeys: list[bytes], sums_text: str, signature: bytes) -> bool:
+    """Verify `signature` against ANY key in `pubkeys` (each a raw 32-byte
+    Ed25519 public key). Mirrors bcc_core.verify_catalog_signature's
+    rotation-friendly "any currently-trusted key" semantics, applied to
+    RELEASE_PUBKEYS instead of the catalog's key list. Returns False (never
+    raises) for an empty `pubkeys` list -- fails closed rather than
+    vacuously verifying against nothing."""
+    return any(
+        verify_checksums(base64.b64encode(pk).decode("ascii"), sums_text, signature)
+        for pk in pubkeys
+    )
+
+
 # --------------------------------------------------------------------------- #
 # CLI
 # --------------------------------------------------------------------------- #

tests/test_catalog_review.py

@@ -334,13 +334,29 @@ def test_acknowledge_gating_requires_every_entry():
     assert r.all_entries_acknowledged(session) is True
 
 
-def test_no_acknowledge_all_function_exists():
-    """Deliberate: there must be no shortcut to acknowledge every entry at
-    once. See the comment in catalog_review.py above SignDecision."""
+def test_no_acknowledge_all_shortcut_and_gate_is_real():
+    """Two things, both load-bearing (issue #68: the original version of
+    this test asserted ONLY the first half, and passed the entire time the
+    gate below it was vacuously satisfiable -- 'no function named
+    acknowledge_all' is worthless if signing doesn't actually require
+    acknowledgement in practice).
+
+    1. No bulk-acknowledge shortcut exists (see the comment in
+       catalog_review.py above SignDecision -- deliberate friction).
+    2. The gate that friction protects is actually enforced: with entries
+       still unacknowledged, can_sign() must refuse, not just "some GUI
+       checkbox happens to be unticked".
+    """
     names = [n for n in dir(r) if "acknowledge" in n.lower()]
     assert "acknowledge_all" not in names
     assert "acknowledge_all_entries" not in names
 
+    session = r.start_review("sha1", _catalog(), _catalog(_entry(id="a"), _entry(id="b")))
+    r.acknowledge_entry(session, "a")  # only one of two -- not a bulk call
+    decision = r.can_sign(session, "sha1")
+    assert decision.ok is False
+    assert "acknowledged" in decision.reason.lower()
+
 
 # --------------------------------------------------------------------------- #
 # can_sign: TOCTOU blob pinning + acknowledge gating combined
@@ -377,6 +393,130 @@ def test_can_sign_blob_mismatch_takes_priority_message():
     assert "blob" in decision.reason.lower() or "changed" in decision.reason.lower()
 
 
+def test_can_sign_false_on_empty_changeset():
+    """The exact bug behind issue #68 finding 1: 'main' loaded against
+    itself diffs to [], and an empty changeset used to leave can_sign()
+    with nothing to refuse on (set() <= set() is vacuously True). Commit
+    b08cf21 signed 19 entries through precisely this path -- zero of them
+    were ever reviewed. An empty diff must mean 'nothing to sign', never
+    'sign unlocked'."""
+    same_catalog = _catalog(_entry())
+    session = r.start_review("sha1", same_catalog, same_catalog)
+    assert session.changes == []  # diff_catalogs(x, x) -> []
+    assert r.all_entries_acknowledged(session) is True  # vacuously -- this is the trap
+    decision = r.can_sign(session, "sha1")  # blob matches, "everything" acknowledged
+    assert decision.ok is False
+    assert "nothing to sign" in decision.reason.lower()
+
+
+def test_can_sign_false_with_outstanding_blocking_risk_even_if_acknowledged():
+    """can_sign() must itself refuse a blocking risk finding -- today a
+    blocking finding only disables the GUI checkbox, so the pure gate must
+    not simply trust that the caller never acknowledged a blocking entry.
+    Acknowledge it directly here (bypassing any GUI checkbox-disable logic
+    entirely) to prove the gate catches it independently of the GUI."""
+    session = r.start_review(
+        "sha1",
+        _catalog(),
+        _catalog(_entry(config={"command": "bash", "args": ["-c", "evil"]})),
+    )
+    r.acknowledge_entry(session, "filesystem")
+    assert r.all_entries_acknowledged(session) is True
+    decision = r.can_sign(session, "sha1")
+    assert decision.ok is False
+    assert "blocking" in decision.reason.lower()
+
+
+# --------------------------------------------------------------------------- #
+# sign_precondition: the ref-resolution seam that used to hardcode "main"
+# --------------------------------------------------------------------------- #
+def test_sign_precondition_resolves_against_loaded_ref_not_hardcoded_main():
+    """The regression test for issue #68 finding 1's first bug:
+    ReviewWindow._on_sign used to hardcode fetch_ref(repo, "main") as the
+    TOCTOU comparison ref. For a PR review, _on_load pins the PR HEAD's
+    blob SHA, so comparing against main's SHA differs by definition and
+    Sign could never fire on the PR path.
+
+    The fake resolver below returns a DIFFERENT (deliberately wrong) SHA for
+    "main" than for the PR ref that was actually loaded. If
+    sign_precondition ever resolves against "main" instead of
+    session.loaded_ref, this test fails -- both via the recorded `calls`
+    list and via decision.ok flipping to False.
+    """
+    pr_ref = "refs/pull/42/head"
+    session = r.start_review("pr-blob-sha", _catalog(), _catalog(_entry()), loaded_ref=pr_ref)
+    r.acknowledge_entry(session, "filesystem")
+
+    calls: list[str] = []
+
+    def fake_resolver(ref: str) -> str:
+        calls.append(ref)
+        return {"main": "main-blob-sha-WRONG", pr_ref: "pr-blob-sha"}[ref]
+
+    decision = r.sign_precondition(session, fake_resolver)
+    assert calls == [pr_ref]  # never asked the resolver for "main"
+    assert decision.ok is True
+    assert decision.reason is None
+
+
+def test_sign_precondition_refuses_when_loaded_ref_blob_moved():
+    """Same seam, the negative case: if the loaded ref's blob SHA has moved
+    since review began (a new commit landed on the reviewed PR/branch), the
+    resolver reflects that and sign_precondition must refuse -- proving this
+    isn't just a hardcoded pass-through."""
+    pr_ref = "refs/pull/42/head"
+    session = r.start_review("pr-blob-sha", _catalog(), _catalog(_entry()), loaded_ref=pr_ref)
+    r.acknowledge_entry(session, "filesystem")
+
+    def fake_resolver(_ref: str) -> str:
+        return "pr-blob-sha-AFTER-A-NEW-PUSH"
+
+    decision = r.sign_precondition(session, fake_resolver)
+    assert decision.ok is False
+    assert "mismatch" in decision.reason.lower() or "changed" in decision.reason.lower()
+
+
+def test_sign_precondition_defaults_to_main_when_loaded_ref_unset():
+    """start_review()'s loaded_ref defaults to 'main' for source=main
+    reviews (and backward-compat with callers that don't pass it)."""
+    session = r.start_review("sha1", _catalog(), _catalog(_entry()))
+    assert session.loaded_ref == "main"
+    r.acknowledge_entry(session, "filesystem")
+
+    def fake_resolver(ref: str) -> str:
+        assert ref == "main"
+        return "sha1"
+
+    decision = r.sign_precondition(session, fake_resolver)
+    assert decision.ok is True
+
+
+# --------------------------------------------------------------------------- #
+# find_last_signed_catalog_raw: what source=main diffs against
+# --------------------------------------------------------------------------- #
+def test_find_last_signed_catalog_raw_returns_matching_candidate():
+    """Simulates walking catalog.json's git history: the CURRENT signature
+    covers an OLDER version of the bytes (a later commit changed
+    catalog.json without re-signing -- the exact bypass that produced
+    commit b08cf21). The first candidate that verifies against that
+    signature is 'the last catalog a maintainer actually signed'."""
+    seed, pubkey = r.generate_keypair()
+    old_raw = b'{"schema":1,"version":1,"servers":[]}'
+    new_raw = b'{"schema":1,"version":2,"servers":[]}'
+    sig = r.sign_catalog_bytes(old_raw, seed)  # signature covers the OLD bytes
+    found = r.find_last_signed_catalog_raw([new_raw, old_raw], sig, [pubkey])
+    assert found == old_raw
+
+
+def test_find_last_signed_catalog_raw_none_when_nothing_verifies():
+    seed, _pubkey = r.generate_keypair()
+    _other_seed, other_pubkey = r.generate_keypair()
+    raw = b'{"schema":1,"version":1,"servers":[]}'
+    sig = r.sign_catalog_bytes(raw, seed)
+    # Check against a pubkey list that does NOT include the signer's key.
+    assert r.find_last_signed_catalog_raw([raw], sig, [other_pubkey]) is None
+
+
 # --------------------------------------------------------------------------- #
 # catalog_signing_message: domain separation must match bcc_core exactly
 # --------------------------------------------------------------------------- #

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
@@ -2370,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"