Compare commits
8 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| a73f2e3883 | |||
| 7ff4f6e5c0 | |||
| 7517e16b15 | |||
| 9a0433225e | |||
| fa82d30087 | |||
| 05b00a40c0 | |||
| febd617c56 | |||
| da20eb2fdb |
@@ -19,6 +19,7 @@ from typing import ClassVar
|
||||
from PySide6.QtCore import QRect, QSettings, QSize, Qt, QThread, QTimer, QUrl, Signal
|
||||
from PySide6.QtGui import (
|
||||
QAction,
|
||||
QActionGroup,
|
||||
QColor,
|
||||
QCursor,
|
||||
QDesktopServices,
|
||||
@@ -26,6 +27,7 @@ from PySide6.QtGui import (
|
||||
QIcon,
|
||||
QKeySequence,
|
||||
QPainter,
|
||||
QPalette,
|
||||
QPixmap,
|
||||
)
|
||||
from PySide6.QtWidgets import (
|
||||
@@ -66,89 +68,122 @@ import bcc_core as core
|
||||
# thread during drag-and-drop import, so skip anything larger than this.
|
||||
MAX_DROP_IMPORT_BYTES = 5 * 1024 * 1024 # 5 MB
|
||||
|
||||
# --- One-line rebrand: change this to recolor the whole app --------------- #
|
||||
ACCENT = "#f97316" # warm orange
|
||||
ACCENT_DIM = "#c2570b"
|
||||
BG = "#1b1d23"
|
||||
PANEL = "#23262e"
|
||||
PANEL_2 = "#2b2f39"
|
||||
TEXT = "#e7e9ee"
|
||||
MUTED = "#9aa0ad"
|
||||
BORDER = "#3a3f4b"
|
||||
GOOD = "#4ade80"
|
||||
BAD = "#f87171"
|
||||
WARN = "#fbbf24"
|
||||
# --- Theming (issue #75) -------------------------------------------------- #
|
||||
# The palette lives in bcc_core (testable without a Qt app); these module-level
|
||||
# names are rebound by `apply_palette()` whenever the theme changes.
|
||||
#
|
||||
# Why globals rather than passing a palette around: ~20 inline
|
||||
# `setStyleSheet(f"color: {MUTED}")` calls are scattered through this file, and
|
||||
# an f-string resolves its names when it runs, not when it's compiled. Rebinding
|
||||
# the globals means every one of those call sites picks up the new colour on its
|
||||
# next render, with no change to the call sites themselves.
|
||||
PALETTE = core.DARK_PALETTE
|
||||
ACCENT = ACCENT_DIM = BG = PANEL = PANEL_2 = TEXT = MUTED = BORDER = ""
|
||||
GOOD = BAD = WARN = REMOTE = ON_ACCENT = DISABLED_BG = MONO_BG = SEL_TEXT = ""
|
||||
STATUS_COLORS: dict[str, str] = {}
|
||||
HEALTH_COLORS: dict[str, str] = {}
|
||||
|
||||
STATUS_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": "#60a5fa", "unknown": WARN}
|
||||
STATUS_GLYPH = {"ok": "●", "missing": "●", "warn": "▲", "remote": "◆", "unknown": "○"}
|
||||
STATUS_GLYPH = {
|
||||
"ok": "\u25cf",
|
||||
"missing": "\u25cf",
|
||||
"warn": "\u25b2",
|
||||
"remote": "\u25c6",
|
||||
"unknown": "\u25cb",
|
||||
}
|
||||
|
||||
# Health dot (spawn-test outcome, see core.HealthStatus) shown per row in the
|
||||
# server tables' "Health" column -- distinct from the PATH-dependency Status
|
||||
# column above.
|
||||
HEALTH_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED}
|
||||
HEALTH_GLYPH = {"ok": "●", "failed": "●", "untested": "○"}
|
||||
HEALTH_GLYPH = {"ok": "\u25cf", "failed": "\u25cf", "untested": "\u25cb"}
|
||||
|
||||
STYLESHEET = f"""
|
||||
|
||||
def build_stylesheet(p: core.Palette) -> str:
|
||||
"""Render the global QSS for a palette."""
|
||||
return f"""
|
||||
/* No font-family here on purpose: Qt already uses the native system UI font
|
||||
on every platform (San Francisco / Segoe UI / desktop default). Naming
|
||||
web-CSS aliases like -apple-system forces a costly font-alias scan. */
|
||||
* {{ font-size: 13px; color: {TEXT}; }}
|
||||
QMainWindow, QDialog {{ background: {BG}; }}
|
||||
* {{ font-size: 13px; color: {p.text}; }}
|
||||
QMainWindow, QDialog {{ background: {p.bg}; }}
|
||||
QLabel#h1 {{ font-size: 15px; font-weight: 600; }}
|
||||
QLabel#muted {{ color: {MUTED}; }}
|
||||
QFrame#card {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px; }}
|
||||
QLabel#muted {{ color: {p.muted}; }}
|
||||
QFrame#card {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px; }}
|
||||
QLineEdit, QPlainTextEdit, QComboBox {{
|
||||
background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px;
|
||||
padding: 6px 8px; selection-background-color: {ACCENT}; selection-color: #1a1205;
|
||||
background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px;
|
||||
padding: 6px 8px; selection-background-color: {p.accent}; selection-color: {p.on_accent};
|
||||
}}
|
||||
QLineEdit:focus, QPlainTextEdit:focus, QComboBox:focus {{ border: 1px solid {ACCENT}; }}
|
||||
QLineEdit:focus, QPlainTextEdit:focus, QComboBox:focus {{ border: 1px solid {p.accent}; }}
|
||||
QComboBox::drop-down {{ border: none; width: 22px; }}
|
||||
QComboBox QAbstractItemView {{ background: {PANEL_2}; border: 1px solid {BORDER};
|
||||
selection-background-color: {ACCENT}; outline: none; }}
|
||||
QPushButton {{ background: {PANEL_2}; border: 1px solid {BORDER}; border-radius: 7px;
|
||||
QComboBox QAbstractItemView {{ background: {p.panel_2}; border: 1px solid {p.border};
|
||||
selection-background-color: {p.accent}; outline: none; }}
|
||||
QPushButton {{ background: {p.panel_2}; border: 1px solid {p.border}; border-radius: 7px;
|
||||
padding: 7px 13px; }}
|
||||
QPushButton:hover {{ border: 1px solid {ACCENT}; }}
|
||||
QPushButton:disabled {{ color: {MUTED}; background: {PANEL}; }}
|
||||
QPushButton#primary {{ background: {ACCENT}; border: 1px solid {ACCENT}; color: #1a1205; font-weight: 600; }}
|
||||
QPushButton#primary:hover {{ background: {ACCENT_DIM}; }}
|
||||
QPushButton#primary:disabled {{ background: {PANEL}; color: {MUTED}; border: 1px solid {BORDER}; }}
|
||||
QPushButton#danger:hover {{ border: 1px solid {BAD}; color: {BAD}; }}
|
||||
QTableWidget {{ background: {PANEL}; border: 1px solid {BORDER}; border-radius: 10px;
|
||||
QPushButton:hover {{ border: 1px solid {p.accent}; }}
|
||||
QPushButton:disabled {{ color: {p.muted}; background: {p.panel}; }}
|
||||
QPushButton#primary {{ background: {p.accent}; border: 1px solid {p.accent}; color: {p.on_accent}; font-weight: 600; }}
|
||||
QPushButton#primary:hover {{ background: {p.accent_dim}; }}
|
||||
QPushButton#primary:disabled {{ background: {p.panel}; color: {p.muted}; border: 1px solid {p.border}; }}
|
||||
QPushButton#danger:hover {{ border: 1px solid {p.bad}; color: {p.bad}; }}
|
||||
QTableWidget {{ background: {p.panel}; border: 1px solid {p.border}; border-radius: 10px;
|
||||
gridline-color: transparent; outline: none; }}
|
||||
QTableWidget::item {{ padding: 6px 8px; border: none; }}
|
||||
QTableWidget::item:selected {{ background: {ACCENT}; color: #1a1205; }}
|
||||
QTableWidget::item:selected {{ background: {p.accent}; color: {p.on_accent}; }}
|
||||
/* Inline cell editors: the global QLineEdit padding/radius clips the text
|
||||
inside a table row, so give editors a compact, flat style instead. */
|
||||
QTableWidget QLineEdit {{
|
||||
background: {PANEL_2}; color: {TEXT}; border: 1px solid {ACCENT};
|
||||
background: {p.panel_2}; color: {p.text}; border: 1px solid {p.accent};
|
||||
border-radius: 3px; padding: 0px 4px; margin: 0px;
|
||||
selection-background-color: {ACCENT_DIM}; selection-color: #ffffff;
|
||||
selection-background-color: {p.accent_dim}; selection-color: {p.selection_text};
|
||||
}}
|
||||
QHeaderView::section {{ background: {PANEL}; color: {MUTED}; border: none;
|
||||
border-bottom: 1px solid {BORDER}; padding: 8px; font-weight: 600; }}
|
||||
QHeaderView::section {{ background: {p.panel}; color: {p.muted}; border: none;
|
||||
border-bottom: 1px solid {p.border}; padding: 8px; font-weight: 600; }}
|
||||
QScrollBar:vertical {{ background: transparent; width: 10px; margin: 2px; }}
|
||||
QScrollBar::handle:vertical {{ background: {BORDER}; border-radius: 5px; min-height: 24px; }}
|
||||
QScrollBar::handle:vertical {{ background: {p.border}; border-radius: 5px; min-height: 24px; }}
|
||||
QScrollBar::add-line, QScrollBar::sub-line {{ height: 0; }}
|
||||
QLabel#statusbar {{ color: {MUTED}; padding: 4px 2px; }}
|
||||
QLabel#warnBanner {{ color: #1a1205; background: {WARN}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }}
|
||||
QFrame#noticeBanner {{ background: {PANEL_2}; border: 1px solid {ACCENT}; border-radius: 8px; }}
|
||||
QLabel#noticeText {{ color: {TEXT}; }}
|
||||
QPushButton#noticeClose {{ background: transparent; border: none; color: {MUTED}; font-size: 14px; padding: 2px; }}
|
||||
QPushButton#noticeClose:hover {{ color: {TEXT}; }}
|
||||
QLabel#section {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
|
||||
QLabel#sectionDisabled {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
|
||||
QLabel#placeholder {{ color: {MUTED}; padding: 12px; background: {PANEL_2}; border: 1px dashed {BORDER}; border-radius: 8px; }}
|
||||
QTableWidget#disabledTable {{ background: #202229; }}
|
||||
QTableWidget#disabledTable::item:selected {{ background: {ACCENT}; color: #1a1205; }}
|
||||
QLabel#statusbar {{ color: {p.muted}; padding: 4px 2px; }}
|
||||
QLabel#warnBanner {{ color: {p.on_accent}; background: {p.warn}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }}
|
||||
QFrame#noticeBanner {{ background: {p.panel_2}; border: 1px solid {p.accent}; border-radius: 8px; }}
|
||||
QLabel#noticeText {{ color: {p.text}; }}
|
||||
QPushButton#noticeClose {{ background: transparent; border: none; color: {p.muted}; font-size: 14px; padding: 2px; }}
|
||||
QPushButton#noticeClose:hover {{ color: {p.text}; }}
|
||||
QLabel#section {{ color: {p.muted}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
|
||||
QLabel#sectionDisabled {{ color: {p.muted}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
|
||||
QLabel#placeholder {{ color: {p.muted}; padding: 12px; background: {p.panel_2}; border: 1px dashed {p.border}; border-radius: 8px; }}
|
||||
QTableWidget#disabledTable {{ background: {p.disabled_bg}; }}
|
||||
QTableWidget#disabledTable::item:selected {{ background: {p.accent}; color: {p.on_accent}; }}
|
||||
QPlainTextEdit#diag {{ font-family: "Menlo", "Cascadia Code", "Consolas", "DejaVu Sans Mono", monospace;
|
||||
font-size: 12px; background: #16181d; border: 1px solid {BORDER}; border-radius: 8px; }}
|
||||
font-size: 12px; background: {p.mono_bg}; border: 1px solid {p.border}; border-radius: 8px; }}
|
||||
QFrame#diagCard {{ background: transparent; border: none; }}
|
||||
QSplitter::handle {{ background: transparent; }}
|
||||
QSplitter::handle:hover {{ background: {BORDER}; border-radius: 4px; }}
|
||||
QSplitter::handle:pressed {{ background: {ACCENT}; border-radius: 4px; }}
|
||||
QSplitter::handle:hover {{ background: {p.border}; border-radius: 4px; }}
|
||||
QSplitter::handle:pressed {{ background: {p.accent}; border-radius: 4px; }}
|
||||
"""
|
||||
|
||||
|
||||
def apply_palette(p: core.Palette) -> str:
|
||||
"""Rebind the module-level colour names to `p` and return its stylesheet."""
|
||||
global PALETTE, ACCENT, ACCENT_DIM, BG, PANEL, PANEL_2, TEXT, MUTED, BORDER
|
||||
global GOOD, BAD, WARN, REMOTE, ON_ACCENT, DISABLED_BG, MONO_BG, SEL_TEXT
|
||||
global STATUS_COLORS, HEALTH_COLORS
|
||||
PALETTE = p
|
||||
ACCENT, ACCENT_DIM = p.accent, p.accent_dim
|
||||
BG, PANEL, PANEL_2 = p.bg, p.panel, p.panel_2
|
||||
TEXT, MUTED, BORDER = p.text, p.muted, p.border
|
||||
GOOD, BAD, WARN, REMOTE = p.good, p.bad, p.warn, p.remote
|
||||
ON_ACCENT, DISABLED_BG, MONO_BG, SEL_TEXT = (
|
||||
p.on_accent,
|
||||
p.disabled_bg,
|
||||
p.mono_bg,
|
||||
p.selection_text,
|
||||
)
|
||||
STATUS_COLORS = {"ok": GOOD, "missing": BAD, "warn": WARN, "remote": REMOTE, "unknown": WARN}
|
||||
HEALTH_COLORS = {"ok": GOOD, "failed": BAD, "untested": MUTED}
|
||||
return build_stylesheet(p)
|
||||
|
||||
|
||||
STYLESHEET = apply_palette(core.DARK_PALETTE)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Background reachability tester (keeps the UI responsive during the request)
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -233,7 +268,7 @@ class _SecretMaskDelegate(QStyledItemDelegate):
|
||||
if self.revealed or not option.text:
|
||||
return
|
||||
key_item = self._table.item(index.row(), 0)
|
||||
if key_item and core.is_secret_key(key_item.text()):
|
||||
if key_item and core.should_mask_value(key_item.text(), option.text):
|
||||
option.text = core.MASK
|
||||
|
||||
|
||||
@@ -1643,6 +1678,22 @@ class MainWindow(QMainWindow):
|
||||
|
||||
# --- menu bar ---------------------------------------------------------- #
|
||||
def _build_menu_bar(self):
|
||||
view_menu = self.menuBar().addMenu("&View")
|
||||
theme_menu = view_menu.addMenu("Theme")
|
||||
self._theme_group = QActionGroup(self)
|
||||
self._theme_group.setExclusive(True)
|
||||
current = stored_theme_setting()
|
||||
for setting, label in (
|
||||
(core.THEME_SYSTEM, "Match system"),
|
||||
(core.THEME_LIGHT, "Light"),
|
||||
(core.THEME_DARK, "Dark"),
|
||||
):
|
||||
act = QAction(label, self, checkable=True)
|
||||
act.setChecked(setting == current)
|
||||
act.triggered.connect(lambda _checked=False, s=setting: self._set_theme(s))
|
||||
self._theme_group.addAction(act)
|
||||
theme_menu.addAction(act)
|
||||
|
||||
help_menu = self.menuBar().addMenu("&Help")
|
||||
|
||||
# "Check for updates" used to exist only as a button inside the About
|
||||
@@ -1697,6 +1748,26 @@ class MainWindow(QMainWindow):
|
||||
self.update_banner.hide()
|
||||
self.status.setText(f"You're up to date ({core.__version__}).")
|
||||
|
||||
def _set_theme(self, setting: str):
|
||||
"""Persist the theme choice and repaint the running window."""
|
||||
QSettings("BCC", "BetterClaudeConfig").setValue("ui/theme", setting)
|
||||
app = QApplication.instance()
|
||||
if app is None: # pragma: no cover - only in a headless test harness
|
||||
return
|
||||
app.setStyleSheet(theme_stylesheet_for(app, setting))
|
||||
# The global stylesheet covers most of the UI, but the inline
|
||||
# setStyleSheet calls (status dots, warning labels, update banner) only
|
||||
# pick up the new palette when their widget next renders -- so re-render
|
||||
# them now rather than leaving dark-on-light text behind.
|
||||
self._repaint_themed_widgets()
|
||||
|
||||
def _repaint_themed_widgets(self):
|
||||
"""Re-run the inline-styled bits after a palette change."""
|
||||
self.status.setStyleSheet(f"color: {MUTED};")
|
||||
idx = self._current_index()
|
||||
self._refresh_tables(select_index=idx if idx >= 0 else -1)
|
||||
self._update_status(saved=False)
|
||||
|
||||
def _show_about(self):
|
||||
AboutDialog(self).exec()
|
||||
|
||||
@@ -2045,9 +2116,21 @@ class MainWindow(QMainWindow):
|
||||
return
|
||||
self.full_config = cfg
|
||||
repaired = True
|
||||
# extract_servers tolerates malformed entries rather than raising (#72),
|
||||
# but keep it inside the guard: a load failure must leave the previously
|
||||
# loaded profile intact instead of half-swapping the window's state.
|
||||
try:
|
||||
servers = core.extract_servers(self.full_config)
|
||||
except Exception as exc: # pragma: no cover - defence in depth
|
||||
QMessageBox.critical(
|
||||
self,
|
||||
"Could not read config",
|
||||
f"{profile.path}\n\nThe server list couldn't be read: {exc}",
|
||||
)
|
||||
return
|
||||
self._loaded_stat = core.config_fingerprint(profile.path)
|
||||
self.current_profile = profile
|
||||
self.servers = core.extract_servers(self.full_config)
|
||||
self.servers = servers
|
||||
self.dirty = False
|
||||
self.restart_btn.hide()
|
||||
self._undo_stack.clear()
|
||||
@@ -2370,7 +2453,7 @@ class MainWindow(QMainWindow):
|
||||
entry = self.servers[idx]
|
||||
old_name = entry.name
|
||||
entry.name = self.editor.current_name()
|
||||
entry.data = self.editor.dump_data()
|
||||
entry.set_data(self.editor.dump_data())
|
||||
# The server stays in its section (enable state unchanged), so update
|
||||
# its existing row in place rather than re-rendering.
|
||||
# An edit invalidates any cached "Test all" result -- the server that
|
||||
@@ -2464,7 +2547,7 @@ class MainWindow(QMainWindow):
|
||||
QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
|
||||
)
|
||||
if ans == QMessageBox.StandardButton.Yes:
|
||||
self.servers[existing[name]].data = data
|
||||
self.servers[existing[name]].set_data(data)
|
||||
return False, True
|
||||
name = core.resolve_name_collision(name, {s.name for s in self.servers})
|
||||
self.servers.append(core.ServerEntry(name, data, True))
|
||||
@@ -2539,6 +2622,12 @@ class MainWindow(QMainWindow):
|
||||
self.save_btn.setEnabled(False)
|
||||
return False
|
||||
lint_warnings = core.lint_servers(self.servers)
|
||||
# ${VAR} references are only meaningful if the target client expands
|
||||
# them -- Claude Desktop doesn't, so the same config is fine in one
|
||||
# profile and broken in another (#76). Report against the loaded one.
|
||||
for entry in self.servers:
|
||||
for warning in core.env_ref_warnings(entry.data, self.current_profile):
|
||||
lint_warnings.append(f"'{entry.name}': {warning}")
|
||||
if lint_warnings:
|
||||
self.validation_lbl.setText(f"⚠ {lint_warnings[0]}")
|
||||
self.validation_lbl.setStyleSheet(f"color: {WARN};")
|
||||
@@ -2584,6 +2673,11 @@ class MainWindow(QMainWindow):
|
||||
except Exception as e:
|
||||
QMessageBox.critical(self, "Reload failed", str(e))
|
||||
return
|
||||
# The reload above is the on-disk truth for everything the user
|
||||
# didn't touch -- but it also wipes BCC-authored keys the user
|
||||
# changed in this session (named sets), which apply_servers
|
||||
# doesn't write. Carry them over before saving (#73).
|
||||
contested = core.carry_owned_keys(self.full_config, fresh)
|
||||
core.apply_servers(fresh, self.servers)
|
||||
try:
|
||||
backup = core.write_config(self.current_profile.path, fresh)
|
||||
@@ -2596,8 +2690,13 @@ class MainWindow(QMainWindow):
|
||||
self.dirty = False
|
||||
self.save_btn.setEnabled(False)
|
||||
bnote = f" · backup: {backup.name}" if backup else " · (new file)"
|
||||
cnote = (
|
||||
f" · kept your {', '.join(contested)} (the file on disk had a different copy)"
|
||||
if contested
|
||||
else ""
|
||||
)
|
||||
self.status.setText(
|
||||
f"Merged & saved {self.current_profile.path}{bnote}"
|
||||
f"Merged & saved {self.current_profile.path}{bnote}{cnote}"
|
||||
f" · Restart {self.current_profile.label} to apply."
|
||||
)
|
||||
self._offer_restart_button()
|
||||
@@ -2755,6 +2854,33 @@ class MainWindow(QMainWindow):
|
||||
e.accept()
|
||||
|
||||
|
||||
def system_is_dark(app: QApplication) -> bool:
|
||||
"""Whether the desktop is currently using a dark appearance.
|
||||
|
||||
Read from the style's own window colour rather than per-platform APIs --
|
||||
Qt has already resolved the OS appearance by the time it builds the
|
||||
default palette, so this works the same on all three platforms.
|
||||
"""
|
||||
try:
|
||||
return app.palette().color(QPalette.ColorRole.Window).lightness() < 128
|
||||
except Exception: # pragma: no cover - defensive; never block startup on theming
|
||||
return True
|
||||
|
||||
|
||||
def stored_theme_setting() -> str:
|
||||
"""The user's theme choice, defaulting to following the system."""
|
||||
value = QSettings("BCC", "BetterClaudeConfig").value("ui/theme", core.THEME_SYSTEM)
|
||||
return value if value in core.THEME_CHOICES else core.THEME_SYSTEM
|
||||
|
||||
|
||||
def theme_stylesheet_for(app: QApplication, setting: str | None = None) -> str:
|
||||
"""Resolve setting + OS appearance into a palette, apply it, return the QSS."""
|
||||
if setting is None:
|
||||
setting = stored_theme_setting()
|
||||
theme = core.resolve_theme(setting, system_is_dark(app))
|
||||
return apply_palette(core.palette_for(theme))
|
||||
|
||||
|
||||
def main():
|
||||
if sys.platform == "win32":
|
||||
# Without an explicit AppUserModelID, Windows taskbar groups the app
|
||||
@@ -2773,7 +2899,7 @@ def main():
|
||||
icon = _app_icon()
|
||||
if not icon.isNull():
|
||||
app.setWindowIcon(icon)
|
||||
app.setStyleSheet(STYLESHEET)
|
||||
app.setStyleSheet(theme_stylesheet_for(app))
|
||||
win = MainWindow()
|
||||
win.show()
|
||||
sys.exit(app.exec())
|
||||
|
||||
+426
-9
@@ -15,6 +15,7 @@ from __future__ import annotations
|
||||
|
||||
import base64
|
||||
import contextlib
|
||||
import copy
|
||||
import difflib
|
||||
import functools
|
||||
import glob
|
||||
@@ -49,6 +50,12 @@ DISABLED_KEY = "_disabledMcpServers"
|
||||
# parks the rest under DISABLED_KEY.
|
||||
SETS_KEY = "_bccServerSets"
|
||||
|
||||
# Top-level keys BCC itself authors. They live in the client's config file, but
|
||||
# BCC is their owner, so on a stale-file merge the in-memory copy wins over the
|
||||
# on-disk one (see `carry_owned_keys`). Any future BCC-authored key belongs
|
||||
# here -- forgetting to add one is exactly how #73 happened.
|
||||
BCC_OWNED_KEYS = (SETS_KEY,)
|
||||
|
||||
BACKUP_DIRNAME = ".bcc_backups"
|
||||
MAX_BACKUPS = 15
|
||||
|
||||
@@ -211,16 +218,195 @@ class Profile:
|
||||
self.path = Path(self.path)
|
||||
|
||||
|
||||
class _NoRaw:
|
||||
"""Sentinel for ServerEntry.raw.
|
||||
|
||||
`None` can't do this job: `{"mcpServers": {"foo": null}}` is legal JSON and
|
||||
a real malformed-entry case, so None has to mean "the config said null",
|
||||
not "there was nothing here".
|
||||
"""
|
||||
|
||||
__slots__ = ()
|
||||
|
||||
def __repr__(self) -> str: # keeps ServerEntry reprs readable in test output
|
||||
return "<no raw>"
|
||||
|
||||
|
||||
NO_RAW = _NoRaw()
|
||||
|
||||
|
||||
@dataclass
|
||||
class ServerEntry:
|
||||
"""One server definition.
|
||||
|
||||
`data` is always a dict so every consumer can treat it as one. When the
|
||||
config held something that wasn't a JSON object for this server (a string,
|
||||
a number, a list -- all legal JSON, all wrong here), `data` is empty and
|
||||
the original value is preserved verbatim in `raw` so Save round-trips it
|
||||
instead of silently deleting the user's line. `lint_servers` surfaces it.
|
||||
`raw` defaults to the NO_RAW sentinel rather than None, because a config
|
||||
value of literal `null` is itself a malformed entry worth preserving.
|
||||
|
||||
Assigning `data` means the user replaced the definition through the editor,
|
||||
which retires `raw` -- use `set_data` so that can't be forgotten.
|
||||
"""
|
||||
|
||||
name: str
|
||||
data: dict
|
||||
enabled: bool = True
|
||||
raw: object = NO_RAW
|
||||
|
||||
@property
|
||||
def kind(self) -> str:
|
||||
return "remote" if "url" in self.data and "command" not in self.data else "stdio"
|
||||
|
||||
@property
|
||||
def malformed(self) -> bool:
|
||||
"""True when the config value for this server wasn't a JSON object."""
|
||||
return self.raw is not NO_RAW
|
||||
|
||||
def set_data(self, data: dict) -> None:
|
||||
"""Replace the definition from the editor, clearing any malformed original."""
|
||||
self.data = data
|
||||
self.raw = NO_RAW
|
||||
|
||||
def config_value(self):
|
||||
"""What to write back to the config: the edited dict, or the untouched
|
||||
malformed original when the user never edited it."""
|
||||
return self.data if self.raw is NO_RAW else self.raw
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Theming (issue #75)
|
||||
# --------------------------------------------------------------------------- #
|
||||
THEME_SYSTEM = "system"
|
||||
THEME_LIGHT = "light"
|
||||
THEME_DARK = "dark"
|
||||
THEME_CHOICES = (THEME_SYSTEM, THEME_LIGHT, THEME_DARK)
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class Palette:
|
||||
"""Every colour the UI draws with.
|
||||
|
||||
Deliberately exhaustive: the stylesheet used to inline a handful of
|
||||
near-black literals (`#1a1205` for text on the accent, `#202229` for the
|
||||
disabled table, `#16181d` for the diagnostics pane), which is fine while
|
||||
there's one theme and invisible breakage the moment there are two. Each
|
||||
gets a slot here so a light palette can't silently inherit a dark value.
|
||||
"""
|
||||
|
||||
name: str
|
||||
accent: str
|
||||
accent_dim: str
|
||||
bg: str
|
||||
panel: str
|
||||
panel_2: str
|
||||
text: str
|
||||
muted: str
|
||||
border: str
|
||||
good: str
|
||||
bad: str
|
||||
warn: str
|
||||
remote: str
|
||||
on_accent: str # text drawn on top of an accent fill
|
||||
disabled_bg: str # the parked-servers table
|
||||
mono_bg: str # diagnostics / log panes
|
||||
selection_text: str
|
||||
|
||||
|
||||
# The shipping theme through v1.3.0. These values are carried over verbatim --
|
||||
# adding a light theme must not restyle the dark one.
|
||||
DARK_PALETTE = Palette(
|
||||
name="dark",
|
||||
accent="#f97316",
|
||||
accent_dim="#c2570b",
|
||||
bg="#1b1d23",
|
||||
panel="#23262e",
|
||||
panel_2="#2b2f39",
|
||||
text="#e7e9ee",
|
||||
muted="#9aa0ad",
|
||||
border="#3a3f4b",
|
||||
good="#4ade80",
|
||||
bad="#f87171",
|
||||
warn="#fbbf24",
|
||||
remote="#60a5fa",
|
||||
on_accent="#1a1205",
|
||||
disabled_bg="#202229",
|
||||
mono_bg="#16181d",
|
||||
selection_text="#ffffff",
|
||||
)
|
||||
|
||||
# The semantic colours are NOT the dark ones lightened. #4ade80 / #fbbf24 sit
|
||||
# around 1.7:1 against white -- illegible. These are darkened to clear 4.5:1,
|
||||
# which `test_light_palette_meets_contrast` enforces so nobody "tidies" them
|
||||
# back toward the dark hues later.
|
||||
LIGHT_PALETTE = Palette(
|
||||
name="light",
|
||||
accent="#c2410c",
|
||||
accent_dim="#9a3412",
|
||||
bg="#f6f7f9",
|
||||
panel="#ffffff",
|
||||
panel_2="#eef0f4",
|
||||
text="#1b1d23",
|
||||
muted="#5c6270",
|
||||
border="#d3d7de",
|
||||
good="#15803d",
|
||||
bad="#b91c1c",
|
||||
warn="#a16207",
|
||||
remote="#1d4ed8",
|
||||
on_accent="#ffffff",
|
||||
disabled_bg="#e9ebef",
|
||||
mono_bg="#f0f2f5",
|
||||
selection_text="#ffffff",
|
||||
)
|
||||
|
||||
PALETTES = {DARK_PALETTE.name: DARK_PALETTE, LIGHT_PALETTE.name: LIGHT_PALETTE}
|
||||
|
||||
|
||||
def resolve_theme(setting: str, system_is_dark: bool) -> str:
|
||||
"""Map a stored theme setting + the OS appearance onto a concrete palette name.
|
||||
|
||||
Anything unrecognised (a hand-edited QSettings value, a setting written by
|
||||
a future version) falls back to following the system rather than to a
|
||||
fixed theme -- the user's desktop is the better guess.
|
||||
"""
|
||||
if setting == THEME_DARK:
|
||||
return THEME_DARK
|
||||
if setting == THEME_LIGHT:
|
||||
return THEME_LIGHT
|
||||
return THEME_DARK if system_is_dark else THEME_LIGHT
|
||||
|
||||
|
||||
def palette_for(theme: str) -> Palette:
|
||||
"""Concrete palette by name; unknown names fall back to dark (the historical look)."""
|
||||
return PALETTES.get(theme, DARK_PALETTE)
|
||||
|
||||
|
||||
def _hex_to_rgb(value: str) -> tuple[int, int, int]:
|
||||
v = value.lstrip("#")
|
||||
if len(v) == 3:
|
||||
v = "".join(ch * 2 for ch in v)
|
||||
return int(v[0:2], 16), int(v[2:4], 16), int(v[4:6], 16)
|
||||
|
||||
|
||||
def relative_luminance(color: str) -> float:
|
||||
"""WCAG relative luminance for a #rrggbb colour."""
|
||||
|
||||
def chan(c: int) -> float:
|
||||
srgb = c / 255.0
|
||||
return srgb / 12.92 if srgb <= 0.04045 else ((srgb + 0.055) / 1.055) ** 2.4
|
||||
|
||||
r, g, b = (chan(c) for c in _hex_to_rgb(color))
|
||||
return 0.2126 * r + 0.7152 * g + 0.0722 * b
|
||||
|
||||
|
||||
def contrast_ratio(fg: str, bg: str) -> float:
|
||||
"""WCAG contrast ratio between two #rrggbb colours (1.0 to 21.0)."""
|
||||
a, b = relative_luminance(fg), relative_luminance(bg)
|
||||
lighter, darker = max(a, b), min(a, b)
|
||||
return (lighter + 0.05) / (darker + 0.05)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Discovery
|
||||
@@ -485,13 +671,30 @@ def repair_config_file(path: str | os.PathLike) -> tuple[dict, list[str], str]:
|
||||
return obj, notes, pretty
|
||||
|
||||
|
||||
def _server_entry(name: str, data, enabled: bool) -> ServerEntry:
|
||||
"""Build a ServerEntry, tolerating a value that isn't a JSON object.
|
||||
|
||||
A hand-edited config can legally hold `{"mcpServers": {"foo": "oops"}}` --
|
||||
valid JSON, wrong shape. Calling dict() on that raises, which used to take
|
||||
the whole load down before the linter ever got a look at it (#72). Keep the
|
||||
original instead and let the linter report it.
|
||||
"""
|
||||
if isinstance(data, dict):
|
||||
return ServerEntry(name=name, data=dict(data), enabled=enabled)
|
||||
return ServerEntry(name=name, data={}, enabled=enabled, raw=data)
|
||||
|
||||
|
||||
def extract_servers(cfg: dict) -> list[ServerEntry]:
|
||||
"""Pull enabled (`mcpServers`) and disabled (`_disabledMcpServers`) servers."""
|
||||
"""Pull enabled (`mcpServers`) and disabled (`_disabledMcpServers`) servers.
|
||||
|
||||
Never raises on a structurally-odd config -- malformed entries come back as
|
||||
empty-data entries carrying their original value (see `_server_entry`).
|
||||
"""
|
||||
out: list[ServerEntry] = []
|
||||
for name, data in (cfg.get("mcpServers") or {}).items():
|
||||
out.append(ServerEntry(name=name, data=dict(data), enabled=True))
|
||||
out.append(_server_entry(name, data, True))
|
||||
for name, data in (cfg.get(DISABLED_KEY) or {}).items():
|
||||
out.append(ServerEntry(name=name, data=dict(data), enabled=False))
|
||||
out.append(_server_entry(name, data, False))
|
||||
return out
|
||||
|
||||
|
||||
@@ -583,8 +786,8 @@ def apply_servers(cfg: dict, servers: list[ServerEntry]) -> dict:
|
||||
Write the server list back into `cfg` in place, preserving every other key
|
||||
and the position of `mcpServers`. Returns the same dict for convenience.
|
||||
"""
|
||||
enabled = {s.name: s.data for s in servers if s.enabled}
|
||||
disabled = {s.name: s.data for s in servers if not s.enabled}
|
||||
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
|
||||
if disabled:
|
||||
@@ -597,6 +800,34 @@ def apply_servers(cfg: dict, servers: list[ServerEntry]) -> dict:
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Write (atomic, with rotating backups)
|
||||
# --------------------------------------------------------------------------- #
|
||||
def carry_owned_keys(local_cfg: dict, fresh_cfg: dict) -> list[str]:
|
||||
"""Carry BCC-authored top-level keys from `local_cfg` onto `fresh_cfg`.
|
||||
|
||||
Used by the stale-file "Merge & save" path, which reloads the file from
|
||||
disk and re-applies the user's server edits. That reload used to drop
|
||||
anything BCC owns but `apply_servers` doesn't write -- named server sets
|
||||
vanished without a word (#73). BCC owns these keys, so the in-memory copy
|
||||
wins; mutates `fresh_cfg` in place.
|
||||
|
||||
Returns the keys where the on-disk copy differed and was overwritten, so
|
||||
the caller can tell the user something was actually contested rather than
|
||||
merely carried across.
|
||||
|
||||
Deliberately one-directional: a key absent locally is left alone on disk.
|
||||
We can't tell "user deleted their last set" from "user never had sets and
|
||||
another machine just added some", and silently deleting someone else's
|
||||
data is the worse of the two failures.
|
||||
"""
|
||||
conflicts: list[str] = []
|
||||
for key in BCC_OWNED_KEYS:
|
||||
if key not in local_cfg:
|
||||
continue
|
||||
if key in fresh_cfg and fresh_cfg[key] != local_cfg[key]:
|
||||
conflicts.append(key)
|
||||
fresh_cfg[key] = copy.deepcopy(local_cfg[key])
|
||||
return conflicts
|
||||
|
||||
|
||||
def _make_backup(path: Path) -> Path:
|
||||
bdir = path.parent / BACKUP_DIRNAME
|
||||
bdir.mkdir(exist_ok=True)
|
||||
@@ -662,13 +893,28 @@ def backup_label(backup_path: Path | str) -> str:
|
||||
return f"{ts[:4]}-{ts[4:6]}-{ts[6:8]} {ts[9:11]}:{ts[11:13]}:{ts[13:]}"
|
||||
|
||||
|
||||
def should_mask_value(key: str, value) -> bool:
|
||||
"""Whether an env/header value should be masked for display.
|
||||
|
||||
A ${VAR} reference is NOT a secret -- it's a pointer to one, and it's the
|
||||
thing we want users to adopt. Masking it to dots would make a reference
|
||||
indistinguishable from a stored credential, hiding exactly the distinction
|
||||
that makes the feature worth using (#76).
|
||||
"""
|
||||
if not is_secret_key(key):
|
||||
return False
|
||||
return not is_env_ref(value) if isinstance(value, str) else True
|
||||
|
||||
|
||||
def _redact_server_data(data: dict) -> dict:
|
||||
"""Return a copy of a server definition with secrets masked for display."""
|
||||
out = dict(data)
|
||||
if "args" in out:
|
||||
out["args"] = redact_args(list(out["args"] or []))
|
||||
if "env" in out:
|
||||
out["env"] = {k: (MASK if is_secret_key(k) else v) for k, v in (out["env"] or {}).items()}
|
||||
out["env"] = {
|
||||
k: (MASK if should_mask_value(k, v) else v) for k, v in (out["env"] or {}).items()
|
||||
}
|
||||
return out
|
||||
|
||||
|
||||
@@ -1219,6 +1465,155 @@ MASK = "••••••••"
|
||||
_EMBEDDED_CRED_RE = re.compile(r"://[^:@/\s]+:[^:@/\s]+@")
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Environment-variable references (issue #76)
|
||||
#
|
||||
# Claude Code expands ${VAR} and ${VAR:-default} itself, in command, args, env,
|
||||
# url and headers. So BCC does NOT expand these on write -- resolving them into
|
||||
# the file would put the secret back on disk, which is the whole thing the user
|
||||
# is avoiding, and would defeat a feature the client already implements. BCC
|
||||
# authors, validates and warns.
|
||||
#
|
||||
# Claude Desktop has no documented support, so the same text there is passed to
|
||||
# the server literally. That makes this a per-client capability, not a global
|
||||
# one -- see client_expands_env_refs().
|
||||
# --------------------------------------------------------------------------- #
|
||||
# ${NAME} or ${NAME:-default}. Names follow the shell convention (letter or
|
||||
# underscore first) so a bare "${}" or "${1}" isn't mistaken for a reference.
|
||||
_ENV_REF_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)(?::-([^}]*))?\}")
|
||||
|
||||
# The five fields Claude Code documents as expansion sites.
|
||||
ENV_REF_FIELDS = ("command", "args", "env", "url", "headers")
|
||||
|
||||
|
||||
class EnvRef(NamedTuple):
|
||||
"""One ${VAR} / ${VAR:-default} occurrence found in a server definition."""
|
||||
|
||||
name: str
|
||||
default: str | None
|
||||
field: str # which of ENV_REF_FIELDS it was found in
|
||||
|
||||
@property
|
||||
def has_default(self) -> bool:
|
||||
return self.default is not None
|
||||
|
||||
|
||||
def find_env_refs(text: str, field: str = "") -> list[EnvRef]:
|
||||
"""Every ${VAR} / ${VAR:-default} reference in a single string."""
|
||||
if not isinstance(text, str):
|
||||
return []
|
||||
return [EnvRef(m.group(1), m.group(2), field) for m in _ENV_REF_RE.finditer(text)]
|
||||
|
||||
|
||||
def is_env_ref(value: str) -> bool:
|
||||
"""True when the value contains at least one ${VAR} reference.
|
||||
|
||||
Used to keep placeholders OUT of secret masking: `${API_KEY}` under a
|
||||
secret-looking key is a reference, not a secret, and masking it to dots
|
||||
would hide the one distinction the user needs to see.
|
||||
"""
|
||||
return bool(find_env_refs(value))
|
||||
|
||||
|
||||
def server_env_refs(data: dict) -> list[EnvRef]:
|
||||
"""Every env reference in a server definition, tagged with its field.
|
||||
|
||||
Only inspects the fields Claude Code actually expands; a ${VAR} written
|
||||
into some other key is not a reference and shouldn't be reported as one.
|
||||
"""
|
||||
out: list[EnvRef] = []
|
||||
if not isinstance(data, dict):
|
||||
return out
|
||||
for field in ENV_REF_FIELDS:
|
||||
value = data.get(field)
|
||||
if isinstance(value, str):
|
||||
out.extend(find_env_refs(value, field))
|
||||
elif isinstance(value, list):
|
||||
for item in value:
|
||||
out.extend(find_env_refs(item, field))
|
||||
elif isinstance(value, dict):
|
||||
for v in value.values():
|
||||
out.extend(find_env_refs(v, field))
|
||||
return out
|
||||
|
||||
|
||||
def expand_env_refs(text: str, environ: dict | None = None) -> str:
|
||||
"""Expand ${VAR} / ${VAR:-default} the way Claude Code documents it.
|
||||
|
||||
Provided for previewing what the client will do -- BCC never writes the
|
||||
expanded form back to the config. Unset with no default is left as the
|
||||
literal ${VAR} text, matching Claude Code: the config still loads and the
|
||||
unexpanded text is passed through.
|
||||
"""
|
||||
if not isinstance(text, str):
|
||||
return text
|
||||
env = os.environ if environ is None else environ
|
||||
|
||||
def repl(m: re.Match) -> str:
|
||||
name, default = m.group(1), m.group(2)
|
||||
if name in env:
|
||||
return env[name]
|
||||
return default if default is not None else m.group(0)
|
||||
|
||||
return _ENV_REF_RE.sub(repl, text)
|
||||
|
||||
|
||||
def unresolved_env_refs(data: dict, environ: dict | None = None) -> list[EnvRef]:
|
||||
"""References that would not resolve: variable unset AND no default.
|
||||
|
||||
Best-effort by nature -- BCC's environment isn't necessarily the client's,
|
||||
so this warns rather than blocks, and the warning text says so.
|
||||
"""
|
||||
env = os.environ if environ is None else environ
|
||||
return [r for r in server_env_refs(data) if not r.has_default and r.name not in env]
|
||||
|
||||
|
||||
def client_expands_env_refs(profile: Profile) -> bool:
|
||||
"""Whether the client behind `profile` expands ${VAR} itself.
|
||||
|
||||
Claude Code does, in command/args/env/url/headers, for both project
|
||||
`.mcp.json` and user-scope `~/.claude.json`. Claude Desktop has no
|
||||
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.
|
||||
"""
|
||||
return not profile_targets_claude_desktop(profile)
|
||||
|
||||
|
||||
def env_ref_warnings(
|
||||
data: dict, profile: Profile | None = None, environ: dict | None = None
|
||||
) -> list[str]:
|
||||
"""Advisory warnings about env references in one server definition.
|
||||
|
||||
Two distinct problems, deliberately worded differently:
|
||||
- the target client won't expand them at all (Claude Desktop)
|
||||
- the client will expand them, but a variable looks unset here
|
||||
"""
|
||||
refs = server_env_refs(data)
|
||||
if not refs:
|
||||
return []
|
||||
|
||||
if profile is not None and not client_expands_env_refs(profile):
|
||||
names = ", ".join(sorted({f"${{{r.name}}}" for r in refs}))
|
||||
return [
|
||||
f"{names} will NOT be expanded by Claude Desktop -- it has no "
|
||||
f"documented support for variable references, so the server "
|
||||
f"receives the literal text. Use a real value here, or move this "
|
||||
f"server to a Claude Code config."
|
||||
]
|
||||
|
||||
missing = unresolved_env_refs(data, environ)
|
||||
if not missing:
|
||||
return []
|
||||
names = ", ".join(sorted({r.name for r in missing}))
|
||||
return [
|
||||
f"{names} is not set in this environment and has no ':-default'. "
|
||||
f"Claude Code will pass the reference through unexpanded. "
|
||||
f"(Checked against BCC's environment, which may differ from the "
|
||||
f"client's.)"
|
||||
]
|
||||
|
||||
|
||||
def is_secret_key(name: str) -> bool:
|
||||
"""Does this env-var / header / flag name look like it holds a secret?"""
|
||||
return bool(_SECRET_KEY_RE.search(name or ""))
|
||||
@@ -1235,17 +1630,22 @@ def redact_args(args: list[str]) -> list[str]:
|
||||
--api-key=abc123 -> --api-key=•••••••• (inline flag=value)
|
||||
ghp_abc123 -> •••••••• (well-known token prefix)
|
||||
Everything else passes through untouched.
|
||||
|
||||
${VAR} references are left visible: they name a secret rather than being
|
||||
one, and hiding them would obscure the difference between "this config
|
||||
leaks a token" and "this config points at one" (#76).
|
||||
"""
|
||||
out: list[str] = []
|
||||
mask_next = False
|
||||
for a in args:
|
||||
s = str(a)
|
||||
if mask_next:
|
||||
out.append(MASK)
|
||||
mask_next = False
|
||||
out.append(s if is_env_ref(s) else MASK)
|
||||
continue
|
||||
if s.startswith("-") and "=" in s and is_secret_key(s.split("=", 1)[0]):
|
||||
out.append(s.split("=", 1)[0] + "=" + MASK)
|
||||
flag, value = s.split("=", 1)
|
||||
out.append(f"{flag}={value}" if is_env_ref(value) else f"{flag}={MASK}")
|
||||
continue
|
||||
if s.startswith("-") and is_secret_key(s):
|
||||
out.append(s)
|
||||
@@ -1272,6 +1672,11 @@ def args_secret_warning(data: dict) -> str | None:
|
||||
args = [str(a) for a in (data.get("args") or [])]
|
||||
mask_next = False
|
||||
for a in args:
|
||||
# A ${VAR} reference is the recommended fix for this very warning --
|
||||
# continuing to warn after the user adopts it punishes the fix (#76).
|
||||
if is_env_ref(a):
|
||||
mask_next = False
|
||||
continue
|
||||
if mask_next:
|
||||
mask_next = False
|
||||
if not a.startswith("-"):
|
||||
@@ -1413,9 +1818,21 @@ def lint_server(name: str, data: dict) -> list[str]:
|
||||
|
||||
|
||||
def lint_servers(servers: list[ServerEntry]) -> list[str]:
|
||||
"""Concatenate lint_server warnings across every entry, in order."""
|
||||
"""Concatenate lint_server warnings across every entry, in order.
|
||||
|
||||
Entries whose config value wasn't a JSON object at all are reported here
|
||||
rather than in lint_server, which takes an already-dict `data` (#72).
|
||||
"""
|
||||
out: list[str] = []
|
||||
for s in servers:
|
||||
if s.malformed:
|
||||
nm = s.name.strip() or "(unnamed)"
|
||||
out.append(
|
||||
f"'{nm}': server definition is not an object "
|
||||
f"(found {type(s.raw).__name__}) -- it is preserved as-is; "
|
||||
f"edit it to replace it with a proper definition"
|
||||
)
|
||||
continue
|
||||
out.extend(lint_server(s.name, s.data))
|
||||
return out
|
||||
|
||||
|
||||
@@ -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
|
||||
@@ -2372,6 +2373,261 @@ def test_config_has_unfilled_placeholders_checks_env_too():
|
||||
assert c.config_has_unfilled_placeholders(cfg) is True
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# #72 -- a server value that isn't a JSON object must not take the load down
|
||||
# --------------------------------------------------------------------------- #
|
||||
@pytest.mark.parametrize("bad", ["not-a-dict", 123, ["a", "b"], None, True, 1.5])
|
||||
def test_extract_servers_survives_non_dict_server_value(bad):
|
||||
entries = c.extract_servers({"mcpServers": {"foo": bad}})
|
||||
assert len(entries) == 1
|
||||
assert entries[0].name == "foo"
|
||||
assert entries[0].data == {}
|
||||
assert entries[0].malformed is True
|
||||
assert entries[0].raw == bad
|
||||
|
||||
|
||||
def test_extract_servers_marks_only_the_bad_entry():
|
||||
cfg = {"mcpServers": {"good": {"command": "npx"}, "bad": "oops"}}
|
||||
by_name = {e.name: e for e in c.extract_servers(cfg)}
|
||||
assert by_name["good"].malformed is False
|
||||
assert by_name["good"].data == {"command": "npx"}
|
||||
assert by_name["bad"].malformed is True
|
||||
|
||||
|
||||
def test_extract_servers_handles_malformed_disabled_entry():
|
||||
entries = c.extract_servers({c.DISABLED_KEY: {"parked": ["nope"]}})
|
||||
assert entries[0].enabled is False
|
||||
assert entries[0].malformed is True
|
||||
|
||||
|
||||
def test_malformed_entry_round_trips_through_save_unchanged():
|
||||
"""The cardinal rule: never silently delete what the user had on disk."""
|
||||
cfg = {"mcpServers": {"good": {"command": "npx"}, "bad": "oops"}}
|
||||
servers = c.extract_servers(cfg)
|
||||
out = c.apply_servers(dict(cfg), servers)
|
||||
assert out["mcpServers"]["bad"] == "oops"
|
||||
assert out["mcpServers"]["good"] == {"command": "npx"}
|
||||
|
||||
|
||||
def test_editing_a_malformed_entry_retires_the_raw_value():
|
||||
entry = c.extract_servers({"mcpServers": {"bad": "oops"}})[0]
|
||||
entry.set_data({"command": "npx"})
|
||||
assert entry.malformed is False
|
||||
assert entry.config_value() == {"command": "npx"}
|
||||
assert c.apply_servers({}, [entry])["mcpServers"]["bad"] == {"command": "npx"}
|
||||
|
||||
|
||||
def test_lint_reports_the_malformed_entry_by_name():
|
||||
servers = c.extract_servers({"mcpServers": {"bad": "oops"}})
|
||||
warnings = c.lint_servers(servers)
|
||||
assert len(warnings) == 1
|
||||
assert "'bad'" in warnings[0]
|
||||
assert "not an object" in warnings[0]
|
||||
assert "str" in warnings[0]
|
||||
|
||||
|
||||
def test_lint_still_reports_normal_warnings_alongside_malformed():
|
||||
cfg = {"mcpServers": {"bad": "oops", "sloppy": {"command": "npx", "args": "one two"}}}
|
||||
warnings = c.lint_servers(c.extract_servers(cfg))
|
||||
assert any("not an object" in w for w in warnings)
|
||||
assert any("'args' should be a list" in w for w in warnings)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# #73 -- the stale-file merge must not discard BCC-authored keys
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_carry_owned_keys_moves_sets_onto_the_reloaded_config():
|
||||
local = {"mcpServers": {}, c.SETS_KEY: {"work": ["a", "b"]}}
|
||||
fresh = {"mcpServers": {"external": {"command": "npx"}}}
|
||||
contested = c.carry_owned_keys(local, fresh)
|
||||
assert contested == []
|
||||
assert fresh[c.SETS_KEY] == {"work": ["a", "b"]}
|
||||
assert fresh["mcpServers"] == {"external": {"command": "npx"}}
|
||||
|
||||
|
||||
def test_carry_owned_keys_reports_a_genuine_conflict():
|
||||
local = {c.SETS_KEY: {"work": ["a"]}}
|
||||
fresh = {c.SETS_KEY: {"work": ["a", "b"]}}
|
||||
assert c.carry_owned_keys(local, fresh) == [c.SETS_KEY]
|
||||
assert fresh[c.SETS_KEY] == {"work": ["a"]} # local wins: BCC owns the key
|
||||
|
||||
|
||||
def test_carry_owned_keys_is_quiet_when_both_sides_agree():
|
||||
local = {c.SETS_KEY: {"work": ["a"]}}
|
||||
fresh = {c.SETS_KEY: {"work": ["a"]}}
|
||||
assert c.carry_owned_keys(local, fresh) == []
|
||||
|
||||
|
||||
def test_carry_owned_keys_leaves_disk_alone_when_absent_locally():
|
||||
"""Can't distinguish 'deleted my last set' from 'never had sets'; keep theirs."""
|
||||
fresh = {c.SETS_KEY: {"remote": ["a"]}}
|
||||
assert c.carry_owned_keys({}, fresh) == []
|
||||
assert fresh[c.SETS_KEY] == {"remote": ["a"]}
|
||||
|
||||
|
||||
def test_carry_owned_keys_deep_copies_so_later_edits_do_not_leak():
|
||||
local = {c.SETS_KEY: {"work": ["a"]}}
|
||||
fresh = {}
|
||||
c.carry_owned_keys(local, fresh)
|
||||
local[c.SETS_KEY]["work"].append("b")
|
||||
assert fresh[c.SETS_KEY] == {"work": ["a"]}
|
||||
|
||||
|
||||
def test_merge_flow_preserves_sets_and_external_servers(tmp_path):
|
||||
"""End-to-end shape of the Merge & save path that lost sets in #73."""
|
||||
path = tmp_path / "claude.json"
|
||||
path.write_text(json.dumps({"mcpServers": {"old": {"command": "old"}}}))
|
||||
|
||||
# BCC loads, user saves a named set and edits servers in memory.
|
||||
local = c.load_config(path)
|
||||
servers = c.extract_servers(local)
|
||||
c.save_server_set(local, "work", servers)
|
||||
|
||||
# Something else rewrites the file underneath us.
|
||||
path.write_text(json.dumps({"mcpServers": {"external": {"command": "new"}}, "other": 1}))
|
||||
|
||||
# Merge & save: reload disk, carry BCC keys, re-apply the user's servers.
|
||||
fresh = c.load_config(path)
|
||||
c.carry_owned_keys(local, fresh)
|
||||
c.apply_servers(fresh, servers)
|
||||
c.write_config(path, fresh)
|
||||
|
||||
saved = c.load_config(path)
|
||||
assert saved[c.SETS_KEY] == {"work": ["old"]} # the set survived
|
||||
assert saved["other"] == 1 # unrelated external key preserved
|
||||
assert "old" in saved["mcpServers"] # user's servers re-applied
|
||||
|
||||
|
||||
def test_null_server_value_is_malformed_not_mistaken_for_absent():
|
||||
"""`{"mcpServers": {"foo": null}}` is legal JSON and a real malformed case,
|
||||
so None must not double as the 'nothing here' sentinel."""
|
||||
entry = c.extract_servers({"mcpServers": {"foo": None}})[0]
|
||||
assert entry.malformed is True
|
||||
assert entry.raw is None
|
||||
assert c.apply_servers({}, [entry])["mcpServers"]["foo"] is None
|
||||
|
||||
|
||||
def test_a_normal_entry_is_not_malformed():
|
||||
entry = c.extract_servers({"mcpServers": {"foo": {"command": "npx"}}})[0]
|
||||
assert entry.malformed is False
|
||||
assert entry.raw is c.NO_RAW
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# #75 -- theming
|
||||
# --------------------------------------------------------------------------- #
|
||||
@pytest.mark.parametrize(
|
||||
"setting,system_dark,expected",
|
||||
[
|
||||
(c.THEME_DARK, False, "dark"),
|
||||
(c.THEME_DARK, True, "dark"),
|
||||
(c.THEME_LIGHT, False, "light"),
|
||||
(c.THEME_LIGHT, True, "light"),
|
||||
(c.THEME_SYSTEM, True, "dark"),
|
||||
(c.THEME_SYSTEM, False, "light"),
|
||||
],
|
||||
)
|
||||
def test_resolve_theme_covers_every_setting_and_appearance(setting, system_dark, expected):
|
||||
assert c.resolve_theme(setting, system_dark) == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize("junk", ["", "solarized", None, "DARK", 3])
|
||||
def test_resolve_theme_falls_back_to_following_the_system(junk):
|
||||
"""A hand-edited or future QSettings value should follow the desktop,
|
||||
not pin a fixed theme."""
|
||||
assert c.resolve_theme(junk, True) == "dark"
|
||||
assert c.resolve_theme(junk, False) == "light"
|
||||
|
||||
|
||||
def test_palette_for_known_names():
|
||||
assert c.palette_for("dark") is c.DARK_PALETTE
|
||||
assert c.palette_for("light") is c.LIGHT_PALETTE
|
||||
|
||||
|
||||
def test_palette_for_unknown_name_falls_back_to_dark():
|
||||
assert c.palette_for("chartreuse") is c.DARK_PALETTE
|
||||
|
||||
|
||||
def test_dark_palette_is_unchanged_from_the_shipped_look():
|
||||
"""v1.3.0 shipped these exact colours; adding a light theme must not
|
||||
quietly restyle the dark one."""
|
||||
p = c.DARK_PALETTE
|
||||
assert (p.accent, p.bg, p.panel, p.panel_2) == ("#f97316", "#1b1d23", "#23262e", "#2b2f39")
|
||||
assert (p.text, p.muted, p.border) == ("#e7e9ee", "#9aa0ad", "#3a3f4b")
|
||||
assert (p.good, p.bad, p.warn, p.remote) == ("#4ade80", "#f87171", "#fbbf24", "#60a5fa")
|
||||
assert (p.on_accent, p.disabled_bg, p.mono_bg) == ("#1a1205", "#202229", "#16181d")
|
||||
|
||||
|
||||
def test_both_palettes_define_every_slot():
|
||||
"""A missing slot should fail here rather than render a broken window."""
|
||||
for pal in (c.DARK_PALETTE, c.LIGHT_PALETTE):
|
||||
for f in dataclasses.fields(c.Palette):
|
||||
value = getattr(pal, f.name)
|
||||
assert value, f"{pal.name}.{f.name} is empty"
|
||||
if f.name != "name":
|
||||
assert re.fullmatch(r"#[0-9a-fA-F]{6}", value), f"{pal.name}.{f.name}={value!r}"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("pal_name", ["dark", "light"])
|
||||
@pytest.mark.parametrize("slot", ["text", "muted", "good", "bad", "warn", "remote", "accent"])
|
||||
def test_palette_meets_contrast_on_panel(pal_name, slot):
|
||||
"""Every colour drawn as text/glyph must clear WCAG AA (4.5:1) against the
|
||||
surface it sits on. The light palette's semantic colours are NOT the dark
|
||||
ones lightened -- #4ade80 sits near 1.7:1 on white -- so this guards
|
||||
against someone 'harmonising' them back toward the dark hues."""
|
||||
pal = c.palette_for(pal_name)
|
||||
assert c.contrast_ratio(getattr(pal, slot), pal.panel) >= 4.5
|
||||
|
||||
|
||||
@pytest.mark.parametrize("pal_name", ["dark", "light"])
|
||||
def test_on_accent_is_legible_against_the_accent_fill(pal_name):
|
||||
"""Primary buttons and selected rows draw on_accent on top of accent."""
|
||||
pal = c.palette_for(pal_name)
|
||||
assert c.contrast_ratio(pal.on_accent, pal.accent) >= 4.5
|
||||
|
||||
|
||||
def test_contrast_ratio_endpoints():
|
||||
assert c.contrast_ratio("#000000", "#ffffff") == pytest.approx(21.0, abs=0.01)
|
||||
assert c.contrast_ratio("#123456", "#123456") == pytest.approx(1.0, abs=0.001)
|
||||
assert c.contrast_ratio("#ffffff", "#000000") == pytest.approx(21.0, abs=0.01)
|
||||
|
||||
|
||||
def test_relative_luminance_extremes():
|
||||
assert c.relative_luminance("#000000") == pytest.approx(0.0)
|
||||
assert c.relative_luminance("#ffffff") == pytest.approx(1.0)
|
||||
|
||||
|
||||
def test_stylesheet_builder_has_no_hardcoded_colours():
|
||||
"""Every colour in the QSS must come from the palette.
|
||||
|
||||
Three near-black literals used to be inlined here (#1a1205, #202229,
|
||||
#16181d). Harmless with one theme; with two, they silently render dark
|
||||
chrome on a light window. Reads the source rather than importing bcc,
|
||||
which needs PySide6.
|
||||
"""
|
||||
src = (Path(__file__).resolve().parent.parent / "bcc.py").read_text(encoding="utf-8")
|
||||
start = src.index("def build_stylesheet")
|
||||
body = src[start : src.index("def apply_palette")]
|
||||
assert re.findall(r"#[0-9a-fA-F]{6}", body) == []
|
||||
|
||||
|
||||
def test_every_palette_slot_is_consumed():
|
||||
"""A slot added to Palette but never wired up is dead weight.
|
||||
|
||||
Checks for `p.<slot>` anywhere in bcc.py, which covers both the QSS and
|
||||
apply_palette's global bindings -- not every slot belongs in the
|
||||
stylesheet (`good` and `remote` feed the inline status dots via
|
||||
STATUS_COLORS/HEALTH_COLORS, never the QSS). This won't catch a slot bound
|
||||
to a global that nothing then uses; it does catch the common mistake of
|
||||
extending the dataclass and forgetting to plumb it through.
|
||||
"""
|
||||
src = (Path(__file__).resolve().parent.parent / "bcc.py").read_text(encoding="utf-8")
|
||||
for f in dataclasses.fields(c.Palette):
|
||||
if f.name == "name":
|
||||
continue
|
||||
assert f"p.{f.name}" in src, f"palette slot {f.name!r} is never consumed"
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# #78/#79 -- update notice: when to show it, and what it says
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -2424,3 +2680,138 @@ def test_update_notice_renders_both_versions_the_same_way():
|
||||
assert "1.3.0" in n["text"] and "1.2.0" in n["text"]
|
||||
# the machine-readable field keeps the real tag
|
||||
assert n["version"] == "v1.3.0"
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# #76 -- ${VAR} references. Semantics mirror Claude Code's documented
|
||||
# behaviour: ${VAR} and ${VAR:-default}, expanded in command/args/env/url/
|
||||
# headers, and an unset variable with no default left as literal text.
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_find_env_refs_plain_and_defaulted():
|
||||
refs = c.find_env_refs("${A} and ${B:-fallback}")
|
||||
assert [(r.name, r.default) for r in refs] == [("A", None), ("B", "fallback")]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("text", ["${}", "${1BAD}", "$NOTBRACED", "{NOPE}", "plain", "$${X"])
|
||||
def test_find_env_refs_ignores_non_references(text):
|
||||
assert c.find_env_refs(text) == []
|
||||
|
||||
|
||||
def test_find_env_refs_allows_an_empty_default():
|
||||
"""`${VAR:-}` is a documented way to say 'blank if unset'."""
|
||||
refs = c.find_env_refs("${A:-}")
|
||||
assert refs[0].default == ""
|
||||
assert refs[0].has_default is True
|
||||
|
||||
|
||||
def test_server_env_refs_covers_all_five_documented_fields():
|
||||
data = {
|
||||
"command": "${BIN}",
|
||||
"args": ["--x", "${ARG}"],
|
||||
"env": {"K": "${ENVV}"},
|
||||
"url": "${URL}/mcp",
|
||||
"headers": {"Authorization": "Bearer ${HDR}"},
|
||||
}
|
||||
found = {(r.name, r.field) for r in c.server_env_refs(data)}
|
||||
assert found == {
|
||||
("BIN", "command"),
|
||||
("ARG", "args"),
|
||||
("ENVV", "env"),
|
||||
("URL", "url"),
|
||||
("HDR", "headers"),
|
||||
}
|
||||
|
||||
|
||||
def test_server_env_refs_ignores_unexpanded_fields():
|
||||
"""Claude Code expands five fields; a ${VAR} elsewhere isn't a reference."""
|
||||
assert c.server_env_refs({"description": "${NOPE}", "timeout": "${ALSO_NO}"}) == []
|
||||
|
||||
|
||||
def test_expand_env_refs_matches_documented_semantics():
|
||||
env = {"SET": "value"}
|
||||
assert c.expand_env_refs("${SET}", env) == "value"
|
||||
assert c.expand_env_refs("${MISSING:-dflt}", env) == "dflt"
|
||||
assert c.expand_env_refs("${SET:-dflt}", env) == "value"
|
||||
# unset with no default: left as literal text, exactly as Claude Code does
|
||||
assert c.expand_env_refs("${MISSING}", env) == "${MISSING}"
|
||||
|
||||
|
||||
def test_expand_env_refs_handles_several_in_one_string():
|
||||
assert c.expand_env_refs("${A}/${B:-two}/${C}", {"A": "one"}) == "one/two/${C}"
|
||||
|
||||
|
||||
def test_unresolved_env_refs_only_flags_unset_without_default():
|
||||
data = {"env": {"A": "${SET}", "B": "${UNSET}", "C": "${OTHER:-has_default}"}}
|
||||
assert [r.name for r in c.unresolved_env_refs(data, {"SET": "x"})] == ["UNSET"]
|
||||
|
||||
|
||||
# --- the two interactions that were backwards for this feature ------------
|
||||
def test_placeholder_under_a_secret_key_is_not_masked():
|
||||
"""A ${VAR} names a secret rather than being one. Masking it would make a
|
||||
reference indistinguishable from a stored credential."""
|
||||
assert c.should_mask_value("API_KEY", "${API_KEY}") is False
|
||||
assert c.should_mask_value("API_KEY", "ghp_realsecret") is True
|
||||
assert c.should_mask_value("NOT_SECRET", "${API_KEY}") is False
|
||||
|
||||
|
||||
def test_redacted_display_keeps_placeholders_but_masks_real_secrets():
|
||||
out = c._redact_server_data({"env": {"API_KEY": "${API_KEY}", "TOKEN": "ghp_real"}})
|
||||
assert out["env"]["API_KEY"] == "${API_KEY}"
|
||||
assert out["env"]["TOKEN"] == c.MASK
|
||||
|
||||
|
||||
def test_redact_args_keeps_placeholders_visible():
|
||||
assert c.redact_args(["--token", "${GH_TOKEN}"]) == ["--token", "${GH_TOKEN}"]
|
||||
assert c.redact_args(["--api-key=${K}"]) == ["--api-key=${K}"]
|
||||
# real secrets still masked
|
||||
assert c.redact_args(["--token", "ghp_real"]) == ["--token", c.MASK]
|
||||
assert c.redact_args(["--api-key=sk-real"]) == [f"--api-key={c.MASK}"]
|
||||
|
||||
|
||||
def test_args_secret_warning_is_silenced_by_a_placeholder():
|
||||
"""Moving a token into ${VAR} is the recommended fix for this warning --
|
||||
still warning afterwards would punish the fix."""
|
||||
assert c.args_secret_warning({"args": ["--token", "ghp_real"]}) is not None
|
||||
assert c.args_secret_warning({"args": ["--token", "${GH_TOKEN}"]}) is None
|
||||
|
||||
|
||||
def test_args_secret_warning_still_fires_on_the_arg_after_a_placeholder():
|
||||
"""A placeholder must clear the pending-flag state, not blanket-suppress."""
|
||||
assert c.args_secret_warning({"args": ["${SAFE}", "--token", "ghp_real"]}) is not None
|
||||
|
||||
|
||||
# --- per-client gating ----------------------------------------------------
|
||||
def _profile(path):
|
||||
return c.Profile(label="p", path=Path(path), config_exists=True)
|
||||
|
||||
|
||||
def test_claude_code_profiles_expand_references():
|
||||
assert c.client_expands_env_refs(_profile(Path.home() / ".claude.json")) is True
|
||||
assert c.client_expands_env_refs(_profile("/repo/.mcp.json")) is True
|
||||
|
||||
|
||||
def test_claude_desktop_profile_does_not_expand_references():
|
||||
desktop = _profile(c.app_support_base() / "Claude" / c.CONFIG_FILENAME)
|
||||
assert c.client_expands_env_refs(desktop) is False
|
||||
|
||||
|
||||
def test_desktop_profile_warns_that_references_are_literal():
|
||||
data = {"env": {"API_KEY": "${API_KEY}"}}
|
||||
desktop = _profile(c.app_support_base() / "Claude" / c.CONFIG_FILENAME)
|
||||
warnings = c.env_ref_warnings(data, desktop, {"API_KEY": "set"})
|
||||
assert len(warnings) == 1
|
||||
assert "NOT be expanded" in warnings[0]
|
||||
assert "${API_KEY}" in warnings[0]
|
||||
|
||||
|
||||
def test_claude_code_profile_warns_only_about_unset_variables():
|
||||
code = _profile(Path.home() / ".claude.json")
|
||||
data = {"env": {"A": "${UNSET_ONE}"}}
|
||||
assert c.env_ref_warnings(data, code, {}) != []
|
||||
assert c.env_ref_warnings(data, code, {"UNSET_ONE": "x"}) == []
|
||||
# a default means it always resolves
|
||||
assert c.env_ref_warnings({"env": {"A": "${X:-d}"}}, code, {}) == []
|
||||
|
||||
|
||||
def test_no_references_means_no_warnings():
|
||||
assert c.env_ref_warnings({"command": "npx", "args": ["-y", "pkg"]}, None) == []
|
||||
|
||||
Reference in New Issue
Block a user