Skip to content

Commit b9b768a

Browse files
feat: Phase 3 write capabilities — call_service and bulk_call_service (#1813) (#1921)
* fix: register device operation before dispatch to close the state_changed race control_device_smart stored the pending operation AFTER awaiting call_service, so a fast entity's state_changed could arrive before the op existed and be dropped, leaving the op PENDING until a coincidental later event or timeout. Register before dispatch; on dispatch failure flip the op to FAILED so a later unrelated event can't spuriously complete a write that never happened. * feat: component call_service capability with authoritative ha_mcp_tools domain block The first Phase 3 write capability (issue #1813): an in-process `ha_mcp_tools/call_service` WS command that fires exactly one `hass.services.async_call` and returns the REAL pre->post state transition for the target entities, event-confirmed via an `EVENT_STATE_CHANGED` listener registered BEFORE the dispatch (D5) rather than a hardcoded expected-state guess. All awaiting work lives in the async `_call_service_prep`; `_do_call_service` is a pure formatter (D2). An authoritative component-side domain block refuses `domain == "ha_mcp_tools"` before `has_service`/dispatch (D1), independent of the server-side guard, so this second write path can never be turned into an in-process invoker of the admin-gated `ha_mcp_tools.*` services. A confirmation timeout is `partial`, never a failure (D4); pre-dispatch problems propagate for the server to map (D7). Component side only; the server consumer and bulk_call_service are separate later tasks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FBYnVcu8ueWjeCfVTTDAc * feat: component bulk_call_service capability (register-before-fire batch) The batch write capability (Phase 3, D5a — issue #1813): an in-process `ha_mcp_tools/bulk_call_service` WS command that runs the authoritative D1 `ha_mcp_tools` domain block for EVERY operation FIRST — before any pre-state read, listener registration, or dispatch — so a batch is fail-closed: one refused op (the guarded domain or an unknown service) raises the whole frame and NOTHING is dispatched. No partial batch can smuggle a `ha_mcp_tools.*` op past the guard. It then registers ALL confirmation listeners in one synchronous pass BEFORE any dispatch (register-before-fire is trivially correct for the batch), fires the operations (`parallel` by default via `asyncio.gather(return_exceptions=True)`, or sequentially), and waits on ONE shared deadline for every op's transition. A per-op `async_call` failure under `parallel` is captured on that op's result (`error` + `dispatched: false`) WITHOUT aborting the others; a post-dispatch confirmation timeout is `partial`, never a failure (D4). All awaiting work lives in the async `_bulk_call_service_prep`; `_do_bulk_call_service` is a pure formatter that reuses the single `call_service` guard / transition / diff helpers. Component side only; the server consumer is a separate later task. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FBYnVcu8ueWjeCfVTTDAc * feat: route ha_call_service and ha_bulk_control through the write capabilities * chore: bump component to 1.3.0 for the call_service write capabilities * test: guarantee EVENT_STATE_CHANGED const and isolate caps probe for full-suite runs * fix: address Phase 3 review — post-send at-most-once, loop-thread confirmation listener, non-confirmed result content Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FBYnVcu8ueWjeCfVTTDAc * fix: close Phase 3 write at-most-once gaps from round-2 review - add HomeAssistantCommandNotSent (never-sent subtype) raised only at send_command's two pre-send sites; both write consumers catch it first and fall back to legacy, while a post-send drop stays ambiguous (C1) - treat a malformed/unusable SUCCESS envelope as ambiguous, not legacy, so an already-landed write is never re-fired (I2) - make the component's post-dispatch formatting total: raise-proof attribute diff (_values_differ) plus a try-wrap degrading to a dispatched-but-unconfirmed envelope, so a command error is genuinely pre-dispatch (I1) - ride under the pending 1.2.0 (revert the 1.3.0 over-bump) (I3) - minors: PENDING-only fail_pending_operation, per-op bulk frame timeout, dispatched_unconfirmed status, verbose routes to legacy, None new_state is not confirmed, accurate ambiguous progress message Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FBYnVcu8ueWjeCfVTTDAc * style: ruff format round-2 test file Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FBYnVcu8ueWjeCfVTTDAc * test: raise TypeError in array-like test double for CodeQL gate py/unexpected-raise-in-special-method flags a __bool__ that always raises ValueError; TypeError is the conventional bool-coercion failure and is immaterial to _values_differ (catches any Exception). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FBYnVcu8ueWjeCfVTTDAc * fix: treat a send() failure as ambiguous and close three write-routing bugs - send_command: a send_json_message failure re-raises the ORIGINAL exception (ambiguous — bytes may already be on the socket), not HomeAssistantCommandNotSent; the readiness guard stays the one provably-never-sent site. Consumers then treat a send failure as ambiguous/partial (never re-fired), only the readiness case as legacy - _bulk_via_component: route the whole batch to legacy when an entity_id repeats (the component's per-entity waiter collapses both ops onto the first state_changed) - _maybe_component_call_service: a comma-separated (multi-target) entity_id routes to legacy (the component confirms one literal entity_id, yielding a false partial) - _bulk_frame_timeout: honor an explicit timeout_seconds=0 (only an absent key -> 10) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FBYnVcu8ueWjeCfVTTDAc * fix: confirm component writes on the expected state, not the first event The component confirmed a write on the FIRST state_changed event for a target, a regression vs the legacy expected-state verifier: a multi-phase service (lock -> locking -> locked) confirmed on "locking", an attribute- only noise tick confirmed a state that never changed, and an idempotent no-op (turn_on already-on, no event) waited out the full timeout then falsely reported partial. The server now passes its _SERVICE_TO_STATE.get(service) expected primary state to the component as an optional confirmation-timing hint. The component confirms only on reaching that state (skipping intermediate and noise events), immediate-matches when the current state already equals it (idempotent no-op, no wait), and falls back to today's any-first-event behavior when no hint exists. The returned transition is still the real observed one; the hint governs timing only. _SERVICE_TO_STATE moves to util_helpers as the single source of truth, imported by both the single (tools_service) and bulk (device_control) write paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FBYnVcu8ueWjeCfVTTDAc * style: use dict.fromkeys for constant-value expected_by_entity (C420) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FBYnVcu8ueWjeCfVTTDAc * chore: allowlist _SERVICE_TO_STATE cross-module CodeQL false positive The map moved to the leaf util_helpers module and is imported by tools_service and device_control; CodeQL's single-file analysis misses the cross-module read. Same class as the existing _tools_meta suppression. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FBYnVcu8ueWjeCfVTTDAc --------- Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 692275d commit b9b768a

18 files changed

Lines changed: 5120 additions & 42 deletions

custom_components/ha_mcp_tools/websocket_api.py

Lines changed: 912 additions & 1 deletion
Large diffs are not rendered by default.

scripts/codeql_quality_gate.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,16 @@
113113
"_coerce_tool_states. CodeQL's single-file analysis misses the "
114114
"cross-module import, so the declaration looks dead.",
115115
),
116+
(
117+
"py/unused-global-variable",
118+
"src/ha_mcp/tools/util_helpers.py",
119+
"_SERVICE_TO_STATE",
120+
"Cross-module use: util_helpers is the leaf module that owns the single "
121+
"_SERVICE_TO_STATE map, imported and read by tools_service.py (ha_call_service) "
122+
"and device_control.py (ha_bulk_control) as the confirmation-state hint. "
123+
"CodeQL's single-file analysis misses the cross-module import, so the "
124+
"declaration looks dead.",
125+
),
116126
(
117127
"py/unused-import",
118128
"packaging/binary/pyinstaller_hooks/runtime_hook.py",

src/ha_mcp/client/rest_client.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,24 @@ class HomeAssistantConnectionError(HomeAssistantError):
4848
"""Connection error to Home Assistant."""
4949

5050

51+
class HomeAssistantCommandNotSent(HomeAssistantConnectionError):
52+
"""A WS command that provably never left the process.
53+
54+
Raised by ``HomeAssistantWebSocketClient.send_command`` ONLY at its single
55+
provably-never-sent site: the entry-guard reject (socket not authenticated), where
56+
nothing is transmitted. Subclass of ``HomeAssistantConnectionError`` so every
57+
existing broad handler is unaffected, yet a write consumer can catch this type
58+
FIRST to fall back to the legacy path safely — the write provably never happened,
59+
so a legacy first fire cannot double-apply. A ``send_json_message`` failure is NOT
60+
this type: ``websocket.send()`` raising does not prove the frame was untransmitted
61+
(bytes may already be on the socket when the close surfaces), so send_command
62+
re-raises the original exception and the consumer treats it as ambiguous. A
63+
post-send socket close (mid-await) likewise raises a plain
64+
``HomeAssistantConnectionError`` (the close handler sets it on the pending future),
65+
so type alone distinguishes never-sent from sent-then-dropped/ambiguous.
66+
"""
67+
68+
5169
class HomeAssistantAuthError(HomeAssistantError):
5270
"""Authentication error with Home Assistant.
5371

src/ha_mcp/client/websocket_client.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from ..config import get_global_settings
2424
from .rest_client import (
2525
HomeAssistantCommandError,
26+
HomeAssistantCommandNotSent,
2627
HomeAssistantCommandTimeout,
2728
HomeAssistantConnectionError,
2829
_is_ssl_error,
@@ -616,7 +617,12 @@ async def send_command(self, command_type: str, **kwargs: Any) -> dict[str, Any]
616617
Response from Home Assistant
617618
"""
618619
if not self._state.is_ready:
619-
raise HomeAssistantConnectionError("WebSocket not authenticated")
620+
# PRE-SEND and the ONLY provably-never-sent site: nothing is transmitted at
621+
# this entry guard. Raise the never-sent subtype so an at-most-once write
622+
# consumer can fall back to legacy safely (a subclass of
623+
# HomeAssistantConnectionError, so every existing broad handler is
624+
# unaffected). A later send() failure is NOT never-sent (see below).
625+
raise HomeAssistantCommandNotSent("WebSocket not authenticated")
620626

621627
# Pull the wait timeout out of kwargs rather than making it a positional
622628
# parameter: callers unpack a ``dict[str, object]`` via
@@ -635,6 +641,13 @@ async def send_command(self, command_type: str, **kwargs: Any) -> dict[str, Any]
635641
try:
636642
await self.send_json_message(message)
637643
except Exception:
644+
# AMBIGUOUS, not never-sent: websocket.send() raising (e.g. a
645+
# ConnectionClosed detected mid-write) does NOT prove the frame was not
646+
# transmitted — bytes may already be on the socket when the close surfaces.
647+
# Re-raise the ORIGINAL exception unchanged so an at-most-once write
648+
# consumer treats it like a post-send drop (ambiguous -> partial, never
649+
# re-fired), NOT as never-sent; only the readiness guard above is provably
650+
# never-sent. Still cancel the pending future so it cannot leak.
638651
self.cancel_pending_response(message_id)
639652
raise
640653

0 commit comments

Comments
 (0)