feat(catalog-console): a keys status command, and complete rotation without a red main (#62 , #68 )

Two problems from issue #68's follow-up review: 1. The maintainer -- the only person who will ever use this tool -- cannot reliably tell which of the two signing keys is which or what state either is in. He already pasted a private key into a chat window because a prompt was ambiguous. That's a defect in this tool, not user error. 2. PR #71 rotates bcc_core.CATALOG_PUBKEYS, which makes data/catalog.json.sig (signed by the retired key) stop verifying and the CI catalog-signature job go red. The Console could previously only load/sign against `main`, so the only way through was to merge a red PR and fix main afterwards -- normalizing exactly the alarm fatigue this whole design exists to prevent. Task 1 -- `python catalog_console.py keys`: A plain-English-first status report for BOTH keys: purpose, where the private half lives, whether it exists locally, its fingerprint, whether that fingerprint matches every place its public half is expected to be committed (bcc_core.CATALOG_PUBKEYS, ci.yml's trust anchor, and scripts/sign_checksums.RELEASE_PUBKEYS -- checked independently, since issue #68 finding 4 was exactly bcc_core.py and ci.yml silently drifting apart), and whether data/catalog.json.sig currently verifies -- ending with the exact command to run next. Needs no passphrase and never touches private key bytes: a plaintext public-key cache (store_public_key/load_public_key) is written alongside the existing encrypted private blob at keygen time, precisely so this command can report a fingerprint without decrypting anything. The status/report logic (key_status, render_key_status_report, recommend_next_steps, fingerprint_pubkey, extract_pubkey_list_literal, extract_ci_trust_anchor_pubkey) is pure and lives in catalog_review.py; cmd_keys in catalog_console.py is a thin printer over it, per the project's existing pure-core/thin-GUI split. Task 2 -- rotation completable without a red main: ReviewWindow now offers a "current branch" source (auto-detected via `current_branch()`, or --ref to name one explicitly) alongside "main" and open PRs. Loading it runs the exact same diff-against-last-signed / rotation-detection logic "main" always used (_load_own_ref, extracted from the old hardcoded-to-main _on_load), just parameterized on the ref. Signing now pushes to session.loaded_ref, never a hardcoded "main" (commit_and_push_signed_catalog's branch param was already there -- only the call site was wrong). The ref-list computation itself is a pure function (compute_own_refs) so this seam is unit-testable without git or Qt. None of can_sign()'s guards (empty-diff, acknowledge-all, blocking-risk, TOCTOU) were touched. This lets a rotation branch be reviewed, re-attested (every entry, since the new key never vouched for any of them -- issue #68 finding 5 follow-up), signed, and pushed to ITS OWN branch before it's ever merged. Task 3 -- label the keys everywhere: PassphraseDialog now shows which key (CATALOG vs RELEASE) and its fingerprint before the passphrase field, both in its window title and its prompt text -- the exact ambiguity that led to a private key being pasted into a chat window. cmd_keygen's stored-key confirmation now reads "CATALOG private key encrypted..." / "RELEASE private key encrypted..." instead of a capitalized-lowercase kind. The reattest banner now says "CATALOG signing key" / "CATALOG key" throughout instead of "the key". PySide6's import is now guarded (try/except -> _PYSIDE6_AVAILABLE) and every GUI class definition that depends on it moved under `if _PYSIDE6_AVAILABLE:`. `keygen`, `show-seed-b64`, and the new `keys` command have no GUI dependency and now work (and are testable) in an environment without PySide6 -- which is exactly this repo's own `test` CI job (pytest + cryptography only, no PySide6). `gui` fails with a clear message instead of an ImportError stack trace if it's missing. Tests: 26 new pure-function tests in tests/test_catalog_review.py (fingerprint_pubkey, extract_pubkey_list_literal, extract_ci_trust_anchor_pubkey, key_status, recommend_next_steps, render_key_status_report) and a new tests/test_catalog_console_git.py (14 tests) covering compute_own_refs, current_branch, commit_and_push_signed_catalog's branch targeting, and catalog_sig_status_on_disk against real local git repos -- importing catalog_console.py directly, proving it works without PySide6. 400 passed, 1 skipped (pre-existing). ruff check / ruff format --check clean.
chore(#68 ): rotate signing keys, key-rotation re-attestation mode, harden show-seed-b64
2026-07-13 13:10:36 -04:00 · 2026-07-13 12:24:28 -04:00
14 changed files with 1849 additions and 2958 deletions
@@ -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: "082NOwVB7uURkvfyS3+knJ+40Fk6C9unsF47+2uPKo4="
+          EXPECTED_CATALOG_PUBKEY_B64: "0s24PmkZcTT5yxNDdyTPHl5fyxArrHNPJKBjnXoQd8k="
        run: |
          python - <<'PY'
          import base64, os, pathlib, sys
@@ -31,8 +31,6 @@ The codebase is split into two layers:
 **`bcc_core.py`** — All logic with no GUI imports. Contains:
 - `Profile` / `ServerEntry` dataclasses (the data model)
 - `ClientSpec` (issue #5, cross-client) — one adapter object per MCP host capturing everything client-specific: the top-level `servers_key` (Claude uses `mcpServers`; VS Code will use `servers`), the parking `disabled_key`, config `config_filename`, the capability flags (`expands_env_refs`, `supports_restart`), and a per-server `entry_to_internal`/`entry_from_internal` translation pair (identity for Claude; the seam a differently-shaped client overrides). `CLAUDE_DESKTOP` and `CLAUDE_CODE` are the two shipped specs; `resolve_client(path)` picks one by filename, and each `Profile` carries its resolved `client`. The read/write/diff functions take an optional `spec` and default to Claude's layout, so a call with no spec is unchanged.
 - `discover_profiles()` — scans the platform's app-support directory for `Claude*` folders (Claude Desktop) **and** always adds `~/.claude.json` (Claude Code user scope — what `claude mcp add` writes). `~/.claude/settings.json` is NOT a server config (it rejects `mcpServers` with a schema error) and is only surfaced, labelled legacy, if servers are found parked in it. Project-scope `.mcp.json` files can be opened via Add config…
 - `load_config` / `extract_servers` / `apply_servers` / `write_config` — the read/write pipeline; writes are atomic with rotating timestamped backups in `.bcc_backups/`
 - `parse_pasted_json()` / `parse_pasted_json_verbose()` — accepts three JSON shapes (full config, inner map, or bare server object). Input does not have to be valid JSON: `repair_json_text()` auto-fixes markdown fences, surrounding prose, `//` `/* */` `#` comments, trailing/missing commas, smart quotes, single quotes, unquoted keys, Python/JS literals, and unclosed braces. The verbose variant also returns human-readable notes describing every repair applied (shown live in the paste dialog)
@@ -45,7 +43,7 @@ The codebase is split into two layers:
 - `KeyValueTable` — reusable widget for env vars and headers
 - `ConnTester(QThread)` — background thread for remote reachability tests
-**The cardinal rule**: `apply_servers()` only ever writes the two keys the target client's servers live under — by default `mcpServers` and `_disabledMcpServers`, or whatever the profile's `ClientSpec` declares (`servers_key` + `disabled_key`). All other keys in the user's config are preserved verbatim and in their original order. The rule generalises across clients precisely because it is parameterised by the spec rather than hard-coded.
+**The cardinal rule**: `apply_servers()` only ever writes to `mcpServers` and `_disabledMcpServers`. All other keys in the user's config are preserved verbatim and in their original order.
 Disabled servers are parked under `_disabledMcpServers` (which Claude Desktop ignores) so they can be re-enabled without losing their definition.
@@ -1,50 +0,0 @@
 # PR #87 — "Move to environment variable" (#83): manual test checklist
 The logic is covered by 15 unit tests in CI; what CI **can't** exercise is the GUI (no PySide6). This checklist is only the parts a human needs to click. Should take ~10 minutes.
 ## Setup
 ```bash
 cd ~/Documents/Claude/Projects/BetterClaudeConfig/better-claude-config
 git fetch origin
 git checkout feat/83-move-to-env-var
 git pull                      # ensure you're on 8fdbe90 or later
 source .venv/bin/activate     # or recreate: python3 -m venv .venv && source .venv/bin/activate && pip install -r requirements.txt
 python bcc.py
 ```
 Pick a **Claude Code** profile (e.g. `~/.claude.json`) that has, or add, a server with an env value that looks like a secret — e.g. `env: { "API_KEY": "ghp_test123" }`. (You can use a throwaway value; nothing is sent anywhere.)
 ## The checklist
 ### Gating — where the action appears
 - [ ] Right-click the **value cell** of a secret env row (`API_KEY`) on a **Claude Code** profile → a **"Move to environment variable…"** item appears.
 - [ ] Right-click a **non-secret** row (e.g. `REGION` = `us-east-1`) → the item does **not** appear.
 - [ ] Right-click a row whose value is already a reference (`${API_KEY}`) → the item does **not** appear.
 - [ ] Switch to a **Claude Desktop** profile (a `claude_desktop_config.json`), right-click the same kind of secret row → the item does **not** appear. (Desktop doesn't expand `${VAR}`, so offering it would break the config — this is the important gate.)
 ### The dialog
 - [ ] Trigger the action → dialog opens with **Variable** pre-filled from the key, sanitized to a legal shell name (e.g. `api-key` → `API_KEY`).
 - [ ] Edit the variable name → the shown **shell line updates live** and matches your platform (`export VAR='…'` on macOS/Linux, `setx VAR "…"` on Windows), with the other platform shown in parentheses.
 - [ ] If you type a variable name that **is already set** in your shell environment, the green "already looks set" note appears; if not, it's hidden.
 - [ ] **Cancel** → nothing changes (value still the raw secret, no dirty state).
 ### The conversion
 - [ ] **Move && copy secret** → the cell now shows the reference `${VAR}` (visible, **not** masked to dots), and the window goes dirty (Save enabled).
 - [ ] Paste from your clipboard somewhere → it's the **original secret value** (handed back before removal).
 - [ ] The reference value is **not** flagged as a secret warning anymore (it's the recommended state).
 ### Headers + persistence
 - [ ] Repeat on a **remote server's Headers** table (e.g. an `Authorization` header) → same behavior.
 - [ ] **Save**, then open the config file on disk in a text editor → the servers block holds `${VAR}`, and the **plaintext secret is gone** from the file.
 - [ ] Re-open the profile in BCC → the row still shows `${VAR}` (round-trips).
 ### Undo (nice-to-have)
 - [ ] After a conversion, **Ctrl+Z / Cmd+Z** restores the previous value.
 ## Known scope (not bugs)
 - **Args rows** are out of scope for this PR — the core supports them, but the args editor is a free-text widget, so wiring that UI is a deliberate follow-up. Right-clicking args won't offer the action yet.
 - The "already set" check reads **BCC's** environment, which may differ from the client's — it's advisory, worded that way.
 ## If anything's off
 Tell me which checkbox failed and what you saw; I'll fix on the branch and re-push. If everything passes, approve/merge #87 (or tell me to merge it).
@@ -90,6 +90,21 @@ 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
@@ -110,12 +125,29 @@ 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:
+the RELEASE key, which signs `SHA256SUMS` (release checksums). It does
 **not** sign `data/catalog.json` and is not the key `bcc_core.CATALOG_PUBKEYS`
 trusts:
 ```
-<PLACEHOLDER — AJ: paste the release public key from `catalog_console.py keygen --release` here>
+6BnPgJEHJFyVltFoLTCNadIsehjy00iiW8IRlC1TfhA=
 ```
 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
@@ -190,7 +222,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).
+- `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).
 ## Building from source
@@ -10,7 +10,6 @@ Run:  python mcp_manager.py
 from __future__ import annotations
 import contextlib
 import sys
 import time
 from pathlib import Path
@@ -19,7 +18,6 @@ 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,
@@ -27,7 +25,6 @@ from PySide6.QtGui import (
    QIcon,
    QKeySequence,
    QPainter,
    QPalette,
    QPixmap,
 )
 from PySide6.QtWidgets import (
@@ -68,122 +65,85 @@ import bcc_core as core
 # thread during drag-and-drop import, so skip anything larger than this.
 MAX_DROP_IMPORT_BYTES = 5 * 1024 * 1024  # 5 MB
-# --- Theming (issue #75) -------------------------------------------------- #
+# --- One-line rebrand: change this to recolor the whole app --------------- #
-# The palette lives in bcc_core (testable without a Qt app); these module-level
+ACCENT = "#f97316"  # warm orange
-# names are rebound by `apply_palette()` whenever the theme changes.
+ACCENT_DIM = "#c2570b"
-#
+BG = "#1b1d23"
-# Why globals rather than passing a palette around: ~20 inline
+PANEL = "#23262e"
-# `setStyleSheet(f"color: {MUTED}")` calls are scattered through this file, and
+PANEL_2 = "#2b2f39"
-# an f-string resolves its names when it runs, not when it's compiled. Rebinding
+TEXT = "#e7e9ee"
-# the globals means every one of those call sites picks up the new colour on its
+MUTED = "#9aa0ad"
-# next render, with no change to the call sites themselves.
+BORDER = "#3a3f4b"
-PALETTE = core.DARK_PALETTE
+GOOD = "#4ade80"
-ACCENT = ACCENT_DIM = BG = PANEL = PANEL_2 = TEXT = MUTED = BORDER = ""
+BAD = "#f87171"
-GOOD = BAD = WARN = REMOTE = ON_ACCENT = DISABLED_BG = MONO_BG = SEL_TEXT = ""
+WARN = "#fbbf24"
 STATUS_COLORS: dict[str, str] = {}
 HEALTH_COLORS: dict[str, str] = {}
-STATUS_GLYPH = {
+STATUS_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": "#60a5fa", "unknown": WARN}
-    "ok": "\u25cf",
+STATUS_GLYPH = {"ok": "●", "missing": "●", "warn": "▲", "remote": "◆", "unknown": "○"}
    "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_GLYPH = {"ok": "\u25cf", "failed": "\u25cf", "untested": "\u25cb"}
+HEALTH_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED}
 HEALTH_GLYPH = {"ok": "●", "failed": "●", "untested": "○"}
-
+STYLESHEET = f"""
 def build_stylesheet(p: core.Palette) -> str:
    """Render the global QSS for a palette."""
    return f"""
 /* No font-family here on purpose: Qt already uses the native system UI font
   on every platform (San Francisco / Segoe UI / desktop default). Naming
   web-CSS aliases like -apple-system forces a costly font-alias scan. */
-* {{ font-size: 13px; color: {p.text}; }}
+* {{ font-size: 13px; color: {TEXT}; }}
-QMainWindow, QDialog {{ background: {p.bg}; }}
+QMainWindow, QDialog {{ background: {BG}; }}
 QLabel#h1 {{ font-size: 15px; font-weight: 600; }}
-QLabel#muted {{ color: {p.muted}; }}
+QLabel#muted {{ color: {MUTED}; }}
-QFrame#card {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px; }}
+QFrame#card {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px; }}
 QLineEdit, QPlainTextEdit, QComboBox {{
-    background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px;
+    background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px;
-    padding: 6px 8px; selection-background-color: {p.accent}; selection-color: {p.on_accent};
+    padding: 6px 8px; selection-background-color: {ACCENT}; selection-color: #1a1205;
 }}
-QLineEdit:focus, QPlainTextEdit:focus, QComboBox:focus {{ border: 1px solid {p.accent}; }}
+QLineEdit:focus, QPlainTextEdit:focus, QComboBox:focus {{ border: 1px solid {ACCENT}; }}
 QComboBox::drop-down {{ border: none; width: 22px; }}
-QComboBox QAbstractItemView {{ background: {p.panel_2}; border: 1px solid {p.border};
+QComboBox QAbstractItemView {{ background: {PANEL_2}; border: 1px solid {BORDER};
-    selection-background-color: {p.accent}; outline: none; }}
+    selection-background-color: {ACCENT}; outline: none; }}
-QPushButton {{ background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px;
+QPushButton {{ background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px;
    padding: 7px 13px; }}
-QPushButton:hover {{ border: 1px solid {p.accent}; }}
+QPushButton:hover {{ border: 1px solid {ACCENT}; }}
-QPushButton:disabled {{ color: {p.muted}; background: {p.panel}; }}
+QPushButton:disabled {{ color: {MUTED}; background: {PANEL}; }}
-QPushButton#primary {{ background: {p.accent}; border: 1px solid {p.accent}; color: {p.on_accent}; font-weight: 600; }}
+QPushButton#primary {{ background: {ACCENT}; border: 1px solid {ACCENT}; color: #1a1205; font-weight: 600; }}
-QPushButton#primary:hover {{ background: {p.accent_dim}; }}
+QPushButton#primary:hover {{ background: {ACCENT_DIM}; }}
-QPushButton#primary:disabled {{ background: {p.panel}; color: {p.muted}; border: 1px solid {p.border}; }}
+QPushButton#primary:disabled {{ background: {PANEL}; color: {MUTED}; border: 1px solid {BORDER}; }}
-QPushButton#danger:hover {{ border: 1px solid {p.bad}; color: {p.bad}; }}
+QPushButton#danger:hover {{ border: 1px solid {BAD}; color: {BAD}; }}
-QTableWidget {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px;
+QTableWidget {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px;
    gridline-color: transparent; outline: none; }}
 QTableWidget::item {{ padding: 6px 8px; border: none; }}
-QTableWidget::item:selected {{ background: {p.accent}; color: {p.on_accent}; }}
+QTableWidget::item:selected {{ background: {ACCENT}; color: #1a1205; }}
 /* 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: {p.panel_2}; color: {p.text}; border: 1px solid {p.accent};
+    background: {PANEL_2}; color: {TEXT}; border: 1px solid {ACCENT};
    border-radius: 3px; padding: 0px 4px; margin: 0px;
-    selection-background-color: {p.accent_dim}; selection-color: {p.selection_text};
+    selection-background-color: {ACCENT_DIM}; selection-color: #ffffff;
 }}
-QHeaderView::section {{ background: {p.panel}; color: {p.muted}; border: none;
+QHeaderView::section {{ background: {PANEL}; color: {MUTED}; border: none;
-    border-bottom: 1px solid {p.border}; padding: 8px; font-weight: 600; }}
+    border-bottom: 1px solid {BORDER}; padding: 8px; font-weight: 600; }}
 QScrollBar:vertical {{ background: transparent; width: 10px; margin: 2px; }}
-QScrollBar::handle:vertical {{ background: {p.border}; border-radius: 5px; min-height: 24px; }}
+QScrollBar::handle:vertical {{ background: {BORDER}; border-radius: 5px; min-height: 24px; }}
 QScrollBar::add-line, QScrollBar::sub-line {{ height: 0; }}
-QLabel#statusbar {{ color: {p.muted}; padding: 4px 2px; }}
+QLabel#statusbar {{ color: {MUTED}; padding: 4px 2px; }}
-QLabel#warnBanner {{ color: {p.on_accent}; background: {p.warn}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }}
+QLabel#warnBanner {{ color: #1a1205; background: {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#section {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
-QLabel#noticeText {{ color: {p.text}; }}
+QLabel#sectionDisabled {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
-QPushButton#noticeClose {{ background: transparent; border: none; color: {p.muted}; font-size: 14px; padding: 2px; }}
+QLabel#placeholder {{ color: {MUTED}; padding: 12px; background: {PANEL_2}; border: 1px dashed {BORDER}; border-radius: 8px; }}
-QPushButton#noticeClose:hover {{ color: {p.text}; }}
+QTableWidget#disabledTable {{ background: #202229; }}
-QLabel#section {{ color: {p.muted}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
+QTableWidget#disabledTable::item:selected {{ background: {ACCENT}; color: #1a1205; }}
 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: {p.mono_bg}; border: 1px solid {p.border}; border-radius: 8px; }}
+    font-size: 12px; background: #16181d; border: 1px solid {BORDER}; border-radius: 8px; }}
 QFrame#diagCard {{ background: transparent; border: none; }}
 QSplitter::handle {{ background: transparent; }}
-QSplitter::handle:hover {{ background: {p.border}; border-radius: 4px; }}
+QSplitter::handle:hover {{ background: {BORDER}; border-radius: 4px; }}
-QSplitter::handle:pressed {{ background: {p.accent}; border-radius: 4px; }}
+QSplitter::handle:pressed {{ background: {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)
 # --------------------------------------------------------------------------- #
@@ -268,230 +228,10 @@ class _SecretMaskDelegate(QStyledItemDelegate):
        if self.revealed or not option.text:
            return
        key_item = self._table.item(index.row(), 0)
-        if key_item and core.should_mask_value(key_item.text(), option.text):
+        if key_item and core.is_secret_key(key_item.text()):
            option.text = core.MASK
 class MoveToEnvDialog(QDialog):
    """Confirm moving a plaintext secret out to a ${VAR} reference (#83).
    The secret is about to leave the config file, so this dialog's whole job is
    to hand it back first: it lets the user name the variable, shows the exact
    shell line to set it, and (on accept) the caller copies the secret to the
    clipboard. If the variable already looks set in this environment, it says so
    and drops the urgency.
    """
    def __init__(self, parent, key: str, secret: str):
        super().__init__(parent)
        self.setWindowTitle("Move to environment variable")
        self.setMinimumWidth(460)
        self._secret = secret
        v = QVBoxLayout(self)
        v.setSpacing(10)
        intro = QLabel(
            "This replaces the value in place with a ${VAR} reference. The secret "
            "moves to your shell/OS environment — not this config file, and not the "
            "Environment variables table below. Run the line below to set it there, "
            "or the server won't authenticate."
        )
        intro.setWordWrap(True)
        v.addWidget(intro)
        grid = QGridLayout()
        grid.setSpacing(8)
        lbl = QLabel("Variable:")
        lbl.setObjectName("muted")
        grid.addWidget(lbl, 0, 0)
        self._name_edit = QLineEdit(core.sanitize_env_var_name(key))
        self._name_edit.textChanged.connect(self._refresh)
        grid.addWidget(self._name_edit, 0, 1)
        v.addLayout(grid)
        self._already = QLabel("")
        self._already.setWordWrap(True)
        self._already.setStyleSheet(f"color: {GOOD};")
        v.addWidget(self._already)
        set_lbl = QLabel("Set it with:")
        set_lbl.setObjectName("muted")
        v.addWidget(set_lbl)
        self._cmd = QLabel("")
        self._cmd.setWordWrap(True)
        self._cmd.setTextInteractionFlags(Qt.TextInteractionFlag.TextSelectableByMouse)
        self._cmd.setStyleSheet("font-family: monospace;")
        v.addWidget(self._cmd)
        note = QLabel("The secret will be copied to your clipboard when you continue.")
        note.setObjectName("muted")
        note.setWordWrap(True)
        v.addWidget(note)
        btns = QDialogButtonBox(
            QDialogButtonBox.StandardButton.Ok | QDialogButtonBox.StandardButton.Cancel
        )
        ok = btns.button(QDialogButtonBox.StandardButton.Ok)
        ok.setText("Move && copy secret")
        ok.setObjectName("primary")
        btns.accepted.connect(self.accept)
        btns.rejected.connect(self.reject)
        v.addWidget(btns)
        self._refresh()
    def var_name(self) -> str:
        return core.sanitize_env_var_name(self._name_edit.text())
    def _refresh(self, *_):
        name = self.var_name()
        lines = core.shell_export_lines(name, self._secret)
        if sys.platform == "win32":
            self._cmd.setText(f"{lines['windows']}\n\n(macOS/Linux: {lines['posix']})")
        else:
            self._cmd.setText(f"{lines['posix']}\n\n(Windows: {lines['windows']})")
        if core.is_env_var_set(name):
            self._already.setText(f"{name} already looks set in this environment.")
            self._already.show()
        else:
            self._already.hide()
 class MoveArgToEnvDialog(QDialog):
    """Confirm relocating a secret arg into the env block (#83, kept in file).
    Unlike the reference move, this keeps the value in the config -- it just
    moves it out of the argument list (visible in process listings) and into
    the Environment variables table, where the user can see and edit it. It
    changes how the server is launched, so it says so plainly.
    """
    def __init__(self, parent, key: str, value: str):
        super().__init__(parent)
        self.setWindowTitle("Move into environment variables")
        self.setMinimumWidth(460)
        v = QVBoxLayout(self)
        v.setSpacing(10)
        intro = QLabel(
            "This moves the secret out of the arguments and into the Environment "
            "variables table below, where you can see and edit its value. The value "
            "stays in this config file."
        )
        intro.setWordWrap(True)
        v.addWidget(intro)
        warn = QLabel(
            "⚠  This changes how the server is launched: the flag is dropped and the "
            "value is set as an environment variable instead. It only works if the "
            "server reads this secret from that variable."
        )
        warn.setWordWrap(True)
        warn.setStyleSheet(f"color: {WARN};")
        v.addWidget(warn)
        grid = QGridLayout()
        grid.setSpacing(8)
        lbl = QLabel("Variable:")
        lbl.setObjectName("muted")
        grid.addWidget(lbl, 0, 0)
        self._name_edit = QLineEdit(core.sanitize_env_var_name(key))
        grid.addWidget(self._name_edit, 0, 1)
        v.addLayout(grid)
        btns = QDialogButtonBox(
            QDialogButtonBox.StandardButton.Ok | QDialogButtonBox.StandardButton.Cancel
        )
        ok = btns.button(QDialogButtonBox.StandardButton.Ok)
        ok.setText("Move into env")
        ok.setObjectName("primary")
        btns.accepted.connect(self.accept)
        btns.rejected.connect(self.reject)
        v.addWidget(btns)
    def var_name(self) -> str:
        return core.sanitize_env_var_name(self._name_edit.text())
 class ReferencedVarsDialog(QDialog):
    """Show every ${VAR} the loaded server references and whether it's set (#83).
    After a secret becomes a reference, the variable lives in the user's
    environment, not the config -- so this is where they confirm it exists and
    get the command to set it. Read-only; BCC can't (and shouldn't) store the
    value.
    """
    def __init__(self, parent, data: dict):
        super().__init__(parent)
        self.setWindowTitle("Referenced variables")
        self.setMinimumWidth(560)
        v = QVBoxLayout(self)
        v.setSpacing(10)
        self._usages = core.referenced_env_vars(data)
        if not self._usages:
            v.addWidget(QLabel("This server references no ${VAR} variables."))
            btns = QDialogButtonBox(QDialogButtonBox.StandardButton.Close)
            btns.rejected.connect(self.reject)
            btns.accepted.connect(self.accept)
            v.addWidget(btns)
            return
        intro = QLabel(
            "These references are read from your shell/OS environment when the client "
            "runs. ✓ means it's set in BCC's environment (which may differ from the "
            "client's) or has a default; ✗ means nothing would fill it."
        )
        intro.setWordWrap(True)
        v.addWidget(intro)
        self._table = QTableWidget(len(self._usages), 3)
        self._table.setHorizontalHeaderLabels(["Variable", "Status", "Used in"])
        self._table.horizontalHeader().setSectionResizeMode(0, QHeaderView.ResizeMode.Stretch)
        self._table.horizontalHeader().setSectionResizeMode(2, QHeaderView.ResizeMode.Stretch)
        self._table.verticalHeader().setVisible(False)
        self._table.setSelectionBehavior(QAbstractItemView.SelectionBehavior.SelectRows)
        self._table.setEditTriggers(QAbstractItemView.EditTrigger.NoEditTriggers)
        for r, u in enumerate(self._usages):
            is_set = core.is_env_var_set(u.name)
            status = "✓ set" if is_set else ("✓ default" if u.has_default else "✗ not set")
            self._table.setItem(r, 0, QTableWidgetItem(u.name))
            self._table.setItem(r, 1, QTableWidgetItem(status))
            self._table.setItem(r, 2, QTableWidgetItem(", ".join(u.fields)))
        self._table.selectionModel().selectionChanged.connect(self._refresh_cmd)
        v.addWidget(self._table, 1)
        set_lbl = QLabel("Set the selected variable with:")
        set_lbl.setObjectName("muted")
        v.addWidget(set_lbl)
        self._cmd = QLabel("")
        self._cmd.setWordWrap(True)
        self._cmd.setTextInteractionFlags(Qt.TextInteractionFlag.TextSelectableByMouse)
        self._cmd.setStyleSheet("font-family: monospace;")
        v.addWidget(self._cmd)
        btns = QDialogButtonBox(QDialogButtonBox.StandardButton.Close)
        btns.rejected.connect(self.reject)
        btns.accepted.connect(self.accept)
        v.addWidget(btns)
        self._table.selectRow(0)
    def _refresh_cmd(self, *_):
        rows = self._table.selectionModel().selectedRows()
        if not rows:
            self._cmd.setText("")
            return
        name = self._usages[rows[0].row()].name
        # A placeholder value -- BCC doesn't hold the secret, this shows the shape.
        lines = core.shell_export_lines(name, "<value>")
        if sys.platform == "win32":
            self._cmd.setText(f"{lines['windows']}\n\n(macOS/Linux: {lines['posix']})")
        else:
            self._cmd.setText(f"{lines['posix']}\n\n(Windows: {lines['windows']})")
 class KeyValueTable(QWidget):
    def __init__(self, key_label="Key", val_label="Value", on_change=None, before_change=None):
        super().__init__()
@@ -512,11 +252,6 @@ class KeyValueTable(QWidget):
        self.table.setSelectionBehavior(QAbstractItemView.SelectionBehavior.SelectRows)
        self.table.setMinimumHeight(90)
        self.table.itemChanged.connect(self._changed)
        # Right-click a secret row to move it out to a ${VAR} reference (#83).
        # Set by the owner (ServerEditor) so the action can gate on the client.
        self.profile_provider = None
        self.table.setContextMenuPolicy(Qt.ContextMenuPolicy.CustomContextMenu)
        self.table.customContextMenuRequested.connect(self._context_menu)
        # Secret-looking values (API_KEY, TOKEN, ...) render masked by default.
        self._mask_delegate = _SecretMaskDelegate(self.table)
        self.table.setItemDelegateForColumn(1, self._mask_delegate)
@@ -540,58 +275,6 @@ class KeyValueTable(QWidget):
        self.reveal_btn.setText("Hide secrets" if on else "Show secrets")
        self.table.viewport().update()
    def _row_key_value(self, row: int):
        key_item = self.table.item(row, 0)
        val_item = self.table.item(row, 1)
        key = key_item.text().strip() if key_item else ""
        # The mask is display-only (a delegate); the model text is the real value.
        value = val_item.text() if val_item else ""
        return key, value
    def _context_menu(self, pos):
        item = self.table.itemAt(pos)
        if item is None:
            return
        row = item.row()
        key, value = self._row_key_value(row)
        # Only a real stored secret is worth moving; nothing to offer otherwise.
        if not core.should_mask_value(key, value):
            return
        profile = self.profile_provider() if self.profile_provider else None
        menu = QMenu(self)
        act = QAction("Replace with a ${VAR} reference (out of file)…", self)
        if core.can_move_value_to_env_ref(key, value, profile):
            act.triggered.connect(lambda: self._move_row_to_env(row))
        else:
            # Show it disabled with the reason rather than an empty menu, so the
            # feature is discoverable and Claude Desktop's gating is explained.
            act.setEnabled(False)
            act.setText("Replace with ${VAR} reference — unavailable for Claude Desktop")
            act.setToolTip(
                "Claude Desktop doesn't expand ${VAR}, so a reference would reach "
                "the server as literal text."
            )
        menu.addAction(act)
        menu.exec(self.table.viewport().mapToGlobal(pos))
    def _move_row_to_env(self, row: int):
        key, secret = self._row_key_value(row)
        if not secret:
            return
        dlg = MoveToEnvDialog(self.window(), key, secret)
        if not dlg.exec():
            return
        var_name = dlg.var_name()
        # Hand the secret back before it leaves the file: clipboard now holds it,
        # and the dialog showed the exact shell line to set it.
        QGuiApplication.clipboard().setText(secret)
        if self._before_change:
            self._before_change()
        val_item = self.table.item(row, 1)
        if val_item is not None:
            # setText fires itemChanged -> _changed -> on_change (dirty + revalidate).
            val_item.setText(f"${{{var_name}}}")
    def _changed(self, item=None, *_):
        if item is not None and item.column() == 0:
            new_key = item.text().strip()
@@ -797,12 +480,6 @@ class ServerEditor(QFrame):
        self.logs_btn = QPushButton("View logs")
        self.logs_btn.setToolTip("Open this server's MCP log in a read-only, auto-tailing viewer")
        self.logs_btn.clicked.connect(self._view_logs)
        self.vars_btn = QPushButton("Variables…")
        self.vars_btn.setToolTip(
            "Show the ${VAR} references this server uses and whether each is set in "
            "your environment"
        )
        self.vars_btn.clicked.connect(self._show_referenced_vars)
        self.details_btn = QPushButton("Details ▸")
        self.details_btn.setCheckable(True)
        self.details_btn.toggled.connect(self._toggle_diag)
@@ -814,7 +491,6 @@ class ServerEditor(QFrame):
        dep.addWidget(self.test_btn)
        dep.addWidget(self.spawn_btn)
        dep.addWidget(self.logs_btn)
        dep.addWidget(self.vars_btn)
        dep.addWidget(self.details_btn)
        dep.addWidget(recheck)
        outer.addLayout(dep)
@@ -915,58 +591,6 @@ class ServerEditor(QFrame):
        v.addWidget(self.headers, 1)
        return w
    def set_profile_provider(self, provider):
        """Let the env/headers tables and the args editor gate the secret-move
        actions on which client the loaded profile targets (#83), and wire the
        args editor's two move actions back to this editor (which owns the whole
        form, since moving an arg into env touches both fields)."""
        self.env.profile_provider = provider
        self.headers.profile_provider = provider
        self.args.profile_provider = provider
        self.args.on_move_to_ref = self.move_arg_to_reference
        self.args.on_move_to_env = self.move_arg_into_env
    # --- secret moves from args (#83) ------------------------------------ #
    def _reload_from_data(self, new_data: dict):
        """Repopulate the form from a transformed data dict and mark dirty."""
        self.load_entry(core.ServerEntry(self.current_name(), new_data, True))
        self._emit()
    def move_arg_to_reference(self, index: int):
        """Args secret -> ${VAR} reference in place (secret leaves the file)."""
        data = self.dump_data()
        args = data.get("args") or []
        if not (0 <= index < len(args)):
            return
        dlg = MoveToEnvDialog(
            self.window(), core.suggested_env_var_for_arg(args, index), args[index]
        )
        if not dlg.exec():
            return
        conv = core.move_value_to_env_ref(data, field="args", index=index, var_name=dlg.var_name())
        if conv is None:
            return
        QGuiApplication.clipboard().setText(conv.secret)
        self._reload_from_data(conv.data)
    def move_arg_into_env(self, index: int):
        """Args secret -> env block, kept in this config (visible/editable)."""
        data = self.dump_data()
        args = data.get("args") or []
        if not (0 <= index < len(args)):
            return
        default_name = core.suggested_env_var_for_arg(args, index)
        dlg = MoveArgToEnvDialog(self.window(), default_name, args[index])
        if not dlg.exec():
            return
        new = core.move_arg_to_env_block(data, index, var_name=dlg.var_name())
        if new is None:
            return
        self._reload_from_data(new)
    def _show_referenced_vars(self):
        ReferencedVarsDialog(self.window(), self.dump_data()).exec()
    # --- model <-> form -------------------------------------------------- #
    def load_entry(self, entry: core.ServerEntry | None):
        self._loading = True
@@ -1244,52 +868,6 @@ class ArgsEdit(QPlainTextEdit):
        self.blockCountChanged.connect(self._update_gutter_width)
        self.updateRequest.connect(self._on_update_request)
        self._update_gutter_width()
        # Wired by ServerEditor: gate on the loaded client, and the two move
        # actions (which the editor performs, since moving an arg into env
        # touches both the args and the env table). Indices are into the
        # non-blank arg list, matching dump_data()'s args.
        self.profile_provider = None
        self.on_move_to_ref = None
        self.on_move_to_env = None
    def contextMenuEvent(self, event):
        menu = self.createStandardContextMenu()  # keep cut/copy/paste
        lines = self.toPlainText().splitlines()
        block = self.cursorForPosition(event.pos()).blockNumber()
        if 0 <= block < len(lines) and lines[block].strip():
            # This editor is one arg per line; map the clicked block to its
            # index among the non-blank args the model actually sees.
            cleaned = [ln for ln in lines if ln.strip() != ""]
            idx = sum(1 for ln in lines[:block] if ln.strip() != "")
            if idx in set(core.secret_arg_indices(cleaned)):
                profile = self.profile_provider() if self.profile_provider else None
                expands = profile is None or core.client_expands_env_refs(profile)
                first = menu.actions()[0] if menu.actions() else None
                # Reference (secret leaves the file) -- needs an expanding client.
                ref_act = QAction("Replace with a ${VAR} reference (out of file)…", self)
                if expands and self.on_move_to_ref:
                    ref_act.triggered.connect(lambda: self.on_move_to_ref(idx))
                else:
                    ref_act.setEnabled(False)
                    ref_act.setText(
                        "Replace with ${VAR} reference — unavailable for Claude Desktop"
                    )
                    ref_act.setToolTip(
                        "Claude Desktop doesn't expand ${VAR}, so a reference would "
                        "reach the server as literal text."
                    )
                # Move into the env block (kept in file) -- works on any client.
                env_act = QAction("Move into Environment variables (kept in this config)…", self)
                if self.on_move_to_env:
                    env_act.triggered.connect(lambda: self.on_move_to_env(idx))
                menu.insertAction(first, ref_act)
                menu.insertAction(first, env_act)
                if first is not None:
                    menu.insertSeparator(first)
        menu.exec(event.globalPos())
    def gutter_width(self) -> int:
        digits = max(1, len(str(self.blockCount())))
@@ -1915,54 +1493,6 @@ 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.
@@ -2020,18 +1550,12 @@ 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)
        split.setHandleWidth(10)
        split.addWidget(self._build_left())
        self.editor = ServerEditor(on_change=self._editor_changed, before_change=self._push_undo)
        self.editor.set_profile_provider(lambda: self.current_profile)
        split.addWidget(self.editor)
        split.setStretchFactor(0, 3)
        split.setStretchFactor(1, 4)
@@ -2061,96 +1585,11 @@ 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()
@@ -2171,9 +1610,10 @@ 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())
-        notice = core.update_notice(core.__version__, release)
+        if core.is_newer_version(core.__version__, release["version"]):
-        if notice:
+            self.status.setText(
-            self._show_update_notice(notice)
+                f"Update available: {release['version']}  ·  Help ▸ About to view it."
            )
    # --- layout persistence ---------------------------------------------- #
    def _restore_layout(self):
@@ -2426,11 +1866,6 @@ class MainWindow(QMainWindow):
        for p in self.profiles:
            tag = "" if p.config_exists else "  (no config yet)"
            self.profile_combo.addItem(f"{p.label}{tag}")
            # Full path in the tooltip so a profile is always verifiable even
            # when two labels look alike (e.g. two repos both named "app").
            self.profile_combo.setItemData(
                self.profile_combo.count() - 1, str(p.path), Qt.ItemDataRole.ToolTipRole
            )
        self.profile_combo.blockSignals(False)
        if self.profiles:
            self.profile_combo.setCurrentIndex(0)
@@ -2504,21 +1939,9 @@ class MainWindow(QMainWindow):
                return
            self.full_config = cfg
            repaired = True
        # extract_servers tolerates malformed entries rather than raising (#72),
        # but keep it inside the guard: a load failure must leave the previously
        # loaded profile intact instead of half-swapping the window's state.
        try:
            servers = core.extract_servers(self.full_config, profile.client)
        except Exception as exc:  # pragma: no cover - defence in depth
            QMessageBox.critical(
                self,
                "Could not read config",
                f"{profile.path}\n\nThe server list couldn't be read: {exc}",
            )
            return
        self._loaded_stat = core.config_fingerprint(profile.path)
        self.current_profile = profile
-        self.servers = servers
+        self.servers = core.extract_servers(self.full_config)
        self.dirty = False
        self.restart_btn.hide()
        self._undo_stack.clear()
@@ -2841,7 +2264,7 @@ class MainWindow(QMainWindow):
        entry = self.servers[idx]
        old_name = entry.name
        entry.name = self.editor.current_name()
-        entry.set_data(self.editor.dump_data())
+        entry.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
@@ -2935,7 +2358,7 @@ class MainWindow(QMainWindow):
                QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
            )
            if ans == QMessageBox.StandardButton.Yes:
-                self.servers[existing[name]].set_data(data)
+                self.servers[existing[name]].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))
@@ -2978,7 +2401,7 @@ class MainWindow(QMainWindow):
        except Exception as e:
            QMessageBox.critical(self, "Copy failed", f"Couldn't read {dest.label}:\n{e}")
            return
-        existing = core.extract_servers(dest_cfg, dest.client)
+        existing = core.extract_servers(dest_cfg)
        names = {s.name for s in existing}
        if src.name in names:
            ans = QMessageBox.question(
@@ -2990,7 +2413,7 @@ class MainWindow(QMainWindow):
                return
            existing = [s for s in existing if s.name != src.name]
        existing.append(core.ServerEntry(src.name, dict(src.data), True))
-        core.apply_servers(dest_cfg, existing, dest.client)
+        core.apply_servers(dest_cfg, existing)
        try:
            backup = core.write_config(dest.path, dest_cfg)
        except Exception as e:
@@ -3010,12 +2433,6 @@ class MainWindow(QMainWindow):
            self.save_btn.setEnabled(False)
            return False
        lint_warnings = core.lint_servers(self.servers)
        # ${VAR} references are only meaningful if the target client expands
        # them -- Claude Desktop doesn't, so the same config is fine in one
        # profile and broken in another (#76). Report against the loaded one.
        for entry in self.servers:
            for warning in core.env_ref_warnings(entry.data, self.current_profile):
                lint_warnings.append(f"'{entry.name}': {warning}")
        if lint_warnings:
            self.validation_lbl.setText(f"⚠ {lint_warnings[0]}")
            self.validation_lbl.setStyleSheet(f"color: {WARN};")
@@ -3050,7 +2467,7 @@ class MainWindow(QMainWindow):
            and disk_stat != self._loaded_stat
        ):
            changed_keys, server_diff = core.external_change_summary(
-                self.full_config, self.current_profile.path, self.current_profile.client
+                self.full_config, self.current_profile.path
            )
            dlg = StaleDialog(self, str(self.current_profile.path), changed_keys, server_diff)
            if not dlg.exec():
@@ -3061,12 +2478,7 @@ 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
+                core.apply_servers(fresh, self.servers)
                # didn't touch -- but it also wipes BCC-authored keys the user
                # changed in this session (named sets), which apply_servers
                # doesn't write. Carry them over before saving (#73).
                contested = core.carry_owned_keys(self.full_config, fresh)
                core.apply_servers(fresh, self.servers, self.current_profile.client)
                try:
                    backup = core.write_config(self.current_profile.path, fresh)
                except Exception as e:
@@ -3078,20 +2490,15 @@ 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}{cnote}"
+                    f"Merged & saved {self.current_profile.path}{bnote}"
                    f"  ·  Restart {self.current_profile.label} to apply."
                )
                self._offer_restart_button()
                return
            # else OVERWRITE: fall through to normal write
-        core.apply_servers(self.full_config, self.servers, self.current_profile.client)
+        core.apply_servers(self.full_config, self.servers)
        try:
            backup = core.write_config(self.current_profile.path, self.full_config)
        except Exception as e:
@@ -3242,33 +2649,6 @@ 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
@@ -3287,7 +2667,7 @@ def main():
    icon = _app_icon()
    if not icon.isNull():
        app.setWindowIcon(icon)
-    app.setStyleSheet(theme_stylesheet_for(app))
+    app.setStyleSheet(STYLESHEET)
    win = MainWindow()
    win.show()
    sys.exit(app.exec())
@@ -16,6 +16,8 @@ new surface" recurring-bug lesson).
 from __future__ import annotations
 import base64
 import hashlib
 import os
 import re
 from collections.abc import Callable
@@ -442,18 +444,43 @@ 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:
-            self.changes = diff_catalogs(self.old_catalog, self.new_catalog)
+            if self.reattest:
                # 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(
@@ -461,12 +488,14 @@ 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,
    )
@@ -531,6 +560,14 @@ 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
@@ -816,3 +853,243 @@ _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)
@@ -1,6 +1,5 @@
 # Runtime (also in requirements.txt)
 PySide6>=6.6
 cryptography>=42.0   # catalog signature verification (bcc_core) + release checksum signing
 # Build / packaging
 pyinstaller>=6.0
@@ -9,3 +8,4 @@ pillow>=10.0        # generates icons/app.ico during CI (Windows build)
 # Test / lint
 pytest>=8.0
 ruff>=0.6
 cryptography>=42.0   # release checksum signing (scripts/sign_checksums.py)
@@ -1,2 +1 @@
 PySide6>=6.6
 cryptography>=42.0   # bcc_core imports it at load (catalog signature verification)
@@ -53,13 +53,17 @@ DOMAIN_PREFIX = b"bcc-release-v1|"
 # the signature on every past release: verification accepts a match against
 # ANY key here.
 #
-# Empty until the maintainer generates the release keypair (separately from
+# Populated by the maintainer via:
 # 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
+# Rotated 2026-07 (issue #68 finding 5 / #68 CI-exposure incident): the
-# like a real key -- release.yml's signing-smoke-test fails closed (loudly)
+# original key was shared with the catalog key and had been exposed to CI,
-# on an empty list rather than silently verifying against nothing.
+# so both keypairs were regenerated as separate, disjoint keys. This list
-RELEASE_PUBKEYS: list[bytes] = []
+# holds only the current release key -- if release.yml's signing-smoke-test
 # 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
@@ -0,0 +1,215 @@
 """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"
@@ -491,6 +491,98 @@ 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
 # --------------------------------------------------------------------------- #
@@ -693,3 +785,245 @@ 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
@@ -1,7 +1,6 @@
 """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
@@ -2371,863 +2370,3 @@ 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"
 # --------------------------------------------------------------------------- #
 # #76 -- ${VAR} references. Semantics mirror Claude Code's documented
 # behaviour: ${VAR} and ${VAR:-default}, expanded in command/args/env/url/
 # headers, and an unset variable with no default left as literal text.
 # --------------------------------------------------------------------------- #
 def test_find_env_refs_plain_and_defaulted():
    refs = c.find_env_refs("${A} and ${B:-fallback}")
    assert [(r.name, r.default) for r in refs] == [("A", None), ("B", "fallback")]
@pytest.mark.parametrize("text", ["${}", "${1BAD}", "$NOTBRACED", "{NOPE}", "plain", "$${X"])
 def test_find_env_refs_ignores_non_references(text):
    assert c.find_env_refs(text) == []
 def test_find_env_refs_allows_an_empty_default():
    """`${VAR:-}` is a documented way to say 'blank if unset'."""
    refs = c.find_env_refs("${A:-}")
    assert refs[0].default == ""
    assert refs[0].has_default is True
 def test_server_env_refs_covers_all_five_documented_fields():
    data = {
        "command": "${BIN}",
        "args": ["--x", "${ARG}"],
        "env": {"K": "${ENVV}"},
        "url": "${URL}/mcp",
        "headers": {"Authorization": "Bearer ${HDR}"},
    }
    found = {(r.name, r.field) for r in c.server_env_refs(data)}
    assert found == {
        ("BIN", "command"),
        ("ARG", "args"),
        ("ENVV", "env"),
        ("URL", "url"),
        ("HDR", "headers"),
    }
 def test_server_env_refs_ignores_unexpanded_fields():
    """Claude Code expands five fields; a ${VAR} elsewhere isn't a reference."""
    assert c.server_env_refs({"description": "${NOPE}", "timeout": "${ALSO_NO}"}) == []
 def test_expand_env_refs_matches_documented_semantics():
    env = {"SET": "value"}
    assert c.expand_env_refs("${SET}", env) == "value"
    assert c.expand_env_refs("${MISSING:-dflt}", env) == "dflt"
    assert c.expand_env_refs("${SET:-dflt}", env) == "value"
    # unset with no default: left as literal text, exactly as Claude Code does
    assert c.expand_env_refs("${MISSING}", env) == "${MISSING}"
 def test_expand_env_refs_handles_several_in_one_string():
    assert c.expand_env_refs("${A}/${B:-two}/${C}", {"A": "one"}) == "one/two/${C}"
 def test_unresolved_env_refs_only_flags_unset_without_default():
    data = {"env": {"A": "${SET}", "B": "${UNSET}", "C": "${OTHER:-has_default}"}}
    assert [r.name for r in c.unresolved_env_refs(data, {"SET": "x"})] == ["UNSET"]
 # --- the two interactions that were backwards for this feature ------------
 def test_placeholder_under_a_secret_key_is_not_masked():
    """A ${VAR} names a secret rather than being one. Masking it would make a
    reference indistinguishable from a stored credential."""
    assert c.should_mask_value("API_KEY", "${API_KEY}") is False
    assert c.should_mask_value("API_KEY", "ghp_realsecret") is True
    assert c.should_mask_value("NOT_SECRET", "${API_KEY}") is False
 def test_redacted_display_keeps_placeholders_but_masks_real_secrets():
    out = c._redact_server_data({"env": {"API_KEY": "${API_KEY}", "TOKEN": "ghp_real"}})
    assert out["env"]["API_KEY"] == "${API_KEY}"
    assert out["env"]["TOKEN"] == c.MASK
 def test_redact_args_keeps_placeholders_visible():
    assert c.redact_args(["--token", "${GH_TOKEN}"]) == ["--token", "${GH_TOKEN}"]
    assert c.redact_args(["--api-key=${K}"]) == ["--api-key=${K}"]
    # real secrets still masked
    assert c.redact_args(["--token", "ghp_real"]) == ["--token", c.MASK]
    assert c.redact_args(["--api-key=sk-real"]) == [f"--api-key={c.MASK}"]
 def test_args_secret_warning_is_silenced_by_a_placeholder():
    """Moving a token into ${VAR} is the recommended fix for this warning --
    still warning afterwards would punish the fix."""
    assert c.args_secret_warning({"args": ["--token", "ghp_real"]}) is not None
    assert c.args_secret_warning({"args": ["--token", "${GH_TOKEN}"]}) is None
 def test_args_secret_warning_still_fires_on_the_arg_after_a_placeholder():
    """A placeholder must clear the pending-flag state, not blanket-suppress."""
    assert c.args_secret_warning({"args": ["${SAFE}", "--token", "ghp_real"]}) is not None
 # --- per-client gating ----------------------------------------------------
 def _profile(path):
    return c.Profile(label="p", path=Path(path), config_exists=True)
 def test_claude_code_profiles_expand_references():
    assert c.client_expands_env_refs(_profile(Path.home() / ".claude.json")) is True
    assert c.client_expands_env_refs(_profile("/repo/.mcp.json")) is True
 def test_claude_desktop_profile_does_not_expand_references():
    desktop = _profile(c.app_support_base() / "Claude" / c.CONFIG_FILENAME)
    assert c.client_expands_env_refs(desktop) is False
 def test_desktop_profile_warns_that_references_are_literal():
    data = {"env": {"API_KEY": "${API_KEY}"}}
    desktop = _profile(c.app_support_base() / "Claude" / c.CONFIG_FILENAME)
    warnings = c.env_ref_warnings(data, desktop, {"API_KEY": "set"})
    assert len(warnings) == 1
    assert "NOT be expanded" in warnings[0]
    assert "${API_KEY}" in warnings[0]
 def test_claude_code_profile_warns_only_about_unset_variables():
    code = _profile(Path.home() / ".claude.json")
    data = {"env": {"A": "${UNSET_ONE}"}}
    assert c.env_ref_warnings(data, code, {}) != []
    assert c.env_ref_warnings(data, code, {"UNSET_ONE": "x"}) == []
    # a default means it always resolves
    assert c.env_ref_warnings({"env": {"A": "${X:-d}"}}, code, {}) == []
 def test_no_references_means_no_warnings():
    assert c.env_ref_warnings({"command": "npx", "args": ["-y", "pkg"]}, None) == []
 # --------------------------------------------------------------------------- #
 # Client adapters (issue #5 — cross-client support, phase 1)
 #
 # The refactor's promise is twofold: (1) the two Claude clients behave exactly
 # as before, and (2) the ClientSpec seam is real — a client with a different
 # servers key and a different per-server shape flows through the same pipeline.
 # A synthetic "VS Code-like" spec stands in for the phase-2 client so the
 # abstraction is proven now, before anything depends on it.
 # --------------------------------------------------------------------------- #
 def test_claude_specs_are_registered_and_mcpservers_shaped():
    assert c.CLAUDE_DESKTOP.servers_key == "mcpServers"
    assert c.CLAUDE_CODE.servers_key == "mcpServers"
    assert c.CLAUDE_DESKTOP.disabled_key == c.DISABLED_KEY
    assert c.CLAUDE_CODE.disabled_key == c.DISABLED_KEY
    # capabilities the old inline filename checks used to compute
    assert c.CLAUDE_DESKTOP.expands_env_refs is False
    assert c.CLAUDE_CODE.expands_env_refs is True
    assert c.CLAUDE_DESKTOP.supports_restart is True
    assert c.CLAUDE_CODE.supports_restart is False
    assert set(c.CLIENT_SPECS) == {c.CLAUDE_DESKTOP, c.CLAUDE_CODE}
    assert c.DEFAULT_CLIENT is c.CLAUDE_DESKTOP
 def test_client_by_key_round_trips_and_misses():
    assert c.client_by_key("claude_desktop") is c.CLAUDE_DESKTOP
    assert c.client_by_key("claude_code") is c.CLAUDE_CODE
    assert c.client_by_key("nope") is None
 def test_resolve_client_matches_the_old_filename_rule():
    assert c.resolve_client("/x/Claude/claude_desktop_config.json") is c.CLAUDE_DESKTOP
    assert c.resolve_client(Path.home() / ".claude.json") is c.CLAUDE_CODE
    assert c.resolve_client("/repo/.mcp.json") is c.CLAUDE_CODE
    assert c.resolve_client(Path.home() / ".claude" / "settings.json") is c.CLAUDE_CODE
 def test_profile_auto_resolves_client_from_path():
    desktop = c.Profile(
        label="Claude", path="/x/Claude/claude_desktop_config.json", config_exists=True
    )
    code = c.Profile(label="Claude Code", path="/home/me/.claude.json", config_exists=True)
    assert desktop.client is c.CLAUDE_DESKTOP
    assert code.client is c.CLAUDE_CODE
 def test_profile_honours_an_explicit_client():
    # An explicit spec is not overridden by the path-based resolver.
    p = c.Profile(
        label="odd",
        path="/somewhere/claude_desktop_config.json",
        config_exists=True,
        client=c.CLAUDE_CODE,
    )
    assert p.client is c.CLAUDE_CODE
 def test_desktop_gating_and_env_expansion_read_off_the_spec():
    desktop = c.Profile(label="d", path="/x/Claude/claude_desktop_config.json", config_exists=True)
    code = c.Profile(label="c", path=Path.home() / ".claude.json", config_exists=True)
    assert c.profile_targets_claude_desktop(desktop) is True
    assert c.profile_targets_claude_desktop(code) is False
    assert c.client_expands_env_refs(desktop) is False
    assert c.client_expands_env_refs(code) is True
 def test_extract_and_apply_default_spec_is_unchanged():
    # No spec argument must behave byte-for-byte like the pre-refactor code.
    cfg = {"mcpServers": {"a": {"command": "x"}}, "_disabledMcpServers": {"b": {"command": "y"}}}
    servers = c.extract_servers(cfg)
    assert {(s.name, s.enabled) for s in servers} == {("a", True), ("b", False)}
    out = c.apply_servers({}, servers)
    assert out == {
        "mcpServers": {"a": {"command": "x"}},
        "_disabledMcpServers": {"b": {"command": "y"}},
    }
 # A stand-in for the phase-2 VS Code adapter: different top-level key
 # ("servers"), a different disabled key, and a per-server shape that carries a
 # `type` field the internal model doesn't. entry_to/from_internal are the only
 # things it overrides — proving that's the whole extension point.
 class _FakeVSCode(c.ClientSpec):
    def entry_to_internal(self, value):
        if not isinstance(value, dict):
            return value
        return {k: v for k, v in value.items() if k != "type"}
    def entry_from_internal(self, data):
        if not isinstance(data, dict):
            return data
        return {"type": "stdio", **data}
 _VSCODE = _FakeVSCode(
    key="vscode_fake",
    label="VS Code (test)",
    servers_key="servers",
    disabled_key="_bccDisabledServers",
 )
 def test_extract_reads_a_custom_servers_key_and_translates_shape():
    cfg = {"servers": {"a": {"type": "stdio", "command": "x", "args": ["-y"]}}}
    servers = c.extract_servers(cfg, _VSCODE)
    assert len(servers) == 1
    # the `type` field was translated out of the internal model
    assert servers[0].data == {"command": "x", "args": ["-y"]}
 def test_apply_writes_a_custom_key_translates_back_and_keeps_other_keys():
    original = {"servers": {"old": {"type": "stdio", "command": "z"}}, "keepMe": {"x": 1}}
    servers = c.extract_servers(original, _VSCODE)
    out = c.apply_servers(original, servers, _VSCODE)
    # round-trips through the custom key with the shape restored
    assert out["servers"] == {"old": {"type": "stdio", "command": "z"}}
    # the cardinal rule generalises: mcpServers is never introduced, and every
    # unrelated key survives verbatim
    assert "mcpServers" not in out
    assert out["keepMe"] == {"x": 1}
 def test_apply_uses_the_custom_disabled_key():
    servers = [
        c.ServerEntry("on", {"command": "a"}, True),
        c.ServerEntry("off", {"command": "b"}, False),
    ]
    out = c.apply_servers({}, servers, _VSCODE)
    assert out["servers"] == {"on": {"type": "stdio", "command": "a"}}
    assert out["_bccDisabledServers"] == {"off": {"type": "stdio", "command": "b"}}
    assert c.DISABLED_KEY not in out
 def test_spec_with_no_disabled_key_drops_disabled_and_never_parks():
    no_park = c.ClientSpec(
        key="nopark", label="No Park", servers_key="mcpServers", disabled_key=None
    )
    servers = [
        c.ServerEntry("on", {"command": "a"}, True),
        c.ServerEntry("off", {"command": "b"}, False),
    ]
    out = c.apply_servers({}, servers, no_park)
    assert out == {"mcpServers": {"on": {"command": "a"}}}
    assert c.DISABLED_KEY not in out
    assert no_park.section_keys() == ("mcpServers",)
 def test_section_keys_reports_both_when_a_disabled_key_exists():
    assert c.CLAUDE_DESKTOP.section_keys() == ("mcpServers", c.DISABLED_KEY)
    assert _VSCODE.section_keys() == ("servers", "_bccDisabledServers")
 def test_malformed_entry_round_trips_through_the_default_spec():
    # #72's non-object server value must still be preserved verbatim on save.
    cfg = {"mcpServers": {"bad": "oops", "good": {"command": "x"}}}
    servers = c.extract_servers(cfg)
    assert any(s.malformed and s.name == "bad" for s in servers)
    out = c.apply_servers({}, servers)
    assert out["mcpServers"]["bad"] == "oops"
 def test_server_sections_and_change_summary_follow_a_custom_key(tmp_path):
    loaded = {"servers": {"a": {"type": "stdio", "command": "x", "env": {"API_KEY": "sekret"}}}}
    sections = c._server_sections(loaded, _VSCODE)
    assert "servers" in sections
    assert "mcpServers" not in sections
    # secret masking still applies through the custom key
    assert sections["servers"]["a"]["env"]["API_KEY"] == c.MASK
    disk = {"servers": {"a": {"type": "stdio", "command": "CHANGED"}}}
    p = tmp_path / "vscode.json"
    p.write_text(json.dumps(disk), encoding="utf-8")
    changed_keys, diff = c.external_change_summary(loaded, p, _VSCODE)
    assert "servers" in changed_keys
    assert diff  # a server-section change under the custom key is diffed
 # --------------------------------------------------------------------------- #
 # Project profile label disambiguation (issue #74)
 # --------------------------------------------------------------------------- #
 def test_disambiguate_labels_no_collision_uses_basename():
    dirs = [Path("/home/me/work/api"), Path("/home/me/work/web")]
    labels = c.disambiguate_project_labels(dirs)
    assert labels[Path("/home/me/work/api")] == "Project: api"
    assert labels[Path("/home/me/work/web")] == "Project: web"
 def test_disambiguate_labels_widens_only_colliding_basenames():
    dirs = [
        Path("/home/me/work/app"),
        Path("/home/me/personal/app"),
        Path("/home/me/notes"),
    ]
    labels = c.disambiguate_project_labels(dirs)
    # the two "app"s widen by one parent; the unique "notes" stays plain
    assert labels[Path("/home/me/work/app")] == "Project: work/app"
    assert labels[Path("/home/me/personal/app")] == "Project: personal/app"
    assert labels[Path("/home/me/notes")] == "Project: notes"
 def test_disambiguate_labels_widens_further_when_parent_also_collides():
    dirs = [Path("/a/x/app"), Path("/b/x/app")]
    labels = c.disambiguate_project_labels(dirs)
    assert labels[Path("/a/x/app")] == "Project: a/x/app"
    assert labels[Path("/b/x/app")] == "Project: b/x/app"
 def _write_project(tmp_path, name, mcp_content):
    d = tmp_path / name
    d.mkdir(parents=True)
    if mcp_content is not None:
        (d / ".mcp.json").write_text(mcp_content, encoding="utf-8")
    return d
 def _claude_json_with_projects(tmp_path, dirs):
    cj = tmp_path / ".claude.json"
    cj.write_text(json.dumps({"projects": {str(d): {} for d in dirs}}), encoding="utf-8")
    return cj
 def test_discover_project_configs_disambiguates_same_basename(tmp_path):
    d1 = _write_project(tmp_path / "work", "app", '{"mcpServers": {}}')
    d2 = _write_project(tmp_path / "personal", "app", '{"mcpServers": {}}')
    cj = _claude_json_with_projects(tmp_path, [d1, d2])
    profiles = c.discover_project_configs(cj)
    labels = sorted(p.label for p in profiles)
    assert labels == ["Project: personal/app", "Project: work/app"]
 def test_discover_project_configs_skips_non_object_and_garbage(tmp_path):
    good = _write_project(tmp_path, "good", '{"mcpServers": {}}')
    array = _write_project(tmp_path, "arr", "[1, 2, 3]")
    garbage = _write_project(tmp_path, "junk", "not json at all")
    missing = tmp_path / "nofile"
    missing.mkdir()
    cj = _claude_json_with_projects(tmp_path, [good, array, garbage, missing])
    profiles = c.discover_project_configs(cj)
    paths = {str(p.path) for p in profiles}
    assert str(good / ".mcp.json") in paths
    assert str(array / ".mcp.json") not in paths
    assert str(garbage / ".mcp.json") not in paths
    assert str(missing / ".mcp.json") not in paths
 # --------------------------------------------------------------------------- #
 # Move to environment variable (issue #83)
 # --------------------------------------------------------------------------- #
@pytest.mark.parametrize(
    "raw,expected",
    [
        ("API_KEY", "API_KEY"),
        ("api-key", "API_KEY"),
        ("x.y z", "X_Y_Z"),
        ("2fa", "_2FA"),
        ("", "VAR"),
        ("***", "VAR"),
        ("clé", "CL_"),  # non-ASCII becomes _
    ],
 )
 def test_sanitize_env_var_name(raw, expected):
    assert c.sanitize_env_var_name(raw) == expected
 def test_shell_export_lines_quote_safely():
    lines = c.shell_export_lines("TOKEN", "ab'cd")
    assert lines["posix"] == "export TOKEN='ab'\\''cd'"
    assert lines["windows"] == 'setx TOKEN "ab\'cd"'
 def test_can_move_gate_requires_secret_and_expanding_client():
    desktop = c.Profile(label="d", path="/x/Claude/claude_desktop_config.json", config_exists=True)
    code = c.Profile(label="c", path=Path.home() / ".claude.json", config_exists=True)
    # real secret on an expanding client -> offer
    assert c.can_move_value_to_env_ref("API_KEY", "ghp_abc", code) is True
    assert c.can_move_value_to_env_ref("API_KEY", "ghp_abc", None) is True
    # non-secret key -> no
    assert c.can_move_value_to_env_ref("REGION", "us-east-1", code) is False
    # already a reference -> no
    assert c.can_move_value_to_env_ref("API_KEY", "${API_KEY}", code) is False
    # non-expanding client (Claude Desktop) -> refuse even a real secret
    assert c.can_move_value_to_env_ref("API_KEY", "ghp_abc", desktop) is False
 def test_move_env_value_replaces_with_reference_and_returns_secret():
    data = {"command": "x", "env": {"API_KEY": "ghp_secret", "REGION": "us"}}
    conv = c.move_value_to_env_ref(data, field="env", key="API_KEY")
    assert conv is not None
    assert conv.var_name == "API_KEY"
    assert conv.reference == "${API_KEY}"
    assert conv.secret == "ghp_secret"
    assert conv.data["env"]["API_KEY"] == "${API_KEY}"
    # non-secret row untouched
    assert conv.data["env"]["REGION"] == "us"
    # input never mutated
    assert data["env"]["API_KEY"] == "ghp_secret"
 def test_move_derives_and_sanitises_var_name_from_key():
    data = {"headers": {"x-api-key": "sekret"}}
    conv = c.move_value_to_env_ref(data, field="headers", key="x-api-key")
    assert conv.var_name == "X_API_KEY"
    assert conv.data["headers"]["x-api-key"] == "${X_API_KEY}"
 def test_move_honours_explicit_var_name():
    data = {"env": {"tok": "sekret"}}
    conv = c.move_value_to_env_ref(data, field="env", key="tok", var_name="GITHUB_TOKEN")
    assert conv.reference == "${GITHUB_TOKEN}"
    assert conv.data["env"]["tok"] == "${GITHUB_TOKEN}"
 def test_move_args_by_index():
    data = {"command": "x", "args": ["--token", "ghp_secret"]}
    conv = c.move_value_to_env_ref(data, field="args", index=1, var_name="GH_TOKEN")
    assert conv.secret == "ghp_secret"
    assert conv.data["args"] == ["--token", "${GH_TOKEN}"]
 def test_move_returns_none_on_missing_or_nonstring_or_already_ref():
    data = {"env": {"API_KEY": "${API_KEY}", "N": 5}}
    assert c.move_value_to_env_ref(data, field="env", key="ABSENT") is None
    assert c.move_value_to_env_ref(data, field="env", key="N") is None  # not a string
    assert c.move_value_to_env_ref(data, field="env", key="API_KEY") is None  # already a ref
    assert c.move_value_to_env_ref({}, field="bogus") is None
    assert c.move_value_to_env_ref({"args": ["a"]}, field="args", index=9) is None
 def test_is_env_var_set():
    assert c.is_env_var_set("FOO", {"FOO": "x"}) is True
    assert c.is_env_var_set("FOO", {"FOO": ""}) is False
    assert c.is_env_var_set("FOO", {}) is False
 # --------------------------------------------------------------------------- #
 # Args secret indices + suggested var name (issue #83, args surface)
 # --------------------------------------------------------------------------- #
 def test_secret_arg_indices_flags_token_and_flag_value():
    args = ["--port", "8080", "ghp_deadbeef", "--token", "sk-abc", "--flag=val"]
    idxs = c.secret_arg_indices(args)
    assert 2 in idxs  # ghp_ token prefix
    assert 4 in idxs  # value following --token
    assert 1 not in idxs  # 8080
    assert 5 not in idxs  # --flag=val inline pair
 def test_secret_arg_indices_flags_embedded_url_credentials():
    args = ["postgres://user:pass@host/db"]
    assert c.secret_arg_indices(args) == [0]
 def test_secret_arg_indices_excludes_existing_references():
    args = ["--token", "${GH_TOKEN}"]
    assert c.secret_arg_indices(args) == []
 def test_suggested_env_var_for_arg_uses_preceding_flag():
    args = ["--api-key", "sk-secret"]
    assert c.suggested_env_var_for_arg(args, 1) == "API_KEY"
 def test_suggested_env_var_for_arg_falls_back_when_no_flag():
    args = ["ghp_secret"]
    assert c.suggested_env_var_for_arg(args, 0) == "SECRET"
 # --------------------------------------------------------------------------- #
 # Referenced-variables readout + args->env relocation (issue #83, "both")
 # --------------------------------------------------------------------------- #
 def test_referenced_env_vars_dedupes_and_reports_status():
    data = {
        "command": "npx",
        "args": ["--token", "${GH_TOKEN}", "${GH_TOKEN}"],
        "env": {"API_KEY": "${API_KEY}", "REGION": "${REGION:-us-east-1}"},
    }
    usages = c.referenced_env_vars(data, environ={"GH_TOKEN": "x"})
    by = {u.name: u for u in usages}
    assert set(by) == {"GH_TOKEN", "API_KEY", "REGION"}
    # GH_TOKEN appears only in args, deduped to one entry, set in env -> resolved
    assert by["GH_TOKEN"].fields == ("args",)
    assert by["GH_TOKEN"].resolved is True
    # API_KEY not set, no default -> unresolved
    assert by["API_KEY"].resolved is False
    # REGION has a default -> resolved regardless of environment
    assert by["REGION"].has_default is True
    assert by["REGION"].resolved is True
    # sorted by name
    assert [u.name for u in usages] == sorted(u.name for u in usages)
 def test_referenced_env_vars_empty_when_no_refs():
    assert c.referenced_env_vars({"command": "npx", "args": ["-y", "pkg"]}) == []
 def test_move_arg_to_env_block_removes_flag_and_value():
    data = {"command": "x", "args": ["--api-key", "sk-secret", "run"], "env": {"KEEP": "1"}}
    out = c.move_arg_to_env_block(data, 1)
    assert out["args"] == ["run"]  # flag + value both gone
    assert out["env"]["API_KEY"] == "sk-secret"
    assert out["env"]["KEEP"] == "1"  # existing env preserved
    # input not mutated
    assert data["args"] == ["--api-key", "sk-secret", "run"]
 def test_move_arg_to_env_block_bare_positional_keeps_no_flag():
    data = {"command": "x", "args": ["ghp_secret", "serve"]}
    out = c.move_arg_to_env_block(data, 0, var_name="GITHUB_TOKEN")
    assert out["args"] == ["serve"]
    assert out["env"] == {"GITHUB_TOKEN": "ghp_secret"}
 def test_move_arg_to_env_block_none_on_bad_target():
    assert c.move_arg_to_env_block({"args": ["a"]}, 5) is None
    assert c.move_arg_to_env_block({"args": ["${REF}"]}, 0) is None  # already a ref
    assert c.move_arg_to_env_block({}, 0) is None
`@@ -1,2 +1 @@`
	`PySide6>=6.6`	`PySide6>=6.6`
	`cryptography>=42.0 # bcc_core imports it at load (catalog signature verification)`