diff --git a/bcc.py b/bcc.py index 37ad0e1..4b902b2 100644 --- a/bcc.py +++ b/bcc.py @@ -1104,7 +1104,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._loaded_stat: core.ConfigStat | None = None self.dirty = False self._suppress_table = False self._suppress_sel = False @@ -1380,7 +1380,7 @@ class MainWindow(QMainWindow): return self.full_config = cfg repaired = True - self._loaded_mtime = core.config_mtime(profile.path) + self._loaded_stat = core.config_fingerprint(profile.path) self.current_profile = profile self.servers = core.extract_servers(self.full_config) self.dirty = False @@ -1723,11 +1723,14 @@ class MainWindow(QMainWindow): return # Stale-file check: if the file changed on disk since we loaded it, prompt. - disk_mtime = core.config_mtime(self.current_profile.path) + # Compare mtime AND size (not mtime alone) so a concurrent external write + # that lands within the mtime resolution window, or that restores the + # original mtime, still gets caught. + disk_stat = core.config_fingerprint(self.current_profile.path) if ( - disk_mtime is not None - and self._loaded_mtime is not None - and disk_mtime != self._loaded_mtime + disk_stat is not None + and self._loaded_stat is not None + and disk_stat != self._loaded_stat ): changed_keys, server_diff = core.external_change_summary( self.full_config, self.current_profile.path @@ -1748,7 +1751,7 @@ class MainWindow(QMainWindow): QMessageBox.critical(self, "Save failed", str(e)) return self.full_config = fresh - self._loaded_mtime = core.config_mtime(self.current_profile.path) + self._loaded_stat = core.config_fingerprint(self.current_profile.path) self.current_profile.config_exists = True self.dirty = False self.save_btn.setEnabled(False) @@ -1766,7 +1769,7 @@ class MainWindow(QMainWindow): except Exception as e: QMessageBox.critical(self, "Save failed", str(e)) return - self._loaded_mtime = core.config_mtime(self.current_profile.path) + self._loaded_stat = core.config_fingerprint(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 1edc990..129eb80 100644 --- a/bcc_core.py +++ b/bcc_core.py @@ -29,6 +29,7 @@ import threading import time from dataclasses import dataclass from pathlib import Path +from typing import NamedTuple from urllib.parse import urlparse CONFIG_FILENAME = "claude_desktop_config.json" @@ -399,6 +400,30 @@ def config_mtime(path: Path | str) -> float | None: return None +class ConfigStat(NamedTuple): + """A snapshot of a config file's mtime + size. + + Pairing size with mtime hardens stale-file detection beyond bare mtime + equality: a concurrent external write can land within the filesystem's + mtime resolution (e.g. same-second writes on ext4/HFS+) or have its mtime + restored by the writing process, in which case mtime alone would miss the + change. Comparing both fields catches those cases without the cost of a + full content hash. + """ + + mtime: float + size: int + + +def config_fingerprint(path: Path | str) -> ConfigStat | None: + """Return the file's (mtime, size) snapshot, or None if it does not exist.""" + try: + st = Path(path).stat() + except OSError: + return None + return ConfigStat(st.st_mtime, st.st_size) + + 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. @@ -493,7 +518,7 @@ def _normalize_unicode(text: str, notes: list[str]) -> str: out = text for junk in _JUNK_CHARS: out = out.replace(junk, "") - out = out.replace(" ", " ") # non-breaking space + out = out.replace(chr(0xA0), " ") # non-breaking space (defensive: avoid a literal char here) for smart, ascii_q in _QUOTE_MAP.items(): out = out.replace(smart, ascii_q) if out != text: diff --git a/tests/test_core.py b/tests/test_core.py index 75a3352..eb830c2 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -2,6 +2,7 @@ proper test functions with tmp_path/monkeypatch fixtures).""" import json +import os import sys import pytest @@ -520,6 +521,42 @@ def test_config_mtime_returns_none_for_missing_file(tmp_path): assert c.config_mtime(tmp_path / "nonexistent.json") is None +def test_config_fingerprint_returns_mtime_and_size_for_existing_file(tmp_path): + f = tmp_path / "cfg.json" + f.write_text("{}") + fp = c.config_fingerprint(f) + assert isinstance(fp, c.ConfigStat) + assert isinstance(fp.mtime, float) + assert fp.size == f.stat().st_size + + +def test_config_fingerprint_returns_none_for_missing_file(tmp_path): + assert c.config_fingerprint(tmp_path / "nonexistent.json") is None + + +def test_config_fingerprint_detects_size_change_when_mtime_is_unchanged(tmp_path): + """ + Guards against the exact hole bare-mtime comparison has: an external write + that lands within the filesystem's mtime resolution (or that restores the + original mtime) must still be caught, because the file's size differs. + """ + f = tmp_path / "cfg.json" + f.write_text('{"mcpServers": {}}') + original = c.config_fingerprint(f) + original_mtime_ns = f.stat().st_mtime_ns + + # Overwrite with materially different (larger) content, then force the + # mtime back to its original value -- simulating a same-second external + # write, or a writer that preserves mtime. + f.write_text('{"mcpServers": {"new-server": {"command": "node", "args": ["a", "b", "c"]}}}') + os.utime(f, ns=(original_mtime_ns, original_mtime_ns)) + + updated = c.config_fingerprint(f) + assert updated.mtime == original.mtime # mtime alone would say "unchanged" + assert updated.size != original.size # size catches what mtime missed + assert updated != original # the fingerprint as a whole detects the change + + def test_external_change_summary_detects_non_server_key_change(tmp_path): cfgpath = tmp_path / "cfg.json" original = {"numStartups": 1, "mcpServers": {"s": {"command": "node"}}}