diff --git a/first_time_install.sh b/first_time_install.sh index 471f0516..1a25bb25 100644 --- a/first_time_install.sh +++ b/first_time_install.sh @@ -1759,6 +1759,21 @@ else mkdir -p /var/log/journal systemd-tmpfiles --create --prefix /var/log/journal >/dev/null 2>&1 || true systemctl restart systemd-journald >/dev/null 2>&1 || true + + # Drop-ins are applied in lexical order, so a locally added file that sorts + # after ledmatrix-persistent.conf (zz-local.conf and friends) still wins. + # Writing the file is not evidence it took effect -- re-read and say so + # plainly rather than reporting success we cannot confirm. + journald_now="$(journald_effective | grep -E '^[[:space:]]*Storage=' | tail -n1 | cut -d= -f2 | tr -d '[:space:]')" + if [ "$journald_now" = "persistent" ]; then + echo " Persistent journald storage active" + else + echo " WARNING: journald storage is still '${journald_now:-unset}' after" + echo " writing /etc/systemd/journald.conf.d/ledmatrix-persistent.conf." + echo " Another drop-in that sorts later is overriding it. Check:" + echo " systemd-analyze cat-config systemd/journald.conf | grep -n Storage=" + echo " Logs will not survive a reboot until that is resolved." + fi fi # Ensure dtparam=audio=off in config.txt (idempotent) diff --git a/src/plugin_system/plugin_health.py b/src/plugin_system/plugin_health.py index 39b5be3e..d9b4f2c5 100644 --- a/src/plugin_system/plugin_health.py +++ b/src/plugin_system/plugin_health.py @@ -139,9 +139,19 @@ class PluginHealthTracker: if field in cls._COUNTER_FIELDS: ok = isinstance(value, int) and not isinstance(value, bool) and value >= 0 elif field in cls._TIMESTAMP_FIELDS: - ok = value is None or isinstance(value, (int, float)) + # bool is a subclass of int, so True would pass as a timestamp + # and then compare as 1.0 -- expiring a cooldown the instant it + # opens, or (False) making the elapsed check never fire. + ok = value is None or ( + isinstance(value, (int, float)) and not isinstance(value, bool) + ) elif field == 'circuit_state': - ok = value in {member.value for member in CircuitState} + # Membership first requires the value to be hashable: a list or + # dict here would raise TypeError out of the repair itself, + # which is the crash this whole path exists to prevent. + ok = isinstance(value, str) and value in { + member.value for member in CircuitState + } else: # last_error ok = value is None or isinstance(value, str) if ok: diff --git a/test/test_plugin_health.py b/test/test_plugin_health.py index 4b1a7636..f418061c 100644 --- a/test/test_plugin_health.py +++ b/test/test_plugin_health.py @@ -161,7 +161,31 @@ def test_newer_fields_are_carried_through(): def test_recording_against_a_repaired_state_does_not_raise(): - """The actual failure: record_failure indexing a field that was not there.""" - tracker = _tracker_reading({"circuit_state": "closed"}) + """The actual failure: record_failure indexing a field that was not there. + + The seed deliberately omits circuit_state. Seeding a record that *has* it + would pass against the old raw-return behaviour too -- the counters are + read with .get(), so circuit_state is the only field whose absence used to + raise. + """ + tracker = _tracker_reading({"total_failures": 2, "total_successes": 1}) tracker.record_failure("p", Exception("boom")) tracker.record_success("p") + + +def test_unhashable_or_boolean_values_are_repaired(): + """Values that break the repair itself rather than a later caller. + + An unhashable circuit_state raises TypeError inside a set membership test, + and bool is a subclass of int, so True would pass as a timestamp and then + compare as 1.0 -- expiring a cooldown the moment it opens. + """ + for bad_state in ({"circuit_state": []}, {"circuit_state": {}}): + state = _tracker_reading(bad_state).get_health_state("p") + assert state["circuit_state"] == CircuitState.CLOSED.value + + state = _tracker_reading({ + "circuit_opened_time": True, "last_success_time": False, + }).get_health_state("p") + assert state["circuit_opened_time"] is None + assert state["last_success_time"] is None