Commit 6c3c0ac
feat: persistent settings UI for stdio mode (#1381)
* feat: persistent settings UI for stdio mode (#863)
Spawn a detached, long-lived settings UI sidecar on stdio startup so
Claude Desktop / Claude Code / default-Docker users can reach the
tool config page at any time — not only while the MCP subprocess
happens to be alive. The sidecar shares route handlers with the
existing HTTP modes (build_settings_handlers in settings_ui.py) so
the page is the same one HTTP/addon users already see — no second
surface to maintain.
- New stdio_settings_sidecar module: detached subprocess (POSIX
start_new_session, Windows DETACHED_PROCESS), random free port,
auto-generated secret path, security middleware (Host validation
for DNS-rebinding guard, Origin validation for CSRF on mutating
methods), 0600 perms on URL/pid files.
- HA_MCP_DISABLE_SETTINGS_UI env var + ~/.ha-mcp/settings_ui_disabled
sentinel for opt-out. POST /api/settings/shutdown endpoint drops
the sentinel and stops the sidecar in one step.
- ha_get_overview surfaces the sidecar URL as settings_url when
present, so the LLM can hand it to the user on request.
- Tool metadata dumped to ~/.ha-mcp/tool_metadata.json on every
parent stdio start; sidecar reads from cache (avoids constructing
a full FastMCP server in the child).
* ci: align build-haos-test-image triggers with cache-key inputs
The GHCR-publish workflow only triggered on `tests/haos_image_build/**`
and its own file, but the cached qcow2 actually bakes in four input
trees (hashed into haos-e2e-tests.yml's cache key). A master commit
changing any of the other three left GHCR stale, so every subsequent
PR that didn't itself touch a bake input pulled the outdated image and
failed the corresponding test. Most recently #1374 (knx allowlist)
broke every open PR's HAOS E2E run until a manual dispatch.
Mirror the cache-key paths in the publish trigger so any master commit
that invalidates the cache also republishes GHCR. Add INVARIANT
comments on both sides naming the other file as the must-stay-in-sync
counterpart so the next contributor doesn't drift them again.
* ci: wait for hassio supervisor/api WS handler after Core restart
Build flaked on a retry of build-haos-test-image (run 26175883676):
``supervisor/api post /store/reload`` came back ``Unknown command``
moments after a Core restart. That message is HA Core's WS dispatcher
saying the command type isn't registered — not Supervisor saying the
endpoint doesn't exist. The ``hassio`` integration calls
``async_load_websocket_api`` during its setup, which is what registers
the ``supervisor/api`` handler; reconnect() returned as soon as the
HTTP layer was up but before that integration finished loading, so
the next call landed in a race window of typically 1–10s.
Block in ``HAWebSocket.reconnect()`` until ``supervisor/api`` actually
dispatches (probe with the cheap ``/supervisor/info`` read), with a
60s outer timeout. Same root cause would silently bite any post-
restart code path; this centralises the wait in the one method every
caller already uses.
Unrelated transients (Supervisor 5xx, real ``unknown_command`` from
Supervisor for a renamed endpoint, etc.) propagate immediately so a
real regression isn't masked as "still booting".
* test(haos-e2e): wait for Node-RED to be running before strict assertion
test_proxy_http_request_headers_pass_through asserts on status_code as
an int but never waits for the addon to leave Supervisor's startup
phase. The bake installs Node-RED with start=True (build_image.py),
but the container takes 20-60s on a slow runner to enter started; if
the test fires inside that window ha_manage_addon short-circuits with
{"success": false, "error": ..., "state": "startup"} and the
assertion fails. Same race already flaked CI on caae6c4 and
ce661e8.
Add _wait_addon_running() that polls ha_get_addon until state ==
started (120s outer timeout) and call it from the strict test. Other
tests in the file tolerate the error path so they don't need it.
Mirrors AGENTS.md's wait_for_entity_registered discipline that
already governs tests acting on freshly created entities.
* fix(#863): address review findings (Gemini + toolkit)
Security + correctness:
- _write_pid_url: use os.open(O_WRONLY|O_CREAT|O_TRUNC, 0o600) for
atomic-secure file creation (closes TOCTOU between write_text and
chmod that briefly exposed the URL-with-secret at 0o644). Write pid
before url and roll the pid back on partial failure so the next
maybe_spawn() never reads a URL pointing at a dead listener.
- Remove redundant custom SIGTERM/SIGINT handler; uvicorn's default
Server.run() install does the same thing (sets should_exit). The
prior comment claimed we disabled uvicorn's handlers but the code
didn't — removing both clears the contradiction.
- build_settings_handlers(is_sidecar=True): sidecar's settings_info
forces is_addon=False regardless of inherited SUPERVISOR_TOKEN.
Prevents the served HTML from rendering a "Restart Add-on" button
that POSTs to a route the sidecar doesn't expose.
Silent failures + diagnostics:
- _shutdown_endpoint: return structured 500 + keep the sidecar
running when the disable sentinel write fails. Silently exiting
without the sentinel would leave the user thinking they'd
disabled the sidecar while it respawns on the next stdio start.
- _wait_addon_running: catch transient ToolError from ha_get_addon
and continue polling. Pre-fix, one Supervisor 5xx during boot
failed the full 120s wait with a misleading stack.
- HAWebSocket: raise typed WSCommandError(code=...) so callers can
branch on the structured error code instead of substring-matching
on str(e). _wait_supervisor_api_ready uses e.code == 'unknown_command'
now, future-proof against HA message-text changes.
- _maybe_spawn_settings_sidecar: log type(e).__name__ in the
best-effort except blocks so ops can distinguish server-init
failures from cache I/O from event-loop conflicts.
- load_tool_metadata_cache: exc_info=True on JSONDecodeError so a
truncated write is distinguishable from a corrupted-mid-file write.
Logging hygiene:
- _wait_supervisor_api_ready: per-attempt log line at DEBUG, not
INFO (project convention; matches Gemini styleguide).
CI:
- haos-e2e-tests.yml: add custom_components/ha_mcp_tools/** and
homeassistant-addon-webhook-proxy/mcp_proxy/** to PR-trigger paths
so a PR touching only those (both baked into the qcow2) still runs
HAOS E2E. Symmetric to the build-haos-test-image.yml fix in this
PR's earlier commit — both lists now match the cache-key inputs.
Tests:
- ha_get_overview: assert settings_url surfaces when sidecar URL
file present, is absent when no URL. Closes the critical coverage
gap surfaced by the review (settings_url is the only path the LLM
sees the URL through).
- Sidecar settings_info: assert is_addon=False even with
SUPERVISOR_TOKEN set (pins the restart-button fix above).
- maybe_spawn cleanup: assert stale pid+url unlinked BEFORE Popen
call. Catches a reordering regression that would surface a stale
URL to ha_get_overview between cleanup and listener bind.
- dump_tool_metadata_cache: assert False return on OSError.
Comment corrections per analyzer review:
- tools_search.py settings_url block: softened claim of "stdio
mode only" — file presence is what actually gates it; a leftover
URL file from a prior stdio run could be surfaced under HTTP mode
(acceptable: still gated by the random secret path).
* fix(#863): close deferred items from review pass
- Cold-start perf: gate the heavy metadata-cache dump on the same
conditions maybe_spawn() checks (disabled / existing sidecar
alive). Warm restarts that already have a sidecar running pay
zero cold-start tax — no FastMCP server build, no asyncio.run,
no cache I/O.
- In-page Stop Sidecar button: HTML + JS that posts to the
/shutdown endpoint, gated by a new ``is_sidecar`` field on
/api/settings/info. HTTP modes never render the button (clicking
Stop there would kill the MCP server).
- run_main() wiring test: mock uvicorn.Server, monkeypatch the
free-port pick, assert ui.url + ui.pid land with the expected
shape (host + port + secret prefix + /settings suffix; pid
matches current process). Second test covers the
disable-sentinel early-return path.
- Comment polish: dropped the rot-prone coerce_bool_param
reference; reworded the "double-fork without boilerplate"
inaccuracy; clarified the CTRL_C_EVENT direction (parent →
child) on Windows; removed the orphan asyncio import + the
stale "Disable uvicorn's own signal handlers" config comment
left behind by the earlier handler removal.
* test(#863): fix two unit-test regressions from e90ba15
- test_host_header_accepted: assertion was {is_addon: False}; the new
is_sidecar field added by the in-page Stop-button work made this
test fail. Updated to expect {is_addon: False, is_sidecar: True}
(matches what _build_app actually returns).
- test_run_main_respects_disable_sentinel: MagicMock forbids setting
__getattr__ as a magic method. Replaced with a plain _TrackingProxy
class that records whether uvicorn.* attribute access happened;
same semantics, no MagicMock restriction.
* fix(#863): move uvicorn import inside disable-sentinel guard
run_main() imported uvicorn at the top of the function, BEFORE the
_is_disabled() check. The disable path therefore paid the uvicorn
import cost even though uvicorn isn't touched. Caught by the new
test_run_main_respects_disable_sentinel which tracks whether
uvicorn's attribute machinery is touched on the disable path —
the import statement itself queries __spec__, which the test's
TrackingProxy treats as an access.
Move the import after the disable check so the fast-exit path
never touches uvicorn.
* docs(site): mention the new stdio settings UI sidecar (#863)
The stdio sidecar from this PR spawns silently — users who only follow
the quick-start won't know the page exists unless they think to ask
the AI. Small additions across the docs to surface it:
- faq.astro: new "How do I change which tools are enabled / pinned?"
entry under General Questions, plus HA_MCP_DISABLE_SETTINGS_UI in
the env-var table. Cross-linked from the other doc additions.
- setup.astro: stdio-conditional "After it's running" tip block
appended to the generated instructions (covers any uvx local
install, not just Claude Desktop).
- guide-macos.astro + guide-windows.astro: matching tip step before
the closing Feedback section. Windows variant uses %USERPROFILE%
for the ui.url path.
The pages reuse the existing withBase() helper for the in-site links
and the setup-script branch hardcodes the /ha-mcp/ prefix to match
the sibling block at line 1245.
* fix(#863): serialize concurrent spawn + pin discoverability flow
Addresses Patch76's review on #1381.
**Spawn atomicity.** Two parent stdio processes starting in rapid
succession could both clear `_existing_sidecar_alive()` and `Popen`
a child — the loser's child then raced on `bind()` and died into
`sidecar.log`. Wrapped the alive-check + Popen window in a
non-blocking `fcntl.flock` (POSIX) / `msvcrt.locking` (Windows)
held on `~/.ha-mcp/spawn.lock`. A second concurrent `maybe_spawn()`
sees the lock held and skips with a log line; by the time the lock
releases, the alive-check inside the lock catches the just-spawned
sidecar. Falls back to unlocked spawn on exotic platforms where the
lock primitive fails — better the rare race than refusing to spawn.
**Discoverability flow test.** Unit tests pinned the `settings_url`
field shape (present/absent) but never verified the URL actually
responds. Added two integration-style tests that wire the producer
(`run_main`'s URL-file writer) to the consumer (the `_build_app`
Starlette routes) via the same `read_sidecar_url()` path
`ha_get_overview` consumes. Catches: secret-path prefix changing in
one place but not the other, route suffix drift, URL truncation —
all of which would surface to users as a 404 with no test failure.
Also adds three lock-semantics tests: concurrent context-manager
re-entry yields False, lock releases on exit, `maybe_spawn()`
short-circuits when another holder has the lock.
* fix(#863): suppress Windows CMD window + surface loading failures
Two issues from real-world Claude Desktop testing:
**Empty CMD window on Windows.** subprocess.Popen with
DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP still gives the child
python.exe (a console app) a fresh console window — it just isn't
the parent's. Add CREATE_NO_WINDOW to suppress it. The flag combo
is supported and the Popen docs explicitly allow it together with
DETACHED_PROCESS.
**Settings page stuck on "Loading..." with no tools.** The JS
swallowed fetch / parse / render errors silently and the initial
<span>Loading...</span> never got replaced. Multiple discrete
failure modes (network, non-2xx, malformed JSON, empty cache,
render exception) now each surface as a distinct status message
so a user with no devtools open can still tell what broke and
where to look. Cache-empty case names the cache path and points
at sidecar.log so debugging is one cat away.
Server side: log a one-liner from the stdio parent after the
metadata dump ("N tools dumped, write succeeded/FAILED") so the
MCP-client log panel surfaces whether the cache made it to disk;
and log a warning from the sidecar's _get_tools handler whenever
it reads an empty cache, naming the path and pointing at the
likely parent-side failure mode.
* style: ruff format on PR-touched files
CI's Ruff Lint job runs `ruff format --check` on changed files; my
local pre-push runs ran `ruff check` only and missed the formatter
deltas. No semantic changes — line-wrap, trailing-comma, and string-
quote normalization on the six files this PR touches.
* fix(#863): use pythonw.exe + drop DETACHED_PROCESS to actually suppress Windows console
Real-world Claude Desktop test on Windows showed the previous fix
was a no-op: a cmd window still popped and closing it killed the
sidecar. Root cause in Python's subprocess docs:
"CREATE_NO_WINDOW... is ignored if you specify CREATE_NEW_CONSOLE
or DETACHED_PROCESS."
DETACHED_PROCESS on a CUI binary (python.exe) auto-allocates a fresh
visible console — and that's the cmd window the user saw. The closed-
window killing the server is because the auto-allocated console's
process group sends CTRL_CLOSE_EVENT to the child when X is clicked.
Drop DETACHED_PROCESS entirely; prefer pythonw.exe (GUI subsystem,
never allocates a console) over python.exe; keep CREATE_NEW_PROCESS_GROUP
(blocks CTRL_C / CTRL_CLOSE propagation from any console that might
attach during the python.exe fallback path) and CREATE_NO_WINDOW
(belt-and-suspenders for the fallback, harmless when pythonw is in
use). pythonw.exe ships alongside python.exe in every standard CPython
install including uv-managed ones.
* fix(#863): belt-and-suspenders Windows console hide + JS error surfacing
User reported on Windows + Claude Desktop that even after the pythonw
fix in b0268d7, the cmd window still pops AND the settings page
still says "Loading..." indefinitely.
**Windows console:** add ``STARTUPINFO`` with ``wShowWindow=SW_HIDE``
on top of pythonw preference + ``CREATE_NO_WINDOW``. uv-managed
Pythons sometimes strip pythonw.exe, falling back to python.exe.
``CREATE_NO_WINDOW`` alone leaves a window in some console-allocation
paths under GUI parents; ``STARTF_USESHOWWINDOW`` + ``SW_HIDE``
force-hides whatever console does get allocated.
**JS error surfacing:** if any function definition in the settings-
page script throws during top-level evaluation (e.g. a runtime error
referencing a missing DOM element), the script aborts before
``loadTools()`` is ever called and the status bar stays at the
initial ``Loading...`` literal. Add ``window.addEventListener('error',
...)`` + ``unhandledrejection`` so the next time the page hangs at
"Loading", the actual error message appears in the status bar
instead — no devtools required.
* fix(settings-ui): unblock tool list rendering + always-emit settings_url
Two regressions surfaced while testing PR #1381 against Claude Desktop.
1. Settings page stuck on "Loading...".
stopSidecar()'s JS confirm() prompt used a single-quoted string
'Stop the settings server?\n\n' inside the Python triple-quoted
_SETTINGS_HTML. Python consumed the \n\n as literal newlines, so
the rendered <script> contained a JS string spanning two physical
lines — an unrecoverable SyntaxError that aborted the entire
script before loadTools() could run. The in-page error handler
(window.addEventListener('error', ...)) cannot catch parse-time
errors, so the user saw only the initial "Loading..." indicator
with no diagnostic. Escape the backslashes in the Python source
so the JS engine sees the intended \n\n escape sequence.
2. settings_url invisible to fields=-projecting callers.
An LLM that minimized payload via fields=["system_info"] (or any
narrow projection) would lose the settings_url field, since the
projection ran *after* settings_url was added to the result. With
the field absent the LLM cannot hand the URL to the user even
when it knows the user is asking for it. Move the settings_url
emission to *after* project_fields so it survives every
projection, and surface it in the main tool docstring (which
LLMs read first) instead of relying on the fields= enum
description that less-attentive LLMs may skip.
Regression tests:
- test_rendered_script_parses_as_javascript shells out to
`node --check` against the rendered <script> body so any future
raw-newline-in-JS-string regression fails fast with a precise
parser diagnostic. Skipped when node is not on PATH (the test
matrix installs node already).
- test_settings_url_survives_fields_projection pins the new
always-emit-regardless-of-projection contract.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(settings-ui): install-aware restart copy + danger-styled disable confirm
Two UX gaps spotted while testing the sidecar in Claude Desktop.
1. Post-save banner read "Restart the add-on for changes to take
effect" regardless of how the user actually runs ha-mcp. Claude
Desktop / Docker / standalone users were told to do something that
doesn't exist in their install. The banner text now reads from
/api/settings/info and rewrites itself per mode:
* is_addon → "Click 'Restart Add-on'" (the button is right there)
* is_sidecar → "Fully quit and reopen your MCP client (Claude
Desktop: tray icon → Quit, then relaunch; Claude Code: close
the terminal session)"
* otherwise → "Restart your ha-mcp process (Docker container,
systemd service, or however you launch it)"
2. The sidecar's "Stop settings server" button was an accent-blue
primary button sitting near the page's routine toggles. A
misclick silently writes ~/.ha-mcp/settings_ui_disabled, which
then prevents the sidecar from respawning on *every* subsequent
Claude Desktop / ha-mcp launch — the only recovery is manual
filesystem cleanup. Two mitigations:
* Renamed to "Permanently disable settings server" and given a
new .danger-btn class (red border + danger-red text on a
transparent fill) so the destructive semantic is visible
without reading the label.
* confirm() now leads with the permanence and spells out the
two-step recovery (delete the marker file AND unset
HA_MCP_DISABLE_SETTINGS_UI). The old wording read like a soft
"stop for now, autostart later" — the new wording reads like
a commitment.
The new \n sequences in the confirm() prompt go through the JS
syntax regression test added in 249bcd1
(test_rendered_script_parses_as_javascript) so any future raw-newline
slip-up still fails fast.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(settings-ui): editable feature flags + env-lock indicator
Surfaces six runtime-editable feature flags in the existing
/settings panel so users can toggle them without rebuilding their
addon / docker / claude-desktop env:
* enable_tool_search ENABLE_TOOL_SEARCH
* tool_search_max_results TOOL_SEARCH_MAX_RESULTS (int 2-10)
* enable_yaml_config_editing ENABLE_YAML_CONFIG_EDITING
* enable_lite_docstrings ENABLE_LITE_DOCSTRINGS
* enable_filesystem_tools HAMCP_ENABLE_FILESYSTEM_TOOLS
* enable_custom_component_integration HAMCP_ENABLE_CUSTOM_COMPONENT_INTEGRATION
Two of those (filesystem, custom-component) previously read
``os.getenv`` directly from inside ``tools_filesystem.py`` /
``tools_mcp_component.py``. Promoted them to first-class
``Settings`` fields so every UI-editable flag goes through the
same precedence path; the legacy callers now read through
``get_global_settings()``.
## Backend
Mirrors the BACKUP_OVERRIDE_FIELDS pattern from PR #1403 so the
addon-config / env-var / override-file / default precedence is
uniform across both runtime-editable surfaces:
* ``FEATURE_FLAG_FIELDS`` enumerates field/env/type tuples
* ``get_feature_flag_origin(env_name)`` returns
``"addon" | "env" | "file" | "default"``
* ``_apply_feature_flag_overrides(settings)`` patches
Settings in place after pydantic construction, only when
not in addon mode AND env var not set
* ``_FEATURE_FLAG_INT_BOUNDS`` enforces range constraints on
file-supplied values so a corrupt JSON file cannot push
``tool_search_max_results`` outside its pydantic 2-10 bound
* ``get_global_settings()`` calls the apply hook on first read
* ``_reset_global_settings()`` is now a publish seam — the
POST handler invalidates the singleton so subsequent
``get_global_settings()`` reads see the new file value
## Endpoints
``GET /api/settings/features`` returns ``{flags: {<field>: {
value, origin, editable, type, env_var, min?, max?}}}``. Mounted
on the FastMCP server (HTTP / addon modes, both addon-root and
secret-prefixed paths) and on the stdio sidecar's Starlette app.
``POST /api/settings/features`` validates types + bounds +
edit-lock; rejects env-/addon-locked fields with the env var
name in the message so the UI can surface the exact unlock
action. Merges with the existing override file so a partial POST
only updates the keys it carries (the front-end POSTs one
toggle per change).
## UI
New collapsible "Server Settings" panel above the tool list:
* Renders per-row using ``FEATURE_META`` (display label +
help copy lives in the front-end so the API stays terse)
* Bool fields render as the existing toggle switch; int
fields as a constrained number input
* Locked rows are dim + show a yellow note pointing at the
env var name and the addon-config alternative
* Edits debounce-save individually and trigger the existing
restart notice so the user sees the action required
## Tests
* ``TestFeatureFlagsEndpoint`` (5 cases) covers the GET shape
on a clean env, env-var locking on GET + POST, file-write
+ singleton-reset on POST, and out-of-range int rejection.
* The handler-keys roster test was updated to include
``get_feature_flags`` + ``save_feature_flags``.
* The JS-syntax regression test (added in 249bcd1) catches
any future ``\n``-in-Python-string regression in the new
UI block.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(settings-ui): migration regressions + clearer locked-row copy
Three things uncovered by CI / my earlier "rough edge" hand-wave.
1. Feature-flag-migration tests broke for filesystem flag.
* ``test_disabled_with_empty_string``: pydantic's bool parser
raises on ``""``, but the legacy ``os.getenv("FLAG", "")
.lower() in (...)`` semantics treated empty as False. Added a
``@field_validator(mode="before")`` on
``enable_filesystem_tools`` + ``enable_custom_component_
integration`` that maps blank strings to False, restoring the
pre-migration contract.
* ``test_disabled_by_default``: ``patch.dict(os.environ, {},
clear=True)`` blows away ``HOME``/``USERPROFILE``, so
``Path.home()`` (called from ``utils.data_paths.get_data_dir``)
raises ``RuntimeError``. The old direct-``os.getenv`` path
never touched the data dir; the override layer regressed
this. ``_read_feature_flag_override_file()`` now catches
``RuntimeError`` alongside the FS errors.
* Added an autouse fixture to ``test_tools_filesystem.py`` that
resets the cached Settings singleton between tests. Without
it the singleton stays frozen at first construction and the
``patch.dict`` env mutations never take effect.
2. Server Settings panel collapsed by default.
First-time users had no visible cue that there was a panel
above the tool list. Made the panel open on initial render
(chevron + body both get ``.open`` in the HTML); collapse-on-
click still works for users who want to hide it.
3. Addon-mode locked-row copy pointed at the wrong place.
Old text suggested "unset <ENV_VAR> or change the add-on
configuration" for every locked row regardless of origin.
Split per origin:
* ``env`` → "Set via environment variable — unset it to edit
here. (<ENV_VAR>)"
* ``addon`` → "Managed by the add-on Configuration tab —
open Settings → Add-ons → ha-mcp → Configuration to edit."
Addon users hit the *canonical* config surface (the add-on
page in HA) instead of the parallel-but-disabled UI row, which
matches how every other addon-managed setting works in HA.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(settings-ui): copy verbatim addon descriptions into FEATURE_META
The terse one-liners I'd put on each row glossed over the real
trade-offs (Sonnet/Opus tool-search conflict, lite-docstrings
degradation risk, filesystem-access danger). The add-on's
Configuration tab already carries the full warning text in
``homeassistant-addon-dev/translations/en.yaml``, and a user who
flips between the web UI and the add-on panel shouldn't see two
different descriptions for the same toggle.
Lifted each label + description verbatim from that translations
file:
* enable_tool_search → full Sonnet/Opus warning + when to use
* tool_search_max_results → range + token-saving guidance
* enable_yaml_config_editing → whitelist + backup + restart notes
* enable_lite_docstrings → degradation caveat + MCP-resource note
* enable_filesystem_tools → "sensitive direct file access" warning
* enable_custom_component_integration → scope clarification
(does NOT control filesystem tool loading)
Marked the source-of-truth file in a comment above the const so a
future translation change updates the addon and this dict together.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(overview): always emit notifications + repairs as lists
The ``fields=`` parameter docstring on ``ha_get_overview`` advertises
``notifications`` and ``repairs`` as available top-level keys, but
the implementation only wrote them when non-empty. On a clean HA
instance an LLM that asks for either via ``fields=[...]`` hits
``project_fields()``'s typo-guard, which appends a "key not found
— available keys: [...]" warning that the LLM dutifully relays
back to the user as "notifications is not in available keys
anymore."
The fix: emit both keys as empty lists by default, and let the
WS-call branches overwrite them with the populated lists when
there's data. The error / dismissed-only branches now leave
``repairs == []`` alongside the existing ``repairs_error`` /
``dismissed_repair_count`` fields — which is what the docstring
already promised.
Updated tests that previously asserted ``"repairs" not in result``
to assert ``result["repairs"] == []`` — that was an "existing
contract" test comment, but the contract conflicted with the
public docstring promise the LLM relies on, so the test was
codifying the bug.
Added ``TestHaGetOverviewAlwaysEmittedKeys`` (3 cases) pinning:
- notifications == [] on a clean instance
- repairs == [] on a clean instance
- ``fields=["notifications","repairs"]`` on a clean instance
returns both empty AND does not raise a project_fields warning.
The third case is the regression guard for the user complaint
that triggered this fix.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(settings-ui): server settings as a tab (matches #1403's pattern)
Restructured the page from a single scrollable column into the
``.tabs`` / ``.panel`` layout PR #1403 uses for its Backups tab,
so the two PRs stack as parallel tabs without HTML/CSS/JS merge
conflicts.
After both merge the user sees three tabs:
Tools | Server Settings | Backups
Specific structural changes:
* H1 generalized: "Tool Settings" → "HA-MCP Settings"
* .tabs / .tab / .panel CSS lifted verbatim from #1403 (same
declarations → identical-block merge resolves automatically)
* Tools content (readonly-notice, pin-notice, restart-notice,
summary, search, groups) wrapped in #panel-tools (active by
default — preserves landing-page behavior).
* Server settings (formerly a collapsible features-panel block
above the tool list) lifted out of the collapsible wrapper and
into #panel-server. The chevron + click-to-expand machinery is
gone; the tab IS the show/hide control now.
* "Permanently disable settings server" button moved into
#panel-server — thematically it belongs with the rest of the
server-management surface.
* Tab-switching JS copied byte-for-byte from #1403 so the merge
treats it as the same block.
JS behavior changes:
* ``saveFeatureFlag()`` no longer toggles the in-tab
``restartNotice`` — that notice lives in #panel-tools and
would be hidden behind a tab the user isn't on after server
saves. The page-level status badge ("Saved — restart
required") is visible across tabs, and the #panel-server
sub-header warns "Changes require an MCP-host restart" up
front, so the in-tab notice is redundant.
* Tool save (saveConfig) keeps its restartNotice toggle —
that flow stays inside #panel-tools where the notice lives.
No test churn — the JS-syntax regression (``node --check`` on
the rendered ``<script>``) still passes, the feature-flag
endpoint coverage is unchanged, and the always-emit
notifications/repairs contract is independent of tab layout.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(settings-ui): point top-of-tools notices at the Server Settings tab
The two notices at the top of the Tools tab still claimed that the
safety toggles + Tool Search "are managed in the add-on configuration
page" — true when the page only existed inside the HA add-on, but
stale after the Server Settings tab made those flags editable in
Claude Desktop / Docker / standalone too.
Reworded both:
* Top notice now: "Server-wide features … live in the Server
Settings tab. Add-on users see those rows as read-only and edit
via the add-on Configuration tab; every other install edits
them directly here. Either path requires an MCP-host restart
to apply." — covers both surfaces honestly and tells the user
which one applies to them.
* Pin notice: dropped the "in the add-on configuration" clause
that suggested Tool Search was add-on-only. The note about
pinning being a no-op without Tool Search stays — that part
didn't change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: review-toolkit findings — atomic writes, self-heal, lock-step logging
PR-review-toolkit raised six legit correctness/UX issues and two test
gaps. Fixed in one commit so reviewers see all of it together.
## Source fixes
* **Atomic ``_atomic_write_0600``** (sidecar): was open+truncate+write
with no rename. A crash mid-write left an empty ``ui.url`` next to
a live ``ui.pid``, the worst-possible state for consumers (read
empty → ``None`` → "no sidecar" while one is still listening).
Now writes to ``<path>.tmp`` then ``os.replace``, atomic on POSIX
and Windows.
* **Stale-PID self-heal** (sidecar): ``_existing_sidecar_alive()``
used to check only ``_pid_alive(pid)``. After PID reuse (crash +
OS reassigns to unrelated process) or crashed-mid-startup (PID
written, URL not), the live-PID check returned True forever and
``maybe_spawn`` permanently refused to spawn. Now also requires
``ui.url`` on disk; missing-URL with live-PID is treated as stale,
WARNING is logged, and the caller respawns.
* **Shutdown rollback on stop() raise** (sidecar): if ``stop()``
raised after the sentinel was written, we returned 200 to the
user while the server kept running with a now-persistent disable
marker — user sees current session intact, next launch refuses
to spawn, no diagnostic. Now wraps ``stop()`` in try/except,
unlinks the sentinel on raise, and returns 500 with the failure
mode in the error message.
* **Override-file read logging** (config): a corrupt or unreadable
``feature_flags.json`` silently fell back to ``{}``, so a user
whose toggles stopped taking effect had no diagnostic. Now splits
failure modes: missing file is silent (normal "never edited"
state), but unreadable/unparseable/non-object now WARNing-log with
the path so ``cat sidecar.log`` tells the user what to fix.
* **Override-apply logging** (config): ``_apply_feature_flag_overrides``
``continue``d silently on bad type, out-of-range int, or
``setattr`` raise — masking exactly the corruption modes the file
layer is meant to defend against. Now logs at WARNING with the
field name and reason. Also broadened the ``setattr`` catch from
``(ValueError, TypeError)`` to ``Exception`` so a weird pydantic
validator raise can never crash every ``get_global_settings()``
consumer.
* **POST corrupt-file refusal + atomic write** (settings_ui): the
feature-flag save handler used to drop to ``existing = {}`` on
``JSONDecodeError`` and then overwrite — silently erasing every
prior toggle persisted before the corruption point. Now returns
409 with a clear message and leaves the file untouched.
``PermissionError`` (raised by an unreadable existing file) now
returns 500 instead of overwriting. The successful write path
also goes tmp + ``os.replace`` to match the sidecar.
## Test fixes
* **Addon-mode short-circuit** (gap T1): new
``TestFeatureFlagAddonMode`` covers both
``get_feature_flag_origin`` returning ``"addon"`` when
``SUPERVISOR_TOKEN`` is set AND
``_apply_feature_flag_overrides`` being a no-op in that mode
(file value is ignored, pydantic default wins).
* **POST validation matrix** (gap T2): six new tests in
``TestFeatureFlagsEndpoint`` covering invalid-JSON body,
body-not-dict, ``flags``-not-dict, unknown field name,
string-for-bool, and bool-for-int. Plus regression tests for the
new corrupt-existing-file refusal and the atomic write (no
``.tmp`` leftover on success).
* **Stale-PID self-heal regression**: new test asserts a PID-live
but URL-missing state respawns AND logs a warning.
* **Shutdown rollback regression**: new test injects a ``stop()``
that raises, asserts 500 + sentinel removed.
* **Override-file read warning regression**: new
``TestFeatureFlagOverrideReadErrors`` asserts missing-file is
silent, corrupt-JSON logs WARNING, non-object root logs WARNING.
## Comment cleanup
Dropped 5 cross-PR references (``BACKUP_OVERRIDE_FIELDS``,
``_apply_backup_overrides``, "PR #1403's Backups tab pattern", etc.)
that would mislead readers — the referenced symbols don't exist
yet, and may not exist by those names if the other PR is renamed
or rebased before merge. Comments now describe the pattern in-line
without naming external symbols.
Also tightened the ``settings_url`` docstring in ``ha_get_overview``
to make the "only when sidecar is running" condition impossible for
an LLM to snip out of context.
158 tests pass locally (1 Windows skip on a POSIX-perms test
unrelated to this PR).
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>1 parent a91f97a commit 6c3c0ac
19 files changed
Lines changed: 3966 additions & 278 deletions
File tree
- .github/workflows
- site/src/pages
- src/ha_mcp
- tools
- tests
- haos_image_build
- src/unit
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
12 | 21 | | |
13 | 22 | | |
14 | 23 | | |
15 | 24 | | |
16 | 25 | | |
17 | 26 | | |
| 27 | + | |
18 | 28 | | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
19 | 33 | | |
20 | 34 | | |
21 | 35 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
22 | 28 | | |
23 | 29 | | |
24 | 30 | | |
| |||
70 | 76 | | |
71 | 77 | | |
72 | 78 | | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
73 | 86 | | |
74 | 87 | | |
75 | 88 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| 28 | + | |
28 | 29 | | |
29 | 30 | | |
30 | 31 | | |
| |||
99 | 100 | | |
100 | 101 | | |
101 | 102 | | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
102 | 114 | | |
103 | 115 | | |
104 | 116 | | |
| |||
475 | 487 | | |
476 | 488 | | |
477 | 489 | | |
478 | | - | |
| 490 | + | |
479 | 491 | | |
480 | 492 | | |
481 | 493 | | |
482 | 494 | | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
483 | 500 | | |
484 | 501 | | |
485 | 502 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
226 | 226 | | |
227 | 227 | | |
228 | 228 | | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
229 | 242 | | |
230 | 243 | | |
231 | 244 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
175 | 175 | | |
176 | 176 | | |
177 | 177 | | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
178 | 191 | | |
179 | 192 | | |
180 | 193 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2253 | 2253 | | |
2254 | 2254 | | |
2255 | 2255 | | |
| 2256 | + | |
| 2257 | + | |
| 2258 | + | |
| 2259 | + | |
| 2260 | + | |
| 2261 | + | |
| 2262 | + | |
| 2263 | + | |
| 2264 | + | |
| 2265 | + | |
| 2266 | + | |
| 2267 | + | |
| 2268 | + | |
| 2269 | + | |
2256 | 2270 | | |
2257 | 2271 | | |
2258 | 2272 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
83 | 83 | | |
84 | 84 | | |
85 | 85 | | |
86 | | - | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
87 | 89 | | |
88 | 90 | | |
89 | 91 | | |
| |||
627 | 629 | | |
628 | 630 | | |
629 | 631 | | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
630 | 638 | | |
631 | 639 | | |
632 | 640 | | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
| 717 | + | |
633 | 718 | | |
634 | 719 | | |
635 | 720 | | |
| |||
875 | 960 | | |
876 | 961 | | |
877 | 962 | | |
| 963 | + | |
878 | 964 | | |
879 | 965 | | |
880 | 966 | | |
| |||
0 commit comments