refactor: introduce ClientSpec adapter; route Claude Desktop + Code through it (#5)

Cross-client support (Cursor / Windsurf / VS Code) was blocked on Claude's
layout being hard-coded throughout the code: servers always under the literal
"mcpServers" key, a fixed set of Claude file locations, and "which client is
this?" answered by sniffing a filename. Adding a client that differs on any of
those axes meant chasing those assumptions through a dozen sites.

This is phase 1 of #5: the keystone refactor, with NO behaviour change. It adds
a ClientSpec adapter that captures the three things that vary across clients --
the top-level servers key, the config discovery paths, and the per-server value
shape -- plus the capability flags that were previously computed inline from a
filename (does the client expand ${VAR}? can we offer Restart?).

- ClientSpec (frozen dataclass): servers_key, disabled_key, config_filename,
  expands_env_refs, supports_restart, and entry_to_internal/entry_from_internal
  -- the per-server translation seam, identity for any mcpServers-shaped client,
  the single point a differently-shaped client (VS Code's type/inputs form)
  overrides.
- CLAUDE_DESKTOP and CLAUDE_CODE specs; both use mcpServers + the existing
  parking key, so their translation is the identity and nothing changes for
  today's users. resolve_client(path) reproduces the old filename rule exactly;
  each Profile now carries its resolved .client.
- extract_servers / apply_servers / _server_sections / external_change_summary
  take an optional spec and default to Claude's layout, so every existing call
  site and test that omits a spec is byte-for-byte unchanged. The cardinal rule
  now generalises: apply_servers only ever writes the client's own two keys,
  parameterised rather than hard-coded.
- profile_targets_claude_desktop and client_expands_env_refs are now thin reads
  off the profile's spec -- one source of truth for client identity instead of
  scattered filename checks -- with identical answers.
- GUI: the load, Copy-to, save and stale-merge paths pass the profile's spec
  into the core calls. Mechanical; no logic moved into bcc.py (which CI can't
  test -- no PySide6).

Tests: +14. Existing suite unchanged and green (behaviour preservation). A
synthetic non-mcpServers spec ("servers" key, a different disabled key, a
per-server `type` field) exercises the whole pipeline -- extract, apply,
masking, external-change diff -- proving the seam actually generalises before
any real client depends on it. 458 passed, 1 skipped; ruff clean.

Refs #5

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

bcc_core.py

@@ -203,19 +203,156 @@ def update_notice(
     }
 
 
+# --------------------------------------------------------------------------- #
+# Client adapters (issue #5 — cross-client support)
+# --------------------------------------------------------------------------- #
+# BCC used to hard-code Claude's layout everywhere: the servers always lived
+# under the literal key "mcpServers", the config was always one of a fixed set
+# of Claude file locations, and "which client is this?" was answered by
+# comparing a filename. Other editors differ along exactly three axes:
+#
+#   * the top-level key the servers map lives under  (VS Code uses "servers")
+#   * where the config files are found               (each client, its own paths)
+#   * the per-server value shape                      (VS Code adds `type`/inputs)
+#
+# A ClientSpec captures all three in one place, plus the small capability flags
+# that used to be answered by filename sniffing (does the client expand ${VAR}
+# itself? can we offer a Restart action?). Everything downstream — tables,
+# editor, backups, stale-file check, secret masking — works against BCC's
+# internal ServerEntry model and never needs to know which client it came from.
+#
+# Phase 1 (this change) introduces the abstraction and routes the two existing
+# clients (Claude Desktop, Claude Code) through it with NO behaviour change:
+# both use "mcpServers" + the _disabledMcpServers parking key, so their entry
+# translation is the identity. Cursor/Windsurf (same shape, new paths) and
+# VS Code (different key + per-server shape) become small, isolated additions
+# on top of this seam — see issue #5's phased plan.
+@dataclass(frozen=True)
+class ClientSpec:
+    """Everything client-specific about one MCP host.
+
+    Frozen so the module-level specs are effectively singletons and safe to
+    share on every Profile. The `entry_*` translators are the extension point
+    for a client whose stored server value isn't already BCC's internal shape;
+    for every mcpServers-shaped client they're the identity. A future VS Code
+    spec overrides them (subclass, or a spec built with translating callables)
+    to map its `type`/inputs form to and from the internal model.
+    """
+
+    key: str
+    label: str
+    servers_key: str = "mcpServers"
+    # The parking key for disabled servers. None means the client has no place
+    # to keep a disabled definition (we'd just drop it); every client so far
+    # supports one.
+    disabled_key: str | None = DISABLED_KEY
+    # Basename that identifies this client's config on disk. Used only to keep
+    # `profile_targets_claude_desktop` answering exactly as it did before.
+    config_filename: str | None = None
+    # The client resolves ${VAR} references itself (Claude Code does; Claude
+    # Desktop does not — see client_expands_env_refs / issue #76).
+    expands_env_refs: bool = False
+    # A "Restart <client>" action makes sense (Claude Desktop only so far).
+    supports_restart: bool = False
+
+    def entry_to_internal(self, value):
+        """Client's stored value for one server -> BCC internal server data.
+
+        Identity for mcpServers-shaped clients. Non-dict values are passed
+        through untouched so `_server_entry` can preserve a malformed entry
+        verbatim (#72) rather than this layer having to know about that case.
+        """
+        return value
+
+    def entry_from_internal(self, data):
+        """BCC internal server data -> the client's stored value for one server."""
+        return data
+
+    def enabled_block(self, cfg: dict) -> dict:
+        """The map of enabled servers from a raw config dict (never None)."""
+        return cfg.get(self.servers_key) or {}
+
+    def disabled_block(self, cfg: dict) -> dict:
+        """The map of parked/disabled servers from a raw config dict."""
+        if not self.disabled_key:
+            return {}
+        return cfg.get(self.disabled_key) or {}
+
+    def section_keys(self) -> tuple[str, ...]:
+        """The top-level config keys this client's servers live under."""
+        if self.disabled_key:
+            return (self.servers_key, self.disabled_key)
+        return (self.servers_key,)
+
+
+CLAUDE_DESKTOP = ClientSpec(
+    key="claude_desktop",
+    label="Claude Desktop",
+    servers_key="mcpServers",
+    disabled_key=DISABLED_KEY,
+    config_filename=CONFIG_FILENAME,
+    expands_env_refs=False,
+    supports_restart=True,
+)
+
+CLAUDE_CODE = ClientSpec(
+    key="claude_code",
+    label="Claude Code",
+    servers_key="mcpServers",
+    disabled_key=DISABLED_KEY,
+    config_filename=None,
+    expands_env_refs=True,
+    supports_restart=False,
+)
+
+# Registry of known clients, and the default used when a caller doesn't supply
+# a spec. The default deliberately matches the pre-refactor constants
+# (mcpServers + _disabledMcpServers) so every existing call site and test that
+# omits a spec behaves exactly as before.
+CLIENT_SPECS: tuple[ClientSpec, ...] = (CLAUDE_DESKTOP, CLAUDE_CODE)
+DEFAULT_CLIENT = CLAUDE_DESKTOP
+
+
+def client_by_key(key: str) -> ClientSpec | None:
+    """Look up a registered ClientSpec by its stable `key`, or None."""
+    for spec in CLIENT_SPECS:
+        if spec.key == key:
+            return spec
+    return None
+
+
+def resolve_client(path: str | os.PathLike) -> ClientSpec:
+    """Pick the ClientSpec for a config path.
+
+    Reproduces the pre-refactor rule exactly: a file named
+    `claude_desktop_config.json` is Claude Desktop; everything else BCC edits
+    (`~/.claude.json`, a project `.mcp.json`, the legacy settings.json) is
+    Claude Code. That one rule is what `profile_targets_claude_desktop` and
+    `client_expands_env_refs` used to compute inline; now it lives here.
+    """
+    return CLAUDE_DESKTOP if Path(path).name == CONFIG_FILENAME else CLAUDE_CODE
+
+
 # --------------------------------------------------------------------------- #
 # Data model
 # --------------------------------------------------------------------------- #
 @dataclass
 class Profile:
-    """A discovered (or manually added) Claude install location."""
+    """A discovered (or manually added) MCP client config location."""
 
     label: str
     path: Path
     config_exists: bool
+    # Which client this config belongs to. Left None by most callers and
+    # resolved from the path (the historical rule), so existing
+    # Profile(label=, path=, config_exists=) construction keeps working and
+    # gets the right adapter for free.
+    client: ClientSpec | None = None
 
     def __post_init__(self):
         self.path = Path(self.path)
+        if self.client is None:
+            self.client = resolve_client(self.path)
 
 
 class _NoRaw:
@@ -623,8 +760,12 @@ def profile_targets_claude_desktop(profile: Profile) -> bool:
     or the legacy ~/.claude/settings.json). Used to gate Desktop-only actions
     like "Restart Claude Desktop" so they never show up for a Claude Code
     profile -- restarting the CLI makes no sense.
+
+    Now a thin read of the profile's resolved ClientSpec -- the identity of the
+    client lives on the spec instead of in scattered filename checks -- but the
+    answer is unchanged: true iff the config is a claude_desktop_config.json.
     """
-    return Path(profile.path).name == CONFIG_FILENAME
+    return profile.client.config_filename == CONFIG_FILENAME
 
 
 # --------------------------------------------------------------------------- #
@@ -684,17 +825,23 @@ def _server_entry(name: str, data, enabled: bool) -> ServerEntry:
     return ServerEntry(name=name, data={}, enabled=enabled, raw=data)
 
 
-def extract_servers(cfg: dict) -> list[ServerEntry]:
-    """Pull enabled (`mcpServers`) and disabled (`_disabledMcpServers`) servers.
+def extract_servers(cfg: dict, spec: ClientSpec | None = None) -> list[ServerEntry]:
+    """Pull enabled and disabled servers for `spec` (default: Claude's layout).
+
+    Reads the client's servers key + parking key and runs each stored value
+    through the client's `entry_to_internal` translator. With no `spec` this is
+    byte-for-byte the old behaviour (`mcpServers` + `_disabledMcpServers`,
+    identity translation).
 
     Never raises on a structurally-odd config -- malformed entries come back as
     empty-data entries carrying their original value (see `_server_entry`).
     """
+    spec = spec or DEFAULT_CLIENT
     out: list[ServerEntry] = []
-    for name, data in (cfg.get("mcpServers") or {}).items():
-        out.append(_server_entry(name, data, True))
-    for name, data in (cfg.get(DISABLED_KEY) or {}).items():
-        out.append(_server_entry(name, data, False))
+    for name, value in spec.enabled_block(cfg).items():
+        out.append(_server_entry(name, spec.entry_to_internal(value), True))
+    for name, value in spec.disabled_block(cfg).items():
+        out.append(_server_entry(name, spec.entry_to_internal(value), False))
     return out
 
 
@@ -781,19 +928,29 @@ def resolve_name_collision(name: str, existing: set[str]) -> str:
     return candidate
 
 
-def apply_servers(cfg: dict, servers: list[ServerEntry]) -> dict:
+def apply_servers(cfg: dict, servers: list[ServerEntry], spec: ClientSpec | None = None) -> dict:
     """
     Write the server list back into `cfg` in place, preserving every other key
-    and the position of `mcpServers`. Returns the same dict for convenience.
-    """
-    enabled = {s.name: s.config_value() for s in servers if s.enabled}
-    disabled = {s.name: s.config_value() for s in servers if not s.enabled}
+    and the position of the client's servers key. Returns the same dict for
+    convenience.
 
-    cfg["mcpServers"] = enabled  # replaces value if key existed; appends otherwise
-    if disabled:
-        cfg[DISABLED_KEY] = disabled
-    else:
-        cfg.pop(DISABLED_KEY, None)
+    The cardinal rule generalises cleanly: this still only ever touches the two
+    keys the client's servers live under (`spec.servers_key` and, if the client
+    has one, `spec.disabled_key`) and leaves everything else verbatim. With no
+    `spec` it writes `mcpServers` + `_disabledMcpServers` exactly as before.
+    """
+    spec = spec or DEFAULT_CLIENT
+    enabled = {s.name: spec.entry_from_internal(s.config_value()) for s in servers if s.enabled}
+    disabled = {
+        s.name: spec.entry_from_internal(s.config_value()) for s in servers if not s.enabled
+    }
+
+    cfg[spec.servers_key] = enabled  # replaces value if key existed; appends otherwise
+    if spec.disabled_key:
+        if disabled:
+            cfg[spec.disabled_key] = disabled
+        else:
+            cfg.pop(spec.disabled_key, None)
     return cfg
 
 
@@ -925,11 +1082,12 @@ def _redact_servers_block(block: dict | None) -> dict:
     return {name: _redact_server_data(data) for name, data in block.items()}
 
 
-def _server_sections(cfg: dict) -> dict:
+def _server_sections(cfg: dict, spec: ClientSpec | None = None) -> dict:
     """Return the masked server sections of a config dict, safe for diff display."""
-    out: dict = {"mcpServers": _redact_servers_block(cfg.get("mcpServers"))}
-    if DISABLED_KEY in cfg:
-        out[DISABLED_KEY] = _redact_servers_block(cfg.get(DISABLED_KEY))
+    spec = spec or DEFAULT_CLIENT
+    out: dict = {spec.servers_key: _redact_servers_block(cfg.get(spec.servers_key))}
+    if spec.disabled_key and spec.disabled_key in cfg:
+        out[spec.disabled_key] = _redact_servers_block(cfg.get(spec.disabled_key))
     return out
 
 
@@ -1039,7 +1197,9 @@ def config_fingerprint(path: Path | str) -> ConfigStat | None:
     return ConfigStat(st.st_mtime, st.st_size)
 
 
-def external_change_summary(original_cfg: dict, path: Path | str) -> tuple[list[str], str]:
+def external_change_summary(
+    original_cfg: dict, path: Path | str, spec: ClientSpec | None = None
+) -> tuple[list[str], str]:
     """
     Compare original_cfg (what BCC loaded) with the current on-disk state.
 
@@ -1048,6 +1208,7 @@ def external_change_summary(original_cfg: dict, path: Path | str) -> tuple[list[
       server_diff  — masked unified diff of server sections (empty if unchanged
                      or the file cannot be read).
     """
+    spec = spec or DEFAULT_CLIENT
     try:
         disk_cfg = load_config(Path(path))
     except Exception:
@@ -1057,12 +1218,12 @@ def external_change_summary(original_cfg: dict, path: Path | str) -> tuple[list[
     changed_keys = sorted(k for k in all_keys if original_cfg.get(k) != disk_cfg.get(k))
 
     server_diff = ""
-    if any(k in {"mcpServers", DISABLED_KEY} for k in changed_keys):
+    if any(k in set(spec.section_keys()) for k in changed_keys):
         before_lines = (
-            json.dumps(_server_sections(original_cfg), indent=2, ensure_ascii=False) + "\n"
+            json.dumps(_server_sections(original_cfg, spec), indent=2, ensure_ascii=False) + "\n"
         ).splitlines(keepends=True)
         after_lines = (
-            json.dumps(_server_sections(disk_cfg), indent=2, ensure_ascii=False) + "\n"
+            json.dumps(_server_sections(disk_cfg, spec), indent=2, ensure_ascii=False) + "\n"
         ).splitlines(keepends=True)
         server_diff = "".join(
             difflib.unified_diff(before_lines, after_lines, fromfile="loaded", tofile="on disk now")
@@ -1576,8 +1737,11 @@ def client_expands_env_refs(profile: Profile) -> bool:
     documented support, so a reference there reaches the server as literal
     text -- which surfaces as a confusing auth failure rather than an obvious
     config error, hence the warning.
+
+    Reads the capability straight off the profile's ClientSpec; the two Claude
+    specs carry the documented answer (Code yes, Desktop no).
     """
-    return not profile_targets_claude_desktop(profile)
+    return profile.client.expands_env_refs
 
 
 def env_ref_warnings(