From df332a06ba4cdd7e440bb7f8223b563fd033a2b1 Mon Sep 17 00:00:00 2001 From: AJ Date: Thu, 2 Jul 2026 01:22:52 -0400 Subject: [PATCH 1/2] feat: backup restore UI (#3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add list_backups / backup_label / backup_diff / restore_backup to bcc_core, RestoreDialog to bcc.py, and a "Restore…" button in the profile top bar. Restore is selective: only mcpServers and _disabledMcpServers are replaced; all other keys in the config (history, project state) are preserved verbatim. Goes through write_config() so a pre-restore backup is always created first. Closes #3 Co-Authored-By: Claude Sonnet 4.6 --- bcc.py | 119 +++++++++++++++++++++++++++++++++++++++++++++ bcc_core.py | 97 ++++++++++++++++++++++++++++++++++++ tests/test_core.py | 115 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 331 insertions(+) diff --git a/bcc.py b/bcc.py index 187767d..3155f2b 100644 --- a/bcc.py +++ b/bcc.py @@ -28,6 +28,8 @@ from PySide6.QtWidgets import ( QHeaderView, QLabel, QLineEdit, + QListWidget, + QListWidgetItem, QMainWindow, QMenu, QMessageBox, @@ -833,6 +835,97 @@ class RepairDialog(QDialog): v.addWidget(btns) +# --------------------------------------------------------------------------- # +# Backup restore dialog: lists available backups, shows a diff of what will +# change, and confirms before writing. Only servers are restored — all other +# keys in the current config are preserved verbatim. +# --------------------------------------------------------------------------- # +class RestoreDialog(QDialog): + def __init__(self, parent, profile: core.Profile, full_config: dict): + super().__init__(parent) + self.setWindowTitle(f"Restore from backup — {profile.label}") + self.resize(700, 480) + self._profile = profile + self._full_config = full_config + + v = QVBoxLayout(self) + v.setSpacing(8) + + intro = QLabel( + "Select a backup to preview the change. Only mcpServers and " + "_disabledMcpServers are restored — all other keys in your config are preserved." + ) + intro.setWordWrap(True) + v.addWidget(intro) + + splitter = QSplitter(Qt.Orientation.Horizontal) + + # --- left: backup list --- + left = QWidget() + lv = QVBoxLayout(left) + lv.setContentsMargins(0, 0, 0, 0) + lv.addWidget(QLabel("Backups (newest first):")) + self.backup_list = QListWidget() + self.backup_list.currentItemChanged.connect(self._on_backup_selected) + lv.addWidget(self.backup_list, 1) + splitter.addWidget(left) + + # --- right: diff preview --- + right = QWidget() + rv = QVBoxLayout(right) + rv.setContentsMargins(0, 0, 0, 0) + rv.addWidget(QLabel("Preview (what will change):")) + self.diff_view = QPlainTextEdit() + self.diff_view.setObjectName("diag") + self.diff_view.setReadOnly(True) + self.diff_view.setPlaceholderText("Select a backup on the left to see the diff.") + rv.addWidget(self.diff_view, 1) + splitter.addWidget(right) + + splitter.setSizes([220, 460]) + v.addWidget(splitter, 1) + + self.btns = QDialogButtonBox() + self.restore_btn = self.btns.addButton("Restore", QDialogButtonBox.ButtonRole.AcceptRole) + self.restore_btn.setObjectName("primary") + self.restore_btn.setEnabled(False) + self.btns.addButton(QDialogButtonBox.StandardButton.Cancel) + self.btns.accepted.connect(self.accept) + self.btns.rejected.connect(self.reject) + v.addWidget(self.btns) + + self._populate() + + def _populate(self): + backups = core.list_backups(self._profile.path) + if not backups: + self.backup_list.addItem("(no backups found)") + return + for bp in backups: + item = QListWidgetItem(core.backup_label(bp)) + item.setData(Qt.ItemDataRole.UserRole, str(bp)) + self.backup_list.addItem(item) + + def _on_backup_selected(self, current: QListWidgetItem | None, _prev): + if current is None or current.data(Qt.ItemDataRole.UserRole) is None: + self.diff_view.setPlainText("") + self.restore_btn.setEnabled(False) + return + bp = current.data(Qt.ItemDataRole.UserRole) + try: + diff = core.backup_diff(self._profile.path, bp, self._full_config) + except Exception as e: + diff = f"(error generating diff: {e})" + self.diff_view.setPlainText(diff) + self.restore_btn.setEnabled(True) + + def selected_backup(self) -> str | None: + item = self.backup_list.currentItem() + if item is None: + return None + return item.data(Qt.ItemDataRole.UserRole) + + # --------------------------------------------------------------------------- # # Paste-JSON dialog # --------------------------------------------------------------------------- # @@ -992,6 +1085,10 @@ class MainWindow(QMainWindow): reload_btn = QPushButton("Reload") reload_btn.clicked.connect(lambda: self.load_profile(self.current_profile, confirm=True)) bar.addWidget(reload_btn) + self.restore_btn = QPushButton("Restore…") + self.restore_btn.setToolTip("Restore servers from a previous backup") + self.restore_btn.clicked.connect(self._restore_from_backup) + bar.addWidget(self.restore_btn) bar.addStretch() self.save_btn = QPushButton("Save") self.save_btn.setObjectName("primary") @@ -1544,6 +1641,28 @@ class MainWindow(QMainWindow): f"Saved {self.current_profile.path}{bnote} · Restart {self.current_profile.label} to apply." ) + def _restore_from_backup(self): + if not self.current_profile: + return + if self.dirty and not self._confirm_discard(): + return + dlg = RestoreDialog(self, self.current_profile, self.full_config) + if not dlg.exec(): + return + bp = dlg.selected_backup() + if not bp: + return + try: + core.restore_backup(self.current_profile.path, bp, current_cfg=self.full_config) + except Exception as e: + QMessageBox.critical(self, "Restore failed", str(e)) + return + # Reload from disk so the UI reflects the restored state + self.load_profile(self.current_profile, confirm=False) + self.status.setText( + f"Restored from {Path(bp).name} · Restart {self.current_profile.label} to apply." + ) + # --- dirty / status -------------------------------------------------- # def _mark_dirty(self): self.dirty = True diff --git a/bcc_core.py b/bcc_core.py index 927681b..f32621f 100644 --- a/bcc_core.py +++ b/bcc_core.py @@ -14,6 +14,7 @@ in its original position. from __future__ import annotations import contextlib +import difflib import functools import glob import json @@ -248,6 +249,102 @@ def write_config(path: str | os.PathLike, cfg: dict) -> Path | None: return backup +# --------------------------------------------------------------------------- # +# Backup utility functions +# --------------------------------------------------------------------------- # +_BACKUP_TS_RE = re.compile(r"(\d{8}-\d{6})") + + +def list_backups(config_path: Path | str) -> list[Path]: + """Return backup files for config_path, newest-first. Returns [] if none exist.""" + p = Path(config_path) + bdir = p.parent / BACKUP_DIRNAME + if not bdir.is_dir(): + return [] + return sorted(bdir.glob(f"{p.stem}.*.json"), reverse=True) + + +def backup_label(backup_path: Path | str) -> str: + """Human-readable label derived from the backup filename timestamp.""" + m = _BACKUP_TS_RE.search(Path(backup_path).name) + if not m: + return Path(backup_path).name + ts = m.group(1) # e.g. "20260702-004124" + return f"{ts[:4]}-{ts[4:6]}-{ts[6:8]} {ts[9:11]}:{ts[11:13]}:{ts[13:]}" + + +def backup_diff( + config_path: Path | str, + backup_path: Path | str, + current_cfg: dict | None = None, +) -> str: + """ + Unified diff showing what the current config would look like after restoring + the backup (servers only — non-server keys are preserved). + + fromfile = current state, tofile = what will be written after restore. + Pass current_cfg to use an already-loaded (possibly repaired) in-memory config + as the baseline instead of re-reading from disk. + """ + p = Path(config_path) + bp = Path(backup_path) + + if current_cfg is None: + try: + current_cfg = load_config(p) + except Exception: + current_cfg = {} + + backup_servers = extract_servers(load_config(bp)) + after_cfg = dict(current_cfg) + apply_servers(after_cfg, backup_servers) + + before_lines = (json.dumps(current_cfg, indent=2, ensure_ascii=False) + "\n").splitlines( + keepends=True + ) + after_lines = (json.dumps(after_cfg, indent=2, ensure_ascii=False) + "\n").splitlines( + keepends=True + ) + + result = "".join( + difflib.unified_diff( + before_lines, + after_lines, + fromfile=f"current: {p.name}", + tofile=f"restore: {bp.name}", + ) + ) + return result or "(no differences — backup matches current servers)" + + +def restore_backup( + config_path: Path | str, + backup_path: Path | str, + current_cfg: dict | None = None, +) -> Path | None: + """ + Restore server entries from backup_path into config_path. + + Only mcpServers and _disabledMcpServers are replaced; all other keys in the + current config (conversation history, project state, etc.) are preserved + verbatim and in their original order. + + Goes through write_config() so a pre-restore backup of the current file is + created automatically. Returns that backup path (or None if no prior file). + """ + p = Path(config_path) + if current_cfg is None: + try: + current_cfg = load_config(p) + except Exception: + current_cfg = {} + + backup_servers = extract_servers(load_config(Path(backup_path))) + cfg = dict(current_cfg) + apply_servers(cfg, backup_servers) + return write_config(p, cfg) + + # --------------------------------------------------------------------------- # # Paste / import parsing # --------------------------------------------------------------------------- # diff --git a/tests/test_core.py b/tests/test_core.py index 253f1fd..5a8c442 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -318,3 +318,118 @@ def test_spawn_test_large_stderr_does_not_deadlock(): assert r["returncode"] == 3 # stderr is capped, not the full 64 KB assert len(r["stderr"]) <= c._STDERR_CAP + + +# --------------------------------------------------------------------------- # +# 8. Backup restore +# --------------------------------------------------------------------------- # +@pytest.fixture +def config_with_backup(tmp_path): + """A config file with two backups already present.""" + cfgpath = tmp_path / "claude_desktop_config.json" + original = { + "globalShortcut": "Cmd+Shift+Space", + "mcpServers": {"server-v1": {"command": "node", "args": ["v1.js"]}}, + } + cfgpath.write_text(json.dumps(original, indent=2)) + + # Create first backup (v1 state) by writing a new version + cfg = c.load_config(cfgpath) + servers = c.extract_servers(cfg) + servers[0] = c.ServerEntry("server-v2", {"command": "node", "args": ["v2.js"]}, True) + c.apply_servers(cfg, servers) + c.write_config(cfgpath, cfg) + + return cfgpath + + +def test_list_backups_returns_newest_first(config_with_backup): + backups = c.list_backups(config_with_backup) + assert len(backups) >= 1 + # Sorted newest-first means descending lexicographic order on timestamps + names = [b.name for b in backups] + assert names == sorted(names, reverse=True) + + +def test_list_backups_empty_when_no_bdir(tmp_path): + cfgpath = tmp_path / "claude_desktop_config.json" + cfgpath.write_text("{}") + assert c.list_backups(cfgpath) == [] + + +def test_backup_label_parses_timestamp(config_with_backup): + backup = c.list_backups(config_with_backup)[0] + label = c.backup_label(backup) + # label should look like "2026-07-02 00:41:24" (date + time, separated by two spaces) + assert "-" in label and ":" in label + assert len(label) == 20 # "YYYY-MM-DD HH:MM:SS" = 10 + 2 + 8 + + +def test_backup_diff_shows_server_change(config_with_backup): + backup = c.list_backups(config_with_backup)[0] + diff = c.backup_diff(config_with_backup, backup) + # The diff should show the transition back to v1 servers + assert "server-v1" in diff or "server-v2" in diff + + +def test_backup_diff_no_diff_when_identical(tmp_path): + cfgpath = tmp_path / "cfg.json" + content = {"mcpServers": {"s": {"command": "node"}}} + cfgpath.write_text(json.dumps(content, indent=2)) + bdir = cfgpath.parent / c.BACKUP_DIRNAME + bdir.mkdir(exist_ok=True) + import time as _time + + stamp = _time.strftime("%Y%m%d-%H%M%S") + backup_path = bdir / f"cfg.{stamp}.json" + backup_path.write_text(json.dumps(content, indent=2)) + diff = c.backup_diff(cfgpath, backup_path) + assert "(no differences" in diff + + +def test_restore_backup_replaces_servers(config_with_backup): + # Get the v1 backup (only backup we have) + backups = c.list_backups(config_with_backup) + assert len(backups) >= 1 + v1_backup = backups[0] + + # Current file has server-v2; restore should bring back server-v1 + c.restore_backup(config_with_backup, v1_backup) + + restored = json.loads(config_with_backup.read_text()) + assert "server-v1" in restored.get("mcpServers", {}) + assert "server-v2" not in restored.get("mcpServers", {}) + + +def test_restore_preserves_non_server_keys(config_with_backup): + backups = c.list_backups(config_with_backup) + c.restore_backup(config_with_backup, backups[0]) + + restored = json.loads(config_with_backup.read_text()) + # Non-server key must survive + assert restored.get("globalShortcut") == "Cmd+Shift+Space" + + +def test_restore_creates_pre_restore_backup(config_with_backup): + before_count = len(c.list_backups(config_with_backup)) + backups = c.list_backups(config_with_backup) + c.restore_backup(config_with_backup, backups[0]) + after_count = len(c.list_backups(config_with_backup)) + # restore_backup goes through write_config which makes a backup first + assert after_count == before_count + 1 + + +def test_restore_with_current_cfg_passed(config_with_backup): + # When current_cfg is passed in (the GUI's in-memory config), it should be + # used instead of re-reading from disk. + backups = c.list_backups(config_with_backup) + # Simulate the GUI passing a repaired in-memory version + in_memory_cfg = { + "globalShortcut": "Cmd+Shift+Space", + "extraKey": "preserved", + "mcpServers": {"server-v2": {"command": "node", "args": ["v2.js"]}}, + } + c.restore_backup(config_with_backup, backups[0], current_cfg=in_memory_cfg) + restored = json.loads(config_with_backup.read_text()) + assert restored.get("extraKey") == "preserved" + assert "server-v1" in restored.get("mcpServers", {}) From 6ab4789a70454249e685555ece1a7322b76759e7 Mon Sep 17 00:00:00 2001 From: AJ Date: Thu, 2 Jul 2026 01:40:24 -0400 Subject: [PATCH 2/2] fix: scope backup_diff to server sections and mask secrets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The diff preview in RestoreDialog was serializing the full config dict, exposing secret env values and token args in cleartext on a pasteable surface. - Add _redact_server_data / _redact_servers_block helpers that apply redact_args to args and mask env values for is_secret_key() keys - Rewrite backup_diff to compare only {mcpServers, _disabledMcpServers} sections (sanitized), not the whole file — also avoids double-serializing multi-MB ~/.claude.json for a servers-only diff - Add clarifying comment in _restore_from_backup about why full_config is the right base after confirm-discard - Add test: backup_diff with secret args/env → MASK in output, raw values absent - Add test: restore_backup restores _disabledMcpServers correctly Co-Authored-By: Claude Sonnet 4.6 --- bcc.py | 2 ++ bcc_core.py | 45 ++++++++++++++++++++++++------- tests/test_core.py | 67 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 10 deletions(-) diff --git a/bcc.py b/bcc.py index 3155f2b..0aa7239 100644 --- a/bcc.py +++ b/bcc.py @@ -1646,6 +1646,8 @@ class MainWindow(QMainWindow): return if self.dirty and not self._confirm_discard(): return + # full_config is the right base: discarded edits were server changes; + # non-server keys from the in-memory state are what we want to preserve. dlg = RestoreDialog(self, self.current_profile, self.full_config) if not dlg.exec(): return diff --git a/bcc_core.py b/bcc_core.py index f32621f..3f2702b 100644 --- a/bcc_core.py +++ b/bcc_core.py @@ -273,18 +273,37 @@ def backup_label(backup_path: Path | str) -> str: return f"{ts[:4]}-{ts[4:6]}-{ts[6:8]} {ts[9:11]}:{ts[11:13]}:{ts[13:]}" +def _redact_server_data(data: dict) -> dict: + """Return a copy of a server definition with secrets masked for display.""" + out = dict(data) + if "args" in out: + out["args"] = redact_args(list(out["args"] or [])) + if "env" in out: + out["env"] = {k: (MASK if is_secret_key(k) else v) for k, v in (out["env"] or {}).items()} + return out + + +def _redact_servers_block(block: dict | None) -> dict: + """Return a sanitized copy of a mcpServers / _disabledMcpServers block.""" + if not block: + return {} + return {name: _redact_server_data(data) for name, data in block.items()} + + def backup_diff( config_path: Path | str, backup_path: Path | str, current_cfg: dict | None = None, ) -> str: """ - Unified diff showing what the current config would look like after restoring - the backup (servers only — non-server keys are preserved). + Unified diff showing what the server sections would look like after restoring + the backup. Only mcpServers and _disabledMcpServers are compared; non-server + keys are excluded entirely (they are preserved by restore, not changed). + Secret values in args and env are masked so the diff is safe to share. fromfile = current state, tofile = what will be written after restore. - Pass current_cfg to use an already-loaded (possibly repaired) in-memory config - as the baseline instead of re-reading from disk. + Pass current_cfg to diff against an already-loaded in-memory config instead + of re-reading from disk. """ p = Path(config_path) bp = Path(backup_path) @@ -299,12 +318,18 @@ def backup_diff( after_cfg = dict(current_cfg) apply_servers(after_cfg, backup_servers) - before_lines = (json.dumps(current_cfg, indent=2, ensure_ascii=False) + "\n").splitlines( - keepends=True - ) - after_lines = (json.dumps(after_cfg, indent=2, ensure_ascii=False) + "\n").splitlines( - keepends=True - ) + def _server_sections(cfg: dict) -> dict: + out: dict = {"mcpServers": _redact_servers_block(cfg.get("mcpServers"))} + if DISABLED_KEY in cfg: + out[DISABLED_KEY] = _redact_servers_block(cfg.get(DISABLED_KEY)) + return out + + before_lines = ( + json.dumps(_server_sections(current_cfg), indent=2, ensure_ascii=False) + "\n" + ).splitlines(keepends=True) + after_lines = ( + json.dumps(_server_sections(after_cfg), indent=2, ensure_ascii=False) + "\n" + ).splitlines(keepends=True) result = "".join( difflib.unified_diff( diff --git a/tests/test_core.py b/tests/test_core.py index 5a8c442..b3b7e77 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -433,3 +433,70 @@ def test_restore_with_current_cfg_passed(config_with_backup): restored = json.loads(config_with_backup.read_text()) assert restored.get("extraKey") == "preserved" assert "server-v1" in restored.get("mcpServers", {}) + + +def test_backup_diff_masks_secrets(tmp_path): + cfgpath = tmp_path / "cfg.json" + current = { + "mcpServers": { + "s": { + "command": "node", + "args": ["--token", "super-secret"], + "env": {"API_KEY": "my-api-key-value"}, + } + } + } + cfgpath.write_text(json.dumps(current, indent=2)) + bdir = cfgpath.parent / c.BACKUP_DIRNAME + bdir.mkdir() + import time as _t + + bp = bdir / f"cfg.{_t.strftime('%Y%m%d-%H%M%S')}.json" + bp.write_text( + json.dumps( + { + "mcpServers": { + "s2": { + "command": "python", + "args": ["--api-key", "another-secret"], + "env": {"SECRET_KEY": "backup-secret-value"}, + } + } + }, + indent=2, + ) + ) + diff = c.backup_diff(cfgpath, bp) + # Raw secret values must not appear in the diff + assert "super-secret" not in diff + assert "my-api-key-value" not in diff + assert "another-secret" not in diff + assert "backup-secret-value" not in diff + # The mask placeholder must appear (secrets were present in both sides) + assert c.MASK in diff + + +def test_restore_backup_restores_disabled_servers(tmp_path): + cfgpath = tmp_path / "cfg.json" + original = { + "mcpServers": {"active": {"command": "node"}}, + c.DISABLED_KEY: {"parked": {"command": "python"}}, + } + cfgpath.write_text(json.dumps(original, indent=2)) + + # Overwrite: add new server, drop the disabled one + cfg = c.load_config(cfgpath) + servers = c.extract_servers(cfg) + servers = [s for s in servers if s.name != "parked"] + servers.append(c.ServerEntry("newcomer", {"command": "uvx"}, True)) + c.apply_servers(cfg, servers) + c.write_config(cfgpath, cfg) + + # Restore from the v1 backup + backups = c.list_backups(cfgpath) + c.restore_backup(cfgpath, backups[0]) + restored = json.loads(cfgpath.read_text()) + + assert "parked" in restored.get(c.DISABLED_KEY, {}) + assert "active" in restored.get("mcpServers", {}) + assert "newcomer" not in restored.get("mcpServers", {})