Stale-file Merge path silently discards named server sets (#52) #73

Closed
opened 2026-07-20 12:07:09 -04:00 by the_og · 0 comments
Owner

What

In the stale-file Merge & save branch (bcc.py ~L2475):

fresh = core.load_config(self.current_profile.path)
core.apply_servers(fresh, self.servers)
...
self.full_config = fresh

apply_servers() only writes mcpServers and _disabledMcpServers. Named server sets live in self.full_config under SETS_KEY (_bccServerSets), written there by save_server_set(self.full_config, ...).

So: user saves a named set → an external process touches the config → user hits Save → Merge → the set is never carried into fresh, and then self.full_config = fresh drops it from memory too. No warning, no backup prompt, gone.

Severity

Same class as the drag-and-drop silent-overwrite fixed in #8: quiet data loss on a path the user explicitly chose because it sounded safe. "Merge" implies keeping the user's work.

Note external_change_summary does compare all top-level keys, so _bccServerSets shows up in changed_keys when the disk copy differs — but that reports it as an external change; it doesn't stop the local one being dropped.

Suggested fix

Decide the merge semantics for BCC-owned keys explicitly, rather than by omission:

  • Preferred: carry BCC-owned keys (SETS_KEY, and anything else BCC authors) from self.full_config onto fresh before writing — BCC is the owner of those keys, so local wins.
  • If disk also changed SETS_KEY, that's a genuine conflict worth surfacing in StaleDialog rather than silently picking a side.
  • Consider making this generic: a BCC_OWNED_KEYS tuple consulted by the merge, so the next BCC-authored key doesn't reintroduce the same bug.

Tests

Config with _bccServerSets in memory + an external write to disk + Merge → the set survives in the written file. Add the symmetric case where disk changed the sets too.

## What In the stale-file **Merge & save** branch (bcc.py ~L2475): ```python fresh = core.load_config(self.current_profile.path) core.apply_servers(fresh, self.servers) ... self.full_config = fresh ``` `apply_servers()` only writes `mcpServers` and `_disabledMcpServers`. Named server sets live in `self.full_config` under `SETS_KEY` (`_bccServerSets`), written there by `save_server_set(self.full_config, ...)`. So: user saves a named set → an external process touches the config → user hits Save → Merge → the set is **never carried into `fresh`**, and then `self.full_config = fresh` drops it from memory too. No warning, no backup prompt, gone. ## Severity Same class as the drag-and-drop silent-overwrite fixed in #8: quiet data loss on a path the user explicitly chose because it sounded safe. "Merge" implies keeping the user's work. Note `external_change_summary` *does* compare all top-level keys, so `_bccServerSets` shows up in `changed_keys` when the disk copy differs — but that reports it as an external change; it doesn't stop the local one being dropped. ## Suggested fix Decide the merge semantics for BCC-owned keys explicitly, rather than by omission: - Preferred: carry BCC-owned keys (`SETS_KEY`, and anything else BCC authors) from `self.full_config` onto `fresh` before writing — BCC is the owner of those keys, so local wins. - If disk *also* changed `SETS_KEY`, that's a genuine conflict worth surfacing in `StaleDialog` rather than silently picking a side. - Consider making this generic: a `BCC_OWNED_KEYS` tuple consulted by the merge, so the next BCC-authored key doesn't reintroduce the same bug. ## Tests Config with `_bccServerSets` in memory + an external write to disk + Merge → the set survives in the written file. Add the symmetric case where disk changed the sets too.
the_og added the P1 label 2026-07-20 12:07:09 -04:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: the_og/better-claude-config#73