fix: stop save_options truncating config.json #10

Merged

mysticalsoap merged 1 commit from fix/config-save-crash into trunk

2026-08-17 14:30:02 -04:00

mysticalsoap commented

2026-08-17 14:25:47 -04:00

Owner

Problem

Saving from the options or scripts tab replaced config.json with only the
keys the caller happened to pass, then crashed on the first key that was no
longer there.

cfgupdate replaces config.json wholesale, so save_options merged its
argument onto self.config_dict first. Nothing ever assigns config_dict --
it is the empty class attribute throughout -- so the merge was a no-op and the
caller's partial dict became the entire file.

save_scripts passes a single key, so setting up a provider manually reduced
the config to {"_scripts": {}} (the provider combobox is empty at that point,
hence the empty prefix). save_options then read temp_config["ping"] and
raised KeyError, which PyQt5 turns into a process abort.

Fix

Merge onto config.settings, which is what the no-op merge was reaching for:
load_config already backfills it from default_settings, so it is always
complete.

Callers keep passing partial dicts, which read_option_change needs anyway --
a tristate checkbox reports neither 0 nor 2 and is deliberately omitted so it
keeps its stored value.

Verification

Reproduced the original crash on a fresh install: config.json went from 311
bytes to 16 ({"_scripts": {}}) before the abort, confirming the write at
line 1350 lands ahead of the KeyError at 1365.
Traced config_dict across the whole GUI module: one assignment (the empty
class attribute) and one read (the merge). Nothing else touches it.
Confirmed load_config backfills every key in default_settings, so the new
merge base cannot itself be partial.

Not covered by an automated test: save_options is a method on the main Qt
window and testing it would mean standing up the whole GUI. The behaviour is
exercised by the manual retest of provider setup.

Dropped a debug print(self.config_list) left in read_option_change.
update_cmd in the CalledProcessError handler is undefined and would raise
NameError. The handler is unreachable -- dbus_call swallows
DBusException and never raises CalledProcessError -- so this was latent,
but a dangling name in an error path is worth removing while here.

## Problem Saving from the options or scripts tab replaced `config.json` with only the keys the caller happened to pass, then crashed on the first key that was no longer there. `cfgupdate` replaces `config.json` wholesale, so `save_options` merged its argument onto `self.config_dict` first. Nothing ever assigns `config_dict` -- it is the empty class attribute throughout -- so the merge was a no-op and the caller's partial dict became the entire file. `save_scripts` passes a single key, so setting up a provider manually reduced the config to `{"_scripts": {}}` (the provider combobox is empty at that point, hence the empty prefix). `save_options` then read `temp_config["ping"]` and raised `KeyError`, which PyQt5 turns into a process abort. ## Fix Merge onto `config.settings`, which is what the no-op merge was reaching for: `load_config` already backfills it from `default_settings`, so it is always complete. Callers keep passing partial dicts, which `read_option_change` needs anyway -- a tristate checkbox reports neither 0 nor 2 and is deliberately omitted so it keeps its stored value. ## Verification - Reproduced the original crash on a fresh install: `config.json` went from 311 bytes to 16 (`{"_scripts": {}}`) *before* the abort, confirming the write at line 1350 lands ahead of the `KeyError` at 1365. - Traced `config_dict` across the whole GUI module: one assignment (the empty class attribute) and one read (the merge). Nothing else touches it. - Confirmed `load_config` backfills every key in `default_settings`, so the new merge base cannot itself be partial. Not covered by an automated test: `save_options` is a method on the main Qt window and testing it would mean standing up the whole GUI. The behaviour is exercised by the manual retest of provider setup. ## Related fixes (found along the way, technically out of scope) - Dropped a debug `print(self.config_list)` left in `read_option_change`. - `update_cmd` in the `CalledProcessError` handler is undefined and would raise `NameError`. The handler is unreachable -- `dbus_call` swallows `DBusException` and never raises `CalledProcessError` -- so this was latent, but a dangling name in an error path is worth removing while here.

mysticalsoap added 1 commit

2026-08-17 14:25:47 -04:00

fix: stop save_options truncating config.json 2247cec2ed

Saving anything from the options or scripts tab replaced config.json with
just the handful of keys the caller happened to touch, then crashed on the
first key that was no longer there.

cfgupdate replaces config.json wholesale, so save_options merged its
argument onto self.config_dict first. Nothing ever assigns config_dict --
it is the empty class attribute throughout -- so the merge was a no-op and
whatever the caller passed became the entire file. save_scripts passes a
single key, so setting up a provider manually reduced config.json to
{"_scripts": {}}. save_options then read temp_config["ping"] and raised
KeyError, which PyQt5 turns into an abort.

Merging onto config.settings instead is what the no-op merge was reaching
for: load_config already backfills it from default_settings, so it is
always complete. Callers keep passing partial dicts, which is what
read_option_change needs anyway -- a tristate checkbox reports neither 0
nor 2 and is deliberately left out so it keeps its stored value.

Also drops a debug print of config_list and an undefined name in the
CalledProcessError handler, which would have raised NameError had that
handler been reachable -- dbus_call swallows DBusException and never
raises CalledProcessError.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

mysticalsoap merged commit ae52b998f4 into trunk

2026-08-17 14:30:02 -04:00

mysticalsoap deleted branch fix/config-save-crash

2026-08-17 14:30:02 -04:00

mysticalsoap referenced this pull request

2026-08-17 16:31:36 -04:00

change: name what the Provider tab's Save button saves #16