feat(settings): drop sub-flag cascade-clear; restore advanced_debug_logging translation; clearer Save-button copy (homeassistant-ai#1164)

Three user-reported fixes from stable-addon testing:

1. Stable add-on's ``translations/en.yaml`` was missing the
   ``advanced_debug_logging`` description — the schema declares the
   toggle in ``config.yaml:49`` but no translation ever shipped, so
   the addon Configuration UI showed an unlabelled checkbox. Add the
   missing entry (same wording as dev's translation).

2. Big "Save advanced settings" button used to say "Nothing to save."
   after the user toggled a feature flag (beta master, Tool Search,
   etc.). Feature-flag toggles auto-save on click via
   ``saveFeatureFlag`` — they never enter ``_advancedDirty``, so the
   advanced-save button sees nothing to do. When the restart banner
   is already showing (recent feature-flag save), surface that
   explicitly: "No advanced changes to save — your feature-flag
   toggles already saved on click. Click Restart above to apply
   them." Falls back to the original "Nothing to save." copy when
   there's no pending restart.

3. Drop the master-off cascade-clear behaviour entirely. The runtime
   master gate in ``_apply_feature_flag_overrides`` already forces
   every beta sub-flag to False whenever the master is off, so the
   tools stay disabled at runtime regardless of file state. Leaving
   the sub-flag values in the override file means toggling the
   master off → on restores the user's prior sub-flag selections
   automatically; the previous cascade-clear forced users to
   re-check each sub-flag after every master cycle, which was the
   wrong UX trade for an opt-in beta surface.

The master-gate check is unchanged — it still rejects payloads that
try to enable a sub-flag while the effective master is off, so the
"sub true while master false in same payload" inconsistency still
can't land. The cascade-clear was a separate (now-removed) defence.

Tests updated:
- ``test_save_features_master_off_preserves_subflag_values`` — was
  ``test_save_features_cascade_clears_subflags_when_master_off``;
  asserts the new "preserve" semantics.
- ``test_save_features_master_on_restores_runtime_subflag_values``
  — new round-trip test for master off → on restoring sub-flags.
- ``test_save_features_payload_master_false_sub_true_rejected_by_gate``
  — renamed; asserts gate rejection still covers the inconsistent
  payload.
- ``test_save_features_cascade_reads_override_file_not_post_gate_settings``
  — deleted (no cascade, no reason to test cascade's read path).
- ``test_save_features_master_off_applied_dict_contains_only_master``
  — new no-cascade pin; ``applied`` carries only the master flip.

Author: kingpanther13

homeassistant-addon/translations/en.yaml

@@ -71,3 +71,10 @@ configuration:
       certificate or hostname mismatch. Disabling weakens transport
       security — leave on unless you know you need it. Requires restart
       to take effect.
+  advanced_debug_logging:
+    name: Advanced debug logging
+    description: >-
+      Captures extra diagnostic info on kill/shutdown — sender PID
+      and memory state. Useful when reporting unexplained add-on
+      stops or crash loops. Off by default to keep logs concise.
+      Requires restart to take effect.

src/ha_mcp/settings_ui.py

@@ -2338,15 +2338,14 @@ def apply_tool_visibility(
         // sub-row dimming reflects the new state immediately. The
         // save POST still proceeds in the background.
         //
-        // Sub-flag VALUES are intentionally NOT flipped here. The
-        // server-side cascade in ``_save_feature_flags`` clears them
-        // on disk, but the live UI keeps the user's prior sub-flag
-        // selections visible (checked-but-dimmed-with-disabled-input)
-        // until the next page load so flipping master off doesn't
-        // visually wipe state the user might want context on. After
-        // refresh, the cascaded-cleared values show through. If save
-        // fails (CONNECTION_FAILED etc.), the visible checked state
-        // matches the actual on-disk state, which is the right UX.
+        // Sub-flag VALUES are intentionally NOT flipped here. Neither
+        // is the server's persisted state — the runtime gate in
+        // ``_apply_feature_flag_overrides`` is the only thing that
+        // forces sub-flags off when master is off, and it does so
+        // without mutating the saved values. Result: turning the
+        // master off then back on restores the user's prior sub-flag
+        // selections automatically, which is the intended UX for an
+        // opt-in beta surface.
         if (isMaster) {
           if (_lastFeatureFlags[fieldName]) {
             _lastFeatureFlags[fieldName] = {
@@ -3546,7 +3545,24 @@ def apply_tool_visibility(
     return;
   }
   if (Object.keys(_advancedDirty).length === 0) {
-    _setAdvSaveStatus('Nothing to save.');
+    // Feature-flag toggles (master beta, Tool Search, etc.) auto-save
+    // on click via ``saveFeatureFlag`` — they don't pass through
+    // ``_advancedDirty``. If a feature-flag save just landed,
+    // ``restartNotice`` is showing and the user should click Restart,
+    // not Save again. Tell them that explicitly so the big Save
+    // button doesn't look broken when they were toggling beta flags
+    // (#1164 follow-up).
+    const restartNotice = document.getElementById('restartNotice');
+    const restartShowing =
+      restartNotice && restartNotice.classList.contains('show');
+    if (restartShowing) {
+      _setAdvSaveStatus(
+        'No advanced changes to save — your feature-flag toggles already ' +
+        'saved on click. Click Restart above to apply them.'
+      );
+    } else {
+      _setAdvSaveStatus('Nothing to save.');
+    }
     return;
   }
   _setAdvSaveDisabled(true);
@@ -4455,9 +4471,6 @@ async def _save_feature_flags(request: Request) -> JSONResponse:
         from .config import (
             BETA_FEATURE_FIELDS as _BETA_SUB,
         )
-        from .config import (
-            _read_feature_flag_override_file,
-        )
 
         effective_master = bool(
             raw_flags.get(
@@ -4571,38 +4584,23 @@ async def _save_feature_flags(request: Request) -> JSONResponse:
                     status_code=400,
                 )
 
-        # Master-off cascade clear (#1164 follow-up): flipping the master
-        # OFF also writes False for every beta sub-flag in the same
-        # save. Without this, sub-flags stay True in the override file
-        # (or in addon options) and resume the prior state the moment
-        # the master is flipped back on — the user would have to
-        # uncheck each sub-flag individually to "really" disable the
-        # beta block.
-        #
-        # Read the persisted override file directly here (NOT
-        # get_global_settings() — the master gate in
-        # _apply_feature_flag_overrides already forced sub-flags to
-        # False on the resolved Settings object, so we'd miss stale-
-        # true override values). Cascade writes False for every
-        # sub-flag that's not already False at the persistence layer.
+        # Master-off no longer cascades into sub-flag values (#1164
+        # follow-up). The runtime master gate in
+        # ``_apply_feature_flag_overrides`` continues to force every
+        # beta sub-flag to False whenever the master is off, so the
+        # tools stay disabled at runtime regardless of file state.
+        # Leaving the sub-flag values in the override file means
+        # re-enabling the master restores the user's prior sub-flag
+        # selections automatically — without it the user had to
+        # re-check each sub-flag individually after every
+        # master-off / master-on cycle, which is the wrong UX trade
+        # for an opt-in beta surface.
         #
-        # ALSO force False even if the payload set the sub-flag to True
-        # — accepting an explicit ``{master: false, sub: true}`` would
-        # land an inconsistent persisted state that the runtime gate
-        # then ignores, leaving a "true-but-ignored" trap on the UI.
-        if new_overrides.get("enable_beta_features") is False:
-            overrides_on_disk = _read_feature_flag_override_file()
-            for sub in _BETA_SUB:
-                explicit = new_overrides.get(sub)
-                if explicit is False:
-                    continue  # already correctly False in this payload
-                # Persisted truthy OR payload-explicit-True both get
-                # force-cleared. Inspect both sources so the file
-                # cascade doesn't miss an override the user set
-                # earlier in standalone mode.
-                file_truthy = bool(overrides_on_disk.get(sub, False))
-                if explicit is True or file_truthy:
-                    new_overrides[sub] = False
+        # The master-gate check above still rejects payloads that try
+        # to enable a sub-flag while the effective master is off, so
+        # users can't land a "sub=true while master=false in same
+        # payload" inconsistency. Pre-existing sub-flag truthy values
+        # in the file are kept verbatim.
 
         # Addon-mode writes go to Supervisor instead of the override file:
         # ``start.py`` reads ``config.yaml`` options on every boot and

tests/src/unit/test_settings_ui.py

@@ -2767,16 +2767,17 @@ async def test_save_features_rejects_subflag_when_payload_turns_master_off(
         _reset_global_settings()
 
     @pytest.mark.asyncio
-    async def test_save_features_cascade_clears_subflags_when_master_off(
+    async def test_save_features_master_off_preserves_subflag_values(
         self, monkeypatch, tmp_path
     ):
-        """Master-off cascade clear (#1164 follow-up).
-
-        Flipping the master OFF in isolation must also write False for
-        every beta sub-flag currently truthy. Without the cascade,
-        sub-flags stay True in the override file and resume the
-        moment the master is flipped back on — UX bug the user
-        reported as "having to turn off every toggle individually."
+        """Master-off does NOT cascade into sub-flag values (#1164
+        follow-up update). The runtime master gate forces sub-flags off
+        at runtime, but the persisted values stay so flipping the
+        master back on restores the user's prior sub-flag selections.
+
+        The previous cascade-clear behavior forced users to re-check
+        every sub-flag after every master-off/on cycle, which is the
+        wrong UX trade for an opt-in beta surface.
         """
         from ha_mcp.config import (
             BETA_FEATURE_FIELDS,
@@ -2804,31 +2805,29 @@ async def test_save_features_cascade_clears_subflags_when_master_off(
         req.json = AsyncMock(return_value={"flags": {"enable_beta_features": False}})
         resp = await handlers["save_feature_flags"](req)
         assert resp.status_code == 200, json.loads(resp.body)
-        # Verify the cascade landed in the override file: every beta
-        # sub-flag must now read False.
+        # Master is now off in the file.
         on_disk = json.loads((tmp_path / "feature_flags.json").read_text())
         assert on_disk["enable_beta_features"] is False
+        # Sub-flag values are PRESERVED so toggling the master back on
+        # later restores them automatically.
         for sub in BETA_FEATURE_FIELDS:
-            assert on_disk[sub] is False, (
-                f"cascade missed {sub} — stays {on_disk.get(sub)!r} after master-off save"
+            assert on_disk[sub] is True, (
+                f"sub-flag {sub} was clobbered on master-off — should have stayed True"
             )
         get_data_dir.cache_clear()
         _reset_global_settings()
 
     @pytest.mark.asyncio
-    async def test_save_features_cascade_clears_subflag_even_when_payload_says_true(
+    async def test_save_features_master_on_restores_runtime_subflag_values(
         self, monkeypatch, tmp_path
     ):
-        """In-payload ``{master: false, sub: true}`` would land an
-        inconsistent persisted state — runtime gate forces sub False
-        but the file still says True, looking on the UI like the
-        cascade didn't run. The cascade now force-clears any sub
-        explicitly set True in the same payload as master=false
-        (#1164 follow-up review).
+        """Round-trip: master off → master on → previously-truthy sub-flags
+        come back through the runtime master gate (no manual re-toggle).
         """
         from ha_mcp.config import (
             FEATURE_FLAG_FIELDS,
             _reset_global_settings,
+            get_global_settings,
         )
         from ha_mcp.settings_ui import build_settings_handlers
         from ha_mcp.utils.data_paths import get_data_dir
@@ -2839,57 +2838,50 @@ async def test_save_features_cascade_clears_subflag_even_when_payload_says_true(
         for _fname, ename, _ftype in FEATURE_FLAG_FIELDS:
             monkeypatch.delenv(ename, raising=False)
         monkeypatch.delenv("SUPERVISOR_TOKEN", raising=False)
-        # Pre-existing state: master on, both sub-flags on.
+        # File: master off + sub-flags true. Runtime gate forces sub
+        # to False on read.
         (tmp_path / "feature_flags.json").write_text(
             json.dumps(
                 {
-                    "enable_beta_features": True,
+                    "enable_beta_features": False,
                     "enable_yaml_config_editing": True,
                     "enable_filesystem_tools": True,
                 }
             )
         )
         _reset_global_settings()
+        settings = get_global_settings()
+        assert settings.enable_beta_features is False
+        assert settings.enable_yaml_config_editing is False  # gated off
+        assert settings.enable_filesystem_tools is False  # gated off
+        # User flips master back on.
         handlers = build_settings_handlers(server=None)
         req = MagicMock()
-        # Caller's payload tries the inconsistent state — master off
-        # but yaml_config still asserted true. Master-gate check
-        # rejects this first (because beta sub-flag write is gated
-        # behind effective_master), so the response should 409 with
-        # the gate message, NOT silently land the inconsistent state.
-        req.json = AsyncMock(
-            return_value={
-                "flags": {
-                    "enable_beta_features": False,
-                    "enable_yaml_config_editing": True,
-                }
-            }
-        )
+        req.json = AsyncMock(return_value={"flags": {"enable_beta_features": True}})
         resp = await handlers["save_feature_flags"](req)
-        assert resp.status_code == 409, json.loads(resp.body)
-        # Sanity check: file unchanged.
-        on_disk = json.loads((tmp_path / "feature_flags.json").read_text())
-        assert on_disk["enable_yaml_config_editing"] is True
+        assert resp.status_code == 200, json.loads(resp.body)
+        _reset_global_settings()
+        settings = get_global_settings()
+        # Sub-flags resume their prior True state because the file
+        # still has them.
+        assert settings.enable_beta_features is True
+        assert settings.enable_yaml_config_editing is True
+        assert settings.enable_filesystem_tools is True
         get_data_dir.cache_clear()
         _reset_global_settings()
 
     @pytest.mark.asyncio
-    async def test_save_features_cascade_reads_override_file_not_post_gate_settings(
+    async def test_save_features_payload_master_false_sub_true_rejected_by_gate(
         self, monkeypatch, tmp_path
     ):
-        """Cascade-clear must read the persisted override file directly,
-        not ``get_global_settings()`` — the master gate already forced
-        sub-flags to False on the resolved Settings, so a read of
-        Settings would think there's nothing to clear and miss the
-        stale-true override (#1164 follow-up review).
-
-        Scenario: master was previously off in the override file but a
-        sub-flag was True (perhaps written via a manual edit before
-        the cascade-clear feature landed). User saves master=false.
-        After save, the file must show the sub-flag = False.
+        """In-payload ``{master: false, sub: true}`` is rejected by the
+        master-gate check (effective_master=false → sub-flag write
+        not allowed). The persisted file is unchanged. Sub-flag values
+        are no longer cascade-cleared, so the gate rejection is the
+        only thing preventing the user from landing an inconsistent
+        "sub true while master will be false at runtime" state.
         """
         from ha_mcp.config import (
-            BETA_FEATURE_FIELDS,
             FEATURE_FLAG_FIELDS,
             _reset_global_settings,
         )
@@ -2902,42 +2894,48 @@ async def test_save_features_cascade_reads_override_file_not_post_gate_settings(
         for _fname, ename, _ftype in FEATURE_FLAG_FIELDS:
             monkeypatch.delenv(ename, raising=False)
         monkeypatch.delenv("SUPERVISOR_TOKEN", raising=False)
-        # Master previously off, sub-flag True on disk (the master
-        # gate would force sub_X to False on the live Settings, so
-        # the old cascade impl reading Settings would miss this).
+        # Pre-existing state: master on, both sub-flags on.
         (tmp_path / "feature_flags.json").write_text(
             json.dumps(
                 {
-                    "enable_beta_features": False,
+                    "enable_beta_features": True,
+                    "enable_yaml_config_editing": True,
                     "enable_filesystem_tools": True,
                 }
             )
         )
         _reset_global_settings()
         handlers = build_settings_handlers(server=None)
         req = MagicMock()
-        req.json = AsyncMock(return_value={"flags": {"enable_beta_features": False}})
+        # Caller's payload tries the inconsistent state — master off
+        # but yaml_config still asserted true. Master-gate check
+        # rejects this first (because beta sub-flag write is gated
+        # behind effective_master), so the response should 409 with
+        # the gate message, NOT silently land the inconsistent state.
+        req.json = AsyncMock(
+            return_value={
+                "flags": {
+                    "enable_beta_features": False,
+                    "enable_yaml_config_editing": True,
+                }
+            }
+        )
         resp = await handlers["save_feature_flags"](req)
-        assert resp.status_code == 200, json.loads(resp.body)
+        assert resp.status_code == 409, json.loads(resp.body)
+        # Sanity check: file unchanged.
         on_disk = json.loads((tmp_path / "feature_flags.json").read_text())
-        assert on_disk["enable_filesystem_tools"] is False, (
-            f"cascade missed stale-true override; file: {on_disk}"
-        )
-        # Other sub-flags untouched (no stale-true to clear).
-        for sub in BETA_FEATURE_FIELDS:
-            if sub == "enable_filesystem_tools":
-                continue
-            assert sub not in on_disk or on_disk[sub] is False
+        assert on_disk["enable_yaml_config_editing"] is True
         get_data_dir.cache_clear()
         _reset_global_settings()
 
     @pytest.mark.asyncio
-    async def test_save_features_master_off_does_not_clobber_already_false_subflags(
+    async def test_save_features_master_off_applied_dict_contains_only_master(
         self, monkeypatch, tmp_path
     ):
-        """Cascade only flips truthy sub-flags. False ones are left
-        alone (no redundant writes that would cause the response
-        ``applied`` dict to balloon).
+        """Master-off save's ``applied`` dict carries only the master
+        change — no synthetic sub-flag entries from a cascade. Pins
+        the no-cascade behavior so a future regression that re-adds
+        cascade-clear shows up in this test's diff.
         """
         from ha_mcp.config import (
             BETA_FEATURE_FIELDS,
@@ -2953,7 +2951,6 @@ async def test_save_features_master_off_does_not_clobber_already_false_subflags(
         for _fname, ename, _ftype in FEATURE_FLAG_FIELDS:
             monkeypatch.delenv(ename, raising=False)
         monkeypatch.delenv("SUPERVISOR_TOKEN", raising=False)
-        # Master + one sub-flag on; rest at default (False).
         (tmp_path / "feature_flags.json").write_text(
             json.dumps(
                 {"enable_beta_features": True, "enable_yaml_config_editing": True}
@@ -2967,14 +2964,11 @@ async def test_save_features_master_off_does_not_clobber_already_false_subflags(
         assert resp.status_code == 200, json.loads(resp.body)
         body = json.loads(resp.body)
         applied = body.get("applied", {})
-        # Master flipped + one sub-flag cleared. No noise for already-
-        # False sub-flags.
-        assert applied.get("enable_beta_features") is False
-        assert applied.get("enable_yaml_config_editing") is False
+        assert applied == {"enable_beta_features": False}, (
+            f"expected applied to contain only the master flip; got {applied}"
+        )
+        # Sub-flag values are NOT touched.
         for sub in BETA_FEATURE_FIELDS:
-            if sub != "enable_yaml_config_editing":
-                assert sub not in applied, (
-                    f"cascade clobbered already-False {sub} — wasted write"
-                )
+            assert sub not in applied
         get_data_dir.cache_clear()
         _reset_global_settings()