chore(deps): update ghcr.io/home-assistant/home-assistant docker tag to v2025.12.5 - #2
Merged
kingpanther13 merged 1 commit intoFeb 3, 2026
Conversation
kingpanther13
added a commit
that referenced
this pull request
Feb 4, 2026
* feat: merge labels and voice assistant exposure into ha_set_entity Extend ha_set_entity with two new optional parameters: - labels: list of label IDs (replace/set semantics, consistent with aliases) - expose_to: dict mapping assistant IDs to booleans for voice assistant exposure This enables single-call updates for all entity metadata including area, name, icon, enabled, hidden, aliases, labels, and voice assistant exposure. Add deprecation notices to ha_manage_entity_labels and ha_expose_entity docstrings, pointing users to ha_set_entity for single-entity operations. Includes 14 unit tests covering labels parameter, expose_to parameter, combined operations, partial failure handling, and JSON string parsing. Closes homeassistant-ai#481 https://claude.ai/code/session_01LL5wZ2K7KQUU2AmMyKzNWV * Update src/ha_mcp/tools/tools_entities.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update src/ha_mcp/tools/tools_entities.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * feat: merge labels and voice assistant exposure into ha_set_entity Extend ha_set_entity with two new optional parameters: - labels: list of label IDs (replace/set semantics, consistent with aliases) - expose_to: dict mapping assistant IDs to booleans for voice assistant exposure control Remove ha_manage_entity_labels and ha_expose_entity tools since their functionality is now covered by ha_set_entity. This reduces tool count and cognitive load for AI agents. Update all E2E and unit tests to use ha_set_entity instead of the removed tools. Add 14 new unit tests for the labels and expose_to parameters. Add "Tool Consolidation" principle to CLAUDE.md: remove redundant tools rather than deprecating them. Closes homeassistant-ai#481 https://claude.ai/code/session_01LL5wZ2K7KQUU2AmMyKzNWV * chore: sync AGENTS.md with CLAUDE.md tool consolidation principle https://claude.ai/code/session_01LL5wZ2K7KQUU2AmMyKzNWV * fix: resolve E2E test double-parsing bug in test_label_operations Tests were calling parse_mcp_result() then passing the already-parsed dict to assert_mcp_success(), which internally calls parse_mcp_result() again. A plain dict lacks the .content attribute, causing the second parse to return {"error": "No content in result"}. Fixed by passing raw MCP results directly to assert_mcp_success(), matching the pattern used in other working E2E tests. https://claude.ai/code/session_01LL5wZ2K7KQUU2AmMyKzNWV * fix: use assert_mcp_success for ha_get_state check in integrity test ha_get_state returns {"data": {...}} without a top-level "success" key, so parse_mcp_result + .get("success") always evaluates to None. Use assert_mcp_success which properly validates the response envelope. https://claude.ai/code/session_01LL5wZ2K7KQUU2AmMyKzNWV * fix: move json import to top of file per PEP 8 Move local `import json as _json` to the module level, addressing PR review feedback about import placement. https://claude.ai/code/session_01LL5wZ2K7KQUU2AmMyKzNWV * fix: address PR review — exposure tracking, error handling, and test coverage Addresses all critical, high, and medium issues from code review: - CRITICAL #1: Track per-assistant success/failure in exposure loop. On partial failure, response now includes exposure_succeeded and exposure_failed dicts showing exactly which assistants were updated. - HIGH #2/#7: Handle fetch failure on expose-only path. If entity registry get fails after exposure, return error with exposure_applied instead of silently returning empty entity_entry. - HIGH #3: Wrap coerce_bool_param calls for enabled/hidden in try/except ValueError, returning VALIDATION_INVALID_PARAMETER instead of falling through to generic Exception handler. - MEDIUM #6: Replace inline json.loads with parse_json_param utility, removing the import json dependency. - MEDIUM #8: Standardize no-updates error to use "suggestions" (plural list) matching all other error responses. - MEDIUM #9: Track actual server-confirmed exposures via succeeded dict instead of echoing back user input. - MEDIUM #10: Extract _format_entity_entry helper to eliminate duplicated entity_entry dict construction. Adds 8 new unit tests covering: - Expose-only failure (no partial flag) - Mixed partial failure with succeeded tracking - Expose-only entity not found - Invalid enabled/hidden values ("maybe") - All 3 assistants in single call - List type for expose_to (rejected) - Registry failure with labels https://claude.ai/code/session_01LL5wZ2K7KQUU2AmMyKzNWV * fix: rename exposure_applied to exposure_succeeded for consistency Addresses Gemini code review feedback: standardize response key names for exposure tracking across success and failure paths. https://claude.ai/code/session_01LL5wZ2K7KQUU2AmMyKzNWV * feat: add label_operation and bulk entity support to ha_set_entity Add full feature parity with removed ha_manage_entity_labels tool: - Add label_operation parameter: "set" (default), "add", "remove" - "set": replaces all labels (existing behavior) - "add": adds labels to existing without duplicates - "remove": removes specified labels from existing - Add bulk entity_id support (str | list[str]) - Bulk operations support labels and expose_to parameters only - Single-entity parameters (area_id, name, etc.) blocked for bulk - Parallel processing with asyncio.gather - Aggregated results with success/failure counts - Add _get_entity_labels() helper for fetching current labels - Refactor into _update_single_entity() for cleaner bulk support - Add comprehensive unit tests (31 total, 9 new tests) https://claude.ai/code/session_01LL5wZ2K7KQUU2AmMyKzNWV * test(e2e): add label_operation tests for add and remove Add E2E tests for new label_operation parameter in ha_set_entity: - test_add_labels_to_existing: verify 'add' preserves existing labels - test_remove_specific_labels: verify 'remove' only removes specified - test_add_prevents_duplicates: verify no duplicate labels when adding https://claude.ai/code/session_01LL5wZ2K7KQUU2AmMyKzNWV * chore: update uv.lock for version 6.5.0 https://claude.ai/code/session_01LL5wZ2K7KQUU2AmMyKzNWV * fix: use set for O(1) label removal membership check Convert parsed_labels to set before list comprehension for improved performance when removing multiple labels: O(M+N) vs O(M*N). https://claude.ai/code/session_01LL5wZ2K7KQUU2AmMyKzNWV --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
kingpanther13
added a commit
that referenced
this pull request
Feb 14, 2026
Resolve all merge conflicts from upstream/master (Parts 1-4 merged). Address Gemini review comments: - Context propagation now handled by upstream's context=context params (fixes comments #1, #2, #3 on helpers.py) - Replace asyncio.sleep(1.0) with polling loop in test_entity_rename.py (fixes comment #4) Fix callers that assign exception_to_structured_error() result to add explicit raise_error=False (tools_config_dashboards, tools_search) since the default is now True. Fix duplicate ToolError imports in tools_config_helpers and tools_entities. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kingpanther13
pushed a commit
that referenced
this pull request
Apr 13, 2026
…meassistant-ai#976) * fix(history): add query_params echo to _fetch_statistics response _fetch_history echoes its effective parameters in a query_params block (noted as desirable by maintainers on homeassistant-ai#964). _fetch_statistics had no equivalent, creating an asymmetry between the two source branches of ha_get_history. Adds query_params to the statistics response: - statistic_types: raw caller-supplied value (not resolved default), so the block reflects what was requested, not internal expansion. period is intentionally excluded — period_type at the top level already carries this value; duplication would contradict the stated design principle (time_range excluded for the same reason). Known edge case: statistic_types=[] is falsy, so all_stat_types falls back to the full six-type default while query_params.statistic_types would show []. Pre-existing behavior, not introduced here. Tracked as a follow-up item. Test: test_get_statistics_query_params_in_response asserts query_params present with statistic_types key and period absent. * fix(history): add limit and offset to _fetch_statistics query_params echo Extends the query_params block to include all three caller-controllable parameters, matching _fetch_history symmetry: - statistic_types: raw caller input (not normalized stat_types_list) - limit: effective_limit after coerce_int_param - offset: effective_offset after coerce_int_param Addresses KP13 review Critical #1. * test(history): remove false-green E2E statistics query_params test The test used sun.sun as entity — it has no state_class, so HA recorder generates no long-term statistics. On a stock fixture, inner.get("success") is falsy, the early-exit fires, and assertions never execute (false-green). Also fixes: "year" re-added to valid_periods list in period-iteration test (was incorrectly dropped — production code at L583 already includes it). Unit coverage for query_params echo is added in test_history_pagination.py. Addresses KP13 review Critical #2, #3, Important #6. * test(history): add unit tests for _fetch_statistics query_params echo Adds two tests to TestStatisticsPagination following the established pattern in this file (HistoryTools mock + patch get_connected_ws_client): - test_statistics_query_params_default: asserts statistic_types=None, limit=_DEFAULT_HISTORY_LIMIT, offset=0 on a default call - test_statistics_query_params_roundtrip: asserts exact values when statistic_types=["mean"], limit=10, offset=5 are passed explicitly Both tests call ha_get_history (public tool layer), not _fetch_statistics directly, matching the sibling test_history_pagination.py pattern. Addresses KP13 review Critical #2, #3, Important #4. --------- Co-authored-by: LB-Agent <lb@local>
kingpanther13
added a commit
that referenced
this pull request
May 4, 2026
Addresses Patch76's CHANGES_REQUESTED review on PR homeassistant-ai#1120 plus the audit-table walk-through showed several legit nuggets I'd skipped on the first pass. Adding all of them now. Patch76 #1 — Cloudflared HA-addon `additional_hosts` block: Added a <details> "Running on Home Assistant OS? Use the Cloudflared add-on" section to the existing Cloudflare Tunnel block, with the brenner-tobias add-on badge link and the `additional_hosts:` YAML pointing at port 9583. Patch76 #2 + #3 — github-copilot-agents org-deployment notes: New "Org & Repository Deployment" instruction-block (any transport) documenting: - Repository-wide config via Settings → Copilot → Coding agent → MCP configuration on github.com (applies to all users with repo access, alternative to per-user .vscode/mcp.json) - Operational prerequisite: the "MCP servers" policy must be enabled for the org/enterprise — admins disable it by default. JetBrains — extended clientNote with the "Import from Claude" button tip (Settings → Tools → AI Assistant → MCP Servers) for users migrating from Claude Desktop. Audit-table gaps closed: - Antigravity: new instruction-block with the UI nav steps to the raw config editor (... menu → MCP Servers → Manage MCP Servers → View raw config) plus an HTTP-transport caveat (gated to !isStdio) about "connection closed" / "SSE stream failed to reconnect" errors with a recommendation to switch to stdio. - Codex: extended Management Commands block with a Codex Desktop walkthrough (Settings → MCP → Add Server with field names) and an OAuth-2.0-for-remote-servers note (gated to !isStdio). - VS Code: secure-input-prompts block now also mentions the vscode:mcp/install?<config> deep-link install pattern. - Copilot CLI: Notes & extras section after both the stdio and HTTP step blocks — Server Type legend (1/2/3/4 for Local/STDIO/HTTP/SSE), KEY=VALUE env-var format, * vs comma-list Tools format, COPILOT_HOME override, /mcp interactive command, "GitHub MCP server included by default" reminder. - OpenCode: Management Commands block extended with OPENCODE_CONFIG env-var path override, project-vs-global config-merge precedence (with link to opencode.ai/docs), {env:VAR} headers interpolation pattern for Bearer auth, oauth: false opt-out flag, home-assistant_* tool namespacing, and a 92+-tools context-size warning recommending a dedicated OpenCode agent for HA-heavy workflows. - Webhook Proxy: "How It Works" section before the install steps with the routing chain (AI client → HTTPS → reverse proxy → HA :8123 → webhook /api/webhook/<id> → MCP add-on). Plus a comparison table vs Cloudflare Tunnel (setup/cost/routing/best-for) after the steps. - Continue: clientNote with the Agent-Mode-required gotcha — MCP only works when Continue is in Agent Mode (use the agent selector near the chat input). - Claude.ai: clientNote with the Pro/Max/Team/Enterprise subscription requirement and the "Search and tools" button tip for per-conversation tool toggling. - Claude Code: clientNote noting config changes take effect immediately (no restart needed) — useful contrast with the restart-required clients. - Linux quick-test: new FAQ item ("Test ha-mcp without configuring a client") with the public-demo-server one-liner from the deleted linux.md body. TOC entry added. - AGENTS.md: appended a sentence to the "adding a new entry" recipe noting that arrays should be kept ordered by `order` (the wizard renders in array order without re-sorting). Intentional skips (with reason, in case anyone re-audits): - JetBrains Node 18+ requirement: only relevant for npm-based MCP servers; ha-mcp uses uvx, so this is a non-applicable constraint. - JetBrains 2025.2+ built-in MCP server: about the IDE itself acting as an MCP server, not relevant to ha-mcp client setup. - Zed Bearer-header HTTP shape: Zed is in stdioOnlyClients, so the wizard routes Zed users through mcp-proxy for HTTP — the isZed HTTP branch in the JSON builder is dead code for ha-mcp users going through the wizard. Adding a Bearer-auth alternative would contradict the httpNote. - uvx Python 3.10+ claim: source body was wrong (faq says 3.13+, matches pyproject.toml requires-python = "==3.13.*"). Body deletion auto-resolved. Build verified clean (npm run build, 7 pages, 18.4s).
kingpanther13
added a commit
that referenced
this pull request
May 5, 2026
…s, drop content collections (homeassistant-ai#1120) * feat(site): add transport-agnostic clientNote field, fix multi-line configLocation rendering Two foundational wizard fixes prerequisite for the content-collection cleanup: 1. clientNote (homeassistant-ai#1106 / Patch76 S2): new optional frontmatter field on the clients schema, rendered unconditionally in the wizard's notes pane. Distinct from httpNote, which only renders on non-stdio paths and so misses dependencies that apply across all transports (e.g. github-copilot agents requires the Copilot extension regardless of stdio vs sse). 2. configLocation multi-line rendering (homeassistant-ai#1106 / Patch76 S1): inline <code> collapsed YAML newlines into one unreadable string. Now splits at newline boundaries, rendering as a bulleted list when multi-line and keeping the previous single-<code> form when single-line. Affects github-copilot-agents and claude-desktop frontmatter today. * feat(site): migrate transport-agnostic notes to clientNote, add troubleshooting nuggets Migrates ~20 nuggets from unrendered content-collection bodies into surfaces users actually see (homeassistant-ai#1097, homeassistant-ai#1106). clients/*.md frontmatter — add clientNote (or rename httpNote → clientNote for cases where the dependency applies regardless of transport): - cursor.md: restart-after-config - windsurf.md: restart-after-config - jetbrains.md: IDE 2025.1+ + AI Assistant plugin + restart - vscode.md: GitHub Copilot extension prereq + reload - github-copilot-agents.md: extension + coding-agent-mode (was httpNote, but applies to stdio path too) - open-webui.md: v0.6.31+ MCP support - raycast.md: v1.98.0+/1.100.0+ MCP+HTTP version reqs, Pro/BYOK faq.astro: - new "Antigravity client troubleshooting" item (5 bullets) - new "Claude.ai connection issues" item - new "Keep ha-mcp-web running in the background" item (nohup pattern) - new "Docker port configuration" item (dual-port caveat: -p second number must equal MCP_PORT) - extended existing "uvx not found" item with Claude Desktop PATH not-inherited workaround guide-macos.astro: - new optional Quick Test step using public demo server * feat(site): migrate UI/CLI client wizard nuggets Extends existing wizard branches with content from unrendered bodies (homeassistant-ai#1097, homeassistant-ai#1106). All additions follow the existing structural patterns of the affected branch (extending <ol> for UI clients, pushing instruction-block divs for CLI clients). ChatGPT (UI block, configFormat=ui): - Add 4th step with the per-conversation activation flow: + → More → Developer Mode → enable connector each conversation, root-path "/" requirement, UUID client ID requirement. Raycast (UI block, configFormat=ui): - Add @home-assistant @-mention usage (Quick AI / AI Chat) - Note deeplink-install alternative - Add HTTP-transport block (gated on !isStdio): enable HTTP in AI Settings, httpHeaders Bearer auth shape - Link to manual.raycast.com/model-context-protocol Open WebUI (UI block, configFormat=ui): - Correct nav path: Admin Panel → Settings → Tools → Manage Tool Servers (was Admin Settings → External Tools — body now reflects the current v0.6.31+ flow); call out the user-side "External Tools" so users don't pick the wrong setting - Drop the now-stale "Set Type: MCP (Streamable HTTP)" step (the current flow no longer asks for it per the body) - Add URL discovery patterns block (host.docker.internal, local network, cloudflared) and mcpo proxy note for stdio servers - Link to docs.openwebui.com/features/mcp/ Claude Code (CLI, configFormat=cli): - Push a Management Commands instruction-block: - SSE alternative `claude mcp add --transport sse ...` (gated on !isStdio — primary stays http to match the JSON path's intent) - claude mcp list / claude mcp remove - --scope user note for cross-project sharing Gemini CLI (CLI, configFormat=cli): - Push a Management Commands instruction-block: - SSE alternative gated on !isStdio (primary stays --transport http to match the wizard's JSON config which uses httpUrl / streamableHttp — keeping primary consistent with the other path) - gemini mcp list / remove - /mcp and /mcp refresh slash commands inside the chat session Codex (CLI, configFormat=cli): - Push a Management Commands instruction-block: codex mcp list / get / remove Antigravity (JSON, configFormat=json, isStdio): - New stdio branch in the JSON builder (placed before the generic mcpServers fallback). Sets FASTMCP_SHOW_SERVER_BANNER=false in env to suppress the startup banner that triggers Antigravity's "Unexpected server output" errors. Handles both uvx (env object) and Docker (-e flag in args) shapes. Build verified clean (npm run build, 7 pages, 4.84s). * feat(site): migrate VS Code/github-copilot-agents secure-inputs + OpenCode mgmt cmds Adds 3 alternative-instruction blocks to the JSON-config branch of the wizard (homeassistant-ai#1097, homeassistant-ai#1106). All three follow the existing instruction-block pattern (<div class="instruction-block"> + <h4 class="instruction-title">) used by the CLI clients' Management Commands blocks. VS Code (json/stdio): - "Alternative: Secure Input Prompts" block. Shows the mcp.inputs[] + ${input:ha-url} / ${input:ha-token} variant from vscode.md so users can opt into VS Code's prompt-on-first-use credential UX instead of putting the URL+token in the config file. GitHub Copilot Agents (json): - "Alternative: Secure Input Prompts" block (gated on the client id, any transport). Shows the bare top-level inputs[] + servers{} shape from github-copilot-agents.md. - Includes an inline note explaining the wizard emits the .vscode/mcp.json (repository) shape, and the personal settings.json form just wraps the same content under an extra mcp: { ... } key. OpenCode (json): - "Management Commands & Quirks" block: opencode mcp list / auth / logout / debug, plus the corrective trio for users coming from other clients (top-level mcp not mcpServers, single command array no args field, environment not env). Build verified clean (npm run build, 7 pages, 24.26s). * feat(site): migrate platform/deployment nuggets into wizard Targeted edits to existing wizard branches in setup.astro for nuggets that fit cleanly into the current structure (homeassistant-ai#1097, homeassistant-ai#1106). Linux uvInstall block (platformId === 'linux'): - Add an "Or via your package manager" line citing pacman -S uv (Arch) and apk add uv (Alpine), matching the macOS branch's existing one-liner that lists curl as a fallback. HA Add-on deployment block (derivedDeployment === 'ha-addon'): - Add an intro line stating the auto-discovery / auto-secret / auto-token behaviour so users understand why this path is the easiest. - Refine the "check the logs" step to specify the exact nav path (Settings → Add-ons → Home Assistant MCP Server → Logs) and clarify these are the add-on logs, not main HA logs. - Show an example log line with the actual URL shape so users know what to look for. - Add a tiny footer note: port 9583 default, 128-bit secret path, persisted across restarts. Docker deployment block (derivedDeployment === 'docker'): - Add a "Container management" details block alongside the existing "Production hardening (docker compose)" details: docker logs -f, stop, rm, pull. Same details/summary pattern as the hardening one. Cloudflared persistent-tunnel block (state.proxy === 'cloudflared', inside the existing "Want a permanent URL?" callout): - Append the ~/.cloudflared/config.yml YAML so users have the actual ingress mapping needed to wire the named tunnel to localhost:8086. Keeps the existing 4-command sequence and adds the config file next to it (the body's order: create → config → route → run). Build verified clean (npm run build, 7 pages, 17.18s). * refactor(site): convert content collections from .md (frontmatter-only) to .yaml Closes the loop on homeassistant-ai#1097 / homeassistant-ai#1106. The bodies under site/src/content/{clients,platforms,connections,deployment}/*.md were unrendered (the wizard's only consumer reads c.data via getCollection and never invokes .render() / .body). Once the body-content migration landed in setup.astro, faq.astro, and guide-macos.astro, the .md shell stopped serving any purpose. Converting to .yaml is the honest representation of what these files have always been since the wizard's introduction: pure metadata. No fake markdown wrapper, no dangling --- markers, the file is the data. Astro's content-layer glob loader supports YAML natively, so this is a transparent swap from the consumer's perspective — state.client.id, state.client.configLocation, etc. all resolve the same way. Changes: - 31 .md files → 31 .yaml files (19 clients/, 4 platforms/, 3 connections/, 5 deployment/). Each .yaml is the unwrapped frontmatter content. - content.config.ts: 4 loader patterns updated **/*.md → **/*.yaml (one per collection). - setup.astro: cross-ref comment updated opencode.md → opencode.yaml. Net deletion: ~1900 lines (the unrendered body content) across the two commits that delivered the migration sweep. Build verified clean (npm run build, 7 pages, 4.78s). * docs: add Site Content Collections section to AGENTS.md Documents the post-homeassistant-ai#1097/homeassistant-ai#1106 contract of site/src/content/*/*.yaml so future contributors don't reintroduce unrendered body prose into the metadata files. Names the rendered surfaces where setup content actually lives (setup.astro, faq.astro, guide-*.astro), explains the "don't author body content here" rule, and points back to the issue trail for context. * refactor(site): inline wizard data into setup.astro, drop content collections Per-issue resolution of homeassistant-ai#1097 / homeassistant-ai#1106: the four Astro content collections (clients, platforms, connections, deployment) under site/src/content/ existed solely to feed metadata to setup.astro via getCollection. Body content was unrendered (verified in homeassistant-ai#1097), and after the migration sweep that put the ~110 unique setup-instruction nuggets into setup.astro, faq.astro, and guide-macos.astro (commits 70361d1..1043e3a), the collection files held only frontmatter — pure metadata duplicating what the wizard already needed at hand. This commit collapses the indirection: - All 31 entries (19 clients + 4 platforms + 3 connections + 5 deployments) inlined directly into setup.astro as four pre-sorted JS arrays at the top of the frontmatter block. Same field names, same order, no shape change visible to the wizard's downstream code beyond dropping the intermediate `c.data` accessor (the markup blocks for the picker tiles are updated to use `client.transports` / `client.logo` / etc. instead of `client.data.transports`). - Logos pre-baked through `withBase()` via a single `.map()` so the template doesn't need the helper anymore. - `import { getCollection } from 'astro:content'` removed from setup.astro. - Entire `site/src/content/` directory deleted (4 subdirs, 31 yaml files). - `site/src/content.config.ts` deleted (no collections to define). - AGENTS.md "Site Content Collections" section rewritten as "Setup Wizard" — single source of truth, no separate content-collection layer. Net effect: contributors edit one file (setup.astro) instead of needing to coordinate edits across a metadata yaml + a wizard branch. The unrendered-body trap that homeassistant-ai#1097 was filed about can no longer occur, since there is no body surface to author into. Build verified clean (npm run build, 7 pages, 3.46s). Closes homeassistant-ai#1097. Closes homeassistant-ai#1106. * feat(site): migrate remaining legitimate nuggets into wizard Follow-up to the body-content sweep — items previously marked deferred that on closer review represent real user-facing setup needs (homeassistant-ai#1097, homeassistant-ai#1106). Zed inline data: - Multi-line configLocation (macOS/Linux ~/.config path, Windows %APPDATA%\\Zed\\settings.json, project-specific .zed/settings.json override). Now renders as a 3-bullet list via the Patch76 S1 fix. - New clientNote: "Settings file supports JSON with // comments" (a Zed-specific quirk users coming from strict JSON tools hit). Cloudflare Tunnel proxy block: - Add a one-line intro callout naming the three concrete advantages the body authored: no port forwarding, free tier, works behind CGNAT. Plus the Cloudflare Access-on-top option for SSO/policy. Custom Reverse Proxy block: - Add Caddy-specific note (automatic HTTPS via Let's Encrypt, needs public IP / dynamic DNS for ACME) and Nginx note pointing at certbot for cert issuance. The existing block listed Caddy / Nginx / Traefik in passing but didn't surface their setup differences. Continue (configFormat=yaml, !isStdio): - "Alternative: With Authentication Headers" instruction-block with the requestOptions.headers shape. Continue's primary YAML has no auth slot today, so users with Bearer-protected MCP servers need this variant explicitly. Gemini CLI (configFormat=cli, !isStdio): - "Alternative: With Authentication Headers" instruction-block. The `gemini mcp add --transport http` command has no flag for custom headers, so authenticated remote servers require manual ~/.gemini/settings.json (or .gemini/settings.json project-level) edits. Show the httpUrl + headers JSON shape. Codex (configFormat=cli, !isStdio): - "Alternative: With Authentication Headers" instruction-block with the TOML [mcp_servers.<name>.headers] shape — same rationale as Gemini, codex mcp add has no header flag. - Existing Codex Management Commands block extended with a one- line note that the TOML config file is shared between the CLI and the Codex Desktop app (Settings → MCP). Build verified clean (npm run build, 7 pages, 3.73s). * docs(agents): hoist deprecated content-collection warning to top of section The Setup Wizard section's history line buried the warning that the deprecated site/src/content/*.md path is gone. Future contributors who encounter references to that path in old issues, PRs, comments, or blog posts need to know explicitly that the files no longer exist — that was the original purpose of touching AGENTS.md per homeassistant-ai#1097's resolution discussion. Replaces the trailing one-line history with a callout block at the top of the section: states the path is gone, explains what was deprecated and why, and forbids re-creating the content collection (which is the exact regression mode homeassistant-ai#1097 was filed to prevent). * docs(agents): drop AGENTS.md additions Master AGENTS.md never mentioned site/src/content/*.md files, so the "Setup Wizard" section I'd been editing was net-new content (a warning against a baseline AGENTS.md never documented). The right move is to leave AGENTS.md untouched — anyone investigating an old reference to the deprecated path can read git history or this PR's commit log. Reverts c6957d4 + 480c62c (the AGENTS.md hunk only) + 6f1e4ce in their entirety. * docs(agents): add Setup Wizard section as a pointer for future site work Short navigational note: where the wizard data lives (4 inline arrays at the top of setup.astro), where instruction templates live (JS template literals keyed off state.client.id / etc. in the <script> block), where cross-cutting troubleshooting content goes (faq.astro, guide-*.astro), and the 3-step recipe for adding a new client / platform / connection / deployment. Informational only — no warnings, no deprecation callouts (master never documented the deprecated content-collection path, so flagging its absence here would be inventing context). Future contributors get the breadcrumbs they need without AGENTS.md trying to retroactively police a path that was never documented in the first place. * fix(site): address Gemini review + comment-analyzer findings Six small follow-ups from the review pass on PR homeassistant-ai#1120: 1. Drop the stale `site/src/content/clients/opencode.yaml — keep aligned` cross-reference comment on the isOpenCode stdio branch (Gemini flagged this; the file no longer exists). The remaining comment about the deliberate Docker-vs-uvx asymmetry stays. 2. Gate the github-copilot-agents secure-input-prompts alternative to `&& isStdio`. The block was firing on any transport, but the inputs[]/command/args/env example is stdio-shaped — for HTTP/SSE users the wizard's primary already emits the correct {url, transport:"sse"} form, so the alternative was incoherent. 3. Replace internal "Task 1/2/3" comment prefixes (migration scratchpad numbering) with intent-only descriptions: "VS Code: alternative secure-input-prompts config (stdio path only…)" / "GitHub Copilot Agents: alternative secure-input-prompts config — only the stdio shape uses inputs[]…" / "OpenCode: management commands + corrective notes for users coming from other clients". 4. Soften two negative-existence claims about third-party CLIs (`gemini mcp add` / `codex mcp add` "does not accept custom headers") — these were comment-rot vectors. Now just present the manual config-file path as the route for Bearer-token auth without explicitly comparing it to the CLI flag set. 5. Drop two tautological sub-comments inside the antigravity stdio branch ("Docker: add env var as an additional -e flag" / "uvx: merge into env object"). The code below each was self-evidently doing exactly that. 6. Trim the Antigravity "EOF errors" FAQ bullet from "Use absolute paths… does not resolve relative paths in the same working directory as your shell" to just "Use absolute paths… not relative paths" — matches the source body's wording. The working-directory specifics were authored copy not in the source. Build verified clean (npm run build, 7 pages, 19.09s). * fix(site): docker run command was malformed when secret path enabled The docker-deployment branch built a one-line `docker run` command but constructed the secret-path fragment with a trailing `\<newline>` (intended as a shell-line-continuation) and then `.trim()`-ed only the newline before splicing it back into the same line. The trailing backslash survived into the rendered command: ... -e MCP_SECRET_PATH=/private_xxx \ ghcr.io/... Bash parses `\<space>` as an escaped space, joining the leading whitespace onto the next token — so docker was invoked with an image argument of " ghcr.io/homeassistant-ai/ha-mcp:latest" (note the leading space), which fails with `invalid reference format`. Users who enabled the secret path on the Docker deployment got a broken copy-paste command. Drop the trailing `\\\n` from secretPathEnv (no continuation needed inline) and remove the matching .trim() at the splice site. The emitted command is now: docker run -d --name ha-mcp -p 8086:8086 -e ... -e ... -e MCP_SECRET_PATH=/private_xxx ghcr.io/... Pre-existing bug in master (predates homeassistant-ai#1106). Boy Scout fix bundled here since the surrounding deployment block is being heavily touched by this PR. Build verified clean (npm run build, 7 pages, 41s). * feat(site): close audit-table gaps + Patch76 review Addresses Patch76's CHANGES_REQUESTED review on PR homeassistant-ai#1120 plus the audit-table walk-through showed several legit nuggets I'd skipped on the first pass. Adding all of them now. Patch76 #1 — Cloudflared HA-addon `additional_hosts` block: Added a <details> "Running on Home Assistant OS? Use the Cloudflared add-on" section to the existing Cloudflare Tunnel block, with the brenner-tobias add-on badge link and the `additional_hosts:` YAML pointing at port 9583. Patch76 #2 + #3 — github-copilot-agents org-deployment notes: New "Org & Repository Deployment" instruction-block (any transport) documenting: - Repository-wide config via Settings → Copilot → Coding agent → MCP configuration on github.com (applies to all users with repo access, alternative to per-user .vscode/mcp.json) - Operational prerequisite: the "MCP servers" policy must be enabled for the org/enterprise — admins disable it by default. JetBrains — extended clientNote with the "Import from Claude" button tip (Settings → Tools → AI Assistant → MCP Servers) for users migrating from Claude Desktop. Audit-table gaps closed: - Antigravity: new instruction-block with the UI nav steps to the raw config editor (... menu → MCP Servers → Manage MCP Servers → View raw config) plus an HTTP-transport caveat (gated to !isStdio) about "connection closed" / "SSE stream failed to reconnect" errors with a recommendation to switch to stdio. - Codex: extended Management Commands block with a Codex Desktop walkthrough (Settings → MCP → Add Server with field names) and an OAuth-2.0-for-remote-servers note (gated to !isStdio). - VS Code: secure-input-prompts block now also mentions the vscode:mcp/install?<config> deep-link install pattern. - Copilot CLI: Notes & extras section after both the stdio and HTTP step blocks — Server Type legend (1/2/3/4 for Local/STDIO/HTTP/SSE), KEY=VALUE env-var format, * vs comma-list Tools format, COPILOT_HOME override, /mcp interactive command, "GitHub MCP server included by default" reminder. - OpenCode: Management Commands block extended with OPENCODE_CONFIG env-var path override, project-vs-global config-merge precedence (with link to opencode.ai/docs), {env:VAR} headers interpolation pattern for Bearer auth, oauth: false opt-out flag, home-assistant_* tool namespacing, and a 92+-tools context-size warning recommending a dedicated OpenCode agent for HA-heavy workflows. - Webhook Proxy: "How It Works" section before the install steps with the routing chain (AI client → HTTPS → reverse proxy → HA :8123 → webhook /api/webhook/<id> → MCP add-on). Plus a comparison table vs Cloudflare Tunnel (setup/cost/routing/best-for) after the steps. - Continue: clientNote with the Agent-Mode-required gotcha — MCP only works when Continue is in Agent Mode (use the agent selector near the chat input). - Claude.ai: clientNote with the Pro/Max/Team/Enterprise subscription requirement and the "Search and tools" button tip for per-conversation tool toggling. - Claude Code: clientNote noting config changes take effect immediately (no restart needed) — useful contrast with the restart-required clients. - Linux quick-test: new FAQ item ("Test ha-mcp without configuring a client") with the public-demo-server one-liner from the deleted linux.md body. TOC entry added. - AGENTS.md: appended a sentence to the "adding a new entry" recipe noting that arrays should be kept ordered by `order` (the wizard renders in array order without re-sorting). Intentional skips (with reason, in case anyone re-audits): - JetBrains Node 18+ requirement: only relevant for npm-based MCP servers; ha-mcp uses uvx, so this is a non-applicable constraint. - JetBrains 2025.2+ built-in MCP server: about the IDE itself acting as an MCP server, not relevant to ha-mcp client setup. - Zed Bearer-header HTTP shape: Zed is in stdioOnlyClients, so the wizard routes Zed users through mcp-proxy for HTTP — the isZed HTTP branch in the JSON builder is dead code for ha-mcp users going through the wizard. Adding a Bearer-auth alternative would contradict the httpNote. - uvx Python 3.10+ claim: source body was wrong (faq says 3.13+, matches pyproject.toml requires-python = "==3.13.*"). Body deletion auto-resolved. Build verified clean (npm run build, 7 pages, 18.4s). * fix(site): address idiot-check findings on third-party UI/path claims Five accuracy fixes flagged by a fresh-eyes pass on the audit-table-gap commit (11d15d2). All are doc-text corrections, no logic changes. 1. Cloudflared HA-addon `additional_hosts.service` — `localhost` was wrong. The Cloudflared add-on tunnels from inside its own container, so `localhost` doesn't reach the MCP add-on. Switched the example to `homeassistant.local:9583` and the footnote to spell out that users should match whatever IP/hostname they see in the MCP add-on logs (since the working value depends on their HA networking). 2. github-copilot-agents Repository-wide config menu path — added a parenthetical noting the node is sometimes labeled "Cloud agent" depending on UI version (GitHub renamed it; both are in the wild). 3. github-copilot-agents org policy name — was `"MCP servers"`, the canonical full label is `"MCP servers in Copilot"`. Also softened the unsourced "admins disable it by default" assertion to "your admin may have it scoped or disabled." 4. JetBrains AI Assistant menu label — official path is `Settings → Tools → AI Assistant → Model Context Protocol (MCP)`, not "MCP Servers". Updated both configLocation and the clientNote "Import from Claude" reference. 5. Claude Code clientNote — "no restart needed" overstated it. New servers added via `claude mcp add` may not appear in an active session until /mcp reconnect or a new session. Reworded to reflect actual behaviour. Build verified clean (npm run build, 7 pages, 18.8s). --------- Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com>
kingpanther13
added a commit
that referenced
this pull request
May 23, 2026
…t infrastructure Outcome of going through all 60 findings from Gemini Code Assist + pr-review-toolkit (code-reviewer, pr-test-analyzer, silent-failure- hunter, comment-analyzer). 3 wrong (skipped: PATH-resolved node binary mis-flagged as hardcoded; project-relative esbuild path mis-flagged as hardcoded; theoretical FakeBroadcastChannel constructor-throw). Remaining real items addressed: Harness: - vm.runInContext replaces window.eval / indirect eval to clear the "no eval()" style-guide flag (Gemini #1, #2). - Timer-callback and broadcast-listener throws now record into the errors list instead of being silently swallowed (homeassistant-ai#30, homeassistant-ai#32). - SAFETY_CAP exhaustion records a clear "runaway setInterval" error instead of breaking silently (#4, homeassistant-ai#31). - Non-navigation jsdomErrors route to errors (not console) so tests asserting `not result.errors` catch them (homeassistant-ai#38). - Transpile failure short-circuits init eval to avoid cascading syntax errors from un-transpiled TS (homeassistant-ai#34). - FakeBroadcastChannel.postMessage now delivers to peer same-name channels in the same context per spec (#5). - Time-faked surface documented accurately (Date.now / setTimeout / setInterval only; new Date / performance.now still wall-time) (homeassistant-ai#46). - New broadcastChannelUnavailable param simulates the `typeof BroadcastChannel === 'undefined'` browsing context so the production null-guard branch is exercised (homeassistant-ai#15). - Dead comments and rot-prone duplications removed (homeassistant-ai#47, homeassistant-ai#49, homeassistant-ai#51, homeassistant-ai#56, homeassistant-ai#57, homeassistant-ai#58, homeassistant-ai#59, homeassistant-ai#66). extract_astro_vars.mjs: - vm.runInContext replaces (0, eval) (Gemini #2). - Multi-line `import { a, b } from 'x';` now stripped robustly (#7). - Eval errors wrapped with the source path for actionable failures (homeassistant-ai#35). _js_harness.py: - Wrong test file name and workflow path in docstring fixed (homeassistant-ai#41, homeassistant-ai#42). - _strip_astro_frontmatter raises ValueError when frontmatter opens but never closes (homeassistant-ai#36). - discover_script_surfaces raises when site/src/ is missing instead of silently producing partial results (homeassistant-ai#37). - extract_script_body accepts source_label for actionable errors (homeassistant-ai#40). - Astro `<script lang="js">` is no longer mis-tagged as TypeScript (#9). - Inert chr(92) Windows backslash replace removed (homeassistant-ai#14). - Field docstrings on ScriptSurface trimmed to the one that earns its keep (homeassistant-ai#52). - _PY_RENDERERS registry refactor + accurate enumeration comment (homeassistant-ai#45). test_settings_ui_js_behavior.py: - Rot-bait PR/issue numbers removed from module docstring (homeassistant-ai#43). - _TOP_LEVEL_ELEMENT_IDS + import-time drift check replaces the "refresh this manually" comment (homeassistant-ai#55). - _assert_clean_init helper called at the top of every test so init failures surface as init errors, not as misleading "side effect didn't fire" failures (homeassistant-ai#33). - 4xx restartBtn assertion now reads disabled state via JS and snaps to body.dataset instead of OR-shortcircuiting against a wiped DOM (homeassistant-ai#27). - New test_script_boots_without_broadcastchannel_global covers the null-guard branch (homeassistant-ai#15). - Assertion-restating comments removed (homeassistant-ai#60). test_astro_setup_js_behavior.py: - Rot-bait homeassistant-ai#1422 reference removed from module docstring (homeassistant-ai#44). - _section_has_hidden_class replaces fragile substring slicing (#6). - test_initial_state_only_client_section_visible now asserts on the promised visibility, not just absence of errors (homeassistant-ai#26). - Per-client smoke now captures config-output text AND instructions HTML into body.dataset and asserts on non-empty content, catching a typo that drops the whole per-client branch (homeassistant-ai#21). test_astro_tools_js_behavior.py: - _card_class helper replaces ±200-char substring slicing (#12). - test_design_mode_toggle now asserts design-only elements lose 'hidden' class, not just the button label flip (homeassistant-ai#25). - New tests cover .filter-btn / .cat-btn / .size-filter-btn / group-category|file|none / sort-alpha / expand-all wiring (homeassistant-ai#22, homeassistant-ai#23, homeassistant-ai#24) — the adjacent coverage gaps issue homeassistant-ai#1422 didn't name but that fit the harness's same regression-class. test_consent_form_js_behavior.py: - _build_form_dom docstring fixed (said "three", listed four) (homeassistant-ai#54). test_rendered_scripts_parse.py: - Missing-dependency skip flips to fail when CI=true so a workflow drift that drops the install step doesn't silently lose parse coverage (homeassistant-ai#29). - Subsumed-test-class reference removed from module docstring. AGENTS.md: - "60s probe windows take milliseconds" wording fixed; time-faked surface documented (homeassistant-ai#13). - Per-surface module naming guidance updated; reflects actual files (#10). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kingpanther13
added a commit
that referenced
this pull request
May 24, 2026
…ge for every rendered <script> (homeassistant-ai#1425) * test(internal): JSDOM behaviour harness + auto-discovery parse coverage for every rendered <script> Closes homeassistant-ai#1422. Adds a JSDOM harness (tests/js/harness.mjs + tests/src/unit/_js_harness.py) that drives real rendered <script> bodies through stubbed fetch / BroadcastChannel / virtual timers / DOM and reports observed side effects. A discovery walker auto-picks-up every <script> surface in the repo (src/ha_mcp/settings_ui.py, src/ha_mcp/auth/consent_form.py, every site/src/**/*.astro) so parse coverage extends as new UI surfaces ship — no registration needed. Behavioural coverage landed for the surfaces named in homeassistant-ai#1422: * settings_ui — restartInProgress concurrency guard, 4xx-suppress-reload branch, 5xx fall-through, instance_id-flip probe, BroadcastChannel restart-required + restart-initiated listeners, saveFeatureFlag JSON-parse fallback. * setup.astro — state-machine progression (local / network / remote), plus a parametrised per-client smoke that drives the wizard to config generation for every id in the real clientsData array. * tools.astro — search/filter pipeline + design-mode toggle (TypeScript; esbuild strips types in the harness before eval). * Layout.astro — copy-button idempotency across re-init. * consent_form — submit handler disable + spinner state. The legacy TestRenderedHTMLJsSyntax in test_settings_ui.py is removed — the auto-discovery parse test in test_rendered_scripts_parse.py subsumes it (and extends to the four other surfaces it never covered). CI: unit-tests job in pr.yml installs nodejs + jsdom + esbuild via apt-get / npm ci. Local devs without tests/js/node_modules/ get clean skips, matching the original parse guard's behaviour. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): skip Astro frontmatter in script extraction; drain microtasks before clock advance CI surfaced two harness bugs the local smoke-tests didn't catch: 1. `extract_script_body` and the discovery walker greedily matched the first `<script>` substring in the source, which in setup.astro is actually a frontmatter comment: `// below in the <script> block keyed off the entry's id.` That made the "script body" start mid-frontmatter and the extracted text wasn't valid JS — esbuild and JSDOM both rejected it with "Unexpected identifier 'keyed'". Fix: strip the `--- ... ---` Astro frontmatter block before searching for `<script>` tags. Plain .py and .html sources have no frontmatter and pass through unchanged. 2. `clock.advance(settleMs)` returned immediately when no timers were yet scheduled, but the script under test often awaits a chain of stubbed-fetch promises BEFORE hitting its first `setTimeout`. With only one microtask drain between eval and advance, those promises hadn't resolved yet, so no timers existed, advance was a no-op, and the script stayed suspended — `restartAddon`'s POST to /api/settings/restart never fired and the `alert(msg)` in the 4xx branch never ran. Fix: drain microtasks aggressively at the start of advance() so pending promises get to schedule their timers, and drain again when the timer queue temporarily empties (a promise resolution may queue new timers). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(internal): expand initial DOM fixtures to cover every top-level addEventListener target CI surfaced this via the test_5xx test (the only one whose assertion included the harness errors list): the settings_ui script aborts during init at `document.getElementById('backupRefresh').addEventListener(...)` because the test DOM is missing the backup table / modal markup. With init aborted, the invoke step never runs — `restartAddon` is never called, POSTs never fire, `alert()` never runs, and all three restart- flow tests silently fail. The setup.astro tests had the same shape: `generateConfig` queries `config-section` (distinct from `section-config`) to show/hide the inner code block. Without it, the proxy click handler in the remote- flow test threw and the `document.body.dataset.beforeProxy` assignment never landed. Fixes: - settings_ui MIN_DOM now includes backupBulkDelete, backupDomain, backupEntity, backupList, backupRefresh, backupState, featuresBody, modalBackdrop / modalBody / modalClose / modalTitle. Set built from `grep -h "document.getElementById" settings_ui.py` so future top-level handlers will surface as the same pattern. - setup.astro DOM now includes config-section alongside section-config. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): run JSDOM eval at global scope; capture body attrs in dom snapshot Two harness bugs the prior CI rounds didn't surface until init-stage crashes were resolved: 1. Wrapping the rendered script in an `async () => { ... }()` IIFE confined top-level `function` declarations to the IIFE scope. `function restartAddon() {...}` never landed on `window`, so `invoke: "window.restartAddon();"` threw `is not a function`. A real browser hoists inline-script function decls to the global window — match that by running prelude + script body at global scope and keeping the IIFE for `invoke` alone (so awaits inside invoke still work). 2. `document.body.innerHTML` returns body's children but not body's own attrs, so tests that wrote `document.body.dataset.foo = 'bar'` as a side-channel for in-page state had no way to assert on it — `result.dom` came back without the attr. Serialise `document.documentElement.outerHTML` instead so html/head/body tags and their own attributes round-trip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(internal): sequence /api/settings/info responses for 5xx restart probe The test_5xx flow hits the info endpoint three times — loadTools init, restartAddon's pre-POST baseline capture, and _probeAddonRestarted after the POST. The old single-response fixture returned the SAME instance_id every time, so the probe never saw the flip and looped until timeout, leaving reloads=0. Adds a `responses: [...]` shape to the harness fetch_map: each match on a URL pattern advances a per-pattern counter; the last entry sticks after exhaustion (matches "the addon came back online and stays online"). Test now provides baseline → baseline → flipped so the probe terminates with restarted=true and the reload fires. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci(test): cache apt downloads and node_modules for the unit-tests job The unit-tests "Install git and Node.js" step was 44 s — almost all of it network download of the nodejs / npm .deb. The "Install JS test dependencies" step is 1 s when node_modules is fresh but can grow as deps change. - Cache /var/cache/apt/archives keyed on a stable string (apt package set rarely changes). Disable docker-clean and set Keep-Downloaded- Packages so the cached .debs survive install for the next run. apt install still runs (unpacks from local cache, ~3-5 s) but skips the network leg. - Cache tests/js/node_modules keyed on package-lock.json so dep bumps invalidate cleanly. `npm ci` short-circuits when the tree matches. Expected first-cold-cache run: unchanged (~45 s install). Cache hits: ~5 s for both steps combined. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(internal): address Gemini + pr-review-toolkit findings on JS test infrastructure Outcome of going through all 60 findings from Gemini Code Assist + pr-review-toolkit (code-reviewer, pr-test-analyzer, silent-failure- hunter, comment-analyzer). 3 wrong (skipped: PATH-resolved node binary mis-flagged as hardcoded; project-relative esbuild path mis-flagged as hardcoded; theoretical FakeBroadcastChannel constructor-throw). Remaining real items addressed: Harness: - vm.runInContext replaces window.eval / indirect eval to clear the "no eval()" style-guide flag (Gemini #1, #2). - Timer-callback and broadcast-listener throws now record into the errors list instead of being silently swallowed (homeassistant-ai#30, homeassistant-ai#32). - SAFETY_CAP exhaustion records a clear "runaway setInterval" error instead of breaking silently (#4, homeassistant-ai#31). - Non-navigation jsdomErrors route to errors (not console) so tests asserting `not result.errors` catch them (homeassistant-ai#38). - Transpile failure short-circuits init eval to avoid cascading syntax errors from un-transpiled TS (homeassistant-ai#34). - FakeBroadcastChannel.postMessage now delivers to peer same-name channels in the same context per spec (#5). - Time-faked surface documented accurately (Date.now / setTimeout / setInterval only; new Date / performance.now still wall-time) (homeassistant-ai#46). - New broadcastChannelUnavailable param simulates the `typeof BroadcastChannel === 'undefined'` browsing context so the production null-guard branch is exercised (homeassistant-ai#15). - Dead comments and rot-prone duplications removed (homeassistant-ai#47, homeassistant-ai#49, homeassistant-ai#51, homeassistant-ai#56, homeassistant-ai#57, homeassistant-ai#58, homeassistant-ai#59, homeassistant-ai#66). extract_astro_vars.mjs: - vm.runInContext replaces (0, eval) (Gemini #2). - Multi-line `import { a, b } from 'x';` now stripped robustly (#7). - Eval errors wrapped with the source path for actionable failures (homeassistant-ai#35). _js_harness.py: - Wrong test file name and workflow path in docstring fixed (homeassistant-ai#41, homeassistant-ai#42). - _strip_astro_frontmatter raises ValueError when frontmatter opens but never closes (homeassistant-ai#36). - discover_script_surfaces raises when site/src/ is missing instead of silently producing partial results (homeassistant-ai#37). - extract_script_body accepts source_label for actionable errors (homeassistant-ai#40). - Astro `<script lang="js">` is no longer mis-tagged as TypeScript (#9). - Inert chr(92) Windows backslash replace removed (homeassistant-ai#14). - Field docstrings on ScriptSurface trimmed to the one that earns its keep (homeassistant-ai#52). - _PY_RENDERERS registry refactor + accurate enumeration comment (homeassistant-ai#45). test_settings_ui_js_behavior.py: - Rot-bait PR/issue numbers removed from module docstring (homeassistant-ai#43). - _TOP_LEVEL_ELEMENT_IDS + import-time drift check replaces the "refresh this manually" comment (homeassistant-ai#55). - _assert_clean_init helper called at the top of every test so init failures surface as init errors, not as misleading "side effect didn't fire" failures (homeassistant-ai#33). - 4xx restartBtn assertion now reads disabled state via JS and snaps to body.dataset instead of OR-shortcircuiting against a wiped DOM (homeassistant-ai#27). - New test_script_boots_without_broadcastchannel_global covers the null-guard branch (homeassistant-ai#15). - Assertion-restating comments removed (homeassistant-ai#60). test_astro_setup_js_behavior.py: - Rot-bait homeassistant-ai#1422 reference removed from module docstring (homeassistant-ai#44). - _section_has_hidden_class replaces fragile substring slicing (#6). - test_initial_state_only_client_section_visible now asserts on the promised visibility, not just absence of errors (homeassistant-ai#26). - Per-client smoke now captures config-output text AND instructions HTML into body.dataset and asserts on non-empty content, catching a typo that drops the whole per-client branch (homeassistant-ai#21). test_astro_tools_js_behavior.py: - _card_class helper replaces ±200-char substring slicing (#12). - test_design_mode_toggle now asserts design-only elements lose 'hidden' class, not just the button label flip (homeassistant-ai#25). - New tests cover .filter-btn / .cat-btn / .size-filter-btn / group-category|file|none / sort-alpha / expand-all wiring (homeassistant-ai#22, homeassistant-ai#23, homeassistant-ai#24) — the adjacent coverage gaps issue homeassistant-ai#1422 didn't name but that fit the harness's same regression-class. test_consent_form_js_behavior.py: - _build_form_dom docstring fixed (said "three", listed four) (homeassistant-ai#54). test_rendered_scripts_parse.py: - Missing-dependency skip flips to fail when CI=true so a workflow drift that drops the install step doesn't silently lose parse coverage (homeassistant-ai#29). - Subsumed-test-class reference removed from module docstring. AGENTS.md: - "60s probe windows take milliseconds" wording fixed; time-faked surface documented (homeassistant-ai#13). - Per-surface module naming guidance updated; reflects actual files (#10). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): seed data-transports on wizard tiles; add NODE/ESBUILD env overrides; FakeBroadcastChannel ctor guard CI surfaced a real test-fixture bug exposed by the new jsdomError → errors routing: setup.astro's connection-click handler reads `card.dataset.transports` via JSON.parse, but the wizard DOM stubs were emitting `<button data-client="...">` without the matching `data-transports` attribute. JSON.parse(undefined) threw "undefined is not valid JSON" on the jsdomError channel, which the previous silent-handling code dropped — now correctly surfaced as a test failure. Fix: serialise the real `transports` array from the clientsData entry onto each tile. Also addressing the items previously marked deferred / skipped during the Gemini + pr-review-toolkit triage: - NODE_BINARY env override (Gemini #3): _node_binary() helper checks the env var before falling back to PATH-resolved `node`. Default unchanged. - ESBUILD_BINARY env override (Gemini #8): _esbuild_binary() returns the env-var path when set, else the project-local install. Default unchanged so the lockfile-pinned install stays the reproducible default. - FakeBroadcastChannel constructor guard (sf-hunter #L1): wraps the `new` in try/catch and records construction failures into errors before re-raising. - Trim TestWizardStateMachine class docstring (comment-analyzer homeassistant-ai#50). - Tighten the info-call enumeration comment in the 5xx test to describe the harness's "last entry sticks" semantics rather than pinning a specific call count (comment-analyzer homeassistant-ai#48). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): stub layout-dependent JSDOM APIs (scrollIntoView, scrollTo, matchMedia) The new timer-callback-error routing surfaced a real JSDOM limitation: `section.scrollIntoView()` (called from the wizard's `scrollToSection` helper inside a setTimeout) is not implemented in JSDOM. Every per-client setup-flow test failed with ``timer callback: TypeError: section.scrollIntoView is not a function`` — production behaviour is fine, but the harness's noise filter wasn't distinguishing real script bugs from JSDOM-missing-API noise. Adds a defensive no-op stub for scrollIntoView (Element + HTMLElement prototypes), scrollTo on window, and matchMedia — the three most common layout-dependent APIs production UI scripts touch. Future rendered scripts that lean on other layout APIs (IntersectionObserver, etc.) can extend the list when needed. Also relaxes the per-client smoke's bare `assert not result.errors` to rely on `_assert_clean_init` (init/transpile/invoke/jsdom errors) plus the content-shape assertion. Timer-callback errors from missing JSDOM APIs are noise; the content-shape check still catches the regression class the test is named for. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): seed .tool-chevron on tool-card fixture for tools.astro expand-all test The expand-all handler queries `card.querySelector('.tool-chevron')!` (TypeScript non-null assertion). The runtime `!` doesn't actually check; chevron is null in the test DOM and `chevron.classList.add(...)` throws. Production cards include the chevron; our fixture didn't. Add it alongside `.tool-details` in `_build_tools_dom`. 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>
kingpanther13
added a commit
that referenced
this pull request
May 25, 2026
…d findings (homeassistant-ai#1164) Round-3 review pass landed 13 verified findings; the cosmetic "persisted-value visible in UI after F5" item was explicitly skipped per user direction (UI shows post-gate value when master off; user accepts this since beta tools are actually disabled at runtime — the preserve-across-master-cycle UX is at the data layer, not the visual). Source fixes: - **Save button copy is now source-blind** — the previous "your feature-flag toggles already saved on click" claim only held when ``saveFeatureFlag`` raised ``restartNotice``; tool-config pin saves, backup-config saves, and cross-tab ``restart-required`` broadcasts also raise it. New copy: "a restart is pending. Click Restart above to apply your prior changes." (#2) - **Stale F.37 test docstring** describing the deleted server-side cascade rewritten. (#3) - **Beta-gate INFO log noise** — cascade-clear removal meant the gate could fire its "forcing %s=False" line every Settings rebuild, spamming addon logs once a user had truthy sub-flags persisted. Dedup via ``_BETA_GATE_LOGGED`` set per process, cleared on ``_reset_global_settings``. (#9) - **Lazy-lock docstring** updated to reflect Python 3.13 semantics (``asyncio.Lock()`` no longer takes a loop arg; the lazy pattern still serves test fixtures and single-loop deployment, with the invariant documented). (#10) - **Addon-mode carve-out comment** clarified to distinguish dev (master in schema) from stable (master web-UI-only). (homeassistant-ai#14) - **probe-div null branch** in F.37 now writes ``data-error`` so a failing test points at "selector missed" vs "value flipped" unambiguously. (homeassistant-ai#17) Tests added: - ``test_translations_cover_every_schema_key`` — parity check that every ``schema:`` key has a non-empty translation ``name`` and ``description``. Parameterised across stable + dev addons. Pins the class of silent gap that this PR's ``advanced_debug_logging`` fix addressed. (#4) - ``test_save_features_acquires_override_file_lock`` + ``test_save_advanced_acquires_override_file_lock`` — counting-lock wrapper asserts ``async with _get_override_file_lock()`` runs exactly once in each file-mode write path. Pin against a regression that silently bypasses concurrent-save serialisation. (#5) - ``test_dual_save_buttons_mirror_disabled_and_status_on_post_failure`` — exercises the 500-response branch of the dual-save mirror so a regression that broke ``_setAdvSaveStatus``/``_setAdvSaveDisabled`` for error paths only would still fail. (#6) - ``test_save_features_master_on_restores_subflag_values_in_addon_mode`` — addon-mode round-trip mirror of the existing standalone restore test; asserts the Supervisor merge-and-post call carries only the master flip-on and never zeroes out sub-flag values. (#7) - ``test_save_button_nothing_to_save_when_no_dirty_and_no_restart`` + ``test_save_button_restart_pending_hint_when_dirty_empty_but_restart_showing`` — both branches of the empty-dirty Save click are exercised; the restart-pending branch asserts the copy is source-blind. (#8) Deferred per user direction: - #1 (file-vs-Settings visual after F5): user accepts the current behavior (UI shows post-gate value; runtime tools actually disabled when master off; data-layer preserve still works end-to-end). - #11/#12/homeassistant-ai#13 (code-simplifier helper extractions): skipped as complicated to implement without behavior risk. - homeassistant-ai#15 (pre-homeassistant-ai#1164 users with already-cleared sub-flags): release-note concern, not a code change. - homeassistant-ai#16 (lock fragility under future thread-pool dispatch): speculative future-risk; not actionable today.
kingpanther13
added a commit
that referenced
this pull request
May 27, 2026
…t rot) Codex findings: - #2: Bump custom_components/ha_mcp_tools manifest 0.4.0 -> 0.5.0 and detect old installed components explicitly. ha-mcp's bootstrap helper now pre-flights /api/services and raises a structured COMPONENT_NOT_INSTALLED ToolError with an "update via HACS" message if get_caller_token is missing, rather than landing in the generic "no usable token" branch downstream. Closes the unclear-error case where a stale component install would otherwise fail downstream with a confusing 400. - #3: Filesystem wrappers (ha_list_files, ha_read_file, ha_write_file, ha_delete_file) now raise ToolError on success=false like ha_config_set_yaml already does, instead of returning an error-shaped "successful" result. E2E tests using safe_call_tool continue to work because that helper unwraps ToolError back to a dict. - #4: test_registers_tools_when_enabled was vacuous - it mocked mcp.tool (legacy decorator) while production uses mcp.add_tool (via register_tool_methods in helpers.py). Test rewritten to assert mcp.add_tool fired 4 times with the expected method names. Also adds test_raises_component_too_old_when_bootstrap_service_missing covering the new bootstrap pre-flight rejection path. Two existing tests updated to give their mocks the new get_services() shape. Not pushing the HAOS E2E (inaddon) failure as a code change - it's a "Client is not connected" cascade on one worker (gw1) that started before the source-refresh test ran, with the previous PR commit passing the same inaddon CI. Almost certainly flake. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kingpanther13
added a commit
that referenced
this pull request
May 28, 2026
…+ ha_call_service refusal) (homeassistant-ai#1459) * feat(ha_mcp_tools): require caller token + refuse ha_call_service domain bypass The five ha_mcp_tools.* services (list_files, read_file, write_file, delete_file, edit_yaml_config) registered into HA's service registry are callable by anything authenticated as HA admin — Developer Tools, automations, scripts, REST API, other integrations, conversation agents, and the ha_call_service MCP tool itself. That last path is the bypass skialpine reported in issue homeassistant-ai#1451: even with the ha-mcp wrapper toggle flipped off, a sufficiently determined LLM can call ha_call_service directly to land yaml edits. Two-part fix: 1. Custom component handlers now require a caller-token field (`_ha_mcp_token`). Generated by secrets.token_urlsafe(32) on first async_setup_entry, persisted to .storage/ha_mcp_tools_auth, kept in hass.data for fast handler access. Each dangerous-service handler gates on secrets.compare_digest at the top; mismatch returns a structured `error_code=unauthorized` response. A new `ha_mcp_tools.get_caller_token` service exposes the token to the ha-mcp server's bootstrap fetcher (admin-auth-only, like every other ha_mcp_tools service). 2. ha-mcp wrapper tools (filesystem.* and yaml_config) now route through a new `call_mcp_tools_service` helper that fetches + caches the token, injects it on every call, and refetches once on `error_code=unauthorized` (handles token rotation). `ha_call_service` itself rejects domain=ha_mcp_tools with a ToolError that points the LLM at the dedicated wrappers. Threat model: the token is admin-readable via the bootstrap service, so this isn't a defense against a compromised HA admin token. It is defense against the casual-caller path: automations, Lovelace, the built-in conversation agent, other integrations, and other MCP servers will not "fetch token, then call" — they go straight to the service registry and get rejected. The only legitimate caller (ha-mcp) does the bootstrap automatically; no user-facing config required. Token leak tolerance is explicitly accepted: anyone with HA admin auth already had write access to /config/www via the existing service registry, so an admin-readable token doesn't widen exposure. Tests: new tests/src/unit/test_caller_token_auth.py covers the handler check (5 cases), the unauthorized response shape (3 cases), the wrapper bootstrap/cache/refetch flow (4 cases), and the ha_call_service refusal of ha_mcp_tools domain (2 cases). Existing yaml_config tool tests updated to account for the bootstrap fetch call. test_custom_component_filesystem.py gains the homeassistant.helpers.storage stub now needed for module import. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fixup: address Gemini review + CI failures + clarify threat table Gemini code review findings (all applied): - Defensive type-check before .get()/.pop() on hass.data[DOMAIN] in three places (_caller_token_ok, get_caller_token handler, async_unload_entry). Only our code writes to the slot, but the isinstance guard is one line and keeps the call sites consistent. - Unify _CALLER_TOKEN_CACHE on int keys (id(client)) to match _CALLER_TOKEN_LOCKS — removes the str() conversion that had no reason to be there. Updated the cache-seeding test to match. CI failures (both green now): - Unit Tests: 3 dashboard tests in test_yaml_dashboards.py called the edit_yaml_config handler directly and didn't supply the token. Updated both class-scoped hass fixtures to seed hass.data and both call_factory fixtures to auto-inject _ha_mcp_token so all 13+ existing tests pass transparently. Added the missing homeassistant.components / persistent_notification / helpers.storage module stubs (the new __init__.py imports those). - Ruff Lint: ruff format --check found 4 files needing format. Reformatted; ruff check + format --check both pass on the touched files now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fixup: address Codex review (component version + raise-on-error + test rot) Codex findings: - #2: Bump custom_components/ha_mcp_tools manifest 0.4.0 -> 0.5.0 and detect old installed components explicitly. ha-mcp's bootstrap helper now pre-flights /api/services and raises a structured COMPONENT_NOT_INSTALLED ToolError with an "update via HACS" message if get_caller_token is missing, rather than landing in the generic "no usable token" branch downstream. Closes the unclear-error case where a stale component install would otherwise fail downstream with a confusing 400. - #3: Filesystem wrappers (ha_list_files, ha_read_file, ha_write_file, ha_delete_file) now raise ToolError on success=false like ha_config_set_yaml already does, instead of returning an error-shaped "successful" result. E2E tests using safe_call_tool continue to work because that helper unwraps ToolError back to a dict. - #4: test_registers_tools_when_enabled was vacuous - it mocked mcp.tool (legacy decorator) while production uses mcp.add_tool (via register_tool_methods in helpers.py). Test rewritten to assert mcp.add_tool fired 4 times with the expected method names. Also adds test_raises_component_too_old_when_bootstrap_service_missing covering the new bootstrap pre-flight rejection path. Two existing tests updated to give their mocks the new get_services() shape. Not pushing the HAOS E2E (inaddon) failure as a code change - it's a "Client is not connected" cascade on one worker (gw1) that started before the source-refresh test ran, with the previous PR commit passing the same inaddon CI. Almost certainly flake. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fixup: remove trailing comma in manifest.json + exclude *.json from ruff The previous commit (806e082) ran ruff format on manifest.json as part of the format sweep; ruff 0.15+ formats .json files as JSONC and adds a trailing comma after the last value. HA's manifest loader uses Python's strict json.load() which rejects trailing commas — the integration failed to register with a ConfigEntryError, _assert_mcp_tools_available returned False everywhere, and every E2E test that exercises ha_mcp_tools.* failed with COMPONENT_NOT_INSTALLED. All 4 failing CI lanes (E2E Validation x2, HAOS E2E, HAOS inaddon) show the identical signature. Earlier HAOS "Client is not connected" cascade is gone with this signature unified. Fixes: 1. Drop the trailing comma in manifest.json (the actual breakage). 2. Add `custom_components/**/*.json` to extend-exclude in pyproject.toml so future `ruff format` invocations skip HA manifests entirely. CI's ruff lint step only checks *.py files so this also catches the case where a contributor runs ruff locally over a wider scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fixup: address Patch76 review (admin gate + case-norm + cache + comment) - custom_components: gate get_caller_token explicitly via _caller_is_admin helper. HA service registry has no built-in admin requirement (verified against HA core); the supported deployments (addon supervisor user mapped to admin-forced hassio_user, standalone admin LLAT) all pass the gate. - custom_components: add error_code "not_initialized" to the bootstrap not-yet-initialized branch so callers can detect it the same way they detect "unauthorized". - custom_components: reword the constant-time-compare comment — token is 256-bit (token_urlsafe(32)), the original "low-entropy" framing was backwards. - tools_filesystem: switch _fetch_caller_token's "no usable token" branch from a bare RuntimeError to raise_tool_error so it becomes a structured ToolError (the bare RuntimeError was being caught by the outer except Exception and remapped to a generic INTERNAL_ERROR). - tools_filesystem: replace id(client)-keyed module-level dicts with weakref.WeakKeyDictionary so the cache self-evicts on client GC and can't briefly inherit a stale entry from id() reuse. - tools_service: normalize the ha_call_service refusal to match HA core's domain.lower() fallback lookup so mixed-case "HA_MCP_TOOLS" and whitespace variants can't slip past the exact-string check. Unit tests (test_caller_token_auth.py): - TestCallerTokenOk: parametrized non-string presented-token coverage. - TestCallerIsAdmin: four cases (admin, non-admin, unknown user_id, no-user-context-trusted). - TestCallMcpToolsServiceInjectsToken: second-unauthorized-no-retry case so the no-loop / no-silent-success guarantee is pinned. - TestHaCallServiceRefusesMcpToolsDomain: parametrized case + whitespace variants of the domain string. E2E tests (workflows/services/test_ha_mcp_tools_refusal.py, cross-lane — testcontainer + HAOS external + HAOS inaddon): - ha_call_service literal-domain refusal end-to-end. - ha_call_service case + whitespace variant refusal (UPPER, mixed, padded-lower, padded-upper). - ha_call_service narrow refusal — persistent_notification still works. - Bootstrap-and-inject positive smoke — ha_list_files round-trips, implicitly exercising bootstrap → admin gate → token inject → handler accept across all three lanes. * fixup: tighten admin-gate-deferral note in refusal e2e module Drop the multi-line justification paragraph; state the deferral as a single factual sentence (initial_test_state has no non-admin user, so the negative path stays on the unit tier in test_caller_token_auth.py TestCallerIsAdmin). * fixup: drop ticket/PR refs + seed non-admin user for admin-gate e2e - Drop "issue homeassistant-ai#1451" / "PR homeassistant-ai#1459" / "skialpine" references from comments, docstrings, and test fixtures. Those belong in the PR description and rot as the codebase evolves. - Seed a second non-admin user (system-users group) in tests/initial_test_state/.storage/auth, with a matching long_lived_access_token refresh_token. - Bake the pre-signed access JWT for that LLAT into tests/test_constants.py::NON_ADMIN_TEST_TOKEN (HS256 over a fixed jwt_key stored in the seed, iss = refresh_token id, 10-year exp matching TEST_TOKEN). - New e2e test class TestCallerTokenAdminGate calls HA's REST /api/services/ha_mcp_tools/get_caller_token directly with the non-admin LLAT and asserts the handler returns success=false / error_code="unauthorized" — the negative branch of the admin gate that unit tests covered at the function level is now covered at the live HA layer too. Runs cross-lane; skips cleanly when the custom component isn't installed. * fixup: drop pre-existing ticket refs in touched files; fail-loud on missing component - tools_filesystem.py / tools_service.py / test_yaml_dashboards.py: drop the three remaining (issue #...) / (PR #...) references in files this PR was already editing. Boy Scout — pre-existing rot in a file we're in is still rot to clean. - test_ha_mcp_tools_refusal.py: remove the COMPONENT_NOT_INSTALLED skip paths from both the bootstrap-smoke and admin-gate tests. The testcontainer conftest auto-installs ha_mcp_tools and HAOS lanes bake it in, so a missing component means the install path regressed and we want it to fail loudly. The deliberate not-installed scenario is already covered by workflows/filesystem/test_file_operations.py::TestMcpToolsComponentNotInstalled. --------- Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kingpanther13
added a commit
that referenced
this pull request
May 28, 2026
…jects Folds together three small things that share the same custom- component file: * Manifest bump 0.5.0 → 0.5.1 (was 0.6.0 in the earlier push; this is a small patch-shape change, not new capability — the PACKAGES_ONLY_YAML_KEYS branch is a routing addition, no new surface). Pattern mirrors homeassistant-ai#1459's bump on the same file. * Patch76 follow-up #1 (PR comment 2026-05-28): spell each storage-mode tool out individually in the rejection guidance instead of the compact slash-form ``ha_config_set_automation/script/scene``. An agent reading the rejection at call time would otherwise parse that as one malformed tool name and fail to route. Locations: * ``__init__.py`` reject message * ``tools_yaml_config.py`` ``yaml_path`` parameter description * Patch76 follow-up #2: ``TestHandleEditYamlConfigPathTraversal`` pins the layering of ``os.path.normpath`` before the ``fnmatch`` package check so a crafted ``packages/../configuration.yaml`` cannot smuggle a PACKAGES_ONLY key (``automation``) into ``configuration.yaml``. Belt-and-suspenders against a future refactor reordering those two steps; defense is already correct by construction. Updated tests for the spell-out: * ``test_yaml_config.py`` E2E rejection asserts each tool name individually instead of the combined slash-string. * ``test_yaml_dashboards.py::test_rejects_packages_only_key_in_configuration_yaml`` does the same at the unit layer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kingpanther13
pushed a commit
that referenced
this pull request
May 29, 2026
…cate detection (homeassistant-ai#1442) * ci(internal): simplify issue bot — remove deep analysis, keep completeness check only - Remove the full investigation & fix-suggestion step (the expensive one) - Keep only the "Information Needed" path that asks for missing details - Remove the "working..." placeholder comment (no heavy step to wait for) - Upgrade model from gemini-3-flash-preview to gemini-3.5-flash (GA) - Reduce timeout from 12 min → 3 min - Always re-run completeness check on author reply (removed skip_preanalysis shortcut) The deep analysis was costing ~$30/month and often produced wrong or misleading suggestions that confused users and misled downstream Claude agents. The completeness check remains useful for nudging users to provide version numbers, error messages, and install methods before maintainers invest time triaging the issue. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(internal): remove maintainer exclusion from issue triage bot Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(internal): skip triage for maintainers in production, allow in dev repo Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(internal): restore classification labels (bug/enhancement/question) in completeness check Extend the evaluate prompt to output COMPLETE:<label> on the complete path and <!-- CLASSIFY:<label> --> on the incomplete path. Labels are stripped from the visible comment and applied to the issue alongside triaged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(ci): issue bot v2 — needs-info, duplicate detection, priority/size, project board - Add needs-info label to incomplete path; 10-day auto-close via close-needs-info.yml - BM25 candidate retrieval (500 issues, inline JS, no npm install) + Gemini duplicate verification; flags likely duplicates with needs-info instead of auto-closing - Priority/size assignment from recent changelogs (P0/P1/P2 for bugs, S/M/L for enhancements, good first issue for P2 bugs + S enhancements) - Project board field updates: Status (To triage / Backlog), Priority, Size via GraphQL - mark_done job sets project Status = Done when issues are closed - Harden should_run to skip closed events (routed to mark_done instead) - Increase triage timeout to 10 min (3 Gemini steps + BM25 + changelog fetch) Project board IDs hardcoded (homeassistant-ai, project #2): PVT_kwDODdb5sM4BPM6N — Status/Priority/Size field + option IDs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ci): add ha_report_issue tip to Information Needed comments for bugs Suggested by @kingpanther13 — maintainers regularly ask users to run this tool. Adding it as a tip in the bot comment saves that back-and-forth. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(ci): consolidate to 1 Gemini call with structured JSON output - Replace 3 separate Gemini calls (completeness + duplicate + priority/size) with a single unified call using maxSessionTurns:1, temperature:0 - Structured JSON output eliminates all fragile text parsing (COMPLETE: prefix, CLASSIFY sentinel, list-shape detection, JSON strip heuristics) - missing_info is now a structured array → formatted as clean bullet list - BM25 threshold raised 1.5 → 60: at corpus size ~500 the 8th-ranked non-duplicate scores 9–93, so 1.5 was a no-op; 60 skips ~55% of non-duplicate Gemini calls while keeping recall@8 at 66.7% - Fix changelog fetch: use git compare API for unreleased commits instead of CHANGELOG.md at master (which has no unreleased section) - Remove mark_done job: GitHub Projects native "Item closed" workflow already sets Status=Done; bot setting it would trigger "Auto-close issue" bidirectionally - Rephrase ha_report_issue tip to "either/or" (run tool OR reply manually) - Reduce timeout 10 min → 6 min (1 Gemini call instead of 3) - Exempt needs-info from close-inactive-issues.yml stale bot (our 10-day close-needs-info.yml handles those issues) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ci): fix YAML block scalar indentation in prompt builder Template literal content lines were at 0 indentation inside the script: | block scalar, which terminated the block early and broke YAML parsing. Replaced with array-join pattern so all content stays at the required 12-space indentation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(internal): unify triage bot — 1 Gemini call, keyword-based BM25, tool-list context Key changes: - Consolidate 3 Gemini calls → 1 unified call with structured JSON output - Add LLM keyword extraction (GitHub Models gpt-4.1-nano) to improve BM25 duplicate detection signal — extracts 8-14 weighted discriminative terms per issue - Pre-inject tool list + changelog context into prompt (Approach A, tested vs B) - Use commit-compare API for unreleased changes (CHANGELOG.md has no unreleased section) - Remove mark_done job (GitHub Projects native workflow handles Status=Done) - Approach B (Gemini file tools) tested: same quality, 2-3x slower, unpredictable cost Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(internal): replace Gemini CLI with GitHub Models gpt-4.1 (fallback gpt-4.1-mini) - No GEMINI_API_KEY secret required — uses GITHUB_TOKEN via models: read permission - Same actions/github-script fetch() pattern as keyword extraction step - Primary: gpt-4.1 (~50 req/day limit); fallback: gpt-4.1-mini on 429 - Evaluate step: ~2.5s vs ~22s with Gemini CLI (no CLI install overhead) - Tested: correct triage on duplicate bug + enhancement issues Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(internal): bump BM25 candidate body truncation 300→600 chars 300 chars cut off mid-sentence before error messages; 600 chars captures the full bug description opening. Worst-case prompt stays ~5.8K tokens, well under the 8K gpt-4.1 free/pro limit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: load issue templates at runtime for type detection Replace hard-coded textarea label strings with dynamic parsing of .github/ISSUE_TEMPLATE/*.yml from the prod repo. Labels shared across multiple templates (e.g. "bug") are dropped as ambiguous; specific labels (runtime-bug, agent-behavior, etc.) map unambiguously to a type. Falls back to body section header detection for unlabeled issues, reading the actual first textarea label from each template file. Also removes the now-dead Checkout step — the workflow no longer needs workspace files since all data is fetched via API. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: strip template boilerplate before BM25 tokenization and keyword extraction Section headers like "### 💬 What Happened?" appear in every template-based issue. Without stripping them, tokens like "happened", "response", "what", "went", "wrong" get high document frequency, suppressing their IDF and causing false-similarity matches across unrelated issues. Apply cleanBody() to corpus bodies and query body in the BM25 step, and to the issue body in the keyword extraction step. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: replace Projects v2 board fields with native org issue fields Projects v2 GraphQL mutations required org-level project access that GITHUB_TOKEN cannot get via workflow permissions. Native issue fields (Priority, Effort) are set via REST issues.update with issues: write, which we already have. Mappings from /orgs/homeassistant-ai/issue-fields: Priority: P0→Urgent, P1→High, P2→Medium Effort: S→Low, M→Medium, L→High Status field is dropped entirely — native project automation (Auto-add, Item added, Item reopened) handles project board status without bot intervention. Removes repository-projects: write from permissions (was wrong scope for Projects v2 anyway). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address PR review comments - close-inactive-issues.yml: revert exempt-issue-labels addition (not needed) - close-needs-info.yml: remove EXEMPT_LABELS check; drop 'reopen' from close message (users cannot reopen) - gemini-triage.yml: replace hardcoded MAINTAINERS list with org 'maintainers' team API lookup Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove duplicate cleanBody declaration in BM25 step Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: post acknowledgement comment on complete issues Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address code review findings - Template fetch: per-file catch so one failed file doesn't nuke the whole detection map; skip null entries in the processing loop - BM25 keywords: validate each entry has a string 'term' field before use; warn and fall back to full-text if schema is wrong - Maintainer check: log non-404 errors as warnings instead of swallowing them silently (404 = not a member, anything else = misconfiguration) - close-needs-info.yml: add concurrency group to prevent simultaneous scheduled + manual runs from double-processing issues Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: use native field label names for priority and effort Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: expand product context, restore template headers, improve completeness rules Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: add Low priority option (field id 63981588) — was missing from options map Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: restrict needs-info reset to issue author replies only Matches gemini-triage.yml which only re-runs on author comments. A maintainer/bystander comment was silently stripping needs-info without triggering re-triage, letting issues escape the stale-close path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address kingpanther review findings - Use process.env + parseInt for issue number in all steps (injection safety) - addLabels now has try/catch; labels applied before comment in all flows - priority/effort validated against option map; warns on unrecognised values - Remove stale 'Skip on cancellation' comment (no longer applies) - 70%+ -> '3 of 4' (requirement count shrunk from 5 to 4) - Add lookup comment on magic org field/option IDs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: document BM25 threshold provenance (calibrated via sub-agent testing) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address kingpanther review — error handling, retry, markers, rename - Fail closed on maintainer team-check non-404 errors - core.error on template fetch failure (was warning — now visible in failed runs) - Exp backoff (2s, 4s) on 5xx for both evaluate and keyword extraction steps - BM25 pagination wrapped in try/catch; empty corpus upgraded to warning - Fetch tool list and changelogs emit warnings on empty output - keyword-summary empty catch now logs core.info - Flow B addLabels wrapped in try/catch (was missing, A and C already had it) - setIssueFields now calls core.setFailed (undocumented param, silent failure unacceptable) - result.related filtered to numbers; result.missing_info filtered to strings - HTML comment anchors in triage bot comments; close-needs-info matches anchors not display text - close-needs-info: try/catch on createComment, skip issues.update if comment failed - Step renamed to 'Post result, apply labels and set fields' - gemini-triage.yml renamed to issue-triage.yml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: increase char caps to fit P95 issues; reduce candidates to 5 - Triage prompt body: 1500 → 8000 chars (covers P95 of 7093 chars) - BM25 tokenization: 2000 → 4000 chars - Keyword extraction: 1200 → 3000 chars - Candidate body: 600 → 1200 chars; count 8 → 5 Total worst-case prompt stays well under 16K token limit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: bump triage prompt body cap to 15000 chars Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: 6 candidates × 2500 chars = 15k total candidate budget Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: maximize prompt towards 16K token limit Body: 16000 chars (covers max observed issue), candidates: 6×4500=27000 chars Estimated total: ~15,950 tokens (~5200 fixed + 4000 body + 6750 candidates) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: reduce candidates to 3500 chars each (~14.5K tokens total) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: pass option name (string) not numeric ID to issue_field_values The API expects value=\Medium\ not value=63981587. Also removes now-redundant option ID constants. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
kingpanther13
added a commit
that referenced
this pull request
May 29, 2026
…ant-ai#1164) (homeassistant-ai#1431) * feat(policy): scaffold policy package for per-tool approval (#966) * feat(policy): add Predicate/Rule/Policy data models (#966) * feat(addon): expose enable_per_tool_approval option (#966) * feat(config): add enable_per_tool_approval setting (#966) * feat(policy): atomic load/save for tool_policy.json (#966) * feat(addon): wire enable_per_tool_approval through start.py + docs (#966) * feat(policy): args-hash + remember-cache for approval queue (#966) * feat(policy): predicate evaluator (eq/in/regex/exists/...) (#966) * feat(policy): pending entries with TTL, decisions, and event signalling (#966) * feat(policy): PolicyMiddleware happy-path branches (#966) * test(policy): cover block/deny/timeout/recall/remember branches (#966) * feat(policy): /api/policy/* Starlette handlers (#966) * fix(policy): wrap ValidationError, scope contains op, regex doc, test bind (#966) * feat(policy): Policies tab in web UI + sidecar route wiring (#966) * feat(policy): register PolicyMiddleware on the FastMCP server (#966) * fix(policy): return 400 on malformed approve/deny bodies (#966) * feat(toolsearch): unpin yaml-edit and code-mode tools, gated by approval middleware (#966) * chore: ruff format + lint cleanup for policy package (#966) - ruff format reflow on PR-touched files (case statements split to two lines, function signatures, line continuations). - UP042: Verdict now inherits from StrEnum instead of (str, Enum). - E402: hoist `import anyio` to the top of test_approval_queue.py. - I001: sort imports in test_evaluator.py and test_model.py. No behavior change. * fix(policy): mypy narrowing for evaluator comparisons (#966) `Predicate.value` is `Any | None` and `extract_path` returns `Any`, so `val == pv`, `val > pv`, etc. inherit `Any` and trip the project's `warn_return_any` mypy setting on functions declared `-> bool`. Wrap the comparison branches in `bool(...)` to make the narrowing explicit. Also guard the `regex` branch with `isinstance(pv, str)` so `re.search` receives a definite `str` instead of `Any | None`; a non-string regex value now returns False instead of raising TypeError at evaluation time, which is the only sensible behavior for a malformed pattern. No change to any test's expected outcome. * docs: credit @L1AD and PolicyLayer for #966 inspiration * docs(addon): fix wrong YAML example in enable_per_tool_approval section (#966) * fix(policy): CI green + critical bugs from reviewer cycle (#966) - expires_in_seconds: use time-remaining not total TTL - middleware: fail-closed on corrupt policy load (was crashing all gated calls) - handlers: 500-with-corrupt-flag on get_config when policy invalid - approval URL: wire secret_prefix via lazy getattr so HTTP-standalone emits a usable absolute path - approve/deny: return bool, 409 on already-decided - Predicate: field validators for op/value compatibility (Gemini) - persistence: explicit UTF-8 encoding (Gemini) - middleware: reuse tools/helpers safe_progress - handlers comment: fix wrong "next call" claim - test_middleware: unwrap ExceptionGroup for pytest.raises (CI fix) - test_stdio_settings_sidecar: include new policy_* handler keys (CI fix) * refactor(policy): drop default_action + tighten Rule.tool_name (#966) - Policy schema simplified: no default_action field. System is always "allow unless a rule matches; rule = require approval". The previous default_action='require_approval' option was a bricked-config trap since rules can't grant allow-overrides. - Rule.tool_name now rejects empty string; wildcard '*' documented in the docstring. - Evaluator simplified to match. - Tests updated/added. * refactor(policy): encapsulate decision state + clean naming (#966) - PendingApproval.decide() encapsulates the decision/event coupling; property guards read-only access to decision. - __post_init__ validates expires_at > created_at. - ApprovalQueue docstring spells out single-process scope and restart-loses-tokens semantics. - Rename args_preview -> args throughout (it was always the full unmodified args; "preview" was misleading). - Remove on_policy_change dead parameter from build_policy_handlers. * fix(toolsearch): default-pinned tools should be user-unpinnable (#966) - Computed pinned set now respects tool_config.json — explicit "enabled" state removes from defaults. - Remove ha_restart / ha_reload_core from DEFAULT_PINNED_TOOLS (recovery actions, low frequency, low value in default LLM tool surface). - Add ha_manage_backup to MANDATORY_TOOLS (operational essential). - Server now declares _settings_secret_prefix on __init__ for pyright. * refactor(policy): rename feature to "Tool Security Policies" (#966) User-facing rename: addon config option, env var, Settings attribute, UI tab label, addon DOCS sections, translations, server method. Internal Python naming (policy/ package, tool_policy.json, /api/policy/* routes, class names like PolicyMiddleware/ApprovalQueue) unchanged for less churn. * docs: small comment polish for policy review nits (#966) * feat(ui): per-tool security-gated toggle in Tools tab (#966) * test(policy): integration + timing-isolation coverage gaps (#966) - test_server_policy_wiring: assert _apply_tool_security_policies attaches middleware + approval_queue when enabled, neither when disabled. - test_settings_ui_handler_selection: parametrize the live-vs-stub branch in build_settings_handlers (sidecar / no server / no queue / live). - test_middleware wait-loop timing: assert event-wake exit, not polling. - test_middleware multi-rule precedence: first-match wins for remember_minutes. * feat(ui): rewrite Tool Security Policies tab — per-tool cards + predicate editor (#966) * feat(config): expose enable_tool_security_policies as a feature flag (#966) Wires the new addon-config toggle into FEATURE_FLAG_FIELDS so it appears in the Server Settings tab and rides PR #1420's _save_feature_flags + _supervisor_merge_and_post_options + _schedule_supervisor_self_restart flow when toggled in addon mode. * fix(policy): address all verified review findings + CI failures (#966) CI fixes: - ruff: drop dead noqa: SLF001 suppression - unit test: test_missing_path_never_matches_except_exists uses op-compatible values so Predicate field_validator doesn't reject Review findings: - FEATURE_META entry for enable_tool_security_policies (toggle now renders in Server Settings tab) - Approval URL points to /settings?tab=tool-security-policies (was POST-only /api/policy/approve which 405'd on browser open) - policyDecide surfaces network errors + 409 current_decision - Policy gains version field for optimistic concurrency; PUT 409s on version mismatch; client surfaces 'reload before saving' - _apply_tool_security_policies failure logs spell out security impact (TOOL SECURITY GATING IS NOT ACTIVE) and include data_dir/env-var context - Validator rejects value on op='exists'; gt/lt TypeError degrades to False - ToolVisibilityResult -> UserToolStateOverrides, fields are frozenset, disjointness asserted - PendingApproval.event private; expose async wait() - _SupervisorOptionsError gains transport()/validation() classmethods encoding kind->status_code pairing - Wiring test binds queue identity; handler-selection covers all 3 live routes - Comment + doc polish (audit-trail claim, e2e docstring path, internal Task references) * fix(policy): CI green + real e2e test for the approval flow (#966) - ruff format: tests/src/unit/test_settings_ui.py - test_save_and_roundtrip: account for save_policy version bump - test_serialized_shape_is_stable: include 'version' in expected keys - test_addon_save_returns_500_when_server_is_none: guard server._settings_secret_prefix assignment with None check (regression from #4's secret-prefix wiring) - tests/src/e2e/policy/test_approval_flow.py: real e2e exercising block -> approve -> re-call cycle with strict args-binding rejection on mutated args. Skip-stub replaced with real test driving the live middleware via mcp_client + /api/policy/* HTTP. * fix(ui): broken quote escaping in predicate-form placeholder breaks JS parse (#966) The Python source `'placeholder=\\'\"lock\"...\\\\'>'` rendered as JS `'placeholder='\"lock\"..'>'` — the single quote inside the HTML attribute value closed the outer JS string literal, and subsequent tokens (\"lock\", 'or', '[', ...) broke parsing. With a syntax error in the inline <script>, the browser stopped executing — Tools tab stuck on 'Loading...', tabs unclickable. Switched to a JS-safe double-quoted attribute with " for the embedded double quotes in the placeholder hint. * fix(ui): gated toggle reads addon-config flag, not Policy.enabled (#966) The per-tool 'security gated' toggle was grayed out even when the user had enable_tool_security_policies turned ON in the addon config + the Server Settings tab toggle, because the JS was reading Policy.enabled (the file field) instead of the addon-config feature flag — which is the single source of truth for whether the middleware is active. loadPolicyState now reads enable_tool_security_policies from /api/settings/features (same place renderFeatureFlags consumes from). * fix(policy): Policy.extra=ignore so old persisted files load cleanly (#966) Persisted tool_policy.json files from an earlier revision of this PR carry default_action (since dropped) and rejected with ValidationError on load — surfacing as 'Could not load policy: 500' when the user clicked the per-tool gated toggle. Predicate/Rule keep extra=forbid (typo catching at construction). * fix(policy): drop Policy.enabled — addon-config flag is the sole switch (#966) The middleware's server-side gate was checking `policy.enabled` (a file field with no UI surface), so it returned ALLOW on every call regardless of rules. The addon-config flag (`enable_tool_security_policies`) was supposed to be the only switch — and the middleware is only registered when that flag is true — so the inner `policy.enabled` check was both redundant and broken. Remove the field, remove both server-side checks (middleware + evaluator), update tests, and refresh the JS comment that referred to it. * fix(policy): drop approve_url, instruct LLM to send user to settings page (#966) The relative-path approve_url doesn't resolve cleanly through cloudflared or other reverse-proxy deployment modes — the LLM can't safely hand it to the user. The user already knows where the Tool Security Policies tab is (they set the rule from it), and that page lists all pending approvals, so a per-request URL is unnecessary noise. - Drop approve_url from USER_APPROVAL_REQUIRED context; keep `token` so a caller could correlate but the user doesn't need to act on it. - Update message + progress text to instruct the LLM to tell the user to open the settings UI Tool Security Policies tab. - Drop the now-unused approval_url_builder param + the _settings_secret_prefix plumbing in server.py / settings_ui.py. Also fix the failing test_defaults (asserted dropped Policy.enabled field) and the e2e test PUT body that still carried `"enabled": True`. * feat(policy): schema-driven condition builder for write/destructive tools (#966) The previous "Add predicate" UX required users to type both the dotted arg path (e.g. `args.domain`) and the value as JSON. Two problems: 1. They need to know what fields each tool takes. 2. They need to know what values are legal (which HA domains exist, which entities, etc.). Replace the free-text path input with a dropdown sourced from the tool's JSON schema, and replace the free-text value input with a (multi-)select sourced from HA when the path has a known value source (domain, service, entity_id today; trivially extensible). Free-text is still available via an "(other — type a path)" escape hatch and as the automatic fallback for ops that don't pair with a registry (regex / contains / gt / lt). Server: - New `/api/policy/tool-schema?name=...` returns `{paths: [...], value_sources: {path: source_key}}`. Read-only tools return empty paths so the UI falls back to free-text (gating those is low-value but still permitted manually). - New `/api/policy/value-source?source=...` resolves a source key to a live list of choices. In-process 30s TTL cache avoids hammering HA when the user explores paths. - value_sources.py registry maps (tool_name, arg_path) → source_key for the common write/destructive surface (call_service, set_entity, set_integration_enabled, get_history, etc.). New mappings are one dict entry plus, if a new source key, one fetcher. - Both endpoints mount in addon + secret-prefix routes. Sidecar serves 503 stubs (no FastMCP registry / HA client in that process). UI: rename user-facing "predicate" → "condition" (CS jargon → SQL/JIRA terminology users actually recognise; internal Pydantic class stays `Predicate` so the wire format is unchanged). Form fetches the schema lazily on first open, caches it on the card, refetches value choices when path/op changes. Includes test_schema_handlers.py covering: missing-name 400, sidecar 503, unknown-tool 404, read-only empty-paths, write-tool paths + registry, JSON-schema enum passthrough, value-source 400 paths, both HA-services payload shapes, domain filtering for entities/services, and upstream-fetch 502 mapping. * test: include new policy handler keys in sidecar all-keys assertion (#966) * fix(ui): clearer condition-builder labels, optional value, bareword input (#966) User feedback on the new form was: 1. "args.foo" path placeholder is gibberish; no real label on path/value 2. value box should not be mandatory for ops where backend allows None 3. typing `lock` into the value box errored with "Invalid JSON" — every normal-looking input has to be quoted 4. for ha_call_service `data` was the only arg without an obvious meaning Changes: - Real `<label>`s on the form rows: "Argument:", "Match when:", "Value:". - Op dropdown shows friendly text ("is present (any value)", "equals", "is one of", "matches regex", etc.); wire values unchanged. - Hint line under the value row reflects the current op so users know whether a value is required and roughly what shape it should take. - Value is now OPTIONAL for ops where the backend accepts a missing field (exists, eq, neq, contains). Submitting an empty value omits the `value` key from the predicate entirely. - Bareword inputs auto-coerce: `lock` → `"lock"`, `lock,alarm` → list, `42` → number, `true` → bool. Falls back to a clearer error if even the smart-coercion can't make JSON. - Path dropdown options now carry the schema `description` as a `title` tooltip, so `data` reads as "Service data dict" on hover instead of being a mystery. - Schema-declared enums render as a value dropdown automatically (no registry entry needed) when the path's JSON-schema has `enum`. Also fix /tmp/extract_js.py — naive paren-counter broke once form strings started containing parens; switch to ast.parse so future edits don't silently break the harness. * feat(policy): wildcard path "args.*" + clearer empty-value semantics (#966) User asked for a catch-all "any argument equals X" condition and called out that the previous "Leave blank to gate on null" hint was nonsensical — a blank value should mean "any value" to a normal user, not "match the null literal". Backend: - Refactor evaluator: `extract_path` → `iter_path_values`, which yields every value the dotted path resolves to. A `*` segment fans out across the current node (dict values for dicts, items for lists). A path like `args.*` thus yields every top-level arg; `args.config.*` yields every leaf of the config sub-dict. - `match_predicate` rewrites to "ANY matching value satisfies the op", which collapses to the previous single-value semantics for non-wildcard paths. So `path=args.*, op=eq, value="lock"` gates whenever any arg of the tool call equals "lock". UI: - New "(any argument)" option at the top of the path dropdown, fills `args.*` and carries a tooltip explaining the semantic. - VALUE_OPTIONAL_OPS shrinks to just `exists` — blank value is no longer silently accepted for eq/neq/contains. Instead, the value-required error fires, and the hint text under the field tells the user to switch op to `is present` if they wanted "any value". - Hint copy revised across all ops so the "what happens with this op + blank value" question has a clear answer at each step. Tests: - New TestIterPathValues covering top-level, nested, missing, and the three wildcard shapes (dict values, list items, empty). - New TestWildcardPredicate covering eq/in/exists/regex matching via `args.*` plus an end-to-end evaluate() test. - Existing tests still pass with the refactored matcher; signatures of the public functions are unchanged. * fix(ui): default condition path to '(any argument)'; relabel error (#966) - Drop the '(pick an argument)' placeholder; default the path dropdown to '(any argument)' so the form is immediately submittable. - 'path is required' error reads 'argument is required' if it ever fires (it won't on the happy path now). * feat(policy): auto-save conditions + surface matched_rule in approval error (#966) UI: - Drop the manual "Save changes" button on each rule card. Conditions now PUT to disk the moment the user clicks "Save condition", clicks the × on a condition row, or edits the remember-minutes field (debounced 500ms). The only feedback is a small "Saving…" / "Saved." status line next to the card. - Removed the now-dead .policy-save-rule CSS and the markDirty helper. Server: - USER_APPROVAL_REQUIRED error context now carries `matched_rule` with the rule's tool_name + when[]. Lets the user (and the LLM) tell at a glance which rule fired, instead of guessing whether their condition saved correctly. * feat(policy): case-insensitive string comparison in all ops (#966) Security gates shouldn't fire differently based on whether the LLM capitalised its argument — 'Lock' and 'LOCK' and 'lock' are the same operationally. eq/neq/in/not_in/contains lower-case both sides before comparing when both are strings; regex uses re.IGNORECASE. Non-string types pass through unchanged so int(1) != str('1') still holds. * fix(policy): mypy bool cast + broaden e2e coverage (#966) mypy: bool(_ci(val) == _ci(pv)) — _ci returns Any (passes non-strings through unchanged), so eq/neq comparisons need an explicit bool wrap. Tests: previous e2e only covered the happy block→approve→re-call path. Add four more cases against the live testcontainer: - wildcard `args.*` gates when any arg matches the value - wildcard `args.*` passes through when no arg matches - case-insensitive matching (rule 'lock' gates caller 'LOCK') - deny → middleware raises USER_DENIED, tool never runs - remember_minutes>0: second call within the window skips the queue * refactor(policy): address review-cycle findings (#966) Gemini (6 unresolved threads): - Migrate POLICY_LOAD_FAILED / USER_DENIED / USER_APPROVAL_REQUIRED off manual `ToolError(json.dumps(...))` onto the canonical `raise_tool_error(create_error_response(...))` pattern. Added the three error codes to ErrorCode enum. - Hoist sync `load_policy()` off the event loop via `anyio.to_thread.run_sync` in the middleware's policy provider call. - Add justification comment on `local_provider._list_tools()` (same rationale that's already documented in `settings_ui.py`'s tool enumerator: public `list_tools()` filters disabled tools but operators may still want to author gating rules for them). Code-reviewer findings: - ApprovalQueue TOCTOU: two concurrent `on_call_tool` coroutines with identical (tool, args_hash) could both miss `find()` and create duplicate pending entries; approving one would leave the other waiter blocked. Introduce `find_or_create(...)` serialised behind an `anyio.Lock`; middleware now uses it. - ApprovalQueue had no pending-entries cap → memory exhaustion under an LLM retry-loop with mutated args. Add `PENDING_CAP = 1000` with FIFO eviction of oldest entries when the cap is hit. Silent-failure-hunter findings: - handlers.py: `get_tool_schema` and `get_value_source` now `logger.exception` before returning 500/502 so FastMCP version bumps or HA outages leave a traceable signal instead of opaque client errors. - value_sources fetchers `logger.warning` on unexpected HA response shapes (would otherwise silently return empty dropdowns). - value_sources cache no longer stores empty results — a transient HA glitch returning [] would otherwise pin the dropdown blank for 30s. PR-test-analyzer findings (the critical one): - test_persistence.py's `test_save_and_roundtrip` passed `Policy(enabled=True, ...)` for a field that no longer exists; `extra="ignore"` silently dropped it so the test was a no-op assertion. Replace with real round-tripped fields (wait_seconds / approval_ttl_minutes / remember_minutes) and add an explicit `test_load_drops_unknown_fields` exercising the extra="ignore" back-compat contract with a JSON file carrying `default_action` + `enabled`. - Add wildcard scalar/None tests (`args.x.*` against scalar yields nothing; doesn't crash). - Add ApprovalQueue tests: concurrent `find_or_create` shares one pending entry; `create` evicts oldest at PENDING_CAP. - Add handler tests: sidecar value-source returns 503, tool-schema 500 on `_list_tools` exception, value-source cache key separates per params, `_extract_arg_paths` skips malformed property entries. Comment-analyzer findings: - Grammar fix in handlers.py `_is_write_or_destructive` docstring. - model.py docstring "older version of this PR" → "older builds". - Strip the `(#966)` / `(issue #966)` parentheticals from module docstrings, settings_ui CSS/HTML/comments — git blame and the commit message carry the link. * fix(policy): UI surface fetch failures + middleware reissues swept pending (#966) - Middleware: after _wait_for_decision returns without a verdict, check whether the pending entry was swept (TTL elapsed during the wait). If so, create a fresh entry before raising USER_APPROVAL_REQUIRED so the LLM isn't told to re-call against a dead token. - UI: policyLoadConfig now surfaces fetch failures in a visible error banner instead of silently rendering blank — picks up the policy_file_corrupt:true repair hint from the server's 500 response. - UI: loadValueChoices records the failure (lastValueSourceError) so renderHint can show it under the value row. The dropdown still downgrades to free-text, but the user can now tell a transient HA outage from "no value source registered for this path". - UI: renderValueControl uses an autoincrement seq so rapid path/op edits don't let an earlier slow fetch's DOM mutation land after a newer one's (similar to the autoSave pattern). * fix(policy): logger.info on silent decide-False; debug log on gt/lt type-mismatch; strengthen event-wake test (#966) - ApprovalQueue.approve/deny: emit logger.info when the call returns False (unknown token or already decided) — was silent. Helps debug the case where the middleware's consume_and_maybe_remember races with an out-of-band decide. - Evaluator gt/lt TypeError fallback now logs at debug so a user whose 'battery_level < 20' rule never fires can see that the arg came in as a string and tighten the rule. - test_event_wakes_waiter now measures elapsed wait time and asserts < 200ms, ruling out a hidden poll-loop impl that would still pass the previous decision-only check. * style: ruff format evaluator.py for CI's 0.15.13 (#966) Local ruff 0.15.7 didn't wrap the multi-arg logger.debug call; CI's ruff 0.15.13 does. Upgrading local toolchain to match. * feat(policy): clear remember-cache on save, clearer disabled-state UX, mirror master toggle (#966) Three things: 1. Remember-cache invalidation on policy save (B2). ApprovalQueue.clear_remember_cache() drops every remembered approval; put_config calls it after a successful save. Without this, tightening a rule was silently bypassed by any in-flight remembered approvals until their window expired. 2. Better 503 / "unavailable" messaging (B8 + the broader issue). The stub handler's 503 message used to read "Live approvals unavailable in this mode (sidecar)" even when the real cause was the feature being turned off in addon config — the user had no way to tell from the UI. Updated to call out all three causes (feature off, sidecar, ImportError) and point at the addon log. The pending-list JS now checks policyState.enabled first and shows "Tool Security Policies is turned off" when that's the actual reason, falling back to the server's 503 message otherwise. 3. Mirror the master toggle onto the Tool Security Policies tab. Was only exposed in Server Settings before — users on the Policies tab had to navigate away to find the on/off switch. New checkbox at the top of the tab posts to the same /api/settings/features endpoint, so the two surfaces are live mirrors of the same addon-config flag. * test(policy): fix JS-harness drift guard + lock policy-tab behaviour (#966) The merged-in JSDOM behaviour test (#1425) failed collection because its hardcoded _TOP_LEVEL_ELEMENT_IDS list didn't yet know about the policy-tab handlers this PR adds (policy-master-toggle, policy-save-global-btn). Add them, plus matching DOM stubs in _build_min_dom so the init pass doesn't throw on the addEventListener calls. While the file is open, add three behavioural tests that pin the new condition-builder UX wiring: - Master toggle change POSTs to /api/settings/features with the enable_tool_security_policies flag (so the on-tab toggle stays a true mirror of the Server-Settings checkbox). - /api/policy/pending 503 renders "Tool Security Policies is turned off" when the addon flag is off (avoids the old misleading "sidecar / unavailable" copy). - /api/policy/pending 503 propagates the server's addon-log message verbatim when the flag IS on but the queue is unreachable (so users know where to look for ImportError details). The parse-coverage path catches syntax breaks already; these tests catch behavioural regressions on top of it. * refactor(policy): address 2nd-round review findings (#966) Verified all 23 findings from the 2nd pr-review-toolkit pass against the code; fixed 22 (skipping #13 — the JSDOM seq-cancel race test is high-effort to author reliably and the production guard is small enough that bench-level review catches regressions). ## Correctness / silent-failure - ApprovalQueue PENDING_CAP eviction now sorts by `(decision == "pending", created_at)` so resolved entries evict first. When a still-pending entry MUST be evicted (cap full, no resolved to drop), `.set()` its event so any waiter in `_wait_for_decision` wakes immediately instead of blocking the full wait_seconds against a row that no longer exists. - Middleware: log INFO with old + new token on the reissue-after-sweep branch so operators can correlate "approval row keeps reappearing" with the actual cause. - Middleware: scope `clear_remember_cache` to "rules actually changed" — editing only wait_seconds / approval_ttl_minutes no longer blows away in-flight remembered approvals. - Policy: model_validator requires `wait_seconds < approval_ttl_minutes * 60` so the middleware can't repeatedly issue fresh pending entries because the wait outlasted the TTL. - value_sources: cache key uses `urllib.parse.urlencode` so a future param value containing `=`/`&` can't collide with another key. - ApprovalQueue: `approve`/`deny` on unknown token now logs WARNING (security-gating endpoint, suggests UI bug or token probing). Already-decided stays INFO (legitimate race). ## UI - settings_ui.policyState gains an `enabledKnown` tri-state bit so downstream branches (`policyLoadPending`'s "feature off" copy, master-toggle revert) don't false-confidently route to the "disabled" message when the features fetch actually failed. - Master-toggle change handler reverts the checkbox on save failure AND syncs from `policyState.enabled` after a successful load — no more "UI says on, server says off" drift. - `policyLoadConfig` appends "(response body unparseable)" when the 500 body isn't JSON (e.g. HTML error page from a misrouted sidecar), so the operator sees more than just "HTTP 500". - `policyLoadPending` surfaces fetch errors inline ("Lost contact with server, retrying") instead of silently freezing the list. - `fetchToolSchema` records `lastValueSourceError` on failure so the hint banner explains why the value dropdown silently downgraded to free text. ## Tests - test_handlers: `test_put_config_clears_remember_cache_when_rules_change` + `test_put_config_preserves_remember_cache_when_only_timing_changes` lock in the scoped invalidation. - test_approval_queue: `test_create_evicts_resolved_entries_before_pending`, `test_evicting_pending_wakes_its_waiter`, `test_create_after_sweep_still_evicts_when_pending_fills_cap` cover the new eviction rules. The strengthened `test_find_or_create_lock_blocks_concurrent_create_under_real_race` inserts a yield point inside the lock body so the lock actually matters to the assertion (the previous test would pass even without the lock under anyio's cooperative scheduler). - test_middleware: `test_swept_pending_during_wait_is_reissued_with_fresh_token` exercises the previously-untested reissue branch. - test_schema_handlers: `test_value_source_empty_result_not_cached` proves the empty-result no-cache guard actually triggers a refetch. - test_settings_ui_js_behavior: master-toggle test now JSON-parses the POST body and structurally asserts `flags.enable_tool_security_policies is True` instead of loose substring matching. ## Comments - approval_queue.py PENDING_CAP docstring now matches implementation (was promising "resolved first" before the implementation actually did it). - evaluator.py: gt-branch comment example uses ">" not "<". - handlers.py: dropped cross-reference to settings_ui that would rot. - middleware.py: trimmed "fast on warm disk" speculation. - value_sources.py: trimmed "(WebSocket reconnect, auth lapse)" speculation in the empty-cache comment. - settings_ui.py: four comments still saying "predicates" updated to "conditions" to match user-facing terminology. * fix(ui): blank value on eq/in/etc coerces to op=exists (#966) User expects 'leave value blank to gate on the argument's mere presence regardless of value' to work across the equality-ish ops, not just op=exists. Earlier I had the form reject blank value for anything other than exists with a 'value is required' error. Now: for eq / neq / in / not_in / contains / exists, leaving the value blank silently coerces the predicate to op=exists on save. The condition row then reads as 'args.* exists' which is the right description of what's stored. Ops that genuinely need a value (regex / gt / lt) still raise 'value is required for op=...'. The hint text under each op updated to call out 'Leave blank to gate on any value' where it applies. * docs(addon): drop beta tag from Tool Security Policies (#966) The feature is stable enough to ship as a default-supported addon config option, not a beta. Also corrects two pieces of doc drift that landed here originally: - 'approval URL' wording → 'tell the user to open the Tool Security Policies tab' (the URL field was dropped earlier in this PR) - 'predicates' → 'conditions' (matches the user-facing terminology the UI now uses) Touches both prod and dev addon directories (config.yaml-driven UI text + the rendered DOCS.md). * docs(beta): describe 3-path enabling (dev addon, stable + web UI, env vars) (#1164) * feat(config): advanced settings registry + beta master toggle field (#1164) - Add ``ADVANCED_SETTINGS_FIELDS`` registry (21 fields across connection, search, operations, diagnostics, tools_surface, beta_codemode sections) - Add ``_ADVANCED_SETTINGS_BOUNDS`` and ``_ADVANCED_SETTINGS_CHOICES`` dicts for UI/POST validation - Add ``BETA_FEATURE_FIELDS`` tuple for master-gate enforcement - Add ``enable_beta_features`` Settings field (alias ENABLE_BETA_FEATURES, default False) as the master beta toggle - Update ``FEATURE_FLAG_FIELDS``: add ``enable_beta_features`` at front, ``enable_code_mode`` at end; reorder for UI grouping - Extend ``BACKUP_OVERRIDE_FIELDS`` from 3 to 5 entries (add ``auto_backup_dir`` and ``auto_backup_calendar_lookahead_days``) - Extend ``_apply_backup_overrides`` to handle ``str`` type; widen ``coerced`` annotation to ``bool | int | str``; add bounds check for ``auto_backup_calendar_lookahead_days`` (1..365) - Add coverage gate test asserting every Settings env alias is registered in one of the three panel registries (or in the explicit ALLOWLIST) - Add ``test_enable_beta_features_default_false`` * refactor(config): code-review fixups — docstring clarity, stricter bool reject, null-byte guard (#1164) * feat(settings-ui): per-tool env-pin for DISABLED_TOOLS / PINNED_TOOLS (#1164 addendum) Add env_pinned_tools() and effective_tool_config() helpers so tools listed in DISABLED_TOOLS / PINNED_TOOLS env vars stay read-only at runtime even after tool_config.json has been written. The _get_tools GET handler now includes env_pinned metadata per tool entry; _save_tools rejects incoming flips of env-pinned tools with HTTP 409. Server.py startup path updated to use effective_tool_config() so env pins apply at boot. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(config): master beta gate + advanced overrides apply (#1164) - Rewrite _apply_feature_flag_overrides: lift addon-mode short-circuit for beta fields (enable_beta_features + BETA_FEATURE_FIELDS), add master gate that forces all 5 beta sub-flags to False when master is off regardless of env/file state - Update get_feature_flag_origin: beta fields never return "addon" — they follow standalone precedence in either mode - Add _apply_advanced_overrides: reads feature_flags.json for all editable ADVANCED_SETTINGS_FIELDS entries; skips display-only fields; validates types, bounds, and choices before setattr - Wire _apply_advanced_overrides into get_global_settings (runs after feature-flag + backup passes) - Add 18 unit tests covering master gate semantics, addon-mode carve-out, advanced-override int/str/float/display-only/out-of-bounds/invalid- choice cases, and backward-compat (pre-master override files) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(settings-ui): code-review fixups — drop dup env_pinned, settings param, msg + tests (#1164) Address code-quality review feedback on 32d67b4d: - Drop the redundant per-tool-entry `env_pinned` field from the GET /api/settings/tools response. The top-level `env_pinned` map is the single source of truth; UI does O(1) lookups against it. - `effective_tool_config()` now accepts an optional `settings` parameter (mirrors `load_tool_config()`); restores dependency injection at the server.py startup callsite (`self.settings`). - 409 rejection message uses comma-joined names instead of Python list repr for better human readability; structured `context.rejected` remains for programmatic access. - Add `test_get_tools_includes_env_pinned_map` test covering the new GET response field. - Symmetrize `get_data_dir.cache_clear()` calls at both ends of each tmp_path test so cross-test cache pollution can't leak in. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(config): code-review fixups — docstring accuracy, top-level import, null-byte test (#1164) - Correct _apply_advanced_overrides docstring: most advanced fields ARE in the addon config.yaml schema (backup_hint, verify_ssl, enabled_tool_modules, etc.) and are handled correctly via the env- var-wins check because start.py exports them. Only code_mode_* and mcp_server_version are file-only in either mode. - Move `from typing import Any` from function body to module-level imports (stdlib typing — no need to defer). - Add test_advanced_override_str_field_with_null_byte_rejected to cover the previously-unexercised null-byte reject branch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(settings-ui): render env-pinned tool rows as read-only + update addon translations (#1164) - Add `toolEnvPinned` module-level map; populate from `data.env_pinned` in `loadTools()` - Tool rows for env-pinned tools get `.env-pinned` class, all inputs disabled, and a `feature-locked-note` banner naming the env var (DISABLED_TOOLS or PINNED_TOOLS) - Group master toggle excludes env-pinned tools from bulk enable/disable - Update `pinNotice` copy to describe the env-pinned lock-until-unset semantics - Update `homeassistant-addon-dev/translations/en.yaml` descriptions for `disabled_tools` and `pinned_tools` to reflect that they are operator-level locks, not seed-only values - Add JSDOM behavioural tests for env-pinned disabled and pinned tool rows * feat(settings-ui): /api/settings/advanced GET+POST handlers (#1164) * feat(settings-ui): render advanced settings sections in Server Settings tab (#1164) * feat(settings-ui): beta master toggle + nested sub-row gating + 409 rejection (#1164) * feat(settings-ui): nest code-mode sub-numerics under enable_code_mode (#1164) * test(addon): assert start.py auto-enables master beta in dev addon mode + stable schema absence (#1164) * feat(addon): start.py auto-enables ENABLE_BETA_FEATURES=true when dev addon options carry beta keys (#1164) * fix(settings-ui): post-CI/post-review fixes — backup-config str+range, addon-mode gate skip, e2e env, JSDOM ids, stale comments (#1164) * fix(config): beta-sub-flag origin returns addon in dev mode (env var presence as signal) (#1164) * fix(tests): addon-mode save tests use non-beta flag matching new origin semantics (#1164) * refactor(settings-ui): loadAdvancedSettings error parity + atomic-write helper reuse + beta_sub_flags via API (#1164) * refactor(config): narrower setattr errors + hasattr precheck + Gemini coerced-decl nit (#1164) * refactor(settings-ui): hoist override-file read + display-only warning + missing test coverage (#1164) * test(settings-ui): section-uniqueness + registry-disjoint + tighter env-pin assertions + accurate docstring (#1164) * refactor(addon): extract maybe_auto_enable_beta_master helper + real unit tests (#1164) * test(settings-ui): JSDOM coverage for advanced sections + master live-render; verify file untouched on 400 (#1164) * refactor(config): NamedTuple registries + import-time validator; finish silent-failure + JSDOM nesting tests (#1164) * fix(tests): restore standalone-mode assertion + add adv section containers (#1164) `replace_all` from an earlier sweep had pivoted the standalone-mode save assertion onto a beta sub-flag, so the master-gate guard now rejected the request and the test failed. Restore the non-beta `enable_tool_search` flag here — the assertion is about the unified save-contract shape, not the beta path. JSDOM `TestAdvancedSectionRender` tests were failing because MIN_DOM lacked the five `adv*` section containers; `renderAdvancedSection` would silently no-op (getElementById returned null) and the assertions fired against an empty body. Add `advConnection`, `advSearch`, `advOperations`, `advToolsSurface`, `advDiagnostics` to `_TOP_LEVEL_ELEMENT_IDS` so `_build_min_dom` emits `<div>` containers for them. * fix(tests): adopt master beta gate + new advanced handler keys (#1164) Three failures surfaced after rebasing onto upstream/master: 1. ``test_returns_all_handler_keys`` expected the pre-#1164 handler set. Add ``get_advanced_settings`` / ``save_advanced_settings`` to the expected keys. 2. ``test_tools_filesystem.TestFeatureFlag::test_enabled_with_*`` broke because the master beta gate now forces every beta sub-flag False at runtime when ``ENABLE_BETA_FEATURES`` is unset. Set both env vars together in the enabling tests so they exercise the sub-flag bool parsing in isolation, and add an explicit test for the gated behavior so a future regression in ``_apply_feature_flag_overrides`` surfaces here too. 3. ``test_yaml_config_tool.enable_flag`` fixture sets ``ENABLE_YAML_CONFIG_EDITING`` but didn't set the master, so the cached settings landed with the sub-flag forced False — would have broken next-up after the filesystem tests. Set ``ENABLE_BETA_FEATURES`` alongside. * fix(policy): contains operator case-insensitive on list-membership branch Pre-fix: case "contains": if isinstance(val, str) and isinstance(pv, str): return pv.lower() in val.lower() # CI return isinstance(val, (list, tuple, set)) and pv in val # case-SENSITIVE The string-in-string branch was already case-insensitive (matching the ``_ci``-equivalent treatment that ``eq`` / ``in`` / ``not_in`` apply), but the list-membership branch fell through to Python's default ``in`` operator. A rule listing ``["light.kitchen"]`` would not fire on an LLM passing ``["Light.Kitchen"]`` — silent gate failure. Bring it in line with the other string-op branches via per-element ``_ci``, which passes non-string entries through unchanged so mixed-type collections still get natural equality semantics. Caught by Gemini Code Assist on #1431; addressing inline rather than opening a follow-up because the policy module is now in master (#1421) and any reviewer running the suite would see the case-sensitivity asymmetry in the existing TestCaseInsensitive class. * feat(settings-ui): addon-aware locked banner, beta-at-bottom, danger warning, dual save buttons, fork-dev stable copy (#1164) Five user-feedback fixes against the Server Settings UI: 1. Locked-banner copy adapts in addon mode. The standalone "Set via env var X — unset it to edit here." copy is misleading in HA addon mode where the operator has no env-var surface (start.py writes the env vars from /data/options.json; Supervisor writes the rest). Endpoints now return is_addon; the JS helper envLockedNoteHtml swaps in addon-aware copy that points users at the addon Configuration tab. Master beta gets an extra hint explaining the auto-enable rule. 2. Beta block rendered into a dedicated bottom-of-panel betaBody container instead of featuresBody. The dangerous block sits last so users see safer settings first; a "Beta features (dangerous)" header in warning color marks the boundary. 3. enable_beta_features help-text rewritten to lead with an explicit danger warning (permanent damage to HA, no warranty, take a backup, own risk). Mirrored as a blockquote at the top of the dev addon's beta-options section in DOCS.md so the addon UI also surfaces the risk. 4. Save button redesigned — primary-CTA styling (bigger, accent background, hover state), duplicated at the top of the panel so a user scrolling either end can hit save, and paired with a prominent two-step note explaining that Save → Restart are both required for changes to take effect. 5. New scripts/fork-dev/copy-stable.sh + restore-dev.sh let maintainers whose only HA test path is the fork-dev addon flip homeassistant-addon-dev/ between dev-flavor and a stable-mirroring "stable test" flavor. Round-trip clean — copy → restore restores the index exactly. JSDOM behavioural coverage added for each of (1)-(4); MIN_DOM updated with the new top-row + beta-section element ids. * fix(tests): JSDOM beta-block tests assert against production HTML / fixed regex (#1164) Three CI failures from the previous commit: - ``test_beta_section_header_present_with_danger_styling`` and ``test_two_step_save_note_present`` asserted on static ``panel-server`` markup that lives in the rendered HTML template, not in any JS-populated container. MIN_DOM doesn't replicate the full panel-server children (by design — it's a minimal handler stub), so the assertions never found their target strings. Switch both tests to assert directly against ``_SETTINGS_HTML``; the presence of the markup at the template level is the property we actually want to lock down. - ``test_beta_rows_render_into_betaBody`` used a regex ``<div id="betaBody">(.*?)</div>\s*<div`` whose ``</div>\s*<div`` boundary matched the very first nested ``</div><div`` *inside* the master row (after the ``.feature-name`` close tag), so ``bb_content`` only captured the header of the master row. Anchor the boundary on ``</div>\s*</body>`` instead — non-greedy capture between the ``betaBody`` open tag and the body close still gives the full container content. Added a fallback path that asserts on class markers + non-leakage to featuresBody if the regex still misses. * feat(settings): fix stuck-master bug, master in dev schema, cascade-clear, drop connection panel, sync backup_hint+verify_ssl (#1164) Six fixes against the Server Settings + addon Configuration surfaces: 1. ``maybe_auto_enable_beta_master`` now requires ``config.get(key) is True`` instead of ``key in config``. The bare presence check fired the moment HA Supervisor merged the dev addon's schema defaults into options.json, locking the master "on" with origin=env on every fresh dev install even when all 5 sub-flags were False. New unit tests cover the truthy / all-false / one-of-many / non-bool-truthy permutations so a future regression on the semantic fails loudly. 2. Master ``enable_beta_features`` moved into the dev addon Configuration tab (schema + options + translations + DOCS.md). Defaults ON in dev — beta tools are the channel's purpose, so a fresh install lights them up without the user round-tripping to the web UI. Stable's schema is unchanged; the standalone web UI master path remains the gate there. start.py writes ENABLE_BETA_FEATURES from options.json only when the key is present; ``get_feature_flag_origin`` now treats the master like the sub-flags (env-var presence in addon mode → origin=addon). ``maybe_auto_enable_beta_master`` is kept as a one-cycle legacy bridge for installs whose options.json pre-dates the master key. 3. Beta sub-flags also default ON in the dev addon options block. 4. Master-off cascade clear: flipping ``enable_beta_features=False`` in ``_save_feature_flags`` now also writes False for every truthy beta sub-flag in the same save. Without this, sub-flags stayed True in the override file and resumed the moment the master was flipped back on — UX bug the user reported as "having to turn off every toggle individually." JS mirrors the cascade so sub-rows visually de-toggle on master-off without a page reload. 5. "Connection (display only)" section removed from the Server Settings panel. The read-only HOMEASSISTANT_URL / TOKEN / SUPERVISOR_TOKEN fields just wasted space — operators already see them in addon logs and configuration. Registry entries kept in ADVANCED_SETTINGS_FIELDS so the API still returns them (env-pin debugging, future surfaces). ``verify_ssl`` moved from ``connection`` to ``operations`` so it still renders in the panel. 6. ``backup_hint`` and ``verify_ssl`` now sync between addon Configuration and the web UI like feature flags do. New ``ADDON_SYNCED_ADVANCED_FIELDS`` set drives the origin helper (returns ``'addon'`` in addon mode for these) and the save handler (routes their writes through Supervisor ``/addons/self/options`` instead of the override file). Cross-surface gate visibility note appended to every beta sub-flag description in ``translations/en.yaml`` (and a top-of-options note on the master) so addon Configuration users know the web UI master gates everything. Locked-banner addon-mode copy from the earlier commit was already in place; tests in this commit re-target fixtures from the removed connection section to ``search``. * fix(addon-dev): beta sub-flags default OFF — only the master defaults ON (#1164) Mis-read of the user's intent in the previous commit. The intended shape for the dev addon's fresh-install defaults is: enable_beta_features: true ← gate unlocked enable_yaml_config_editing: false ← user opts in enable_filesystem_tools: false ← user opts in enable_custom_component_integration: false ← user opts in enable_code_mode: false ← user opts in enable_lite_docstrings: false ← user opts in The previous commit defaulted every sub-flag to true alongside the master, which would have shipped every beta tool live on a fresh dev install — including filesystem writes and the YAML config editor. Each sub-flag mutates the user's HA system, so they remain opt-in even on the dev channel; the master being on just means the gate is open. * revert(scope): drop scripts/fork-dev/ — maintainer tooling, wrong repo (#1164) Pushed these in 69b7a235 as part of task #17. They're personal-fork test tooling for the fork-dev addon workflow — they have no business on master. Removing from the PR; they're still in this branch's git history if anyone needs to fish them back out. * fix(addon+ui): #1431 review pass — restore MCP_HOST, gate sub-flag env writes, sane save (#1164) Round-2 review pass addressed 14 verified findings: **Bugs**: - **MCP_HOST regression** restored. The PR's earlier merge of upstream/master dropped the `bind_host = os.getenv("MCP_HOST", "0.0.0.0")` block introduced by #1434/#1436. `mcp.run(host=...)` now goes through `bind_host` again. - **Beta sub-flag env vars** are now written only when the matching key is present in `/data/options.json`. start.py was writing ENABLE_YAML_CONFIG_EDITING=false (etc.) unconditionally on stable addon, marking those fields origin='addon' in the web UI; the user's save then POSTed to Supervisor which rejected because the keys are not in stable's schema. - **Env-pinned tool save 409**: `_save_tools` now accepts re-sends whose state matches the env-pinned value, rejecting only true mismatches. The JS `saveConfig` POSTs the entire `toolStates` map including env-pinned rows; without this fix every save with `DISABLED_TOOLS` / `PINNED_TOOLS` non-empty would 409. - **Cascade-clear** now reads the persisted override file directly via `_read_feature_flag_override_file()` instead of `get_global_settings()` (whose master gate had already forced sub-flags to False, hiding stale-true overrides). Also force-False sub-flags that are explicitly True in the same payload as master=false, so `{master:false, sub:true}` no longer lands an inconsistent persisted state. - **Master beta-gate check** is now applied uniformly (no more "skip in addon mode" carve-out). The skip existed because the legacy auto-enable wrote ENABLE_BETA_FEATURES from sub-flag presence; now start.py writes the master from its own options key, so the gate is sound to apply in both modes. - **Dev-upgrade silent-disable warning**: start.py logs when master=false but a sub-flag is true in options.json, so an operator who toggled the master off in Configuration after a pre-#1164 dev install sees why their previously-enabled beta tools went away. - **Mixed-batch advanced save** is now split client-side. The server-side guard that returned 500 stays as a defense, but the UI no longer triggers it — `saveAdvancedSettings` partitions `_advancedDirty` into addon-routed and file-routed batches. **Logging / defensive code**: - Supervisor failure in `_save_advanced_settings` now logs before returning, matching the sibling `_save_feature_flags` / `_save_backup_config` handlers. - The three `assert sup_err is not None` sites that would crash under `python -O` now explicitly return INTERNAL_ERROR (covers the addon-route paths in feature flags and advanced settings). - `_apply_advanced_overrides` narrows `except Exception` to `(ValueError, TypeError)`, matching the parallel `_apply_feature_flag_overrides` exception shape. - `maybe_auto_enable_beta_master` now logs which sub-flag(s) triggered the legacy bridge when it fires, with a removal- candidate note in the docstring. **Docs / UX**: - Docstring drift fixed in `get_feature_flag_origin`, `_get_advanced_settings`, `_save_advanced_settings`. - `envLockedNoteHtml` master copy rewritten — origin='env' on the master is now only the legacy-bridge path, not the default dev-addon path. - "Bottom" save button now actually at the bottom of `panel-server` (below the beta block and its code-mode sub-numerics). Second two-step save note duplicated near the bottom row so users editing dangerous beta toggles also see it. Findings deferred to follow-up (legit but bigger than this pass): - A.2 concurrent-save read-modify-write race (needs an `asyncio.Lock` around the override-file path; functionality is safe today because the runtime gate hides the persisted-state inconsistency). - A.4 master-flip via addon Configuration tab → no cascade (the runtime gate + new log warning cover the observable surface). - F.* missing tests for new behaviours — adding in a follow-up commit. * test(settings): cover #1431 review pass fixes (#1164) - ``test_env_pinned_noop_resend_does_not_409`` — JS saveConfig POSTs the entire toolStates map; env-pinned no-op resend must be accepted. - ``test_env_pinned_value_mismatch_still_409s`` — pin true flips still rejected. - ``test_save_features_cascade_clears_subflag_even_when_payload_says_true`` — in-payload {master:false, sub:true} → 409 instead of inconsistent persisted state. - ``test_save_features_cascade_reads_override_file_not_post_gate_settings`` — cascade reads the file directly so it catches stale-true sub-flag overrides hidden by the master gate on the resolved Settings. - ``test_stable_addon_does_not_declare_enable_beta_features`` and ``test_dev_addon_declares_enable_beta_features_master_in_schema`` — lock the schema asymmetry that makes the dev/stable channel distinction work. - ``test_dev_addon_defaults_every_beta_subflag_to_false`` — sub-flags remain opt-in even on dev. * fix(settings): serialise override-file RMW + cover addon-synced advanced save (#1164) A.2 (concurrent-save race): both ``_save_feature_flags`` and ``_save_advanced_settings`` touch the same ``feature_flags.json`` override file. Two near-simultaneous requests could interleave their read/merge/write and clobber each other's persisted state. The runtime master gate hid the inconsistency for beta sub-flags, but other field combinations (advanced + feature-flag in the same window) would have lost state silently. Wrap the RMW window in an ``asyncio.Lock`` (lazy-initialised under the live event loop so module import doesn't bind to no loop). Both handlers acquire the same lock, so saves serialise correctly. F.35 + F.40 (test coverage): - ``test_save_advanced_addon_synced_routes_through_supervisor`` — ``backup_hint`` / ``verify_ssl`` saves in addon mode call ``_supervisor_merge_and_post_options``, return ``mode='addon'``, do NOT write the override file. - ``test_save_advanced_addon_synced_supervisor_4xx_surfaces_validation_failed`` — Supervisor schema rejection surfaces as ``CONFIG_VALIDATION_FAILED`` with the Supervisor status code preserved, not a generic 502. - ``test_origin_for_addon_synced_field_is_addon_in_addon_mode`` — pins the origin matrix: ``backup_hint`` / ``verify_ssl`` come back ``origin='addon, editable'`` in addon mode; non-synced env-pinned fields stay ``origin='env, locked'``. * fix(settings): A.8 preserve sub-flag visual + cover remaining deferred test gaps (#1164) A.8 — JS master-off no longer flips sub-flag checkboxes visually. Previously the cascade-clear set ``_lastFeatureFlags[sub].value = false`` on master-off so the re-render painted sub-rows unchecked. That fought the user's mental model — they expressed intent on individual sub-flags, master-off shouldn't visually wipe that context. Now sub-rows stay checked + dimmed + disabled after the master flips off; the server-side cascade still clears the values on disk, so a refresh shows the cleared state, and a failed save leaves the visible checked state matching the actual on-disk state. F.37 — ``test_master_off_click_dims_subrow_live_without_clobbering_value`` dispatches a real change event on the master input and asserts the sub-row goes dimmed + disabled but keeps its checked attribute. F.38 — three cross-mode origin permutations for the master: - dev-addon env-set → 'addon' - standalone env-set → 'env' - addon mode + file override (no env) → 'file' F.42 — ``test_dual_save_buttons_mirror_disabled_and_status_state`` probes both ``advSaveStatus`` / ``advSaveStatusTop`` text and both buttons' disabled state via a hidden probe div, asserts the mirror holds at save completion. * fix(tests): probe JSDOM properties via probe div; assert mid-save mirror (#1164) Two test bugs in 25b45ab7's new assertions: 1. ``test_master_off_click_dims_subrow_live_without_clobbering_value`` asserted on the literal string ``"checked"`` in the serialised DOM. ``input.checked`` is a DOM property (not an HTML attribute), so JSDOM's serialiser doesn't emit it. The .checked state IS true at the property level — just invisible to the regex. Probe via a hidden ``__sub_state_probe`` div that reads the .checked / .disabled properties and writes them to data-* attributes. 2. ``test_dual_save_buttons_mirror_disabled_and_status_state`` probed AFTER the full save+reload chain, by which point loadAdvancedSettings() had blanked both status text els. Restructure the test to probe SYNCHRONOUSLY after ``click()``, while saveAdvancedSettings is mid-flight (status="Saving…", both buttons disabled). That's the actual mirror invariant we wanted to lock — the helpers ``_setAdvSaveDisabled(true)`` and ``_setAdvSaveStatus('Saving…')`` run synchronously before the first await. * feat(settings): drop sub-flag cascade-clear; restore advanced_debug_logging translation; clearer Save-button copy (#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. * fix(settings): #1431 review pass — address 13 verified findings (#1164) Round-3 review pass landed 13 verified findings; the cosmetic "persisted-value visible in UI after F5" item was explicitly skipped per user direction (UI shows post-gate value when master off; user accepts this since beta tools are actually disabled at runtime — the preserve-across-master-cycle UX is at the data layer, not the visual). Source fixes: - **Save button copy is now source-blind** — the previous "your feature-flag toggles already saved on click" claim only held when ``saveFeatureFlag`` raised ``restartNotice``; tool-config pin saves, backup-config saves, and cross-tab ``restart-required`` broadcasts also raise it. New copy: "a restart is pending. Click Restart above to apply your prior changes." (#2) - **Stale F.37 test docstring** describing the deleted server-side cascade rewritten. (#3) - **Beta-gate INFO log noise** — cascade-clear removal meant the gate could fire its "forcing %s=False" line every Settings rebuild, spamming addon logs once a user had truthy sub-flags persisted. Dedup via ``_BETA_GATE_LOGGED`` set per process, cleared on ``_reset_global_settings``. (#9) - **Lazy-lock docstring** updated to reflect Python 3.13 semantics (``asyncio.Lock()`` no longer takes a loop arg; the lazy pattern still serves test fixtures and single-loop deployment, with the invariant documented). (#10) - **Addon-mode carve-out comment** clarified to distinguish dev (master in schema) from stable (master web-UI-only). (#14) - **probe-div null branch** in F.37 now writes ``data-error`` so a failing test points at "selector missed" vs "value flipped" unambiguously. (#17) Tests added: - ``test_translations_cover_every_schema_key`` — parity check that every ``schema:`` key has a non-empty translation ``name`` and ``description``. Parameterised across stable + dev addons. Pins the class of silent gap that this PR's ``advanced_debug_logging`` fix addressed. (#4) - ``test_save_features_acquires_override_file_lock`` + ``test_save_advanced_acquires_override_file_lock`` — counting-lock wrapper asserts ``async with _get_override_file_lock()`` runs exactly once in each file-mode write path. Pin against a regression that silently bypasses concurrent-save serialisation. (#5) - ``test_dual_save_buttons_mirror_disabled_and_status_on_post_failure`` — exercises the 500-response branch of the dual-save mirror so a regression that broke ``_setAdvSaveStatus``/``_setAdvSaveDisabled`` for error paths only would still fail. (#6) - ``test_save_features_master_on_restores_subflag_values_in_addon_mode`` — addon-mode round-trip mirror of the existing standalone restore test; asserts the Supervisor merge-and-post call carries only the master flip-on and never zeroes out sub-flag values. (#7) - ``test_save_button_nothing_to_save_when_no_dirty_and_no_restart`` + ``test_save_button_restart_pending_hint_when_dirty_empty_but_restart_showing`` — both branches of the empty-dirty Save click are exercised; the restart-pending branch asserts the copy is source-blind. (#8) Deferred per user direction: - #1 (file-vs-Settings visual after F5): user accepts the current behavior (UI shows post-gate value; runtime tools actually disabled when master off; data-layer preserve still works end-to-end). - #11/#12/#13 (code-simplifier helper extractions): skipped as complicated to implement without behavior risk. - #15 (pre-#1164 users with already-cleared sub-flags): release-note concern, not a code change. - #16 (lock fragility under future thread-pool dispatch): speculative future-risk; not actionable today. * feat(settings): version footer + Patch76 review feedback Adds the running ha-mcp version to the settings UI footer (issue #1466). ``info.version`` flows from ``/api/settings/info`` → ``HA_MCP_BUILD_VERSION`` env var on addon builds (set by both stable and dev Dockerfiles) → package metadata fallback. Empty on older deployments without the field. Addresses Patch76's review (no blockers, all bundleable): - ``OverrideField`` folds the structurally-identical ``FeatureFlagField`` and ``BackupOverrideField`` into one NamedTuple; aliases preserve readable construction sites. ``AdvancedField`` keeps its own type since it c…
kingpanther13
added a commit
that referenced
this pull request
Aug 25, 2026
A step id is a caller-controlled key, and these rejections reach the usage log unmasked the same way a value would. Nothing echoed a NESTED caller key before this directive existed — the walker's supplied_keys reports only top-level names — so this was new exposure rather than an existing one. Errors now name the entry's position (step_values entry #2) and its types. That still says which entry is wrong, and cannot carry anything the caller typed. Found by CodeRabbit review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr
kingpanther13
added a commit
that referenced
this pull request
Aug 26, 2026
…meassistant-ai#2256) * fix(flows): keep unnamed fields when editing an existing config entry An options, reconfigure or subentry-reconfigure step arrives pre-filled by Home Assistant's add_suggested_values_to_schema, and saving the UI form posts every box back. The flow walker submitted only the keys the caller named, so voluptuous filled each omitted vol.Optional(k, default=STATIC) with its static default and dropped every no-default optional outright: a one-field patch through ha_set_integration(entry_id=..., config=...) silently reset the rest of the entry. Repro on core workday, where config={"days_offset": 3} reset the workday/exclude lists and wiped province, returning success with no warnings. Thread keep_current_values through the two flow walkers and the form-step consumption. Under it, a declared field the caller named no key for is submitted with the value the step itself carries (suggestion, else a constant's only legal value), including leaves inside sections the caller never named, since the section is a box on the same form. A bare "default" still means omission, exactly as it does for the UI's own form. An explicit null is the opposite request and is honoured as a clear: consumed, then left out of the payload. Backfilled values are the step's data, so they neither count towards the "consumed at least one caller key" guard nor satisfy reconfigure's "consumed EVERY key" one, and a value carrying a redaction sentinel is never written back. The flag is set by update_config_entry_options (ha_set_integration options mode and helper updates), the official reconfigure flow, and the subentry reconfigure branch. Create flows - add integration, create helper, create subentry - are unchanged: there is no stored value to preserve and materializing a field would invent data. Fixes homeassistant-ai#2254 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * refactor(dev): harvest update_source's preserved overrides from the flow schema Replaces the hardcoded _PRESERVED_OPTION_KEYS tuple with a schema-driven harvest (description.suggested_value, the same suggestion-over-default rule as the walker's keep_current_values backfill), so a field added to the component's options flow later cannot be silently wiped by a partial update_source submit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): submit an explicit null for defaulted fields instead of omitting it Omitting a null'd field that carries a voluptuous default let HA substitute the static schema default, so the tool reported success while writing a value the caller never asked for and never cleared the field. Only a field with no default can express a clear by omission; everything else submits the null for HA to validate. Found by Codex review on homeassistant-ai#2256. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * test(e2e): stop the HACS nudge lanes racing their own retry schedule Both positive lanes waited exactly RETRY_DELAYS[0] (30s) for the marker. The nudge's first attempt is immediate, but a launcher can beat HA to registering HACS's WS handlers; that attempt returns unknown_command, which _refresh_with_retries cannot tell apart from 'no HACS' and so retries at RETRY_DELAYS[0] — landing attempt two AT the deadline. Seen on the ubuntu-24.04-arm leg of homeassistant-ai#2256: unknown_command at 02:26:42, retry at 02:27:13, assert at 30s. Derives the wait from the schedule so the two cannot drift apart, and corrects both failure messages, which blamed the lifespan for not scheduling the nudge when it had scheduled it and was mid-retry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * test(e2e): budget the nudge wait for a hanging first attempt The wait covered RETRY_DELAYS[0] but not the time attempt one can spend before it fails: hacs/repositories/list has no explicit timeout, so it waits DEFAULT_COMMAND_WAIT_TIMEOUT before giving up, putting the start of attempt two at the old deadline. Budget both command timeouts and the retry delay, all derived so neither schedule can outgrow the wait. Names send_command's default reply timeout, which callers scheduling retries have to budget around. Found by CodeRabbit review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * test(e2e): pin the homeassistant-ai#2254 partial-options wipe against a real options flow The existing options round-trip could not catch it: group's LIGHT options are all vol.Required, and required fields were already backfilled from the step's suggestion. group_type=sensor is the reachable repro -- its schema adds vol.Optional(ignore_non_numeric, default=False), the optional + static-default shape that voluptuous silently refills when the key is omitted. Complements the unit suite rather than repeating it: those pin the payload against a hand-written copy of HA's serialization and would keep passing if HA changed how it emits suggested_value. Verified to fail pre-fix (field omitted, persisted False) and pass post-fix (True submitted and kept). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * test(e2e): use call_tool_success for the homeassistant-ai#2254 expected-success calls tests/AGENTS.md line 51 is the convention for success-expecting calls; the new test had copied the sibling's older call_tool + assert_mcp_success pattern. call_tool_success also turns a raised ToolError into a named assertion failure instead of an opaque exception. Found by CodeRabbit review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * test(e2e): set the homeassistant-ai#2254 baseline through the options flow ignore_non_numeric is in group's SENSOR_OPTIONS, not its config schema, so setting it on the add call was dropped as an undeclared key and the entry never had it — the baseline wait timed out on every full-suite leg. Set it with a full options submit instead, which also serves as the control for the partial patch that follows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): let an explicit clear beat a required section's prefill _required_section_defaults seeds a REQUIRED section with the step's own values before anything is consumed, and _field_default_value reads description.suggested_value first -- so an optional no-default leaf inside one arrives already holding its stored value. Omitting the cleared key let that seed survive the merge: the old value was submitted while the tool reported a successful clear. A clear now rides through the merge as a _CLEARED sentinel in the key's place and is stripped once the section is assembled, so it wins at any nesting depth and a section holding only clears is dropped rather than submitted. Also stops update_source echoing preserved credentials: the component's options form exposes oauth_client_secret as a suggested_value, so the schema-driven resend picks it up (correctly -- omitting it would clear it), but this path submits the flow directly and never meets the flow-schema redaction. The response now reports only what the caller asked to change. And budgets the HACS nudge wait for send_hacs_repository_refresh's own HACS_REFRESH_TIMEOUT, which runs after the retried list call and writes the marker only once it returns. Found by Codex review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * test(dev): pin update_source's response to the caller's requested change test_non_embedded_never_routes_to_component_write asserted the response echoed the preserved server_url alongside the channel delta — the contract that leaked oauth_client_secret. Its subject is the sync-vs-scheduled routing, so it now asserts the trimmed response AND that the submission still carries the override, pinning both halves of the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): keep the caller's intent across a redeclared optional field begin_step clears only filled, and the caller's key is popped from remaining_config by the first step — so a later step declaring the same optional field reached _redeclared_field_submission with nothing marking it as the caller's, and keep_current_values handed it straight to the backfill. That resubmitted the entry's stored value: a null clear was silently undone, and a caller's new value was overwritten. _ReuseState.recorded_value survives the whole walk (only filled is per-step) and distinguishes a recorded None from no record, so it now decides first: a recorded None keeps the field omitted, a recorded value is resubmitted as the caller's through the existing claim_write path, and only a field the caller never named falls through to the step's own value. The required-field asymmetry predates this PR and is left alone. Found by Patch76 review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * refactor(flows): extract the edit-mode submission decision The caller-intent-before-backfill branches pushed _redeclared_field_submission to C901 12 > 10. Repo policy is extract, never a per-file ignore, and the extracted decision stands on its own: what to submit for an edit-mode field the caller named no key for at THIS step. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): keep the stored value when a revisited step's write is spent claim_write allows one reused write per (step, path). Once spent, an optional edit-mode field fell back to omission — which hands voluptuous the field's STATIC default and overwrites the entry's stored value, the exact wipe this mode exists to stop. A menu loop revisiting the same step is enough to reach it. The step's own value now goes back instead. The required-field branch still omits: there, omission raises HA's own loud 'required key not provided' rather than losing data silently. Found by CodeRabbit review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): make a clear hold past its first encounter What expresses a clear depends on the field's shape, and only one of the two sites deciding it carried the qualifier. _consume_leaf_field submits a null verbatim for a field with a "default" — omitting it there would let voluptuous substitute that default — while _edit_mode_submission omitted any recorded None. So a defaulted field cleared on step one had the static default handed back on its second encounter, reporting success having cleared nothing. Reachable by a later step redeclaring the field or a menu loop revisiting the same one. Both sites now share _clears_by_omission, so they cannot drift again. Also pins the warning the optional reuse path now emits: resubmitting the caller's value is the intended outcome there, so the note rides with it. Found by Patch76 review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * feat(flows): address a field per step encounter via step_values The flat config dict is keyed by field NAME alone, so a flow whose steps declare the same field twice — a later step redeclaring it, or one revisited through a menu loop — could only ever carry one value for both. The walker reused that value everywhere and warned that per-visit values could not be expressed, which is what made the warning a dead end: it reported a limitation with no remedy, on an outcome that was otherwise correct. step_values={'<step_id>': {'<field>': <value>}} supplies the remedy, mirroring the reserved-key convention next_step_id already uses. A step's entry shadows the flat value for that step only and is restored after, so addressing one step never spends the caller's flat value and a step nobody addresses behaves exactly as before. It buys three things the flat dict cannot express: a different value per visit, a value on only some visits, and a clear on one visit but not another. The warning now names the remedy instead of declaring the case impossible, which resolves the review point that it fired on a correct outcome with nothing the caller could do about it. Found by Patch76 review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): report step_values entries the flow never presented A step_values key naming a step the walk never reaches — a typo'd step_id, or a branch the menu selections never took — applied nothing and said so nowhere. Each step's entry is now spent as it is applied, so whatever remains at the end is named in warnings, mirroring how an un-consumed menu selection is reported. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): isolate step_values from reuse and stop it swallowing typos Two defects in the per-step overlay, plus the CodeQL gate: 1. The overlay goes through the normal leaf consumer, which records what it consumed in _ReuseState — and flat/scoped survive the whole walk. A value the caller scoped to one step was therefore reused by a LATER unaddressed step instead of that step's own stored value. _ReuseState now knows which names are step-scoped and keeps them out of flat/scoped, while still marking them filled so nothing is injected over them in their own step. 2. Overlay keys were dropped after the step, so a field the step's schema never declared vanished before ignored-key accounting saw it — and the reserved outer key suppressed any warning, letting the update report success having applied nothing. An unconsumed overlay key is now reported under its own path (step_values.<step_id>.<field>) before being dropped. 3. _PER_STEP_VALUES_KEY moved to config_entry_flow_form, its only user. CodeQL's py/unused-global-variable reads a constant defined in one module and used only from another as dead — the same reason _MENU_SELECTION_KEYS already lives there. A code fix, not an allowlist entry. Found by CodeRabbit review and the CodeQL gate on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): point the empty-forms error at step_values step ids step_values is a directive, not a field, so a config of nothing but a step_values entry naming a step the flow never presents tripped the empty-forms guard and was told to check its field names. The likely mistake there is the step_id. Guidance now branches on what was actually supplied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): scope nested step_values leaves out of reuse too step_scoped holds the overlay's TOP-LEVEL keys, but the guard compared the leaf name — so an overlay key naming a section let every value inside it through. For {'connection': {'province': 'TX'}} the leaf is 'province', which is nowhere in {'connection'}, and a later step declaring connection.province resubmitted TX instead of its stored value. Matched on the root of the declaration path instead. Found by CodeRabbit review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): close the flat-key reuse leak and let step_values loop Two shapes the per-step overlay still got wrong: 1. record() matched only the declaration ROOT against step_scoped, which catches an overlay key naming a section. The mirror case is a FLAT overlay key filling a leaf a section declares — the walker accepts that — where the root is the section name and equally absent. The step-scoped value then survived the whole walk and was submitted for a step nobody addressed. Both the popped key and the root are matched now; either check alone leaks the other shape. 2. A step's entry was spent on its first encounter, so a menu loop got the scoped value once and the step's own suggestion afterwards — fewer encounters than the flat key it replaces, which is what claim_write's note points callers at. A LIST is now consumed one entry per encounter with its tail replacing the key until dry, mirroring next_step_id, which is the only shape that can express a loop or a different value per visit. A dict still applies once. The leftover warning covers both causes: a step never presented, and a tail left because the step ran fewer times than the list expects. Found by Patch76 review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): reject a malformed step_values directive step_values is consumed as a directive, so a malformed one was invisible to ignored-key reporting: an outer value that is not an object, or a list entry that is not one, applied nothing and still returned success. Two of the four malformed shapes warned; the other two were silent. validate_step_values runs at the top of both walkers, before any step is driven, and raises ToolError per the repo's tool-failure rule rather than warning — a malformed directive is caller error, not a degraded result. Valid shapes are unchanged: a dict entry, or a list of dicts for a step the flow presents more than once. Found by CodeRabbit review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): reject an explicit null step_values directive The guard returned early on a falsy directive, so {'step_values': None} passed validation — and because the reserved key is excluded from ignored-key reporting, the walk then applied the caller's other fields and returned a clean success for a directive that did nothing. Key PRESENCE is the test, so an explicit None now falls through to the not-an-object rejection. Found by CodeRabbit review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): keep submitted values out of step_values rejections Three problems with the boundary rejection: 1. The error context echoed the caller's entry verbatim, and a rejected directive can carry a credential the caller is submitting for the FIRST time. Home Assistant does not hold it yet, so no read-back has harvested it and RedactSecretsMiddleware cannot scrub it even when enabled — and raise_tool_error serialises the whole response into the exception message, which @log_tool_usage records as error_message from inside the tool, where only 'parameters' are masked. The value reached plaintext mcp_usage.jsonl. Reports received_type / entry_types now; the shape is what diagnoses a malformed directive anyway. 2. Entries that apply nothing were reported inconsistently: a bare {} left a leftover the warning named, while [] and [{}] were popped at their first encounter and vanished, so the walk reported a clean success for a directive that did nothing. All four shapes reject now. 3. The list form was taught only by the rejection text. It is on all three composing surfaces now, including the resubmission note — the one that fires in the revisited-step case a single dict cannot express. Found by Patch76 review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): report an entry position, not the caller's step id A step id is a caller-controlled key, and these rejections reach the usage log unmasked the same way a value would. Nothing echoed a NESTED caller key before this directive existed — the walker's supplied_keys reports only top-level names — so this was new exposure rather than an existing one. Errors now name the entry's position (step_values entry #2) and its types. That still says which entry is wrong, and cannot carry anything the caller typed. Found by CodeRabbit review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * fix(flows): scope the no-echo rule to values, keep the per-encounter no-op Three points from review, all measured first: - The comment justifying position-over-step-id claimed nothing echoed a nested caller key. Field names do: step_values.<step>.<field> goes into ignored_config_keys, which _unconsumed_reconfigure_keys unions into the reconfigure abort error -- the same logged path. That is kept, because naming the field is the whole diagnostic for a typo and a name is not the secret a value is, so the comment is scoped to values instead. - An empty object INSIDE a longer list is a capability, not a mistake: it means "leave this encounter as it would be without the directive", which nothing else expresses -- omitting the step affects every encounter, and {"field": None} is an explicit clear. Documented in the validator docstring and pinned by a test; entries that apply nothing anywhere are still rejected. - context={} was a no-op (create_error_response gates on truthiness) and the guard reduced to "not any(entries)". Both simplified. Verified live on a real workday options flow before fixing: the [{}, ...] shape is accepted, the empty entry is a genuine no-op, and the tail is reported. Found by Patch76 review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr * test(flows): actually pin the per-encounter no-op The test demonstrated the fallback and guarded the dead shapes in two halves that never met: the demonstrating half drives _handle_form_step, which never calls validate_step_values -- only the walkers do. So nothing put the fallback shape through the validator, and tightening `any` to `all` killed the capability with the whole file still green. Measured: 73 passed under that mutation. One line puts the same shape through the validator. Now 73 passed unmutated, 1 failed under the mutation, failing on the fallback assertion rather than incidentally on a dead shape another test already covers. Found by Patch76 review on homeassistant-ai#2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr --------- Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR contains the following updates:
2025.12.4→2025.12.5Release Notes
home-assistant/core (ghcr.io/home-assistant/home-assistant)
v2025.12.5Compare Source
Configuration
📅 Schedule: Branch creation - "after 3pm on tuesday" in timezone UTC, Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
🔕 Ignore: Close this PR and you won't be reminded about this update again.
This PR has been generated by Renovate Bot.