Compare commits
7 Commits
febd617c56
...
main
| Author | SHA1 | Date | |
|---|---|---|---|
| 7ff4f6e5c0 | |||
| 7517e16b15 | |||
| 9a0433225e | |||
| fa82d30087 | |||
| 05b00a40c0 | |||
| 3068e74e5c | |||
| da20eb2fdb |
@@ -10,6 +10,7 @@ Run: python mcp_manager.py
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import contextlib
|
||||||
import sys
|
import sys
|
||||||
import time
|
import time
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
@@ -141,6 +142,10 @@ QScrollBar::handle:vertical {{ background: {p.border}; border-radius: 5px; min-h
|
|||||||
QScrollBar::add-line, QScrollBar::sub-line {{ height: 0; }}
|
QScrollBar::add-line, QScrollBar::sub-line {{ height: 0; }}
|
||||||
QLabel#statusbar {{ color: {p.muted}; padding: 4px 2px; }}
|
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; }}
|
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#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#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; }}
|
QLabel#placeholder {{ color: {p.muted}; padding: 12px; background: {p.panel_2}; border: 1px dashed {p.border}; border-radius: 8px; }}
|
||||||
@@ -1528,6 +1533,54 @@ class AboutDialog(QDialog):
|
|||||||
QDesktopServices.openUrl(QUrl(self._release_url or core.RELEASES_URL))
|
QDesktopServices.openUrl(QUrl(self._release_url or core.RELEASES_URL))
|
||||||
|
|
||||||
|
|
||||||
|
class NoticeBanner(QFrame):
|
||||||
|
"""A persistent, dismissible notice with an optional action button.
|
||||||
|
|
||||||
|
The status bar is the wrong home for anything the user needs to act on --
|
||||||
|
21 call sites rewrite it, so a message posted there is gone by the next
|
||||||
|
click. That wiped the MSIX warning (#35) and then the update notice (#78).
|
||||||
|
This is the shared mechanism so it doesn't happen a third time.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def __init__(self, parent=None):
|
||||||
|
super().__init__(parent)
|
||||||
|
self.setObjectName("noticeBanner")
|
||||||
|
row = QHBoxLayout(self)
|
||||||
|
row.setContentsMargins(10, 8, 8, 8)
|
||||||
|
row.setSpacing(8)
|
||||||
|
self._label = QLabel("")
|
||||||
|
self._label.setObjectName("noticeText")
|
||||||
|
self._label.setWordWrap(True)
|
||||||
|
row.addWidget(self._label, 1)
|
||||||
|
self._action_btn = QPushButton("")
|
||||||
|
self._action_btn.setCursor(Qt.CursorShape.PointingHandCursor)
|
||||||
|
self._action_btn.hide()
|
||||||
|
row.addWidget(self._action_btn)
|
||||||
|
self._close_btn = QPushButton("\u2715")
|
||||||
|
self._close_btn.setObjectName("noticeClose")
|
||||||
|
self._close_btn.setCursor(Qt.CursorShape.PointingHandCursor)
|
||||||
|
self._close_btn.setFixedWidth(26)
|
||||||
|
self._close_btn.setToolTip("Dismiss")
|
||||||
|
self._close_btn.clicked.connect(self.hide)
|
||||||
|
row.addWidget(self._close_btn)
|
||||||
|
self.hide()
|
||||||
|
|
||||||
|
def show_notice(self, text: str, action_label: str = "", on_action=None):
|
||||||
|
self._label.setText(text)
|
||||||
|
self._label.setToolTip(text)
|
||||||
|
# Reconnect cleanly: a banner reused for a second notice would
|
||||||
|
# otherwise fire the previous notice's action too.
|
||||||
|
with contextlib.suppress(RuntimeError, TypeError):
|
||||||
|
self._action_btn.clicked.disconnect()
|
||||||
|
if action_label and on_action is not None:
|
||||||
|
self._action_btn.setText(action_label)
|
||||||
|
self._action_btn.clicked.connect(lambda _=False: on_action())
|
||||||
|
self._action_btn.show()
|
||||||
|
else:
|
||||||
|
self._action_btn.hide()
|
||||||
|
self.show()
|
||||||
|
|
||||||
|
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# Restart worker: core.restart_claude_desktop() blocks up to ~5 s on macOS
|
# Restart worker: core.restart_claude_desktop() blocks up to ~5 s on macOS
|
||||||
# waiting for the old instance to exit, so it must run off the UI thread.
|
# waiting for the old instance to exit, so it must run off the UI thread.
|
||||||
@@ -1585,6 +1638,11 @@ class MainWindow(QMainWindow):
|
|||||||
self.warn_banner.hide()
|
self.warn_banner.hide()
|
||||||
root.addWidget(self.warn_banner)
|
root.addWidget(self.warn_banner)
|
||||||
|
|
||||||
|
# Update availability gets its own persistent banner rather than a
|
||||||
|
# status-line write, which the next UI action overwrites (#78).
|
||||||
|
self.update_banner = NoticeBanner(self)
|
||||||
|
root.addWidget(self.update_banner)
|
||||||
|
|
||||||
# User-draggable divider between the server list and the editor.
|
# User-draggable divider between the server list and the editor.
|
||||||
split = QSplitter(Qt.Orientation.Horizontal)
|
split = QSplitter(Qt.Orientation.Horizontal)
|
||||||
split.setChildrenCollapsible(False)
|
split.setChildrenCollapsible(False)
|
||||||
@@ -1637,10 +1695,59 @@ class MainWindow(QMainWindow):
|
|||||||
theme_menu.addAction(act)
|
theme_menu.addAction(act)
|
||||||
|
|
||||||
help_menu = self.menuBar().addMenu("&Help")
|
help_menu = self.menuBar().addMenu("&Help")
|
||||||
|
|
||||||
|
# "Check for updates" used to exist only as a button inside the About
|
||||||
|
# dialog, which is not somewhere anyone looks for it (#79).
|
||||||
|
update_action = QAction("Check for updates…", self)
|
||||||
|
# Explicit role: macOS relocates actions it recognises by text, and
|
||||||
|
# some Qt versions treat "update" as application-menu material. Pin it
|
||||||
|
# so the item stays where the menu says it is on every platform.
|
||||||
|
update_action.setMenuRole(QAction.MenuRole.ApplicationSpecificRole)
|
||||||
|
update_action.triggered.connect(self.check_for_updates)
|
||||||
|
help_menu.addAction(update_action)
|
||||||
|
help_menu.addSeparator()
|
||||||
|
|
||||||
about_action = QAction("About Better Claude Config…", self)
|
about_action = QAction("About Better Claude Config…", self)
|
||||||
|
# Qt auto-assigns AboutRole to actions whose text starts with "About",
|
||||||
|
# which moves this into the application menu on macOS. That is the
|
||||||
|
# right home there -- state it explicitly rather than inheriting it by
|
||||||
|
# accident, since the behaviour is invisible from this call site.
|
||||||
|
about_action.setMenuRole(QAction.MenuRole.AboutRole)
|
||||||
about_action.triggered.connect(self._show_about)
|
about_action.triggered.connect(self._show_about)
|
||||||
help_menu.addAction(about_action)
|
help_menu.addAction(about_action)
|
||||||
|
|
||||||
|
def _show_update_notice(self, notice: dict):
|
||||||
|
"""Surface an available update where it survives the next click."""
|
||||||
|
url = notice["url"]
|
||||||
|
self.update_banner.show_notice(
|
||||||
|
notice["text"],
|
||||||
|
action_label="Open releases page",
|
||||||
|
on_action=lambda: QDesktopServices.openUrl(QUrl(url)),
|
||||||
|
)
|
||||||
|
|
||||||
|
def check_for_updates(self):
|
||||||
|
"""Menu-driven check. Unlike the startup check this is never throttled
|
||||||
|
and always reports back -- the user asked, so silence would read as a
|
||||||
|
broken button."""
|
||||||
|
self.status.setText("Checking for updates…")
|
||||||
|
self._menu_update_worker = UpdateCheckWorker()
|
||||||
|
self._menu_update_worker.done.connect(self._on_menu_update_checked)
|
||||||
|
self._menu_update_worker.start()
|
||||||
|
|
||||||
|
def _on_menu_update_checked(self, release: dict | None):
|
||||||
|
self._menu_update_worker = None
|
||||||
|
if release is None:
|
||||||
|
self.status.setText("Couldn't check for updates (offline?).")
|
||||||
|
return
|
||||||
|
QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
|
||||||
|
notice = core.update_notice(core.__version__, release)
|
||||||
|
if notice:
|
||||||
|
self._show_update_notice(notice)
|
||||||
|
self.status.setText(f"Update available: {notice['version']}")
|
||||||
|
else:
|
||||||
|
self.update_banner.hide()
|
||||||
|
self.status.setText(f"You're up to date ({core.__version__}).")
|
||||||
|
|
||||||
def _set_theme(self, setting: str):
|
def _set_theme(self, setting: str):
|
||||||
"""Persist the theme choice and repaint the running window."""
|
"""Persist the theme choice and repaint the running window."""
|
||||||
QSettings("BCC", "BetterClaudeConfig").setValue("ui/theme", setting)
|
QSettings("BCC", "BetterClaudeConfig").setValue("ui/theme", setting)
|
||||||
@@ -1681,10 +1788,9 @@ class MainWindow(QMainWindow):
|
|||||||
if release is None:
|
if release is None:
|
||||||
return # offline/failed check: don't advance lastCheck, allow retry
|
return # offline/failed check: don't advance lastCheck, allow retry
|
||||||
QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
|
QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
|
||||||
if core.is_newer_version(core.__version__, release["version"]):
|
notice = core.update_notice(core.__version__, release)
|
||||||
self.status.setText(
|
if notice:
|
||||||
f"Update available: {release['version']} · Help ▸ About to view it."
|
self._show_update_notice(notice)
|
||||||
)
|
|
||||||
|
|
||||||
# --- layout persistence ---------------------------------------------- #
|
# --- layout persistence ---------------------------------------------- #
|
||||||
def _restore_layout(self):
|
def _restore_layout(self):
|
||||||
@@ -2010,9 +2116,21 @@ class MainWindow(QMainWindow):
|
|||||||
return
|
return
|
||||||
self.full_config = cfg
|
self.full_config = cfg
|
||||||
repaired = True
|
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._loaded_stat = core.config_fingerprint(profile.path)
|
||||||
self.current_profile = profile
|
self.current_profile = profile
|
||||||
self.servers = core.extract_servers(self.full_config)
|
self.servers = servers
|
||||||
self.dirty = False
|
self.dirty = False
|
||||||
self.restart_btn.hide()
|
self.restart_btn.hide()
|
||||||
self._undo_stack.clear()
|
self._undo_stack.clear()
|
||||||
@@ -2335,7 +2453,7 @@ class MainWindow(QMainWindow):
|
|||||||
entry = self.servers[idx]
|
entry = self.servers[idx]
|
||||||
old_name = entry.name
|
old_name = entry.name
|
||||||
entry.name = self.editor.current_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
|
# The server stays in its section (enable state unchanged), so update
|
||||||
# its existing row in place rather than re-rendering.
|
# its existing row in place rather than re-rendering.
|
||||||
# An edit invalidates any cached "Test all" result -- the server that
|
# An edit invalidates any cached "Test all" result -- the server that
|
||||||
@@ -2429,7 +2547,7 @@ class MainWindow(QMainWindow):
|
|||||||
QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
|
QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
|
||||||
)
|
)
|
||||||
if ans == QMessageBox.StandardButton.Yes:
|
if ans == QMessageBox.StandardButton.Yes:
|
||||||
self.servers[existing[name]].data = data
|
self.servers[existing[name]].set_data(data)
|
||||||
return False, True
|
return False, True
|
||||||
name = core.resolve_name_collision(name, {s.name for s in self.servers})
|
name = core.resolve_name_collision(name, {s.name for s in self.servers})
|
||||||
self.servers.append(core.ServerEntry(name, data, True))
|
self.servers.append(core.ServerEntry(name, data, True))
|
||||||
@@ -2549,6 +2667,11 @@ class MainWindow(QMainWindow):
|
|||||||
except Exception as e:
|
except Exception as e:
|
||||||
QMessageBox.critical(self, "Reload failed", str(e))
|
QMessageBox.critical(self, "Reload failed", str(e))
|
||||||
return
|
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)
|
core.apply_servers(fresh, self.servers)
|
||||||
try:
|
try:
|
||||||
backup = core.write_config(self.current_profile.path, fresh)
|
backup = core.write_config(self.current_profile.path, fresh)
|
||||||
@@ -2561,8 +2684,13 @@ class MainWindow(QMainWindow):
|
|||||||
self.dirty = False
|
self.dirty = False
|
||||||
self.save_btn.setEnabled(False)
|
self.save_btn.setEnabled(False)
|
||||||
bnote = f" · backup: {backup.name}" if backup else " · (new file)"
|
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(
|
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."
|
f" · Restart {self.current_profile.label} to apply."
|
||||||
)
|
)
|
||||||
self._offer_restart_button()
|
self._offer_restart_button()
|
||||||
|
|||||||
+150
-6
@@ -15,6 +15,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import base64
|
import base64
|
||||||
import contextlib
|
import contextlib
|
||||||
|
import copy
|
||||||
import difflib
|
import difflib
|
||||||
import functools
|
import functools
|
||||||
import glob
|
import glob
|
||||||
@@ -49,6 +50,12 @@ DISABLED_KEY = "_disabledMcpServers"
|
|||||||
# parks the rest under DISABLED_KEY.
|
# parks the rest under DISABLED_KEY.
|
||||||
SETS_KEY = "_bccServerSets"
|
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"
|
BACKUP_DIRNAME = ".bcc_backups"
|
||||||
MAX_BACKUPS = 15
|
MAX_BACKUPS = 15
|
||||||
|
|
||||||
@@ -163,6 +170,39 @@ def fetch_latest_release(timeout: float = 4.0) -> dict | None:
|
|||||||
return {"version": tag, "url": payload.get("html_url") or RELEASES_URL}
|
return {"version": tag, "url": payload.get("html_url") or RELEASES_URL}
|
||||||
|
|
||||||
|
|
||||||
|
def update_notice(
|
||||||
|
current: str, release: dict | None, url_fallback: str = RELEASES_URL
|
||||||
|
) -> dict | None:
|
||||||
|
"""Decide whether to tell the user about a release, and what to say.
|
||||||
|
|
||||||
|
Returns {"version", "text", "url"} when `release` is newer than `current`,
|
||||||
|
or None when it isn't, when the check failed, or when the payload is
|
||||||
|
malformed. Kept here rather than in the GUI so the wording and the
|
||||||
|
should-we-notify decision are testable -- bcc.py can't be imported by the
|
||||||
|
test suite, which has no PySide6.
|
||||||
|
|
||||||
|
The text deliberately names no menu path. The old status-line notice read
|
||||||
|
"Help > About to view it", which is wrong on macOS: Qt relocates the About
|
||||||
|
action into the application menu (#79). A notice that carries its own
|
||||||
|
action can't drift out of sync with the platform.
|
||||||
|
"""
|
||||||
|
if not isinstance(release, dict):
|
||||||
|
return None
|
||||||
|
version = release.get("version")
|
||||||
|
if not version or not isinstance(version, str):
|
||||||
|
return None
|
||||||
|
if not is_newer_version(current, version):
|
||||||
|
return None
|
||||||
|
# Tags carry a "v" prefix and __version__ doesn't; render both the same way
|
||||||
|
# so the notice doesn't read "Version v1.3.0 ... you're running 1.2.0".
|
||||||
|
shown = version.lstrip("vV")
|
||||||
|
return {
|
||||||
|
"version": version,
|
||||||
|
"text": f"Version {shown} is available. You're running {current.lstrip('vV')}.",
|
||||||
|
"url": release.get("url") or url_fallback,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# Data model
|
# Data model
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
@@ -178,16 +218,63 @@ class Profile:
|
|||||||
self.path = Path(self.path)
|
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
|
@dataclass
|
||||||
class ServerEntry:
|
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
|
name: str
|
||||||
data: dict
|
data: dict
|
||||||
enabled: bool = True
|
enabled: bool = True
|
||||||
|
raw: object = NO_RAW
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def kind(self) -> str:
|
def kind(self) -> str:
|
||||||
return "remote" if "url" in self.data and "command" not in self.data else "stdio"
|
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)
|
# Theming (issue #75)
|
||||||
@@ -584,13 +671,30 @@ def repair_config_file(path: str | os.PathLike) -> tuple[dict, list[str], str]:
|
|||||||
return obj, notes, pretty
|
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]:
|
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] = []
|
out: list[ServerEntry] = []
|
||||||
for name, data in (cfg.get("mcpServers") or {}).items():
|
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():
|
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
|
return out
|
||||||
|
|
||||||
|
|
||||||
@@ -682,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
|
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 `mcpServers`. Returns the same dict for convenience.
|
||||||
"""
|
"""
|
||||||
enabled = {s.name: s.data for s in servers if s.enabled}
|
enabled = {s.name: s.config_value() for s in servers if s.enabled}
|
||||||
disabled = {s.name: s.data for s in servers if not 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
|
cfg["mcpServers"] = enabled # replaces value if key existed; appends otherwise
|
||||||
if disabled:
|
if disabled:
|
||||||
@@ -696,6 +800,34 @@ def apply_servers(cfg: dict, servers: list[ServerEntry]) -> dict:
|
|||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# Write (atomic, with rotating backups)
|
# 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:
|
def _make_backup(path: Path) -> Path:
|
||||||
bdir = path.parent / BACKUP_DIRNAME
|
bdir = path.parent / BACKUP_DIRNAME
|
||||||
bdir.mkdir(exist_ok=True)
|
bdir.mkdir(exist_ok=True)
|
||||||
@@ -1512,9 +1644,21 @@ def lint_server(name: str, data: dict) -> list[str]:
|
|||||||
|
|
||||||
|
|
||||||
def lint_servers(servers: list[ServerEntry]) -> 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] = []
|
out: list[str] = []
|
||||||
for s in servers:
|
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))
|
out.extend(lint_server(s.name, s.data))
|
||||||
return out
|
return out
|
||||||
|
|
||||||
|
|||||||
@@ -2373,6 +2373,146 @@ def test_config_has_unfilled_placeholders_checks_env_too():
|
|||||||
assert c.config_has_unfilled_placeholders(cfg) is True
|
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
|
# #75 -- theming
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
@@ -2486,3 +2626,57 @@ def test_every_palette_slot_is_consumed():
|
|||||||
if f.name == "name":
|
if f.name == "name":
|
||||||
continue
|
continue
|
||||||
assert f"p.{f.name}" in src, f"palette slot {f.name!r} is never consumed"
|
assert f"p.{f.name}" in src, f"palette slot {f.name!r} is never consumed"
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# #78/#79 -- update notice: when to show it, and what it says
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_update_notice_when_a_newer_release_exists():
|
||||||
|
n = c.update_notice("1.2.0", {"version": "v1.3.0", "url": "https://example.test/rel"})
|
||||||
|
assert n is not None
|
||||||
|
assert n["version"] == "v1.3.0"
|
||||||
|
assert n["url"] == "https://example.test/rel"
|
||||||
|
assert "1.3.0" in n["text"] and "1.2.0" in n["text"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_notice_is_silent_when_current():
|
||||||
|
assert c.update_notice("1.3.0", {"version": "v1.3.0"}) is None
|
||||||
|
assert c.update_notice("1.4.0", {"version": "v1.3.0"}) is None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("bad", [None, {}, {"version": ""}, {"version": None}, {"version": 3}, []])
|
||||||
|
def test_update_notice_is_silent_on_a_failed_or_malformed_check(bad):
|
||||||
|
"""fetch_latest_release returns None on any failure; a half-formed payload
|
||||||
|
must not produce a notice pointing at nothing."""
|
||||||
|
assert c.update_notice("1.0.0", bad) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_notice_falls_back_to_the_releases_page_without_a_url():
|
||||||
|
n = c.update_notice("1.0.0", {"version": "v2.0.0"})
|
||||||
|
assert n["url"] == c.RELEASES_URL
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_notice_names_no_menu_path():
|
||||||
|
"""The old status-line text said 'Help > About to view it', which is wrong
|
||||||
|
on macOS -- Qt moves the About action into the application menu (#79). The
|
||||||
|
notice carries its own action, so it must not describe a menu path."""
|
||||||
|
n = c.update_notice("1.0.0", {"version": "v2.0.0"})
|
||||||
|
lowered = n["text"].lower()
|
||||||
|
for phrase in ("help", "about", "menu", "▸", ">"):
|
||||||
|
assert phrase not in lowered, f"notice text should not reference {phrase!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_notice_handles_the_v_prefix_consistently():
|
||||||
|
assert c.update_notice("1.2.0", {"version": "1.3.0"}) is not None
|
||||||
|
assert c.update_notice("v1.2.0", {"version": "v1.3.0"}) is not None
|
||||||
|
assert c.update_notice("1.3.0", {"version": "v1.3.0"}) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_notice_renders_both_versions_the_same_way():
|
||||||
|
"""Tags carry a 'v' prefix, __version__ doesn't -- don't show both forms
|
||||||
|
in one sentence."""
|
||||||
|
n = c.update_notice("1.2.0", {"version": "v1.3.0"})
|
||||||
|
assert "v1.3.0" not in n["text"]
|
||||||
|
assert "1.3.0" in n["text"] and "1.2.0" in n["text"]
|
||||||
|
# the machine-readable field keeps the real tag
|
||||||
|
assert n["version"] == "v1.3.0"
|
||||||
|
|||||||
Reference in New Issue
Block a user