feat: "Move to environment variable" — convert a plaintext secret to ${VAR} (#83)

Follow-up to #76/#82: BCC warns when a config holds a raw credential and
points at ${VAR}, but gave no way to make the change. This adds the
one-click conversion, right-click a secret row in the env or headers table.

The value is about to leave the file, so the action's real job is handing
the secret back before it does:

- Core (pure, tested): sanitize_env_var_name (key -> legal upper-case shell
  name; 'api-key' -> API_KEY, '2fa' -> _2FA, non-ASCII/empty handled),
  shell_export_lines (the exact export/setx line, POSIX single-quoted
  safely), move_value_to_env_ref (data in -> new data out, replaces one
  env/header/args value with ${VAR}, returns the removed secret; never
  mutates the input; None if the target is missing, non-string, or already a
  reference), can_move_value_to_env_ref (offer only a real stored secret, not
  already a ref, AND only on a client that expands references -- offering it
  on Claude Desktop would author a config that reaches the server as literal
  ${VAR}, the exact failure #76 exists to prevent), and is_env_var_set (skip
  the ceremony when the variable already looks set).

- GUI: KeyValueTable gains a context menu gated on can_move_value_to_env_ref
  (so it never appears on a non-secret row or a Claude Desktop profile).
  MoveToEnvDialog lets the user name the variable (defaulting to the
  sanitised key), shows the platform-appropriate shell line live, notes when
  the variable already looks set, and on accept copies the secret to the
  clipboard before the cell is replaced with the reference. Wired through
  ServerEditor.set_profile_provider so the tables know which client is loaded.

Scope note: env and headers rows for now. The core already handles args by
index; wiring the args editor (a free-text widget, not a table) is a small
follow-up, deliberately not bundled here.

Tests: +15 core (name sanitisation incl. non-ASCII/leading-digit/empty,
POSIX quote safety, the gate across secret/non-secret/already-ref/
non-expanding-client, env+headers+args rewrite, input-not-mutated,
missing/non-string/already-ref -> None, is_env_var_set). 478 passed, ruff
clean. GUI is untestable in CI (no PySide6); the decision logic all lives in
bcc_core and is tested there.

Closes #83

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EKwBecy6N83jnqQmw8ezwE

CLAUDE.md

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

bcc.spec

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

requirements-dev.txt

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

requirements.txt

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

tests/test_core.py

@@ -1,6 +1,7 @@
 """Pytest port of the original test_core.py script (same 23 behaviours, now
 proper test functions with tmp_path/monkeypatch fixtures)."""
 
+import dataclasses
 import json
 import os
 import re
@@ -2357,43 +2358,6 @@ def test_catalog_entry_to_paste_json_includes_env_when_present():
     assert c.validate_catalog(malicious) != []
 
 
-def test_catalog_entry_to_paste_json_seeds_env_required_keys():
-    # Regression: env_required is where the seed data actually keeps its
-    # secret VAR NAMES (postgres/github/notion/etc. all declare their secret
-    # here with config.env left empty) -- catalog_entry_to_paste_json must
-    # surface those names as blank env rows, not silently drop them.
-    entry = _minimal_catalog()["servers"][0]
-    entry["env_required"] = {"DATABASE_URI": ""}
-    result = c.catalog_entry_to_paste_json(entry)
-    assert result["widget"]["env"] == {"DATABASE_URI": ""}
-
-
-def test_catalog_entry_to_paste_json_config_env_wins_over_env_required_default():
-    entry = _minimal_catalog()["servers"][0]
-    entry["config"]["env"] = {"GRAFANA_URL": "<GRAFANA_URL>"}
-    entry["env_required"] = {"GRAFANA_URL": "", "GRAFANA_SERVICE_ACCOUNT_TOKEN": ""}
-    result = c.catalog_entry_to_paste_json(entry)
-    assert result["widget"]["env"] == {
-        "GRAFANA_URL": "<GRAFANA_URL>",
-        "GRAFANA_SERVICE_ACCOUNT_TOKEN": "",
-    }
-
-
-def test_catalog_entry_to_paste_json_real_postgres_entry_seeds_database_uri():
-    """End-to-end regression against the actual shipped postgres entry,
-    which needs DATABASE_URI via env_required and has no config.env at
-    all -- this is exactly the shape that was silently dropping the env
-    field before catalog_entry_to_paste_json accounted for env_required."""
-    root = Path(__file__).resolve().parent.parent
-    raw = (root / "data" / "catalog.json").read_bytes()
-    data = c.load_catalog(raw)
-    entry = next(s for s in data["servers"] if s["id"] == "postgres")
-    result = c.catalog_entry_to_paste_json(entry)
-    assert result["postgres"]["env"] == {"DATABASE_URI": ""}
-    # And the focus-target helper now has something to point the user at.
-    assert c.first_unfilled_focus_target(result["postgres"]) == ("env", "DATABASE_URI")
-
-
 def test_config_has_unfilled_placeholders_true_for_token():
     cfg = {"command": "npx", "args": ["-y", "server", "<ALLOWED_DIR>"]}
     assert c.config_has_unfilled_placeholders(cfg) is True
@@ -2410,307 +2374,709 @@ def test_config_has_unfilled_placeholders_checks_env_too():
 
 
 # --------------------------------------------------------------------------- #
-# Browse-catalog dialog helpers (issue #10 phase 2)
+# #72 -- a server value that isn't a JSON object must not take the load down
 # --------------------------------------------------------------------------- #
+@pytest.mark.parametrize("bad", ["not-a-dict", 123, ["a", "b"], None, True, 1.5])
+def test_extract_servers_survives_non_dict_server_value(bad):
+    entries = c.extract_servers({"mcpServers": {"foo": bad}})
+    assert len(entries) == 1
+    assert entries[0].name == "foo"
+    assert entries[0].data == {}
+    assert entries[0].malformed is True
+    assert entries[0].raw == bad
 
 
-# --- catalog_category_group / CATALOG_CATEGORY_GROUPS --------------------- #
+def test_extract_servers_marks_only_the_bad_entry():
+    cfg = {"mcpServers": {"good": {"command": "npx"}, "bad": "oops"}}
+    by_name = {e.name: e for e in c.extract_servers(cfg)}
+    assert by_name["good"].malformed is False
+    assert by_name["good"].data == {"command": "npx"}
+    assert by_name["bad"].malformed is True
+
+
+def test_extract_servers_handles_malformed_disabled_entry():
+    entries = c.extract_servers({c.DISABLED_KEY: {"parked": ["nope"]}})
+    assert entries[0].enabled is False
+    assert entries[0].malformed is True
+
+
+def test_malformed_entry_round_trips_through_save_unchanged():
+    """The cardinal rule: never silently delete what the user had on disk."""
+    cfg = {"mcpServers": {"good": {"command": "npx"}, "bad": "oops"}}
+    servers = c.extract_servers(cfg)
+    out = c.apply_servers(dict(cfg), servers)
+    assert out["mcpServers"]["bad"] == "oops"
+    assert out["mcpServers"]["good"] == {"command": "npx"}
+
+
+def test_editing_a_malformed_entry_retires_the_raw_value():
+    entry = c.extract_servers({"mcpServers": {"bad": "oops"}})[0]
+    entry.set_data({"command": "npx"})
+    assert entry.malformed is False
+    assert entry.config_value() == {"command": "npx"}
+    assert c.apply_servers({}, [entry])["mcpServers"]["bad"] == {"command": "npx"}
+
+
+def test_lint_reports_the_malformed_entry_by_name():
+    servers = c.extract_servers({"mcpServers": {"bad": "oops"}})
+    warnings = c.lint_servers(servers)
+    assert len(warnings) == 1
+    assert "'bad'" in warnings[0]
+    assert "not an object" in warnings[0]
+    assert "str" in warnings[0]
+
+
+def test_lint_still_reports_normal_warnings_alongside_malformed():
+    cfg = {"mcpServers": {"bad": "oops", "sloppy": {"command": "npx", "args": "one two"}}}
+    warnings = c.lint_servers(c.extract_servers(cfg))
+    assert any("not an object" in w for w in warnings)
+    assert any("'args' should be a list" in w for w in warnings)
+
+
+# --------------------------------------------------------------------------- #
+# #73 -- the stale-file merge must not discard BCC-authored keys
+# --------------------------------------------------------------------------- #
+def test_carry_owned_keys_moves_sets_onto_the_reloaded_config():
+    local = {"mcpServers": {}, c.SETS_KEY: {"work": ["a", "b"]}}
+    fresh = {"mcpServers": {"external": {"command": "npx"}}}
+    contested = c.carry_owned_keys(local, fresh)
+    assert contested == []
+    assert fresh[c.SETS_KEY] == {"work": ["a", "b"]}
+    assert fresh["mcpServers"] == {"external": {"command": "npx"}}
+
+
+def test_carry_owned_keys_reports_a_genuine_conflict():
+    local = {c.SETS_KEY: {"work": ["a"]}}
+    fresh = {c.SETS_KEY: {"work": ["a", "b"]}}
+    assert c.carry_owned_keys(local, fresh) == [c.SETS_KEY]
+    assert fresh[c.SETS_KEY] == {"work": ["a"]}  # local wins: BCC owns the key
+
+
+def test_carry_owned_keys_is_quiet_when_both_sides_agree():
+    local = {c.SETS_KEY: {"work": ["a"]}}
+    fresh = {c.SETS_KEY: {"work": ["a"]}}
+    assert c.carry_owned_keys(local, fresh) == []
+
+
+def test_carry_owned_keys_leaves_disk_alone_when_absent_locally():
+    """Can't distinguish 'deleted my last set' from 'never had sets'; keep theirs."""
+    fresh = {c.SETS_KEY: {"remote": ["a"]}}
+    assert c.carry_owned_keys({}, fresh) == []
+    assert fresh[c.SETS_KEY] == {"remote": ["a"]}
+
+
+def test_carry_owned_keys_deep_copies_so_later_edits_do_not_leak():
+    local = {c.SETS_KEY: {"work": ["a"]}}
+    fresh = {}
+    c.carry_owned_keys(local, fresh)
+    local[c.SETS_KEY]["work"].append("b")
+    assert fresh[c.SETS_KEY] == {"work": ["a"]}
+
+
+def test_merge_flow_preserves_sets_and_external_servers(tmp_path):
+    """End-to-end shape of the Merge & save path that lost sets in #73."""
+    path = tmp_path / "claude.json"
+    path.write_text(json.dumps({"mcpServers": {"old": {"command": "old"}}}))
+
+    # BCC loads, user saves a named set and edits servers in memory.
+    local = c.load_config(path)
+    servers = c.extract_servers(local)
+    c.save_server_set(local, "work", servers)
+
+    # Something else rewrites the file underneath us.
+    path.write_text(json.dumps({"mcpServers": {"external": {"command": "new"}}, "other": 1}))
+
+    # Merge & save: reload disk, carry BCC keys, re-apply the user's servers.
+    fresh = c.load_config(path)
+    c.carry_owned_keys(local, fresh)
+    c.apply_servers(fresh, servers)
+    c.write_config(path, fresh)
+
+    saved = c.load_config(path)
+    assert saved[c.SETS_KEY] == {"work": ["old"]}  # the set survived
+    assert saved["other"] == 1  # unrelated external key preserved
+    assert "old" in saved["mcpServers"]  # user's servers re-applied
+
+
+def test_null_server_value_is_malformed_not_mistaken_for_absent():
+    """`{"mcpServers": {"foo": null}}` is legal JSON and a real malformed case,
+    so None must not double as the 'nothing here' sentinel."""
+    entry = c.extract_servers({"mcpServers": {"foo": None}})[0]
+    assert entry.malformed is True
+    assert entry.raw is None
+    assert c.apply_servers({}, [entry])["mcpServers"]["foo"] is None
+
+
+def test_a_normal_entry_is_not_malformed():
+    entry = c.extract_servers({"mcpServers": {"foo": {"command": "npx"}}})[0]
+    assert entry.malformed is False
+    assert entry.raw is c.NO_RAW
+
+
+# --------------------------------------------------------------------------- #
+# #75 -- theming
+# --------------------------------------------------------------------------- #
 @pytest.mark.parametrize(
-    "category,expected_group",
+    "setting,system_dark,expected",
     [
-        ("files", "Files & Dev"),
-        ("dev", "Files & Dev"),
-        ("code-hosting", "Files & Dev"),
-        ("browser", "Files & Dev"),
-        ("database", "Data"),
-        ("data", "Data"),
-        ("search", "Search & AI"),
-        ("ai", "Search & AI"),
-        ("cloud", "Cloud & Infra"),
-        ("infra", "Cloud & Infra"),
-        ("observability", "Cloud & Infra"),
-        ("productivity", "Work"),
-        ("communication", "Work"),
-        ("crm", "Work"),
-        ("finance", "Work"),
-        ("design", "Work"),
-        ("media", "Home & Personal"),
-        ("smart-home", "Home & Personal"),
-        ("personal", "Home & Personal"),
+        (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_catalog_category_group_maps_every_taxonomy_value(category, expected_group):
-    assert c.catalog_category_group(category) == expected_group
+def test_resolve_theme_covers_every_setting_and_appearance(setting, system_dark, expected):
+    assert c.resolve_theme(setting, system_dark) == expected
 
 
-def test_catalog_category_group_unknown_falls_back_to_other():
-    assert c.catalog_category_group("some-future-category-nobody-has-seen-yet") == "Other"
-    assert c.catalog_category_group("") == "Other"
-    assert c.catalog_category_group(None) == "Other"
+@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_catalog_category_group_is_case_insensitive():
-    assert c.catalog_category_group("Files") == "Files & Dev"
-    assert c.catalog_category_group("DATABASE") == "Data"
+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_shipped_catalog_categories_all_have_a_known_group():
-    """Regression: every category actually used in data/catalog.json must
-    collapse to one of the 7 chips, never silently drop an entry."""
-    root = Path(__file__).resolve().parent.parent
-    raw = (root / "data" / "catalog.json").read_bytes()
-    data = c.load_catalog(raw)
-    for entry in data["servers"]:
-        group = c.catalog_category_group(entry["category"])
-        assert group in c.CATALOG_CATEGORY_CHIPS
+def test_palette_for_unknown_name_falls_back_to_dark():
+    assert c.palette_for("chartreuse") is c.DARK_PALETTE
 
 
-# --- catalog_entry_matches_query / filter_catalog_entries ------------------ #
-def _catalog_entries():
-    return [
-        {
-            "id": "filesystem",
-            "display": "Filesystem",
-            "description": "Read/write access to local directories you choose.",
-            "category": "files",
-        },
-        {
-            "id": "postgres",
-            "display": "Postgres MCP Pro",
-            "description": "Query and inspect a PostgreSQL database.",
-            "category": "database",
-        },
-        {
-            "id": "slack",
-            "display": "Slack",
-            "description": "Search messages and send messages from your assistant.",
-            "category": "communication",
-        },
-    ]
+def test_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_catalog_entry_matches_query_empty_matches_everything():
-    entries = _catalog_entries()
-    assert c.filter_catalog_entries(entries, "") == entries
-    assert c.filter_catalog_entries(entries, "   ") == entries
+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}"
 
 
-def test_catalog_entry_matches_query_matches_id():
-    result = c.filter_catalog_entries(_catalog_entries(), "postgres")
-    assert [e["id"] for e in result] == ["postgres"]
+@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
 
 
-def test_catalog_entry_matches_query_matches_display_case_insensitive():
-    result = c.filter_catalog_entries(_catalog_entries(), "SLACK")
-    assert [e["id"] for e in result] == ["slack"]
+@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_catalog_entry_matches_query_matches_description():
-    result = c.filter_catalog_entries(_catalog_entries(), "PostgreSQL database")
-    assert [e["id"] for e in result] == ["postgres"]
+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_catalog_entry_matches_query_matches_category():
-    result = c.filter_catalog_entries(_catalog_entries(), "database")
-    assert [e["id"] for e in result] == ["postgres"]
+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_catalog_entry_matches_query_no_match_returns_empty():
-    assert c.filter_catalog_entries(_catalog_entries(), "kubernetes") == []
+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_catalog_entries_in_group_all_returns_everything():
-    entries = _catalog_entries()
-    assert c.catalog_entries_in_group(entries, "All") == entries
-    assert c.catalog_entries_in_group(entries, "") == entries
-    assert c.catalog_entries_in_group(entries, None) == entries
+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"
 
 
-def test_catalog_entries_in_group_filters_by_collapsed_category():
-    result = c.catalog_entries_in_group(_catalog_entries(), "Data")
-    assert [e["id"] for e in result] == ["postgres"]
-
-    result = c.catalog_entries_in_group(_catalog_entries(), "Work")
-    assert [e["id"] for e in result] == ["slack"]
+# --------------------------------------------------------------------------- #
+# #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"]
 
 
-# --- format_freshness_hint -------------------------------------------------- #
-def test_format_freshness_hint_none_returns_empty_string():
-    assert c.format_freshness_hint(None) == ""
-    assert c.format_freshness_hint("") == ""
+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
 
 
-def test_format_freshness_hint_unparseable_returns_empty_string():
-    assert c.format_freshness_hint("not-a-date") == ""
+@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_format_freshness_hint_this_month():
-    assert (
-        c.format_freshness_hint("2026-07-01", today=c.date(2026, 7, 12))
-        == "Last updated this month"
-    )
+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_format_freshness_hint_one_month_singular():
-    assert (
-        c.format_freshness_hint("2026-06-01", today=c.date(2026, 7, 12))
-        == "Last updated 1 month ago"
-    )
+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_format_freshness_hint_months_ago():
-    # Exactly 14 full months elapsed, no day-of-month remainder to round off.
-    assert (
-        c.format_freshness_hint("2025-01-15", today=c.date(2026, 3, 15))
-        == "Last updated 14 months ago"
-    )
+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_format_freshness_hint_rounds_down_partial_month():
-    # 2025-05-16 -> 2026-07-12 is 13 full months, not 14: the 14th month
-    # would only complete on 2026-07-16.
-    assert (
-        c.format_freshness_hint("2025-05-16", today=c.date(2026, 7, 12))
-        == "Last updated 13 months ago"
-    )
+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"
 
 
-def test_format_freshness_hint_years_ago():
-    assert (
-        c.format_freshness_hint("2024-01-01", today=c.date(2026, 7, 12))
-        == "Last updated 2 years ago"
-    )
+# --------------------------------------------------------------------------- #
+# #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")]
 
 
-def test_format_freshness_hint_23_months_stays_in_months_not_years():
-    # The switch to "N years ago" happens at 24 full months, not 12 -- the
-    # whole point of this hint is the granular "14 months ago" phrasing the
-    # design comment on #10 asked for, so 13-23 months must stay in months.
-    assert (
-        c.format_freshness_hint("2024-08-12", today=c.date(2026, 7, 12))
-        == "Last updated 23 months ago"
-    )
+@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_format_freshness_hint_future_date_returns_empty_string():
-    # A last_release "in the future" relative to `today` is nonsensical --
-    # show nothing rather than a misleading negative offset.
-    assert c.format_freshness_hint("2027-01-01", today=c.date(2026, 7, 12)) == ""
+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
 
 
-# --- first_unfilled_focus_target -------------------------------------------- #
-def test_first_unfilled_focus_target_prefers_placeholder_arg():
+def test_server_env_refs_covers_all_five_documented_fields():
     data = {
-        "command": "npx",
-        "args": ["-y", "server", "<ALLOWED_DIR>"],
-        "env": {"API_KEY": ""},
+        "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"),
     }
-    assert c.first_unfilled_focus_target(data) == ("args", 2)
 
 
-def test_first_unfilled_focus_target_falls_back_to_first_blank_env():
-    data = {"command": "uvx", "args": ["mcp-grafana"], "env": {"GRAFANA_URL": ""}}
-    assert c.first_unfilled_focus_target(data) == ("env", "GRAFANA_URL")
+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_first_unfilled_focus_target_none_when_fully_filled():
-    data = {"command": "npx", "args": ["-y", "server"], "env": {"API_KEY": "sk-real-value"}}
-    assert c.first_unfilled_focus_target(data) is None
+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_first_unfilled_focus_target_none_for_config_with_no_env_or_args():
-    assert c.first_unfilled_focus_target({"command": "npx", "args": []}) is None
+def test_expand_env_refs_handles_several_in_one_string():
+    assert c.expand_env_refs("${A}/${B:-two}/${C}", {"A": "one"}) == "one/two/${C}"
 
 
-# --- load_bundled_catalog_entries ------------------------------------------- #
-def test_load_bundled_catalog_entries_valid_signature(tmp_path, monkeypatch):
-    priv = Ed25519PrivateKey.generate()
-    pub = priv.public_key().public_bytes_raw()
-    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
-
-    raw, sig = _signed(_minimal_catalog(version=1), priv)
-    catalog_path = tmp_path / "catalog.json"
-    sig_path = tmp_path / "catalog.json.sig"
-    catalog_path.write_bytes(raw)
-    sig_path.write_bytes(sig)
-
-    entries = c.load_bundled_catalog_entries(catalog_path, sig_path)
-    assert len(entries) == 1
-    assert entries[0]["id"] == "widget"
+def test_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"]
 
 
-def test_load_bundled_catalog_entries_tampered_payload_returns_empty_list(tmp_path, monkeypatch):
-    priv = Ed25519PrivateKey.generate()
-    pub = priv.public_key().public_bytes_raw()
-    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
-
-    raw, sig = _signed(_minimal_catalog(version=1), priv)
-    tampered = bytearray(raw)
-    tampered[-2] ^= 0xFF  # flip a byte inside the trailing bytes, still valid-ish JSON shape
-    catalog_path = tmp_path / "catalog.json"
-    sig_path = tmp_path / "catalog.json.sig"
-    catalog_path.write_bytes(bytes(tampered))
-    sig_path.write_bytes(sig)
-
-    assert c.load_bundled_catalog_entries(catalog_path, sig_path) == []
+# --- 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_load_bundled_catalog_entries_wrong_key_returns_empty_list(tmp_path, monkeypatch):
-    priv = Ed25519PrivateKey.generate()
-    other_priv = Ed25519PrivateKey.generate()
-    other_pub = other_priv.public_key().public_bytes_raw()
-    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [other_pub])
-
-    raw, sig = _signed(_minimal_catalog(version=1), priv)  # signed by the WRONG key
-    catalog_path = tmp_path / "catalog.json"
-    sig_path = tmp_path / "catalog.json.sig"
-    catalog_path.write_bytes(raw)
-    sig_path.write_bytes(sig)
-
-    assert c.load_bundled_catalog_entries(catalog_path, sig_path) == []
+def test_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_load_bundled_catalog_entries_missing_files_returns_empty_list(tmp_path):
-    assert c.load_bundled_catalog_entries(tmp_path / "nope.json", tmp_path / "nope.json.sig") == []
+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_load_bundled_catalog_entries_missing_sig_returns_empty_list(tmp_path, monkeypatch):
-    priv = Ed25519PrivateKey.generate()
-    pub = priv.public_key().public_bytes_raw()
-    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
-
-    raw, _sig = _signed(_minimal_catalog(version=1), priv)
-    catalog_path = tmp_path / "catalog.json"
-    catalog_path.write_bytes(raw)
-    missing_sig_path = tmp_path / "catalog.json.sig"  # never written
-
-    assert c.load_bundled_catalog_entries(catalog_path, missing_sig_path) == []
+def test_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_load_bundled_catalog_entries_invalid_but_signed_returns_empty_list(tmp_path, monkeypatch):
-    """A payload that verifies but fails validate_catalog() (disallowed
-    command) must still come back empty -- signing is necessary, not
-    sufficient."""
-    priv = Ed25519PrivateKey.generate()
-    pub = priv.public_key().public_bytes_raw()
-    monkeypatch.setattr(c, "CATALOG_PUBKEYS", [pub])
-
-    raw, sig = _signed(_catalog_with({"config": {"command": "bash", "args": []}}), priv)
-    catalog_path = tmp_path / "catalog.json"
-    sig_path = tmp_path / "catalog.json.sig"
-    catalog_path.write_bytes(raw)
-    sig_path.write_bytes(sig)
-
-    assert c.load_bundled_catalog_entries(catalog_path, sig_path) == []
+def test_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
 
 
-def test_load_bundled_catalog_entries_real_shipped_catalog():
-    """End-to-end regression against the actual bundled data/catalog.json +
-    .sig, using the real CATALOG_PUBKEYS (no monkeypatch) -- this is what
-    the Browse dialog actually calls on startup."""
-    root = Path(__file__).resolve().parent.parent
-    entries = c.load_bundled_catalog_entries(
-        root / "data" / "catalog.json", root / "data" / "catalog.json.sig"
+# --- 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
     )
-    assert len(entries) == 19
-    assert {e["id"] for e in entries} >= {"filesystem", "github", "slack", "postgres"}
+    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
+
+
+# --------------------------------------------------------------------------- #
+# 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