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
|
||||
|
||||
import contextlib
|
||||
import sys
|
||||
import time
|
||||
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; }}
|
||||
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; }}
|
||||
@@ -1528,6 +1533,54 @@ class AboutDialog(QDialog):
|
||||
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
|
||||
# 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()
|
||||
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.
|
||||
split = QSplitter(Qt.Orientation.Horizontal)
|
||||
split.setChildrenCollapsible(False)
|
||||
@@ -1637,10 +1695,59 @@ class MainWindow(QMainWindow):
|
||||
theme_menu.addAction(act)
|
||||
|
||||
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)
|
||||
# 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)
|
||||
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):
|
||||
"""Persist the theme choice and repaint the running window."""
|
||||
QSettings("BCC", "BetterClaudeConfig").setValue("ui/theme", setting)
|
||||
@@ -1681,10 +1788,9 @@ class MainWindow(QMainWindow):
|
||||
if release is None:
|
||||
return # offline/failed check: don't advance lastCheck, allow retry
|
||||
QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
|
||||
if core.is_newer_version(core.__version__, release["version"]):
|
||||
self.status.setText(
|
||||
f"Update available: {release['version']} · Help ▸ About to view it."
|
||||
)
|
||||
notice = core.update_notice(core.__version__, release)
|
||||
if notice:
|
||||
self._show_update_notice(notice)
|
||||
|
||||
# --- layout persistence ---------------------------------------------- #
|
||||
def _restore_layout(self):
|
||||
@@ -2010,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()
|
||||
@@ -2335,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
|
||||
@@ -2429,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))
|
||||
@@ -2549,6 +2667,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)
|
||||
@@ -2561,8 +2684,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()
|
||||
|
||||
+150
-6
@@ -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
|
||||
|
||||
@@ -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}
|
||||
|
||||
|
||||
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
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -178,16 +218,63 @@ 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)
|
||||
@@ -584,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
|
||||
|
||||
|
||||
@@ -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
|
||||
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:
|
||||
@@ -696,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)
|
||||
@@ -1512,9 +1644,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
|
||||
|
||||
|
||||
@@ -2373,6 +2373,146 @@ 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
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -2486,3 +2626,57 @@ def test_every_palette_slot_is_consumed():
|
||||
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
|
||||
# --------------------------------------------------------------------------- #
|
||||
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