Cross-client phase 1: ClientSpec adapter, no behavior change (#5) #85
@@ -31,6 +31,8 @@ The codebase is split into two layers:
|
|||||||
|
|
||||||
**`bcc_core.py`** — All logic with no GUI imports. Contains:
|
**`bcc_core.py`** — All logic with no GUI imports. Contains:
|
||||||
- `Profile` / `ServerEntry` dataclasses (the data model)
|
- `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…
|
- `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/`
|
- `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)
|
- `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
|
- `KeyValueTable` — reusable widget for env vars and headers
|
||||||
- `ConnTester(QThread)` — background thread for remote reachability tests
|
- `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.
|
Disabled servers are parked under `_disabledMcpServers` (which Claude Desktop ignores) so they can be re-enabled without losing their definition.
|
||||||
|
|
||||||
|
|||||||
@@ -2120,7 +2120,7 @@ class MainWindow(QMainWindow):
|
|||||||
# but keep it inside the guard: a load failure must leave the previously
|
# but keep it inside the guard: a load failure must leave the previously
|
||||||
# loaded profile intact instead of half-swapping the window's state.
|
# loaded profile intact instead of half-swapping the window's state.
|
||||||
try:
|
try:
|
||||||
servers = core.extract_servers(self.full_config)
|
servers = core.extract_servers(self.full_config, profile.client)
|
||||||
except Exception as exc: # pragma: no cover - defence in depth
|
except Exception as exc: # pragma: no cover - defence in depth
|
||||||
QMessageBox.critical(
|
QMessageBox.critical(
|
||||||
self,
|
self,
|
||||||
@@ -2590,7 +2590,7 @@ class MainWindow(QMainWindow):
|
|||||||
except Exception as e:
|
except Exception as e:
|
||||||
QMessageBox.critical(self, "Copy failed", f"Couldn't read {dest.label}:\n{e}")
|
QMessageBox.critical(self, "Copy failed", f"Couldn't read {dest.label}:\n{e}")
|
||||||
return
|
return
|
||||||
existing = core.extract_servers(dest_cfg)
|
existing = core.extract_servers(dest_cfg, dest.client)
|
||||||
names = {s.name for s in existing}
|
names = {s.name for s in existing}
|
||||||
if src.name in names:
|
if src.name in names:
|
||||||
ans = QMessageBox.question(
|
ans = QMessageBox.question(
|
||||||
@@ -2602,7 +2602,7 @@ class MainWindow(QMainWindow):
|
|||||||
return
|
return
|
||||||
existing = [s for s in existing if s.name != src.name]
|
existing = [s for s in existing if s.name != src.name]
|
||||||
existing.append(core.ServerEntry(src.name, dict(src.data), True))
|
existing.append(core.ServerEntry(src.name, dict(src.data), True))
|
||||||
core.apply_servers(dest_cfg, existing)
|
core.apply_servers(dest_cfg, existing, dest.client)
|
||||||
try:
|
try:
|
||||||
backup = core.write_config(dest.path, dest_cfg)
|
backup = core.write_config(dest.path, dest_cfg)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
@@ -2662,7 +2662,7 @@ class MainWindow(QMainWindow):
|
|||||||
and disk_stat != self._loaded_stat
|
and disk_stat != self._loaded_stat
|
||||||
):
|
):
|
||||||
changed_keys, server_diff = core.external_change_summary(
|
changed_keys, server_diff = core.external_change_summary(
|
||||||
self.full_config, self.current_profile.path
|
self.full_config, self.current_profile.path, self.current_profile.client
|
||||||
)
|
)
|
||||||
dlg = StaleDialog(self, str(self.current_profile.path), changed_keys, server_diff)
|
dlg = StaleDialog(self, str(self.current_profile.path), changed_keys, server_diff)
|
||||||
if not dlg.exec():
|
if not dlg.exec():
|
||||||
@@ -2678,7 +2678,7 @@ class MainWindow(QMainWindow):
|
|||||||
# changed in this session (named sets), which apply_servers
|
# changed in this session (named sets), which apply_servers
|
||||||
# doesn't write. Carry them over before saving (#73).
|
# doesn't write. Carry them over before saving (#73).
|
||||||
contested = core.carry_owned_keys(self.full_config, fresh)
|
contested = core.carry_owned_keys(self.full_config, fresh)
|
||||||
core.apply_servers(fresh, self.servers)
|
core.apply_servers(fresh, self.servers, self.current_profile.client)
|
||||||
try:
|
try:
|
||||||
backup = core.write_config(self.current_profile.path, fresh)
|
backup = core.write_config(self.current_profile.path, fresh)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
@@ -2703,7 +2703,7 @@ class MainWindow(QMainWindow):
|
|||||||
return
|
return
|
||||||
# else OVERWRITE: fall through to normal write
|
# else OVERWRITE: fall through to normal write
|
||||||
|
|
||||||
core.apply_servers(self.full_config, self.servers)
|
core.apply_servers(self.full_config, self.servers, self.current_profile.client)
|
||||||
try:
|
try:
|
||||||
backup = core.write_config(self.current_profile.path, self.full_config)
|
backup = core.write_config(self.current_profile.path, self.full_config)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
|||||||
+191
-27
@@ -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
|
# Data model
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
@dataclass
|
@dataclass
|
||||||
class Profile:
|
class Profile:
|
||||||
"""A discovered (or manually added) Claude install location."""
|
"""A discovered (or manually added) MCP client config location."""
|
||||||
|
|
||||||
label: str
|
label: str
|
||||||
path: Path
|
path: Path
|
||||||
config_exists: bool
|
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):
|
def __post_init__(self):
|
||||||
self.path = Path(self.path)
|
self.path = Path(self.path)
|
||||||
|
if self.client is None:
|
||||||
|
self.client = resolve_client(self.path)
|
||||||
|
|
||||||
|
|
||||||
class _NoRaw:
|
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
|
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
|
like "Restart Claude Desktop" so they never show up for a Claude Code
|
||||||
profile -- restarting the CLI makes no sense.
|
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)
|
return ServerEntry(name=name, data={}, enabled=enabled, raw=data)
|
||||||
|
|
||||||
|
|
||||||
def extract_servers(cfg: dict) -> list[ServerEntry]:
|
def extract_servers(cfg: dict, spec: ClientSpec | None = None) -> list[ServerEntry]:
|
||||||
"""Pull enabled (`mcpServers`) and disabled (`_disabledMcpServers`) servers.
|
"""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
|
Never raises on a structurally-odd config -- malformed entries come back as
|
||||||
empty-data entries carrying their original value (see `_server_entry`).
|
empty-data entries carrying their original value (see `_server_entry`).
|
||||||
"""
|
"""
|
||||||
|
spec = spec or DEFAULT_CLIENT
|
||||||
out: list[ServerEntry] = []
|
out: list[ServerEntry] = []
|
||||||
for name, data in (cfg.get("mcpServers") or {}).items():
|
for name, value in spec.enabled_block(cfg).items():
|
||||||
out.append(_server_entry(name, data, True))
|
out.append(_server_entry(name, spec.entry_to_internal(value), True))
|
||||||
for name, data in (cfg.get(DISABLED_KEY) or {}).items():
|
for name, value in spec.disabled_block(cfg).items():
|
||||||
out.append(_server_entry(name, data, False))
|
out.append(_server_entry(name, spec.entry_to_internal(value), False))
|
||||||
return out
|
return out
|
||||||
|
|
||||||
|
|
||||||
@@ -781,19 +928,29 @@ def resolve_name_collision(name: str, existing: set[str]) -> str:
|
|||||||
return candidate
|
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
|
Write the server list back into `cfg` in place, preserving every other key
|
||||||
and the position of `mcpServers`. Returns the same dict for convenience.
|
and the position of the client's servers key. 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}
|
|
||||||
|
|
||||||
cfg["mcpServers"] = enabled # replaces value if key existed; appends otherwise
|
The cardinal rule generalises cleanly: this still only ever touches the two
|
||||||
if disabled:
|
keys the client's servers live under (`spec.servers_key` and, if the client
|
||||||
cfg[DISABLED_KEY] = disabled
|
has one, `spec.disabled_key`) and leaves everything else verbatim. With no
|
||||||
else:
|
`spec` it writes `mcpServers` + `_disabledMcpServers` exactly as before.
|
||||||
cfg.pop(DISABLED_KEY, None)
|
"""
|
||||||
|
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
|
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()}
|
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."""
|
"""Return the masked server sections of a config dict, safe for diff display."""
|
||||||
out: dict = {"mcpServers": _redact_servers_block(cfg.get("mcpServers"))}
|
spec = spec or DEFAULT_CLIENT
|
||||||
if DISABLED_KEY in cfg:
|
out: dict = {spec.servers_key: _redact_servers_block(cfg.get(spec.servers_key))}
|
||||||
out[DISABLED_KEY] = _redact_servers_block(cfg.get(DISABLED_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
|
return out
|
||||||
|
|
||||||
|
|
||||||
@@ -1039,7 +1197,9 @@ def config_fingerprint(path: Path | str) -> ConfigStat | None:
|
|||||||
return ConfigStat(st.st_mtime, st.st_size)
|
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.
|
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
|
server_diff — masked unified diff of server sections (empty if unchanged
|
||||||
or the file cannot be read).
|
or the file cannot be read).
|
||||||
"""
|
"""
|
||||||
|
spec = spec or DEFAULT_CLIENT
|
||||||
try:
|
try:
|
||||||
disk_cfg = load_config(Path(path))
|
disk_cfg = load_config(Path(path))
|
||||||
except Exception:
|
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))
|
changed_keys = sorted(k for k in all_keys if original_cfg.get(k) != disk_cfg.get(k))
|
||||||
|
|
||||||
server_diff = ""
|
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 = (
|
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)
|
).splitlines(keepends=True)
|
||||||
after_lines = (
|
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)
|
).splitlines(keepends=True)
|
||||||
server_diff = "".join(
|
server_diff = "".join(
|
||||||
difflib.unified_diff(before_lines, after_lines, fromfile="loaded", tofile="on disk now")
|
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
|
documented support, so a reference there reaches the server as literal
|
||||||
text -- which surfaces as a confusing auth failure rather than an obvious
|
text -- which surfaces as a confusing auth failure rather than an obvious
|
||||||
config error, hence the warning.
|
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(
|
def env_ref_warnings(
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
# Runtime (also in requirements.txt)
|
# Runtime (also in requirements.txt)
|
||||||
PySide6>=6.6
|
PySide6>=6.6
|
||||||
|
cryptography>=42.0 # catalog signature verification (bcc_core) + release checksum signing
|
||||||
|
|
||||||
# Build / packaging
|
# Build / packaging
|
||||||
pyinstaller>=6.0
|
pyinstaller>=6.0
|
||||||
@@ -8,4 +9,3 @@ pillow>=10.0 # generates icons/app.ico during CI (Windows build)
|
|||||||
# Test / lint
|
# Test / lint
|
||||||
pytest>=8.0
|
pytest>=8.0
|
||||||
ruff>=0.6
|
ruff>=0.6
|
||||||
cryptography>=42.0 # release checksum signing (scripts/sign_checksums.py)
|
|
||||||
|
|||||||
@@ -1 +1,2 @@
|
|||||||
PySide6>=6.6
|
PySide6>=6.6
|
||||||
|
cryptography>=42.0 # bcc_core imports it at load (catalog signature verification)
|
||||||
|
|||||||
@@ -2815,3 +2815,179 @@ def test_claude_code_profile_warns_only_about_unset_variables():
|
|||||||
|
|
||||||
def test_no_references_means_no_warnings():
|
def test_no_references_means_no_warnings():
|
||||||
assert c.env_ref_warnings({"command": "npx", "args": ["-y", "pkg"]}, None) == []
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user