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
This commit is contained in:
2026-07-20 12:11:49 -04:00
parent cd2ac2f6f8
commit da20eb2fdb
3 changed files with 283 additions and 10 deletions
+26 -4
View File
@@ -1939,9 +1939,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()
@@ -2264,7 +2276,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
@@ -2358,7 +2370,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))
@@ -2478,6 +2490,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)
@@ -2490,8 +2507,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()