Merge PR #77: tolerate non-object server values; keep named sets across a merge (#72, #73)

Two silent-failure bugs found auditing the v1.3.0 features.

#72 -- extract_servers called dict() on every server value, so a config
that was valid JSON but held a non-object server ("foo": "oops", a
number, a list, null) raised on load. The strict parse succeeded, so the
repair path never saw it, and the call sat outside the load try/except:
an unhandled traceback with the window half-swapped to the new profile.
It also made the #54 schema lint unreachable for the most likely
hand-edit mistake -- the load died before the linter ran.

Malformed values are now preserved verbatim on ServerEntry.raw (behind a
NO_RAW sentinel, since a literal JSON null is itself a malformed entry
worth keeping) and written back untouched on Save, so nothing is silently
deleted. lint_servers names the offending entry instead.

#73 -- the stale-file "Merge & save" path reloaded the file from disk and
re-applied the user's servers, but apply_servers only writes mcpServers
and _disabledMcpServers. Named server sets live under _bccServerSets in
the same file, so a set saved that session was dropped from disk and then
from memory, with no warning, on the path the user picks because it
sounds like the safe one.

BCC-owned keys are now declared in BCC_OWNED_KEYS and carried across by
carry_owned_keys, which reports genuinely contested keys so the status
line can say so. Deliberately one-directional: a key absent locally is
left alone on disk, because 'user deleted their last set' and 'another
machine just added sets' are indistinguishable and deleting someone
else's data is the worse failure.

Closes #72
Closes #73

.github/workflows/ci.yml

@@ -120,7 +120,7 @@ jobs:
       # below to the new key's base64 form, as its own reviewed change.
       - name: Verify data/catalog.json.sig
         env:
-          EXPECTED_CATALOG_PUBKEY_B64: "0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k="
+          EXPECTED_CATALOG_PUBKEY_B64: "082NOwVB7uURkvfyS3+knJ+40Fk6C9unsF47+2uPKo4="
         run: |
           python - <<'PY'
           import base64, os, pathlib, sys

README.md

@@ -90,21 +90,6 @@ key, because they protect different things and live in different places:
 | 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` |
 
-**Confused about which key is which, or what state either is in?** Run:
-
-```bash
-python catalog_console.py keys
-```
-
-It needs no passphrase (it never touches private key bytes) and prints a
-plain-English report for both keys: where each private half lives, whether
-it's present on this machine, its fingerprint, whether that fingerprint
-matches what's actually committed in `bcc_core.py`, `ci.yml`'s trust
-anchor, and `scripts/sign_checksums.py`, and whether
-`data/catalog.json.sig` currently verifies — ending with the exact command
-to run next for whatever state it finds. This is the check that would have
-caught [issue #68](../../issues/68)'s finding 5 incident before it happened.
-
 **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
@@ -125,29 +110,12 @@ 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, which signs `SHA256SUMS` (release checksums). It does
+the RELEASE key, not the catalog key:
-**not** sign `data/catalog.json` and is not the key `bcc_core.CATALOG_PUBKEYS`
-trusts:
 
 ```
-6BnPgJEHJFyVltFoLTCNadIsehjy00iiW8IRlC1TfhA=
+<PLACEHOLDER — AJ: paste the release public key from `catalog_console.py keygen --release` here>
 ```
 
-The catalog public key (Ed25519, base64, raw 32 bytes) — this is the key
-that signs `data/catalog.json` and is trusted via `bcc_core.CATALOG_PUBKEYS`
-and the CI trust anchor in `.github/workflows/ci.yml`. It is listed here
-for completeness, not because you need it to verify a download — use the
-*release* key above for that:
-
-```
-0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k=
-```
-
-Both keys above were rotated 2026-07 — see [issue #68](../../issues/68)
-finding 5. The prior (shared) key is retired and is deliberately **not**
-kept in either trust list; retaining a burned key would defeat the point
-of rotating it.
-
 ## Run from source
 
 ```bash
@@ -222,7 +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` (against `main`, an open PR, or the branch you have checked out — `--ref <branch>` to be explicit, e.g. mid key-rotation, so a rotation can be signed and pushed to its own branch *before* it's merged, never forcing a red `main`), generate/manage both signing keys (`keygen`, `keygen --release`), and report on their status (`keys`, no passphrase needed) — see [Signing keys](#signing-keys).
+- `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

@@ -1939,9 +1939,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 +2276,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 +2370,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 +2490,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 +2507,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()

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
 
@@ -178,16 +185,63 @@ 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
+
 
 # --------------------------------------------------------------------------- #
 # Discovery
@@ -452,13 +506,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 +621,8 @@ def apply_servers(cfg: dict, servers: list[ServerEntry]) -> dict:
     Write the server list back into `cfg` in place, preserving every other key
     and the position of `mcpServers`. Returns the same dict for convenience.
     """
-    enabled = {s.name: s.data for s in servers if s.enabled}
+    enabled = {s.name: s.config_value() for s in servers if s.enabled}
-    disabled = {s.name: s.data for s in servers if not s.enabled}
+    disabled = {s.name: s.config_value() for s in servers if not s.enabled}
 
     cfg["mcpServers"] = enabled  # replaces value if key existed; appends otherwise
     if disabled:
@@ -564,6 +635,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 +1479,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
 
@@ -2193,7 +2304,7 @@ CATALOG_DENIED_ENV_KEYS = frozenset(
 # still trust an older key: verify_catalog_signature() accepts a match
 # against ANY key in this list.
 CATALOG_PUBKEYS: list[bytes] = [
-    base64.b64decode("0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k="),
+    base64.b64decode("082NOwVB7uURkvfyS3+knJ+40Fk6C9unsF47+2uPKo4="),
 ]
 
 # Domain-separation prefix for the signed message. The signature covers

catalog_review.py

@@ -16,8 +16,6 @@ new surface" recurring-bug lesson).
 
 from __future__ import annotations
 
-import base64
-import hashlib
 import os
 import re
 from collections.abc import Callable
@@ -444,43 +442,18 @@ class ReviewSession:
     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).
-
-    `reattest` marks a KEY-ROTATION re-attestation pass (issue #68 finding 5
-    follow-up): the currently-trusted `bcc_core.CATALOG_PUBKEYS` key changed
-    and the existing `data/catalog.json.sig` no longer verifies under it.
-    Content-wise nothing may have changed -- `diff_catalogs(old, new)` can be
-    genuinely empty -- but the NEW key has never vouched for any of this
-    catalog before, so every entry needs a first-time attestation under the
-    new key, not a diff against the old one. When `reattest` is set,
-    `changes` is built as "every entry in `new_catalog`, presented as if
-    newly added" (via `diff_catalogs(None, new_catalog)`) instead of a
-    diff against `old_catalog`, so the acknowledge-gate in `can_sign()`
-    requires re-reviewing everything the new key will sign -- which is the
-    intended cost of a key rotation, not a bypass of the empty-diff guard.
     """
 
     pinned_blob_sha: str
     old_catalog: dict
     new_catalog: dict
     loaded_ref: str = "main"
-    reattest: bool = False
     changes: list[EntryChange] = field(default_factory=list)
     acknowledged: set[str] = field(default_factory=set)
 
     def __post_init__(self) -> None:
         if not self.changes:
-            if self.reattest:
+            self.changes = diff_catalogs(self.old_catalog, self.new_catalog)
-                # Every entry in the new catalog is treated as though it
-                # were newly added -- because, under the NEW signing key,
-                # it is: nothing this key signs was ever attested by it
-                # before. Reusing diff_catalogs(None, new) (rather than a
-                # bespoke code path) means the same "added" risk predicate
-                # (risk_new_entry) and the same EntryChange shape the rest
-                # of this module and the GUI already know how to render
-                # apply here unmodified.
-                self.changes = diff_catalogs(None, self.new_catalog)
-            else:
-                self.changes = diff_catalogs(self.old_catalog, self.new_catalog)
 
 
 def start_review(
@@ -488,14 +461,12 @@ def start_review(
     old_catalog: dict,
     new_catalog: dict,
     loaded_ref: str = "main",
-    reattest: bool = False,
 ) -> ReviewSession:
     return ReviewSession(
         pinned_blob_sha=pinned_blob_sha,
         old_catalog=old_catalog,
         new_catalog=new_catalog,
         loaded_ref=loaded_ref,
-        reattest=reattest,
     )
 
 
@@ -560,14 +531,6 @@ def can_sign(session: ReviewSession, current_blob_sha: str) -> SignDecision:
        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.)
-
-       This gate is unaffected by `session.reattest`: a key-rotation
-       re-attestation session's `changes` is built from
-       `diff_catalogs(None, new_catalog)` (see ReviewSession), which is
-       empty ONLY if the catalog itself has zero entries -- a genuinely
-       empty catalog either way. Rotation never manufactures a non-empty
-       changeset out of an empty one; it just changes *what* "non-empty"
-       is computed against.
     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
@@ -853,243 +816,3 @@ _NON_ASCII_RE = re.compile(r"[^\x00-\x7f]")
 
 def contains_non_ascii(s: str) -> bool:
     return bool(_NON_ASCII_RE.search(s))
-
-
-# --------------------------------------------------------------------------- #
-# Key status reporting (issue #62/#68 follow-up: "make key handling
-# comprehensible"). Pure functions only -- `catalog_console.py cmd_keys` is a
-# thin printer that gathers inputs (local key caches, source-file text, the
-# catalog + its .sig) and hands them here. NEVER touches private key bytes:
-# every input/output here is a public key, a fingerprint, or a status string.
-# --------------------------------------------------------------------------- #
-
-
-def fingerprint_pubkey(pubkey: bytes) -> str:
-    """Short, human-comparable fingerprint of a raw Ed25519 public key: the
-    first 16 hex chars of its SHA-256 digest, grouped in 4s (e.g. "3F2A 9C1B
-    44DE 08AA") so two fingerprints can be eyeballed for a mismatch the way a
-    PGP fingerprint is. Deliberately NOT the raw base64 pubkey itself in the
-    default short form (that's available via the full committed value in the
-    report) -- a fixed-width grouped hex string is easier to compare at a
-    glance and to read aloud/type over chat if needed. Never derived from,
-    and never printed alongside, any private key material.
-    """
-    digest = hashlib.sha256(pubkey).hexdigest().upper()[:16]
-    return " ".join(digest[i : i + 4] for i in range(0, len(digest), 4))
-
-
-_PUBKEY_LIST_B64_RE = re.compile(r'base64\.b64decode\(\s*"([^"]+)"\s*\)')
-
-
-def extract_pubkey_list_literal(source_text: str, var_name: str) -> list[bytes]:
-    """Best-effort extraction of a `<var_name>: list[bytes] = [...]` literal
-    (each entry a `base64.b64decode("...")` call, matching the exact style
-    bcc_core.CATALOG_PUBKEYS and scripts.sign_checksums.RELEASE_PUBKEYS are
-    both written in) straight out of Python source TEXT.
-
-    Deliberately a regex over text, not an import: `catalog_console.py keys`
-    must report on whatever ref/branch is checked out at the inspected repo
-    path, which may not be (and need not be) importable from the running
-    process's own sys.path. Returns [] if the variable isn't found in this
-    exact shape -- callers treat that as "nothing committed here", not an
-    error, since a report that can't parse a file should say so plainly
-    rather than crash the whole `keys` command over one malformed file.
-    """
-    match = re.search(
-        rf"{re.escape(var_name)}\s*:\s*list\[bytes\]\s*=\s*\[(.*?)\]", source_text, re.DOTALL
-    )
-    if not match:
-        return []
-    keys: list[bytes] = []
-    for b64 in _PUBKEY_LIST_B64_RE.findall(match.group(1)):
-        try:
-            keys.append(base64.b64decode(b64))
-        except ValueError:
-            continue
-    return keys
-
-
-_CI_TRUST_ANCHOR_RE = re.compile(r'EXPECTED_CATALOG_PUBKEY_B64:\s*"([^"]+)"')
-
-
-def extract_ci_trust_anchor_pubkey(ci_yml_text: str) -> bytes | None:
-    """Best-effort extraction of ci.yml's `EXPECTED_CATALOG_PUBKEY_B64` trust
-    anchor (issue #68 finding 4) from the workflow file's TEXT. Returns None
-    if the constant isn't found -- the `keys` report shows that plainly
-    ("not found in ci.yml") rather than raising.
-    """
-    match = _CI_TRUST_ANCHOR_RE.search(ci_yml_text)
-    if not match:
-        return None
-    try:
-        return base64.b64decode(match.group(1))
-    except ValueError:
-        return None
-
-
-@dataclass(frozen=True)
-class PubkeyLocationCheck:
-    """One place in the source tree a key's public half is expected to be
-    committed, and whether the fingerprint(s) found there match the key
-    stored locally."""
-
-    location: str
-    committed_fingerprints: tuple[str, ...]
-    status: str  # "match" | "mismatch" | "unknown" (no local key to compare against)
-
-
-@dataclass(frozen=True)
-class KeyStatus:
-    """Everything `catalog_console.py keys` reports about ONE signing key.
-    Built by key_status() below; rendered by render_key_status_report().
-    Never carries private key material -- every field here is safe to print.
-    """
-
-    kind: str  # "catalog" | "release"
-    display_name: str  # "CATALOG" | "RELEASE"
-    purpose: str  # one-line plain-English purpose
-    private_key_location: str  # human-readable, e.g. "on this machine, in the OS keychain"
-    local_exists: bool
-    local_fingerprint: str | None
-    locations: tuple[PubkeyLocationCheck, ...]
-    catalog_sig_status: str | None = None  # "valid" | "invalid" | "missing" | None (n/a)
-
-
-def key_status(
-    kind: str,
-    *,
-    display_name: str,
-    purpose: str,
-    private_key_location: str,
-    local_exists: bool,
-    local_pubkey: bytes | None,
-    locations: list[tuple[str, list[bytes]]],
-    catalog_sig_status: str | None = None,
-) -> KeyStatus:
-    """Pure assembly of a KeyStatus from already-resolved inputs (no file or
-    git I/O here -- that's catalog_console.py's job). `locations` is a list
-    of (label, committed_pubkeys) pairs, e.g.
-    [("bcc_core.CATALOG_PUBKEYS", [...]), ("ci.yml trust anchor", [...])],
-    so a key can be checked against every place its public half is expected
-    to be committed, independently -- this is the check that would have
-    caught bcc_core.CATALOG_PUBKEYS and ci.yml's trust anchor silently
-    drifting apart (issue #68 finding 4 was exactly that kind of drift).
-    """
-    checks: list[PubkeyLocationCheck] = []
-    for label, committed_pubkeys in locations:
-        fps = tuple(fingerprint_pubkey(pk) for pk in committed_pubkeys)
-        if local_pubkey is None:
-            status = "unknown"
-        elif local_pubkey in committed_pubkeys:
-            status = "match"
-        else:
-            status = "mismatch"
-        checks.append(
-            PubkeyLocationCheck(location=label, committed_fingerprints=fps, status=status)
-        )
-
-    return KeyStatus(
-        kind=kind,
-        display_name=display_name,
-        purpose=purpose,
-        private_key_location=private_key_location,
-        local_exists=local_exists,
-        local_fingerprint=fingerprint_pubkey(local_pubkey) if local_pubkey is not None else None,
-        locations=tuple(checks),
-        catalog_sig_status=catalog_sig_status,
-    )
-
-
-_LOCATION_STATUS_ICON = {"match": "✅", "mismatch": "❌", "unknown": "⚠️"}
-_LOCATION_STATUS_VERDICT = {
-    "match": "MATCHES the local private key",
-    "mismatch": "DOES NOT MATCH the local private key",
-    "unknown": "cannot compare -- no local key to check against",
-}
-_CATALOG_SIG_STATUS_LINE = {
-    "valid": "✅ data/catalog.json.sig verifies under the committed CATALOG_PUBKEYS.",
-    "invalid": (
-        "❌ data/catalog.json.sig does NOT verify under the committed CATALOG_PUBKEYS -- "
-        "the catalog needs re-signing (Load → acknowledge all → Sign)."
-    ),
-    "missing": (
-        "⚠️ data/catalog.json.sig is missing entirely -- the catalog has never been signed."
-    ),
-}
-
-
-def recommend_next_steps(statuses: list[KeyStatus]) -> list[str]:
-    """The pure "what to do next" logic behind the keys report's closing
-    section -- one concrete, runnable-looking instruction per problem found,
-    naming the exact key involved (never just "the key"). Returns a single
-    reassuring line if nothing needs attention."""
-    steps: list[str] = []
-    for s in statuses:
-        if not s.local_exists:
-            flag = " --release" if s.kind == "release" else ""
-            steps.append(
-                f"{s.display_name} key has never been generated on this machine -- run "
-                f"`python catalog_console.py keygen{flag}`."
-            )
-            continue
-        for loc in s.locations:
-            if loc.status == "mismatch":
-                steps.append(
-                    f"{s.display_name} key's local fingerprint does not match "
-                    f"{loc.location} -- update {loc.location} to the fingerprint shown "
-                    "above (or, if this is unexpected, treat the committed key as "
-                    "untrusted and investigate before doing anything else)."
-                )
-            elif loc.status == "unknown":
-                steps.append(
-                    f"{s.display_name} key's local fingerprint could not be checked against "
-                    f"{loc.location} -- re-run keygen (or, for an older install, unlock the "
-                    "key once) so its public half is cached locally."
-                )
-        if s.kind == "catalog" and s.catalog_sig_status in ("invalid", "missing"):
-            steps.append(
-                "The catalog needs re-signing: run `python catalog_console.py gui --repo .` "
-                "and Load → acknowledge every entry → Sign. If main is red because "
-                "of a key rotation, load the branch with the rotation instead of main "
-                "(current-branch / --ref source) so the fix lands before merge."
-            )
-    if not steps:
-        steps.append("Everything is consistent -- no action needed.")
-    return steps
-
-
-def render_key_status_report(statuses: list[KeyStatus]) -> str:
-    """Render a full, plain-English-first key status report as one string.
-    `catalog_console.py cmd_keys` prints this verbatim -- the CLI is a thin
-    printer over this pure function, which is what makes the report's
-    content (not just its plumbing) unit-testable."""
-    lines: list[str] = []
-    for s in statuses:
-        lines.append(f"=== {s.display_name} KEY ===")
-        lines.append(s.purpose)
-        lines.append(f"Private half lives: {s.private_key_location}")
-        if s.local_exists and s.local_fingerprint:
-            lines.append(f"Exists locally: yes (fingerprint {s.local_fingerprint})")
-        elif s.local_exists:
-            lines.append("Exists locally: yes (fingerprint unknown -- re-run keygen to cache it)")
-        else:
-            lines.append("Exists locally: no")
-        for loc in s.locations:
-            icon = _LOCATION_STATUS_ICON.get(loc.status, "?")
-            fps = (
-                ", ".join(loc.committed_fingerprints)
-                if loc.committed_fingerprints
-                else "(nothing committed here)"
-            )
-            verdict = _LOCATION_STATUS_VERDICT.get(loc.status, loc.status)
-            lines.append(f"  {icon} {loc.location}: {fps} -- {verdict}")
-        if s.catalog_sig_status is not None:
-            lines.append(
-                f"Catalog signature: {_CATALOG_SIG_STATUS_LINE.get(s.catalog_sig_status, s.catalog_sig_status)}"
-            )
-        lines.append("")
-
-    lines.append("What to do next:")
-    for step in recommend_next_steps(statuses):
-        lines.append(f"  - {step}")
-    return "\n".join(lines)

scripts/sign_checksums.py

@@ -53,17 +53,13 @@ DOMAIN_PREFIX = b"bcc-release-v1|"
 # the signature on every past release: verification accepts a match against
 # ANY key here.
 #
-# Populated by the maintainer via:
+# 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
-# Rotated 2026-07 (issue #68 finding 5 / #68 CI-exposure incident): the
+# This is intentionally NOT pre-populated with a placeholder that looks
-# original key was shared with the catalog key and had been exposed to CI,
+# like a real key -- release.yml's signing-smoke-test fails closed (loudly)
-# so both keypairs were regenerated as separate, disjoint keys. This list
+# on an empty list rather than silently verifying against nothing.
-# holds only the current release key -- if release.yml's signing-smoke-test
+RELEASE_PUBKEYS: list[bytes] = []
-# ever sees this list empty, it fails closed (loudly) rather than silently
-# verifying against nothing.
-RELEASE_PUBKEYS: list[bytes] = [
-    base64.b64decode("6BnPgJEHJFyVltFoLTCNadIsehjy00iiW8IRlC1TfhA="),
-]
 
 CHUNK_SIZE = 1024 * 1024
 

tests/test_catalog_console_git.py

@@ -1,215 +0,0 @@
-"""Tests for catalog_console.py's non-Qt git plumbing and ref-resolution
-seam (issue #68 rotation-completability fix).
-
-catalog_console.py is importable here WITHOUT PySide6 -- its Qt import is
-guarded (`_PYSIDE6_AVAILABLE`) precisely so `keygen`, `show-seed-b64`,
-`keys`, and this git plumbing stay usable (and testable) wherever PySide6
-isn't installed, including this CI test job, which never installs it. If
-PySide6 genuinely isn't importable in this environment, that itself
-exercises the guard path -- see test_module_imports_without_pyside6.
-"""
-
-from __future__ import annotations
-
-import subprocess
-import sys
-from pathlib import Path
-
-sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
-
-import catalog_console as cc
-import catalog_review as review
-
-_SEED_CATALOG = b'{"schema": 1, "version": 1, "servers": []}'
-_SEED_SIG = b"\x00" * 64
-
-
-def _run(*args: str, cwd: Path) -> None:
-    subprocess.run(["git", *args], cwd=cwd, check=True, capture_output=True)
-
-
-def _init_bare_and_clone(tmp_path: Path) -> tuple[Path, Path]:
-    """A bare "origin" repo with `main` and `rotation-branch` both seeded
-    with a catalog + (dummy) signature, plus a working clone with `origin`
-    already configured -- mirroring the tokened-remote clone
-    catalog_console.py's git plumbing is always run against."""
-    origin = tmp_path / "origin.git"
-    _run("init", "--bare", str(origin), cwd=tmp_path)
-
-    seed = tmp_path / "seed"
-    _run("clone", str(origin), str(seed), cwd=tmp_path)
-    _run("config", "user.email", "test@example.com", cwd=seed)
-    _run("config", "user.name", "Test", cwd=seed)
-
-    (seed / "data").mkdir()
-    (seed / "data" / "catalog.json").write_bytes(_SEED_CATALOG)
-    (seed / "data" / "catalog.json.sig").write_bytes(_SEED_SIG)
-    _run("add", "-A", cwd=seed)
-    _run("commit", "-m", "seed", cwd=seed)
-    _run("push", "origin", "HEAD:refs/heads/main", cwd=seed)
-    _run("checkout", "-b", "rotation-branch", cwd=seed)
-    _run("push", "origin", "HEAD:refs/heads/rotation-branch", cwd=seed)
-
-    clone = tmp_path / "work"
-    _run("clone", str(origin), str(clone), cwd=tmp_path)
-    _run("config", "user.email", "test@example.com", cwd=clone)
-    _run("config", "user.name", "Test", cwd=clone)
-    return origin, clone
-
-
-# --------------------------------------------------------------------------- #
-# The module must stay importable without PySide6 -- this IS the fix that
-# lets `keys`/`keygen`/`show-seed-b64` (and this whole test file) run
-# somewhere PySide6 isn't installed.
-# --------------------------------------------------------------------------- #
-def test_module_imports_without_pyside6():
-    assert hasattr(cc, "_PYSIDE6_AVAILABLE")
-    # This CI test job never installs PySide6 (see .github/workflows/ci.yml
-    # "Install test dependencies": pytest + cryptography only) -- so on CI,
-    # this assertion is itself proof the guard is doing its job. Locally,
-    # where a maintainer's env DOES have PySide6, it's fine either way; the
-    # only real assertion this test needs is "importing the module never
-    # raises", which happened just by getting this far.
-    assert cc._PYSIDE6_AVAILABLE in (True, False)
-
-
-def test_cmd_gui_fails_soft_without_pyside6(monkeypatch, capsys):
-    if cc._PYSIDE6_AVAILABLE:
-        return  # nothing to prove where PySide6 IS available
-    import argparse
-
-    args = argparse.Namespace(repo=".", ref=None)
-    assert cc.cmd_gui(args) == 1
-    assert "PySide6" in capsys.readouterr().err
-
-
-# --------------------------------------------------------------------------- #
-# compute_own_refs: the PURE ref-resolution seam. No git, no Qt.
-# --------------------------------------------------------------------------- #
-def test_compute_own_refs_defaults_to_main_only():
-    assert cc.compute_own_refs(None, None) == ["main"]
-
-
-def test_compute_own_refs_adds_detected_branch():
-    assert cc.compute_own_refs(None, "chore/68-key-rotation") == [
-        "main",
-        "chore/68-key-rotation",
-    ]
-
-
-def test_compute_own_refs_explicit_ref_overrides_detected_branch():
-    assert cc.compute_own_refs("explicit-branch", "detected-branch") == [
-        "main",
-        "explicit-branch",
-    ]
-
-
-def test_compute_own_refs_does_not_duplicate_main():
-    assert cc.compute_own_refs(None, "main") == ["main"]
-    assert cc.compute_own_refs("main", "some-other-branch") == ["main"]
-
-
-# --------------------------------------------------------------------------- #
-# current_branch: git plumbing, no Qt.
-# --------------------------------------------------------------------------- #
-def test_current_branch_detects_checked_out_branch(tmp_path):
-    _origin, clone = _init_bare_and_clone(tmp_path)
-    _run("fetch", "origin", "rotation-branch", cwd=clone)
-    _run("checkout", "-B", "rotation-branch", "origin/rotation-branch", cwd=clone)
-    assert cc.current_branch(clone) == "rotation-branch"
-
-
-def test_current_branch_none_on_detached_head(tmp_path):
-    _origin, clone = _init_bare_and_clone(tmp_path)
-    commit = cc.fetch_ref(clone, "main")
-    _run("checkout", commit, cwd=clone)
-    assert cc.current_branch(clone) is None
-
-
-# --------------------------------------------------------------------------- #
-# commit_and_push_signed_catalog: MUST target the given branch, never a
-# hardcoded "main" -- issue #68's completability fix. This is exactly the
-# bug that, before the fix, would have made ReviewWindow._on_sign push a
-# PR/branch review's signature straight to main regardless of what was
-# actually reviewed.
-# --------------------------------------------------------------------------- #
-def test_commit_and_push_signed_catalog_targets_the_given_branch_not_main(tmp_path):
-    _origin, clone = _init_bare_and_clone(tmp_path)
-
-    new_raw = b'{"schema": 1, "version": 2, "servers": []}'
-    new_sig = b"\x01" * 64
-    cc.commit_and_push_signed_catalog(clone, new_raw, new_sig, branch="rotation-branch")
-
-    rotation_commit = cc.fetch_ref(clone, "rotation-branch")
-    rotation_raw, _sha = cc.read_catalog_at_commit(clone, rotation_commit)
-    assert rotation_raw == new_raw
-
-    # main on the shared origin must be COMPLETELY untouched by a sign that
-    # was reviewed and pushed against rotation-branch.
-    main_commit = cc.fetch_ref(clone, "main")
-    main_raw, _sha = cc.read_catalog_at_commit(clone, main_commit)
-    assert main_raw == _SEED_CATALOG
-
-
-def test_commit_and_push_signed_catalog_still_defaults_to_main(tmp_path):
-    """Backward-compatible default: callers that don't pass `branch` (there
-    are none left in catalog_console.py itself, but the signature keeps the
-    default for any other caller / test fixture) still push to main."""
-    _origin, clone = _init_bare_and_clone(tmp_path)
-
-    new_raw = b'{"schema": 1, "version": 2, "servers": []}'
-    new_sig = b"\x01" * 64
-    cc.commit_and_push_signed_catalog(clone, new_raw, new_sig)
-
-    main_commit = cc.fetch_ref(clone, "main")
-    main_raw, _sha = cc.read_catalog_at_commit(clone, main_commit)
-    assert main_raw == new_raw
-
-    rotation_commit = cc.fetch_ref(clone, "rotation-branch")
-    rotation_raw, _sha = cc.read_catalog_at_commit(clone, rotation_commit)
-    assert rotation_raw == _SEED_CATALOG  # untouched
-
-
-# --------------------------------------------------------------------------- #
-# catalog_sig_status_on_disk: the check behind `keys`' "does catalog.json.sig
-# currently verify?" line -- this is precisely the check that would have
-# caught the current chore/68-key-rotation state (bcc_core.CATALOG_PUBKEYS
-# rotated, data/catalog.json.sig still signed by the retired key).
-# --------------------------------------------------------------------------- #
-def test_catalog_sig_status_on_disk_valid(tmp_path):
-    seed, pub = review.generate_keypair()
-    raw = b'{"schema": 1, "version": 1, "servers": []}'
-    sig = review.sign_catalog_bytes(raw, seed)
-
-    (tmp_path / "data").mkdir()
-    (tmp_path / "data" / "catalog.json").write_bytes(raw)
-    (tmp_path / "data" / "catalog.json.sig").write_bytes(sig)
-
-    assert cc.catalog_sig_status_on_disk(tmp_path, [pub]) == "valid"
-
-
-def test_catalog_sig_status_on_disk_invalid_when_pubkey_rotated(tmp_path):
-    """The exact chore/68-key-rotation scenario: signed by an OLD key, but
-    the committed pubkey list now only has the NEW key."""
-    old_seed, _old_pub = review.generate_keypair()
-    _new_seed, new_pub = review.generate_keypair()
-    raw = b'{"schema": 1, "version": 1, "servers": []}'
-    sig = review.sign_catalog_bytes(raw, old_seed)
-
-    (tmp_path / "data").mkdir()
-    (tmp_path / "data" / "catalog.json").write_bytes(raw)
-    (tmp_path / "data" / "catalog.json.sig").write_bytes(sig)
-
-    assert cc.catalog_sig_status_on_disk(tmp_path, [new_pub]) == "invalid"
-
-
-def test_catalog_sig_status_on_disk_missing_when_no_sig_file(tmp_path):
-    (tmp_path / "data").mkdir()
-    (tmp_path / "data" / "catalog.json").write_bytes(b"{}")
-    assert cc.catalog_sig_status_on_disk(tmp_path, []) == "missing"
-
-
-def test_catalog_sig_status_on_disk_missing_when_no_catalog_file(tmp_path):
-    (tmp_path / "data").mkdir()
-    (tmp_path / "data" / "catalog.json.sig").write_bytes(b"\x00" * 64)
-    assert cc.catalog_sig_status_on_disk(tmp_path, []) == "missing"

tests/test_catalog_review.py

@@ -491,98 +491,6 @@ def test_sign_precondition_defaults_to_main_when_loaded_ref_unset():
     assert decision.ok is True
 
 
-# --------------------------------------------------------------------------- #
-# ReviewSession(reattest=True): key-rotation re-attestation (issue #68
-# finding 5 follow-up). After rotating bcc_core.CATALOG_PUBKEYS, the
-# existing data/catalog.json.sig no longer verifies under the new key even
-# though catalog CONTENT is unchanged -- diff_catalogs(old, new) would be
-# empty, and an empty changeset must never unlock Sign (can_sign gate 0).
-# Re-attestation mode sidesteps that correctly: instead of diffing against
-# the (now-untrustworthy) last-signed content, it treats every entry as
-# requiring a fresh acknowledgement, exactly like a brand-new catalog.
-# --------------------------------------------------------------------------- #
-def test_reattest_mode_with_unchanged_content_yields_one_change_per_entry():
-    same = _catalog(_entry(id="a"), _entry(id="b"), _entry(id="c"))
-    session = r.start_review("sha1", same, same, reattest=True)
-    assert len(session.changes) == 3
-    assert {c_.entry_id for c_ in session.changes} == {"a", "b", "c"}
-    # Presented "as if newly added" -- old_catalog plays no role here.
-    assert all(c_.status == "added" for c_ in session.changes)
-    assert all(c_.old is None for c_ in session.changes)
-
-
-def test_reattest_mode_can_sign_refuses_until_all_acknowledged_then_permits():
-    same = _catalog(_entry(id="a"), _entry(id="b"), _entry(id="c"))
-    session = r.start_review("sha1", same, same, reattest=True)
-
-    decision = r.can_sign(session, "sha1")
-    assert decision.ok is False
-    assert "acknowledged" in decision.reason.lower()
-
-    r.acknowledge_entry(session, "a")
-    r.acknowledge_entry(session, "b")
-    decision = r.can_sign(session, "sha1")
-    assert decision.ok is False  # "c" still outstanding
-    assert "acknowledged" in decision.reason.lower()
-
-    r.acknowledge_entry(session, "c")
-    decision = r.can_sign(session, "sha1")
-    assert decision.ok is True
-    assert decision.reason is None
-
-
-def test_normal_mode_with_unchanged_content_still_refuses_empty_diff():
-    """The rotation path must NOT become a general bypass of the empty-diff
-    guard: reattest=False (the default) against identical old/new catalogs
-    must behave exactly as before -- can_sign refuses with 'nothing to
-    sign', full stop."""
-    same = _catalog(_entry(id="a"), _entry(id="b"))
-    session = r.start_review("sha1", same, same)  # reattest defaults False
-    assert session.changes == []
-    decision = r.can_sign(session, "sha1")
-    assert decision.ok is False
-    assert "nothing to sign" in decision.reason.lower()
-
-
-def test_reattest_mode_still_enforces_blocking_risk_check():
-    """Re-attestation must not relax the blocking-risk gate: a
-    disallowed-command entry blocks Sign even with every entry
-    acknowledged."""
-    cat = _catalog(
-        _entry(id="a"),
-        _entry(id="evil", config={"command": "bash", "args": ["-c", "rm -rf /"]}),
-    )
-    session = r.start_review("sha1", cat, cat, reattest=True)
-    assert len(session.changes) == 2
-    r.acknowledge_entry(session, "a")
-    r.acknowledge_entry(session, "evil")
-    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()
-
-
-def test_reattest_mode_still_enforces_toctou_pin():
-    """Re-attestation must not relax the TOCTOU blob-SHA pin: acknowledging
-    everything is not enough if the bytes moved underneath the review."""
-    same = _catalog(_entry(id="a"))
-    session = r.start_review("sha1", same, same, reattest=True)
-    r.acknowledge_entry(session, "a")
-    decision = r.can_sign(session, "sha1")
-    assert decision.ok is True  # sanity: matches when blob is unchanged
-
-    decision = r.can_sign(session, "sha2-a-new-commit-landed")
-    assert decision.ok is False
-    assert "mismatch" in decision.reason.lower() or "changed" in decision.reason.lower()
-
-
-def test_reattest_defaults_to_false():
-    """start_review()'s reattest parameter defaults to False -- normal
-    (diff-based) review is the default behaviour, never silently entered."""
-    session = r.start_review("sha1", _catalog(), _catalog(_entry()))
-    assert session.reattest is False
-
-
 # --------------------------------------------------------------------------- #
 # find_last_signed_catalog_raw: what source=main diffs against
 # --------------------------------------------------------------------------- #
@@ -785,245 +693,3 @@ def test_contains_non_ascii_true():
 
 def test_contains_non_ascii_false():
     assert r.contains_non_ascii("package") is False
-
-
-# --------------------------------------------------------------------------- #
-# fingerprint_pubkey
-# --------------------------------------------------------------------------- #
-def test_fingerprint_pubkey_is_deterministic():
-    pub = b"\x01" * 32
-    assert r.fingerprint_pubkey(pub) == r.fingerprint_pubkey(pub)
-
-
-def test_fingerprint_pubkey_differs_for_different_keys():
-    assert r.fingerprint_pubkey(b"\x01" * 32) != r.fingerprint_pubkey(b"\x02" * 32)
-
-
-def test_fingerprint_pubkey_never_contains_the_key_bytes_themselves():
-    pub = b"\x42" * 32
-    fp = r.fingerprint_pubkey(pub)
-    assert pub.hex() not in fp.lower().replace(" ", "")
-
-
-# --------------------------------------------------------------------------- #
-# extract_pubkey_list_literal / extract_ci_trust_anchor_pubkey: text parsing
-# for `catalog_console.py keys`, exercised here with no file I/O.
-# --------------------------------------------------------------------------- #
-def test_extract_pubkey_list_literal_single_key():
-    _seed, pub = r.generate_keypair()
-    import base64
-
-    text = (
-        "CATALOG_PUBKEYS: list[bytes] = [\n"
-        f'    base64.b64decode("{base64.b64encode(pub).decode()}"),\n'
-        "]\n"
-    )
-    assert r.extract_pubkey_list_literal(text, "CATALOG_PUBKEYS") == [pub]
-
-
-def test_extract_pubkey_list_literal_multiple_keys():
-    import base64
-
-    pubs = [r.generate_keypair()[1] for _ in range(2)]
-    body = ",\n".join(f'    base64.b64decode("{base64.b64encode(p).decode()}")' for p in pubs)
-    text = f"RELEASE_PUBKEYS: list[bytes] = [\n{body},\n]\n"
-    assert r.extract_pubkey_list_literal(text, "RELEASE_PUBKEYS") == pubs
-
-
-def test_extract_pubkey_list_literal_missing_variable_returns_empty():
-    assert r.extract_pubkey_list_literal("some unrelated text", "CATALOG_PUBKEYS") == []
-
-
-def test_extract_pubkey_list_literal_does_not_match_a_different_variable():
-    import base64
-
-    _seed, pub = r.generate_keypair()
-    text = f'OTHER_PUBKEYS: list[bytes] = [base64.b64decode("{base64.b64encode(pub).decode()}")]\n'
-    assert r.extract_pubkey_list_literal(text, "CATALOG_PUBKEYS") == []
-
-
-def test_extract_ci_trust_anchor_pubkey_found():
-    import base64
-
-    _seed, pub = r.generate_keypair()
-    text = f'          EXPECTED_CATALOG_PUBKEY_B64: "{base64.b64encode(pub).decode()}"\n'
-    assert r.extract_ci_trust_anchor_pubkey(text) == pub
-
-
-def test_extract_ci_trust_anchor_pubkey_missing_returns_none():
-    assert r.extract_ci_trust_anchor_pubkey("no anchor here") is None
-
-
-# --------------------------------------------------------------------------- #
-# key_status / render_key_status_report / recommend_next_steps
-# --------------------------------------------------------------------------- #
-def _kw(**overrides):
-    base = dict(
-        kind="catalog",
-        display_name="CATALOG",
-        purpose="Signs the catalog.",
-        private_key_location="on this machine",
-        local_exists=True,
-        local_pubkey=b"\x01" * 32,
-        locations=[("bcc_core.CATALOG_PUBKEYS", [b"\x01" * 32])],
-        catalog_sig_status="valid",
-    )
-    base.update(overrides)
-    return base
-
-
-def test_key_status_reports_match_when_local_pubkey_in_committed_list():
-    status = r.key_status(**_kw())
-    assert status.locations[0].status == "match"
-
-
-def test_key_status_reports_mismatch_when_local_pubkey_not_in_committed_list():
-    status = r.key_status(**_kw(locations=[("bcc_core.CATALOG_PUBKEYS", [b"\x02" * 32])]))
-    assert status.locations[0].status == "mismatch"
-
-
-def test_key_status_reports_unknown_when_no_local_pubkey():
-    status = r.key_status(**_kw(local_pubkey=None, local_exists=False))
-    assert status.locations[0].status == "unknown"
-    assert status.local_fingerprint is None
-
-
-def test_key_status_never_carries_a_local_fingerprint_when_key_absent():
-    status = r.key_status(**_kw(local_pubkey=None, local_exists=False))
-    assert status.local_exists is False
-    assert status.local_fingerprint is None
-
-
-def test_key_status_fingerprint_matches_fingerprint_pubkey_helper():
-    pub = b"\x03" * 32
-    status = r.key_status(**_kw(local_pubkey=pub, locations=[("x", [pub])]))
-    assert status.local_fingerprint == r.fingerprint_pubkey(pub)
-
-
-def test_key_status_checks_multiple_locations_independently():
-    """A key can match one committed location and mismatch another -- this
-    is exactly the drift issue #68 finding 4 was about (bcc_core.py and
-    ci.yml silently disagreeing on the trust anchor)."""
-    pub = b"\x04" * 32
-    other = b"\x05" * 32
-    status = r.key_status(
-        **_kw(
-            local_pubkey=pub,
-            locations=[
-                ("bcc_core.CATALOG_PUBKEYS", [pub]),
-                ("ci.yml trust anchor", [other]),
-            ],
-        )
-    )
-    assert status.locations[0].status == "match"
-    assert status.locations[1].status == "mismatch"
-
-
-def test_recommend_next_steps_flags_never_generated_key():
-    status = r.key_status(**_kw(local_exists=False, local_pubkey=None, locations=[]))
-    steps = r.recommend_next_steps([status])
-    assert any("keygen" in s and "CATALOG" in s for s in steps)
-
-
-def test_recommend_next_steps_release_key_uses_release_flag():
-    status = r.key_status(
-        kind="release",
-        display_name="RELEASE",
-        purpose="Signs checksums.",
-        private_key_location="not generated yet",
-        local_exists=False,
-        local_pubkey=None,
-        locations=[],
-    )
-    steps = r.recommend_next_steps([status])
-    assert any("keygen --release" in s for s in steps)
-
-
-def test_recommend_next_steps_flags_mismatch_by_location_name():
-    status = r.key_status(**_kw(locations=[("bcc_core.CATALOG_PUBKEYS", [b"\x99" * 32])]))
-    steps = r.recommend_next_steps([status])
-    assert any("bcc_core.CATALOG_PUBKEYS" in s and "CATALOG" in s for s in steps)
-
-
-def test_recommend_next_steps_flags_invalid_catalog_signature():
-    status = r.key_status(**_kw(catalog_sig_status="invalid"))
-    steps = r.recommend_next_steps([status])
-    assert any("re-signing" in s for s in steps)
-
-
-def test_recommend_next_steps_flags_missing_catalog_signature():
-    status = r.key_status(**_kw(catalog_sig_status="missing"))
-    steps = r.recommend_next_steps([status])
-    assert any("re-signing" in s for s in steps)
-
-
-def test_recommend_next_steps_all_clear_when_nothing_wrong():
-    status = r.key_status(**_kw())
-    steps = r.recommend_next_steps([status])
-    assert steps == ["Everything is consistent -- no action needed."]
-
-
-def test_recommend_next_steps_release_key_has_no_catalog_signature_advice():
-    """A mismatched RELEASE key must never trigger catalog-signing advice --
-    the two keys' remediation paths must not bleed into each other."""
-    status = r.key_status(
-        kind="release",
-        display_name="RELEASE",
-        purpose="Signs checksums.",
-        private_key_location="on this machine",
-        local_exists=True,
-        local_pubkey=b"\x06" * 32,
-        locations=[("scripts/sign_checksums.py RELEASE_PUBKEYS", [b"\x07" * 32])],
-        catalog_sig_status=None,
-    )
-    steps = r.recommend_next_steps([status])
-    assert not any("re-signing" in s for s in steps)
-    assert any("RELEASE" in s for s in steps)
-
-
-def test_render_key_status_report_never_prints_private_key_material():
-    """The report string must be built ONLY from public inputs. Sanity
-    check: no field on KeyStatus/PubkeyLocationCheck is capable of holding
-    private key bytes in the first place (there's no such field to leak),
-    and the render function only touches fields that exist -- this test
-    guards against a future field addition reintroducing that risk."""
-    status = r.key_status(**_kw())
-    text = r.render_key_status_report([status])
-    assert "CATALOG" in text
-    assert (
-        "purpose" not in text.lower() or "Signs the catalog." in text
-    )  # sanity, not a real secret
-    # No 64-hex-char (or longer) run anywhere -- a raw 32-byte seed/sig
-    # would show up as one if it were ever accidentally interpolated in.
-    import re as _re
-
-    assert not _re.search(r"[0-9a-fA-F]{64,}", text)
-
-
-def test_render_key_status_report_names_the_specific_key_not_generic_the_key():
-    status = r.key_status(**_kw())
-    text = r.render_key_status_report([status])
-    assert "CATALOG KEY" in text
-    assert "the key" not in text.lower()
-
-
-def test_render_key_status_report_includes_catalog_signature_line_only_for_catalog():
-    catalog_status = r.key_status(**_kw())
-    release_status = r.key_status(
-        kind="release",
-        display_name="RELEASE",
-        purpose="Signs checksums.",
-        private_key_location="on this machine",
-        local_exists=True,
-        local_pubkey=b"\x08" * 32,
-        locations=[("scripts/sign_checksums.py RELEASE_PUBKEYS", [b"\x08" * 32])],
-        catalog_sig_status=None,
-    )
-    text = r.render_key_status_report([catalog_status, release_status])
-    assert text.count("Catalog signature:") == 1
-
-
-def test_render_key_status_report_ends_with_what_to_do_next_section():
-    status = r.key_status(**_kw())
-    text = r.render_key_status_report([status])
-    assert "What to do next:" in text

tests/test_core.py

@@ -2370,3 +2370,143 @@ 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