diff --git a/bcc.py b/bcc.py index 0aa7239..42b4bc5 100644 --- a/bcc.py +++ b/bcc.py @@ -926,6 +926,80 @@ class RestoreDialog(QDialog): return item.data(Qt.ItemDataRole.UserRole) +# --------------------------------------------------------------------------- # +# Stale-file conflict dialog +# Shown when the config file was modified on disk between load and save. +# --------------------------------------------------------------------------- # +class StaleDialog(QDialog): + MERGE = 0 + OVERWRITE = 1 + + def __init__( + self, + parent, + profile_path: str, + changed_keys: list[str], + server_diff: str, + ): + super().__init__(parent) + self.setWindowTitle("Config changed on disk") + self.resize(680, 420) + self._choice = self.MERGE + + v = QVBoxLayout(self) + v.setSpacing(8) + + intro = QLabel(f"{profile_path} was modified on disk since you loaded it.") + intro.setWordWrap(True) + v.addWidget(intro) + + if changed_keys: + kl = QLabel("Changed keys: " + ", ".join(f"{k}" for k in changed_keys)) + kl.setObjectName("muted") + kl.setWordWrap(True) + v.addWidget(kl) + + if server_diff: + v.addWidget(QLabel("Server section changes (on disk vs loaded):")) + dv = QPlainTextEdit() + dv.setObjectName("diag") + dv.setReadOnly(True) + dv.setPlainText(server_diff) + v.addWidget(dv, 1) + else: + note = QLabel("(Server sections unchanged — the external edit is in other keys.)") + note.setObjectName("muted") + v.addWidget(note) + + desc = QLabel( + "Merge & save applies your server edits on top of the current file " + "on disk — both sets of changes are kept. " + "Overwrite saves your in-memory state, discarding external changes." + ) + desc.setObjectName("muted") + desc.setWordWrap(True) + v.addWidget(desc) + + btns = QDialogButtonBox() + merge_btn = btns.addButton("Merge && save", QDialogButtonBox.ButtonRole.AcceptRole) + merge_btn.setObjectName("primary") + overwrite_btn = btns.addButton( + "Overwrite anyway", QDialogButtonBox.ButtonRole.DestructiveRole + ) + btns.addButton(QDialogButtonBox.StandardButton.Cancel) + merge_btn.clicked.connect(lambda: self._choose(self.MERGE)) + overwrite_btn.clicked.connect(lambda: self._choose(self.OVERWRITE)) + btns.rejected.connect(self.reject) + v.addWidget(btns) + + def _choose(self, choice: int) -> None: + self._choice = choice + self.accept() + + def choice(self) -> int: + return self._choice + + # --------------------------------------------------------------------------- # # Paste-JSON dialog # --------------------------------------------------------------------------- # @@ -1016,6 +1090,7 @@ class MainWindow(QMainWindow): self.full_config: dict = {} self.servers: list[core.ServerEntry] = [] self.current_profile: core.Profile | None = None + self._loaded_mtime: float | None = None self.dirty = False self._suppress_table = False self._suppress_sel = False @@ -1291,6 +1366,7 @@ class MainWindow(QMainWindow): return self.full_config = cfg repaired = True + self._loaded_mtime = core.config_mtime(profile.path) self.current_profile = profile self.servers = core.extract_servers(self.full_config) self.dirty = False @@ -1627,12 +1703,52 @@ class MainWindow(QMainWindow): if not self._validate(): QMessageBox.warning(self, "Can't save yet", "Fix the highlighted problem first.") return + + # Stale-file check: if the file changed on disk since we loaded it, prompt. + disk_mtime = core.config_mtime(self.current_profile.path) + if ( + disk_mtime is not None + and self._loaded_mtime is not None + and disk_mtime != self._loaded_mtime + ): + changed_keys, server_diff = core.external_change_summary( + self.full_config, self.current_profile.path + ) + dlg = StaleDialog(self, str(self.current_profile.path), changed_keys, server_diff) + if not dlg.exec(): + return # user cancelled + if dlg.choice() == StaleDialog.MERGE: + try: + fresh = core.load_config(self.current_profile.path) + except Exception as e: + QMessageBox.critical(self, "Reload failed", str(e)) + return + core.apply_servers(fresh, self.servers) + try: + backup = core.write_config(self.current_profile.path, fresh) + except Exception as e: + QMessageBox.critical(self, "Save failed", str(e)) + return + self.full_config = fresh + self._loaded_mtime = core.config_mtime(self.current_profile.path) + self.current_profile.config_exists = True + self.dirty = False + self.save_btn.setEnabled(False) + bnote = f" · backup: {backup.name}" if backup else " · (new file)" + self.status.setText( + f"Merged & saved {self.current_profile.path}{bnote}" + f" · Restart {self.current_profile.label} to apply." + ) + return + # else OVERWRITE: fall through to normal write + core.apply_servers(self.full_config, self.servers) try: backup = core.write_config(self.current_profile.path, self.full_config) except Exception as e: QMessageBox.critical(self, "Save failed", str(e)) return + self._loaded_mtime = core.config_mtime(self.current_profile.path) self.current_profile.config_exists = True self.dirty = False self.save_btn.setEnabled(False) diff --git a/bcc_core.py b/bcc_core.py index 3f2702b..f2a8f61 100644 --- a/bcc_core.py +++ b/bcc_core.py @@ -290,6 +290,14 @@ def _redact_servers_block(block: dict | None) -> dict: return {name: _redact_server_data(data) for name, data in block.items()} +def _server_sections(cfg: dict) -> dict: + """Return the masked server sections of a config dict, safe for diff display.""" + 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 + + def backup_diff( config_path: Path | str, backup_path: Path | str, @@ -318,12 +326,6 @@ def backup_diff( after_cfg = dict(current_cfg) apply_servers(after_cfg, backup_servers) - 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) @@ -370,6 +372,46 @@ def restore_backup( return write_config(p, cfg) +def config_mtime(path: Path | str) -> float | None: + """Return the file's mtime, or None if the file does not exist.""" + try: + return Path(path).stat().st_mtime + except OSError: + return None + + +def external_change_summary(original_cfg: dict, path: Path | str) -> tuple[list[str], str]: + """ + Compare original_cfg (what BCC loaded) with the current on-disk state. + + Returns: + changed_keys — sorted list of top-level keys whose values differ. + server_diff — masked unified diff of server sections (empty if unchanged + or the file cannot be read). + """ + try: + disk_cfg = load_config(Path(path)) + except Exception: + return [], "" + + all_keys = set(original_cfg) | set(disk_cfg) + changed_keys = sorted(k for k in all_keys if original_cfg.get(k) != disk_cfg.get(k)) + + server_diff = "" + if any(k in {"mcpServers", DISABLED_KEY} for k in changed_keys): + before_lines = ( + json.dumps(_server_sections(original_cfg), indent=2, ensure_ascii=False) + "\n" + ).splitlines(keepends=True) + after_lines = ( + json.dumps(_server_sections(disk_cfg), indent=2, ensure_ascii=False) + "\n" + ).splitlines(keepends=True) + server_diff = "".join( + difflib.unified_diff(before_lines, after_lines, fromfile="loaded", tofile="on disk now") + ) + + return changed_keys, server_diff + + # --------------------------------------------------------------------------- # # Paste / import parsing # --------------------------------------------------------------------------- # diff --git a/tests/test_core.py b/tests/test_core.py index b3b7e77..303b8bf 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -500,3 +500,86 @@ def test_restore_backup_restores_disabled_servers(tmp_path): assert "parked" in restored.get(c.DISABLED_KEY, {}) assert "active" in restored.get("mcpServers", {}) assert "newcomer" not in restored.get("mcpServers", {}) + + +# --------------------------------------------------------------------------- # +# 9 — Stale-file protection +# --------------------------------------------------------------------------- # + + +def test_config_mtime_returns_float_for_existing_file(tmp_path): + f = tmp_path / "cfg.json" + f.write_text("{}") + assert isinstance(c.config_mtime(f), float) + + +def test_config_mtime_returns_none_for_missing_file(tmp_path): + assert c.config_mtime(tmp_path / "nonexistent.json") is None + + +def test_external_change_summary_detects_non_server_key_change(tmp_path): + cfgpath = tmp_path / "cfg.json" + original = {"numStartups": 1, "mcpServers": {"s": {"command": "node"}}} + cfgpath.write_text(json.dumps({"numStartups": 2, "mcpServers": {"s": {"command": "node"}}})) + changed_keys, server_diff = c.external_change_summary(original, cfgpath) + assert "numStartups" in changed_keys + assert server_diff == "" # server sections unchanged + + +def test_external_change_summary_detects_server_section_change(tmp_path): + cfgpath = tmp_path / "cfg.json" + original = {"mcpServers": {"s1": {"command": "node"}}} + cfgpath.write_text(json.dumps({"mcpServers": {"s2": {"command": "python"}}})) + changed_keys, server_diff = c.external_change_summary(original, cfgpath) + assert "mcpServers" in changed_keys + assert "s1" in server_diff or "s2" in server_diff + + +def test_external_change_summary_server_diff_masks_secrets(tmp_path): + cfgpath = tmp_path / "cfg.json" + original = {"mcpServers": {"a": {"command": "node", "env": {"SECRET_KEY": "s3cr3t"}}}} + # command changes (visible in diff) AND secret value changes (both sides masked → + # MASK appears in context lines, raw secret never appears) + cfgpath.write_text( + json.dumps( + {"mcpServers": {"a": {"command": "python", "env": {"SECRET_KEY": "n3w_s3cr3t"}}}} + ) + ) + changed_keys, server_diff = c.external_change_summary(original, cfgpath) + assert "mcpServers" in changed_keys + assert "s3cr3t" not in server_diff + assert "n3w_s3cr3t" not in server_diff + assert c.MASK in server_diff # appears in context for the masked env value + + +def test_external_change_summary_empty_when_unchanged(tmp_path): + cfgpath = tmp_path / "cfg.json" + cfg = {"mcpServers": {"s": {"command": "node"}}} + cfgpath.write_text(json.dumps(cfg)) + changed_keys, server_diff = c.external_change_summary(cfg, cfgpath) + assert changed_keys == [] + assert server_diff == "" + + +def test_merge_and_reapply_preserves_both_edits(tmp_path): + """AC test: external non-server change + in-memory server edit both survive merge.""" + cfgpath = tmp_path / "cfg.json" + original = {"numStartups": 1, "mcpServers": {"s1": {"command": "node"}}} + cfgpath.write_text(json.dumps(original, indent=2)) + + user_servers = [c.ServerEntry("s2", {"command": "python"}, True)] + + # External process bumps numStartups without touching servers + disk_changed = dict(original) + disk_changed["numStartups"] = 42 + cfgpath.write_text(json.dumps(disk_changed, indent=2)) + + # Merge: load fresh from disk, apply user's server edits, write back + fresh = c.load_config(cfgpath) + c.apply_servers(fresh, user_servers) + c.write_config(cfgpath, fresh) + + result = json.loads(cfgpath.read_text()) + assert result["numStartups"] == 42 # external change preserved + assert "s2" in result["mcpServers"] # user's server edit preserved + assert "s1" not in result["mcpServers"] # old server replaced