fix: sync addon settings UI with Supervisor options end-to-end (#1420)

* fix: sync addon settings UI with Supervisor options end-to-end

Four related addon-mode bugs in the settings web UI:

1. Server-settings toggles were rendered with the
   "Managed by the add-on Configuration tab — open Settings → Add-ons
   → ha-mcp → Configuration to edit." note and a disabled checkbox.
   They are now editable: each save POSTs through
   /addons/self/options and schedules a self-restart so start.py
   re-derives env vars from config.yaml on next boot. The two
   surfaces (web UI + Configuration tab) now stay in sync
   bidirectionally.

2. Clicking the explicit "Restart Add-on" button after saving tools
   used to show a false "Restart failed" popup even though ha-mcp
   logs proved the restart succeeded. Root cause: Supervisor killed
   the addon mid-response, HA ingress converted the dropped
   upstream into a 5xx Bad Gateway, and the JS interpreted that as
   failure. Fix: self-restart now fires from a background asyncio
   task with a small delay so the JSON response flushes through
   ingress *before* Supervisor restarts the container. Non-self
   slugs (sibling addons used by the inaddon E2E) keep the
   synchronous error-surfacing path so test assertions still work.

3. ha_manage_custom_tool (gated by enable_code_mode) did not appear
   in the tool list when its toggle was off. Other beta tools like
   ha_config_set_yaml *do* appear with a "Beta — set X" hint via
   FEATURE_GATED_TOOLS stubs. Added a stub for ha_manage_custom_tool
   so the asymmetry is gone — the tool now shows up with the same
   Beta-disabled note regardless of the addon-config toggle state.

4. POST /api/settings/backup-config returned a 400 with
   "addon_configuration_invalid_error: Missing option 'backup_hint'
   in root" because the old code POSTed only the auto-backup fields
   to /addons/self/options, dropping backup_hint (the addon
   schema's only required key). Fix: GET current options first,
   merge changes into the full options dict, then POST the
   merged result — same pattern start.py::persist_addon_options
   already uses for the secret_path persistence.

Architecture:

- Two new module-level async helpers (_supervisor_fetch_current_options
  and _supervisor_merge_and_post_options) capture the merge-then-POST
  contract once and reuse it for backup-config + feature-flag saves.
- _schedule_supervisor_self_restart wraps the background-task pattern
  with explicit task-set retention so RUF006 stays happy and the
  fire-and-forget coroutine isn't GC'd before it can POST.
- The cross-tab restart-required banner is hoisted from inside
  panel-tools to a top-level sibling, so a save from the
  Server Settings or Backups tab surfaces the same visible notice
  regardless of which tab the user is on.

Tests: 80 settings_ui unit tests pass (Windows-only kill_signal /
filesystem-symlink skips are pre-existing and unrelated). The
restart synchronous-error paths now exercise non-self slugs; a new
test pins self-restart's "schedule + return 200" contract.

* fix(settings-ui): address PR review feedback for addon options helpers

Addresses code-reviewer, comment-analyzer, pr-test-analyzer, and
code-simplifier findings from the PR review toolkit pass on
fix/addon-ui-settings-sync.

Critical
- Transport vs validation errors now route to the right ErrorCode
  with the right HTTP status: supervisor 4xx on the merge POST maps
  to CONFIG_VALIDATION_FAILED with supervisor's status code
  preserved (so the UI shows the real 4xx); network / DNS / token /
  malformed-response failures map to CONNECTION_FAILED with 502 so
  the UI surfaces the "is HA reachable" recovery suggestions.
  Collapsing both into a single 502 sent users down the wrong
  recovery path in the previous version. Mechanism: a new
  _SupervisorOptionsError NamedTuple discriminates kind/message/status
  and both _save_feature_flags and _save_backup_config branch on it.

Defense-in-depth
- _supervisor_fetch_current_options and _supervisor_merge_and_post_options
  now catch RuntimeError from make_supervisor_httpx_client (raised
  when SUPERVISOR_TOKEN is unset) and classify it as a transport
  error, so a future third caller missing the env-var gate gets a
  clean 502 instead of an uncaught 500.

Dead-code removal
- Removed the inner `if not os.environ.get("SUPERVISOR_TOKEN")`
  guards in both addon-mode save branches. They are unreachable —
  entry to those branches already requires the token to be set (via
  is_running_in_addon() / get_feature_flag_origin() returning
  "addon"). The helpers' new RuntimeError catch covers the
  defense-in-depth case.

Mixed-origin invariant
- _save_feature_flags now tracks both addon_writes and
  file_or_default_writes and returns INTERNAL_ERROR 500 if a single
  batch ever contains both — currently impossible by construction
  (get_feature_flag_origin returns one mode per environment), but
  the explicit guard makes any future regression fail loudly
  instead of silently routing a file-mode write through Supervisor.

Comments and docstrings
- Dropped the dangling `(#TODO: refer to PR)` placeholder in
  _save_backup_config — rewritten as plain past-tense prose.
- Fixed the _supervisor_fetch_current_options cross-reference: it
  mirrors maybe_persist_secret_path (which spreads existing config
  before posting), not persist_addon_options (which documents that
  callers must do the spread).
- Moved the addon-mode "restart scheduled" comment in
  saveFeatureFlag inside the `if (data.restarting)` branch so it no
  longer reads as unconditional.
- Promoted the 0.3-second restart-flush delay to
  _SUPERVISOR_SELF_RESTART_FLUSH_DELAY_S with a docstring anchoring
  the "tuned conservatively" claim. Tests still override via kwarg.

Tests
- TestSupervisorOptionsHelpers gains
  test_fetch_current_options_accepts_bare_options_dict,
  test_fetch_current_options_runtime_error_returns_transport_error,
  test_merge_and_post_transport_error_returns_transport_kind,
  and the existing supervisor-error tests now assert on the
  NamedTuple's `kind`/`status_code`/`message` shape.
- TestSaveBackupConfigAddonMode and TestSaveFeatureFlagsAddonMode
  gain explicit transport-vs-validation tests that pin the
  error-code routing contract end-to-end.
- TestSaveFeatureFlagsAddonMode also pins the server=None sidecar
  guard.
- Schedule-self-restart tests now wait deterministically via a new
  _drain_background_restart_tasks helper instead of
  asyncio.sleep(0.05) — eliminates CI-runner flakiness.
- test_invalid_slug_in_body_falls_back_to_self switched from
  inline httpx.AsyncClient patching to the class's
  _patch_supervisor_client helper for consistency.

Verified: ruff check, ruff format, mypy src/, and the full
test_settings_ui.py suite (85 passed, 1 Windows-only skip).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(settings-ui): unify restart flow — save commits, button restarts

Every save endpoint (Tools, Server Settings, Backups) now goes through
the same restart UX: persist the change, surface a "Saved — restart
required" toast and the cross-tab restart banner, and let the user
pick the moment to actually fire the supervisor restart via the
global Restart Add-on button. No save handler auto-restarts the addon
anymore. The button auto-reloads the page 10 seconds after the click.

Why
- Adjusting a Server Setting toggle still produced a "Restart failed"
  alert in addon mode. Root cause: the previous fix moved each save's
  supervisor self-restart into a background task, but the user-visible
  outcome was unchanged — every toggle change immediately fired a
  supervisor restart, and if the user toggled multiple settings in
  quick succession, a later POST would race the in-flight restart
  (supervisor kills the addon → ingress returns 5xx → JS shows
  "Restart failed").
- Saving auto-backup settings on the Backups tab jumped straight to
  "addon is restarting" with no preview, no second-chance confirm,
  and no way to bundle multiple edits before paying the restart cost.
- Clicking the explicit Restart button on the Tools tab left the page
  stuck on "Restart initiated — reload page in ~30s" forever; the
  user had to manually F5.

Server changes
- ``_save_feature_flags`` addon branch: drop
  ``_schedule_supervisor_self_restart`` call. Return
  ``{success, applied, mode, restart_required: True}`` instead of
  ``{..., restarting: True}``.
- ``_save_backup_config`` addon branch: same — no auto-restart.
- ``_save_backup_config`` file branch: rename ``restarting: False``
  → ``restart_required: False`` for naming consistency across all
  save endpoints.
- ``_save_feature_flags`` file/default branch: now also returns
  ``restart_required: True`` (matches the FEATURE_META copy on every
  flag, which says "Requires restart to take effect"). The UI
  surfaces the banner; the restart button stays hidden in non-addon
  mode since there is no supervisor to drive it.

JS changes
- ``saveFeatureFlag``: drops the "add-on is restarting…" branch.
  Every successful save shows ``Saved — restart required`` and
  surfaces the cross-tab banner.
- ``saveBackupConfig``: branches on ``data.restart_required`` instead
  of ``data.restarting``. Re-enables the Save button so the user can
  bundle further edits before clicking Restart.
- ``restartAddon``: auto-reloads the page after
  ``RESTART_RELOAD_DELAY_S = 10`` seconds. 4xx responses (genuine
  config error, e.g. SUPERVISOR_TOKEN missing) suppress the reload
  and surface the error inline; 5xx-from-ingress and network drops
  fall through to the reload because both mean the supervisor killed
  our upstream and the restart is in flight.
- ``ORIGIN_INFO_NOTE.addon`` and ``BACKUP_ORIGIN_LABELS.addon``:
  "save will restart the add-on" → "restart required after save".

Test changes
- ``test_addon_save_merges_and_schedules_restart`` renamed to
  ``test_addon_save_merges_without_restart_returns_restart_required``
  for both the backup-config and feature-flags variants. New
  assertions: ``schedule_mock.assert_not_called()``,
  ``body["restart_required"] is True``,
  ``"restarting" not in body``.
- ``test_standalone_writes_file_and_invalidates_cache``: assert the
  new field name.

Verified: ruff check, ruff format, mypy src/, 85 passing in
test_settings_ui.py (1 pre-existing Windows skip).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(settings-ui): robust restart UX — poll-until-ready, cross-tab sync, unified shape

Addresses all reviewer + idiot-checker findings on the previous
unified-restart-flow commit. The intent stays the same: save commits
the change, the global Restart Add-on button is the single restart
path, the page reloads automatically when the addon comes back. The
implementation is now actually robust enough to deliver that.

Server
- ``_save_tools`` now returns the same
  ``{success, applied, mode, restart_required}`` shape as
  ``_save_feature_flags`` and ``_save_backup_config``. The previous
  ``disabled``/``pinned`` count fields had no consumers (JS read
  none; no test asserted on them) and pre-empted the unified
  contract — replaced with ``applied`` (the states dict) and
  ``mode: "file"``.
- ``_save_feature_flags`` file/default branch now also returns the
  unified shape including ``applied`` (was a bare ``{"success":
  True}`` before). Standalone / Docker / Claude Desktop users get
  the same restart banner the addon-mode users get.
- ``_save_feature_flags`` docstring no longer claims the addon
  branch schedules a restart; describes the new pending-then-explicit
  flow.
- ``_save_backup_config`` docstring same fix.
- File-mode feature-flag comment no longer makes a grep-falsifiable
  claim about FEATURE_META text; describes the semantic instead.
- "reported by the user" rot-prone phrasing in
  ``_save_feature_flags`` replaced with the enduring tech reason.
- ``_schedule_supervisor_self_restart`` now catches ``RuntimeError``
  from ``make_supervisor_httpx_client`` (raised when
  ``SUPERVISOR_TOKEN`` is unset). Mirrors the parity catch the two
  supervisor options helpers already had. Without it, a race that
  unsets the token between handler entry and the 300ms-later task
  wakeup would surface only as asyncio's "Task exception was never
  retrieved" at GC time.

JS — restart flow
- 10-second fixed timer replaced with poll-until-ready against
  ``./api/settings/info``. Initial 3-second grace lets supervisor
  actually kill the addon before we start probing (so a too-eager
  first probe doesn't return 200 from the OLD instance and reload
  too early); then poll every 2 seconds for up to 60 seconds.
  Reloads as soon as the new instance answers; falls back to
  "did not come back online — reload manually" if the probe
  times out. Obsoletes the magic-number constant.
- Concurrency guard: module-level ``restartInProgress`` boolean
  checked at function entry. Guards against DevTools / programmatic
  invocation that bypass the button's ``disabled`` attribute.
  Cleared only on a 4xx genuine config error so the user can fix
  the underlying cause and retry; otherwise stays true through the
  full restart cycle.
- ``restartAddon`` flattened to early-return on the 4xx config-error
  branch — removes the ``configError`` flag.
- Optional-chaining for ``err?.error?.message`` (drops the verbose
  nullish chain).
- Empty ``catch (_e) {}`` for the network-drop path now logs a
  ``console.warn`` so DevTools shows what happened — useful when
  debugging "I clicked Restart and nothing happened" reports.

JS — multi-tab sync (BroadcastChannel)
- New ``restartChannel`` on ``BroadcastChannel('ha-mcp-settings')``.
  Falls back to ``null`` on browsers without the API; every channel
  use is null-guarded so older browsers see no regression.
- ``saveConfig`` / ``saveFeatureFlag`` / ``saveBackupConfig`` post
  ``{type: 'restart-required'}`` when their response carries
  ``restart_required: true``. Other open tabs surface the same
  banner.
- ``restartAddon`` posts ``{type: 'restart-initiated'}`` BEFORE
  starting its poll cycle. The listener kicks off the same
  poll-then-reload cycle in every other tab so they all come back
  to the fresh addon instead of dangling on a stale connection.

JS — quality of life
- ``saveFeatureFlag`` JSON-parse fallback: on ``resp.ok`` with a
  truncated / non-JSON body, default to ``{restart_required:
  true}`` so the banner still shows. Silently hiding the banner
  would make the user think the change took effect live.
- ``saveBackupConfig`` no longer calls ``loadBackupConfig()`` after
  a ``restart_required: true`` save. In addon mode the GET reads
  env-derived values that are still stale (Supervisor has the new
  options but ``start.py`` doesn't re-derive env vars until the
  next boot) — reloading the form snapped it back to old values
  and wiped any in-flight edits the user wanted to bundle.

Tests
- ``test_schedule_self_restart_catches_runtime_error`` pins the
  RuntimeError parity catch with a caplog assertion targeting the
  specific "SUPERVISOR_TOKEN unset" message.
- ``TestSaveFeatureFlagsStandaloneMode::test_standalone_save_returns_unified_contract_shape``
  pins the new file-mode response shape.
- ``TestSaveToolsResponseShape::test_save_returns_unified_contract_shape``
  pins the new unified shape on ``_save_tools`` and asserts the
  retired ``disabled``/``pinned`` count fields do not leak through.

Verified: ruff check, ruff format, mypy src/, 88 passing in
test_settings_ui.py (1 pre-existing Windows skip). ``node --check``
on the rendered ``<script>`` still passes — confirms the
BroadcastChannel + polling code is syntactically valid JS.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(settings-ui): detect actual restart via instance_id flip, not just 200

Closes the silent-restart-failure edge case in the new poll-until-
ready cycle. Without this, a supervisor restart that silently
fails (e.g. ``RuntimeError`` from a missing token, supervisor 4xx,
or any future failure mode that logs server-side but doesn't
surface to JS) would leave the OLD addon instance still answering
the JS probe with 200. The page would reload at T+3s to the same
state, the user would see no change, and the only signal would be
in addon logs — exactly the "broken page with no error message"
UX this PR is supposed to eliminate.

Server
- Module-level ``_PROCESS_INSTANCE_ID`` (uuid4 hex, generated at
  import) + ``_PROCESS_STARTED_AT`` (epoch seconds at import).
  Both surfaced via ``GET /api/settings/info``. A fresh Python
  process gets a fresh uuid; an addon restart that actually swaps
  processes flips it.

JS
- Replaced ``_probeAddonReady`` with ``_probeAddonRestarted(prev)``
  that polls ``/api/settings/info`` until ``instance_id`` differs
  from the captured baseline. ``prev=null`` (e.g. when the
  pre-restart fetch fails or an older server build lacks the
  field) falls back to the old "any 200 means up" behavior so we
  degrade gracefully.
- ``restartAddon`` captures ``previousInstanceId`` via a new
  ``_fetchSettingsInfo()`` helper BEFORE firing the supervisor
  POST, then threads it through the reload cycle.
- ``BroadcastChannel`` ``restart-initiated`` message now carries
  ``previousInstanceId`` so every open tab compares against the
  same pre-restart baseline. Without this, other tabs would either
  capture a stale baseline mid-restart or fall through to the old
  status-only probe.

Tests
- ``TestSettingsInfoEndpoint::test_returns_instance_id_and_started_at``
  pins the field presence and types.
- ``TestSettingsInfoEndpoint::test_instance_id_stable_within_process``
  pins the within-process stability invariant — without this, two
  calls in a row could see the value flip, the JS would reload
  immediately, and the restart-detection contract would silently
  break.

Verified: ruff check, ruff format, mypy src/, 90 passing in
test_settings_ui.py (1 pre-existing Windows skip).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(sidecar): widen settings-info equality checks for new fields

Two sidecar tests asserted ``resp.json() == {"is_addon": ...,
"is_sidecar": ...}`` against the full settings-info payload. The
preceding commit added ``instance_id`` and ``started_at`` to that
payload (needed by the restart-then-reload JS cycle to prove a
restart actually happened) which broke the exact-equality checks.

Switch to field-by-field assertions on the deployment-mode fields
those tests are actually about — the new per-process identity
fields are covered separately by
``TestSettingsInfoEndpoint`` in ``test_settings_ui.py``.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(settings-ui): preserve search filter across tool-toggle rebuilds

Reported: typing a query in the Tools tab search bar filtered the
list correctly, but toggling a setting on a visible tool snapped the
full list back into view — even though the search bar still showed
the query.

Root cause: ``render()`` (called after every toggle change via
``scheduleSave(); render();``) does ``container.innerHTML = ''`` and
rebuilds the entire ``.tool`` DOM. The search filter applied the
``hidden`` class imperatively to the OLD nodes; the freshly-built
nodes had none. The ``<input>`` element itself is outside the
rebuilt container, so its value persists — producing the
visible-query-but-no-filter mismatch the user reported.

Fix: extract the filter into ``applyToolSearch()`` that reads the
search input's current value directly and applies / clears the
``hidden`` class on every ``.tool`` node it finds. Wire it to the
input's ``input`` event (was inline before) and also call it at the
end of ``render()`` so any rebuild — toggle change, group-master
toggle, future render triggers — re-applies the active filter
against the new DOM.

No regression risk on the no-search path: an empty query short-
circuits ``!q`` to true and every tool stays visible, same outcome
as before. Group auto-expansion when search has matches still
works (q && visible branch unchanged).

Not directly testable in the current JS test infra (the project
bar is ``node --check`` syntax-only — no JSDOM, no DOM event
simulation). Closing that gap is tracked in #1422; ``node --check``
on the rebuilt script confirms no syntax regression.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: kingpanther13