Skip to content

Commit 6630341

Browse files
fix: correct Tools-tab group master + LLM API toggle rendering (#1859)
* fix: show settings-UI group master toggle as on for all-mandatory groups The per-group master switch in the Tools tab derived its checked state from `anyEnabled`, computed only over *toggleable* tools (non-mandatory, non-env-pinned, non-feature-gated). For a group whose tools are all mandatory — e.g. Search & Discovery (ha_search, ha_get_overview, ha_get_state) — that set is empty, so `anyEnabled` was always false and the switch rendered unchecked+disabled: it read as "the whole group is off" while the count beside it said "3/3 enabled" and the tools cannot be disabled at all. Compute the checked state from the group's real enabled status (`groupEnabled`) when there are no toggleable tools, keeping the existing `anyEnabled` bulk semantics for the interactive case. An all-mandatory group now shows on+disabled; a mixed group keeps its interactive master. Add JSDOM regression tests covering all-mandatory, all-disabled toggleable, and mixed groups. * fix: hide LLM API tool toggle on non-embedded install methods The per-tool "LLM API" toggle offers a tool to Home Assistant conversation agents through the LLM API surface that the ha_mcp_tools custom component registers. That surface only exists on the in-process (embedded) custom-component server; on the add-on, Docker, and standalone servers nothing consumes it, so the toggle was a no-op there yet still rendered for every tool. The /api/settings/tools response now reports `llm_api_available` (= is_embedded()), and the Tools tab renders the LLM API toggle column only when it is true — other install methods drop the column entirely. Add JSDOM coverage (toggle hidden when the flag is absent) and a server-side test that the flag is true under HA_MCP_EMBEDDED. * fix: group master switch reflects fully-enabled state, not any-enabled The non-toggleable branch of the group master switch used `groupEnabled > 0` ("any tool enabled") while the intended invariant is "fully enabled, matching the N/N count". They diverge for a fully-locked group that is only partially enabled — reachable via DISABLED_TOOLS or feature gating (e.g. a mandatory tool beside an env-pinned-off one): the switch rendered checked while the count read "1/2 enabled", the same contradiction this PR fixes, inverted. Use `groupEnabled === tools.length` — identical for every uniform case (all-mandatory, all-env-pinned) and correct for the mixed case. Also tidy the surrounding change: - Extract the LLM API toggle fragment into an `llmToggleHtml` const, matching the gatedNote/envPinnedNote/readOnlyNote idiom in render(). - Drop a duplicated comment and stray RST backticks on llmApiAvailable. - Add regression tests: a partially-enabled locked group (unchecked, fails on `> 0`) and an all-env-pinned-on group (checked); assert the hidden-LLM test against `llm_api_available: False` and the absent key. --------- Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com>
1 parent 294650a commit 6630341

4 files changed

Lines changed: 357 additions & 9 deletions

File tree

src/ha_mcp/settings_ui/__init__.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,6 +1446,12 @@ async def _get_tools(_: Request) -> JSONResponse:
14461446
"read_only_exempt": sorted(READ_ONLY_EXEMPT_TOOLS),
14471447
"llm_api": llm_effective,
14481448
"llm_api_overrides": llm_overrides,
1449+
# LLM API exposure is registered by the ha_mcp_tools custom
1450+
# component (in-process/embedded server). On the add-on,
1451+
# Docker, or standalone server nothing consumes it, so the UI
1452+
# hides the per-tool "LLM API" toggle rather than showing a
1453+
# no-op control.
1454+
"llm_api_available": is_embedded(),
14491455
}
14501456
)
14511457

src/ha_mcp/settings_ui/settings.js

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ let toolEnvPinned = {};
3333
// never flipped keep tracking their defaults across releases.
3434
let toolLlm = {};
3535
let toolLlmOverrides = {};
36+
// True only on the in-process custom-component (embedded) server, which
37+
// registers the LLM API exposure surface. On the add-on / Docker / standalone
38+
// server nothing consumes it, so the per-tool "LLM API" toggle is hidden
39+
// rather than shown as a no-op. Set from data.llm_api_available (see
40+
// settings_ui/__init__.py).
41+
let llmApiAvailable = false;
3642
let saveTimer = null;
3743
let openGroups = new Set();
3844

@@ -169,6 +175,7 @@ async function loadTools() {
169175
toolEnvPinned = data.env_pinned || {};
170176
toolLlm = data.llm_api || {};
171177
toolLlmOverrides = data.llm_api_overrides || {};
178+
llmApiAvailable = !!data.llm_api_available;
172179
READ_ONLY_EXEMPT = new Set(data.read_only_exempt || []);
173180
// Load policy state before the first render so the "security gated"
174181
// toggle reflects current policy.rules. loadPolicyState() never throws
@@ -598,6 +605,19 @@ function render() {
598605
return MANDATORY.includes(t.name) || (!t.disabled_by && s !== 'disabled');
599606
}).length;
600607

608+
// Master-switch checked state. Normally it mirrors "any toggleable tool
609+
// enabled" — it is a bulk control over the tools the user can actually
610+
// flip. But when a group has NO toggleable tools (all mandatory /
611+
// env-pinned / feature-gated), the switch is disabled and purely reflects
612+
// status: show it ON only when the group is FULLY enabled (every tool on),
613+
// matching the "N/N enabled" count. anyEnabled can't be reused here — it is
614+
// always false over an empty toggleable set. So an all-mandatory group
615+
// (e.g. Search & Discovery) reads as ON, while a partially-enabled locked
616+
// group (e.g. a mandatory tool beside an env-pinned-off one) reads as OFF
617+
// rather than contradicting its own "1/N enabled" count.
618+
const masterChecked =
619+
toggleable.length === 0 ? groupEnabled === tools.length : anyEnabled;
620+
601621
const header = document.createElement('div');
602622
header.className = 'group-header';
603623
header.innerHTML = `<div class="group-header-left">` +
@@ -606,7 +626,7 @@ function render() {
606626
`<span class="group-count">${groupEnabled}/${tools.length} enabled</span>` +
607627
`</div>` +
608628
`<label class="switch group-master" title="Enable/disable all tools in this group">` +
609-
`<input type="checkbox" name="tool-group:${escapeHtml(tag)}" ${anyEnabled ? 'checked' : ''} ${toggleable.length === 0 ? 'disabled' : ''}>` +
629+
`<input type="checkbox" name="tool-group:${escapeHtml(tag)}" ${masterChecked ? 'checked' : ''} ${toggleable.length === 0 ? 'disabled' : ''}>` +
610630
`<span class="slider"></span>` +
611631
`</label>`;
612632

@@ -717,6 +737,19 @@ function render() {
717737
: (roExemptActive
718738
? '<div class="feature-locked-note">Read Only Mode: write operations of this tool are blocked; read operations stay available.</div>'
719739
: '');
740+
// LLM API exposure column — rendered only on the embedded custom-component
741+
// server (see llmApiAvailable); dropped elsewhere rather than shown as a
742+
// no-op. Built here as a fragment, matching the *Note consts above.
743+
const llmToggleHtml = llmApiAvailable
744+
? `<div class="toggle-group ${isEnabled ? '' : 'disabled-toggle'}" ` +
745+
`title="Offer this tool to Home Assistant conversation agents through the LLM API. Applies on the agent's next message - no restart. A tool disabled above is unavailable to agents regardless.">` +
746+
`<label class="switch"><input type="checkbox" name="tool:${escapeHtml(t.name)}:llm" data-tool="${escapeHtml(t.name)}" data-field="llm" ` +
747+
`aria-label="${escapeHtml(title)} exposed to the conversation-agent LLM API" ` +
748+
`${(toolLlm[t.name] !== false) ? 'checked' : ''} ${isEnabled ? '' : 'disabled'}>` +
749+
`<span class="slider"></span></label>` +
750+
`<span>LLM API</span>` +
751+
`</div>`
752+
: '';
720753

721754
div.innerHTML = `<div class="tool-info">` +
722755
`<div class="tool-name">${escapeHtml(title)}${badges}</div>` +
@@ -750,14 +783,7 @@ function render() {
750783
`<span class="slider"></span></label>` +
751784
`<span>security gated</span>` +
752785
`</div>` +
753-
`<div class="toggle-group ${isEnabled ? '' : 'disabled-toggle'}" ` +
754-
`title="Offer this tool to Home Assistant conversation agents through the LLM API. Applies on the agent's next message - no restart. A tool disabled above is unavailable to agents regardless.">` +
755-
`<label class="switch"><input type="checkbox" name="tool:${escapeHtml(t.name)}:llm" data-tool="${escapeHtml(t.name)}" data-field="llm" ` +
756-
`aria-label="${escapeHtml(title)} exposed to the conversation-agent LLM API" ` +
757-
`${(toolLlm[t.name] !== false) ? 'checked' : ''} ${isEnabled ? '' : 'disabled'}>` +
758-
`<span class="slider"></span></label>` +
759-
`<span>LLM API</span>` +
760-
`</div>` +
786+
llmToggleHtml +
761787
`</div>`;
762788

763789
const inputs = div.querySelectorAll('input[type="checkbox"]');

tests/src/unit/test_settings_ui.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2577,6 +2577,47 @@ async def test_get_tools_llm_api_map_and_stub_beta_default(
25772577
assert body["llm_api"]["ha_get_state"] is False
25782578
# The stub renders hidden-by-default (feature-gated == beta).
25792579
assert body["llm_api"]["ha_config_set_yaml"] is False
2580+
# Not the in-process custom-component server: the UI hides the LLM API
2581+
# toggle (nothing consumes the exposure on this install method).
2582+
assert body["llm_api_available"] is False
2583+
get_data_dir.cache_clear()
2584+
_reset_global_settings()
2585+
2586+
@pytest.mark.asyncio
2587+
async def test_get_tools_llm_api_available_true_when_embedded(
2588+
self, monkeypatch, tmp_path
2589+
):
2590+
"""On the in-process custom-component (embedded) server the tools
2591+
payload advertises ``llm_api_available: True`` so the UI renders the
2592+
per-tool LLM API toggle."""
2593+
monkeypatch.setenv("HA_MCP_CONFIG_DIR", str(tmp_path))
2594+
monkeypatch.setenv("HA_MCP_EMBEDDED", "1")
2595+
from ha_mcp.utils.data_paths import get_data_dir
2596+
2597+
get_data_dir.cache_clear()
2598+
from ha_mcp import settings_ui as sui
2599+
from ha_mcp.config import _reset_global_settings
2600+
2601+
_reset_global_settings()
2602+
monkeypatch.setattr(
2603+
sui,
2604+
"load_tool_metadata_cache",
2605+
lambda: [
2606+
{
2607+
"name": "ha_get_state",
2608+
"title": "Get State",
2609+
"primary_tag": "Entity",
2610+
"tags": ["Entity"],
2611+
"description": "x",
2612+
"category": "read",
2613+
},
2614+
],
2615+
)
2616+
handlers = sui.build_settings_handlers(server=None)
2617+
resp = await handlers["get_tools"](MagicMock())
2618+
body = json.loads(resp.body)
2619+
2620+
assert body["llm_api_available"] is True
25802621
get_data_dir.cache_clear()
25812622
_reset_global_settings()
25822623

0 commit comments

Comments
 (0)