7 Commits

Author SHA1 Message Date
the_og 7ff4f6e5c0 Merge PR #81: make the update checker visible — persistent banner + Help menu item (#78, #79)
CI / Lint (ruff) (push) Successful in 6s
CI / Tests (py3.10 / ubuntu-latest) (push) Successful in 10s
CI / Tests (py3.12 / ubuntu-latest) (push) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (push) Successful in 10s
CI / Catalog signature (push) Successful in 6s
CI / Tests (py3.12 / windows-latest) (push) Has been cancelled
2026-07-20 12:52:48 -04:00
the_og 7517e16b15 Merge branch 'main' into fix/78-79-update-visibility
CI / Lint (ruff) (pull_request) Successful in 6s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 10s
CI / Catalog signature (pull_request) Successful in 6s
CI / Tests (py3.12 / windows-latest) (pull_request) Has been cancelled
Three conflicts, two of them semantic rather than textual:

- bcc.py QSS: this branch added the noticeBanner rules using the old
  module-level constants ({MUTED}, {ACCENT}); main had since moved the
  stylesheet onto palette slots ({p.muted}). Took main's form and
  translated the notice rules into it -- picking either side wholesale
  would have either dropped the banner styling or reintroduced globals
  that test_stylesheet_builder_has_no_hardcoded_colours now forbids.
- bcc.py methods: both sides appended to MainWindow (update-notice
  handlers vs theme handlers). Additive, kept both.
- tests/test_core.py: the usual EOF append. Kept both blocks.

_build_menu_bar auto-merged cleanly (View menu above, Help menu below);
verified both are present with their menu roles intact.

Verified: 272 test functions = 265 (main) + 7 (this branch), no
duplicates; 421 passed, ruff clean.
2026-07-20 12:51:55 -04:00
the_og 9a0433225e Merge PR #80: light theme + system-following, dark preserved exactly (#75)
CI / Lint (ruff) (push) Successful in 6s
CI / Tests (py3.10 / ubuntu-latest) (push) Successful in 11s
CI / Tests (py3.12 / ubuntu-latest) (push) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (push) Successful in 10s
CI / Catalog signature (push) Successful in 7s
CI / Tests (py3.12 / windows-latest) (push) Has been cancelled
2026-07-20 12:51:01 -04:00
the_og fa82d30087 Merge branch 'main' into feat/75-theming
CI / Lint (ruff) (pull_request) Successful in 7s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 10s
CI / Catalog signature (pull_request) Successful in 7s
CI / Tests (py3.12 / windows-latest) (pull_request) Has been cancelled
Union conflict at the end of tests/test_core.py -- both branches appended a
test block. Kept both, main's #72/#73 block first. Verified: 265 test
functions = 238 baseline + 15 (#77) + 12 (theming), no duplicates.
2026-07-20 12:50:09 -04:00
the_og 05b00a40c0 Merge PR #77: tolerate non-object server values; keep named sets across a merge (#72, #73)
CI / Lint (ruff) (push) Successful in 7s
CI / Tests (py3.10 / ubuntu-latest) (push) Successful in 10s
CI / Tests (py3.12 / ubuntu-latest) (push) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (push) Successful in 11s
CI / Catalog signature (push) Successful in 7s
CI / Tests (py3.12 / windows-latest) (push) Has been cancelled
2026-07-20 12:49:23 -04:00
the_og 3068e74e5c fix: make the update checker visible -- persistent banner + a menu item
CI / Lint (ruff) (pull_request) Successful in 7s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 10s
CI / Catalog signature (pull_request) Successful in 6s
CI / Tests (py3.12 / windows-latest) (pull_request) Has been cancelled
Reported from the field: running v1.2 against a repo with v1.3.0 published
gave no prompt, and there appeared to be no way to check manually. The
checker itself works; it was invisible, for two reasons.

#78 -- the notice was written to the shared status label, which 21 other
call sites rewrite. The check runs off-thread and lands a second or two
after launch, right as the user starts clicking, so the next selection or
refresh wiped it. Exactly the bug fixed for the MSIX warning in #35, which
got a persistent banner; that fix was never carried to the update notice.

Adds NoticeBanner: a persistent, dismissible notice carrying its own action
button. It's a shared widget rather than a second bespoke banner, so the
next thing needing the user's attention doesn't reach for the status bar
again. (The MSIX banner still uses its own QLabel -- migrating it is a
follow-up, deliberately not bundled with a bug fix.)

#79 -- the only 'Check for updates' affordance was a button inside the
About dialog, which is not where anyone looks. Worse, the About action was
created without a menu role, and Qt auto-assigns AboutRole to actions whose
text begins with 'About', relocating it into the macOS application menu --
so the notice's own hint, 'Help > About to view it', pointed at a menu that
on macOS doesn't contain the item.

Help now has its own 'Check for updates...' item with an explicit
ApplicationSpecificRole, and the About action states its AboutRole rather
than inheriting it invisibly. The menu-driven check is never throttled and
always reports back -- the user asked, so silence would read as broken.

The decision and the wording live in core.update_notice() because the test
suite has no PySide6 (CI installs pytest + cryptography only), so anything
in bcc.py is untestable. A test asserts the notice text names no menu path,
which is what went stale here in the first place.

Closes #78
Closes #79
2026-07-20 12:29:37 -04:00
the_og da20eb2fdb fix: tolerate non-object server values on load; keep named sets across a merge
CI / Lint (ruff) (pull_request) Successful in 13s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 12s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 12s
CI / Catalog signature (pull_request) Successful in 8s
CI / Tests (py3.12 / windows-latest) (pull_request) Has been cancelled
Two silent-failure bugs found auditing the v1.3.0 features.

#72 -- extract_servers called dict() on every server value, so a config
that was valid JSON but held a non-object server ("foo": "oops", a
number, a list, null) raised on load. The strict parse succeeded, so the
repair path never saw it, and the call sat outside the load try/except:
an unhandled traceback with the window half-swapped to the new profile.
It also made the #54 schema lint unreachable for the most likely
hand-edit mistake -- the load died before the linter ran.

Malformed values are now preserved verbatim on ServerEntry.raw (behind a
NO_RAW sentinel, since a literal JSON null is itself a malformed entry
worth keeping) and written back untouched on Save, so nothing is silently
deleted. lint_servers names the offending entry instead.

#73 -- the stale-file "Merge & save" path reloaded the file from disk and
re-applied the user's servers, but apply_servers only writes mcpServers
and _disabledMcpServers. Named server sets live under _bccServerSets in
the same file, so a set saved that session was dropped from disk and then
from memory, with no warning, on the path the user picks because it
sounds like the safe one.

BCC-owned keys are now declared in BCC_OWNED_KEYS and carried across by
carry_owned_keys, which reports genuinely contested keys so the status
line can say so. Deliberately one-directional: a key absent locally is
left alone on disk, because 'user deleted their last set' and 'another
machine just added sets' are indistinguishable and deleting someone
else's data is the worse failure.

Closes #72
Closes #73
2026-07-20 12:11:49 -04:00
3 changed files with 480 additions and 14 deletions
+136 -8
View File
@@ -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
View File
@@ -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
+194
View File
@@ -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"