fix: resolve disabled entities via entity_registry in helper deletion (homeassistant-ai#1119)

* fix: resolve disabled entities via entity_registry in helper deletion

Closes homeassistant-ai#1057. Disabled entities are removed from the state machine but
remain in the entity registry (`disabled_by != null`). The existing
state_check guard at the top of the registry-lookup loop hit `continue`
when state_check was None, skipping the registry lookup for all 3
attempts. The code then fell through to direct_id (often a no-op for
disabled helpers) and finally to the `already_deleted` short-circuit,
leaving the registry entry in place while the tool reported success.

Per option 3.1 from the issue thread (minimal blast radius): drop the
`continue` so the registry lookup runs every iteration. The retry/sleep
is preserved as the race-condition guard for entities that just
transitioned state.

Adds a unit test `test_simple_path_disabled_entity_resolves_via_registry`
that pins the registry-fallthrough path: state_check returns None,
registry returns a valid unique_id, and the standard websocket-delete
path runs (rather than misclassifying as already_deleted).

Local: 32/32 unit tests pass; ruff and mypy clean.

* fix: drop redundant state-check sleep on disabled-entity path

Per Gemini G2 review on PR homeassistant-ai#1119: with the `continue` removed in commit
69bb587, the state-check `asyncio.sleep` no longer functions as a
race-condition guard — the registry-fail block below already provides
retry-backoff. Keeping both produced a measurable but pointless
0.5s/attempt latency on the disabled-entity-deletion path:

- common case (registry has uid): pre-fix 1.5s + bug → 69bb587 0.5s + correct
  → this commit 0s + correct
- rare case (registry fails 3x): pre-fix 1.5s + already_deleted → 69bb587
  3.0s + already_deleted → this commit 1.5s + already_deleted (parity)

The state-check itself stays as an informational debug log; only the
sleep is removed. Exception handling at L1000 is unchanged
(`HomeAssistantAPIError` only) — Gemini's suggested broader `except
Exception` was declined because it would swallow auth/connection
errors that the existing comment explicitly says must propagate.

Local: 32/32 unit tests pass; ruff and mypy clean.

* test(e2e): add disabled-entity regression test for homeassistant-ai#1057

End-to-end mirror of the unit test
`test_simple_path_disabled_entity_resolves_via_registry`. Creates an
`input_button` helper, disables its entity via
`ha_set_entity(enabled=False)`, deletes it via
`ha_delete_helpers_integrations`, and asserts the standard
`websocket_delete` path ran (rather than the `already_deleted`
short-circuit that masked the bug pre-fix).

Verifies registry entry actually goes away post-delete — pre-fix the
tool reported success while the registry entry stayed.

Closes the integration-level coverage gap noted in the IS comment.

* docs: tighten state-check comment per Gemini G3 review

* test(e2e): adjust post-delete error-code expectation

The new disabled-helper deletion test asserted that ha_get_entity
returns ENTITY_NOT_FOUND post-delete. The actual error code is
SERVICE_CALL_FAILED (with 'Entity not found' in the message) — the
HA-side error surfaces through the service-call layer rather than the
entity-not-found layer. Loosen the assertion to check the message
content and require success=False; the load-bearing claim is
'entity is gone, not still present', not the specific error code.

* docs: tighten test comment per G3 principle

Mirror the comment-verbosity reduction from 3f764ce to the test-side:
shrink the post-delete-verification comment from 6 lines to 2, keeping
the load-bearing 'why message-check, not code-check' rationale and
dropping verbose padding.

Author: Patch76

src/ha_mcp/tools/tools_integrations.py

@@ -971,7 +971,8 @@ async def _delete_simple_helper(
         )
 
         try:
-            # Try to get unique_id with retry logic (race-condition guard)
+            # Resolve unique_id via the entity registry, with a retry loop
+            # for transient registry failures.
             unique_id = None
             registry_result: dict[str, Any] | None = None
             max_retries = 3
@@ -982,18 +983,18 @@ async def _delete_simple_helper(
                     f"(attempt {attempt + 1}/{max_retries})"
                 )
 
-                # Fast state check first
+                # State check is informational only — disabled entities are
+                # missing from the state machine but resolved via the registry
+                # below (issue #1057). Kept as a debug breadcrumb rather than
+                # removed; full removal is option 3.2 in #1057, deferred to a
+                # separate PR for minimal blast radius here.
                 try:
                     state_check = await client.get_entity_state(entity_id)
                     if not state_check:
-                        if attempt < max_retries - 1:
-                            wait_time = 0.5 * (2**attempt)
-                            logger.debug(
-                                f"Entity {entity_id} not in state, waiting "
-                                f"{wait_time}s before retry..."
-                            )
-                            await asyncio.sleep(wait_time)
-                            continue
+                        logger.debug(
+                            f"Entity {entity_id} not in state; "
+                            "proceeding to registry lookup"
+                        )
                 except HomeAssistantAPIError as e:
                     # State check is best-effort here; an APIError (e.g. 404)
                     # is informational. Auth/connection errors must propagate
@@ -1009,8 +1010,8 @@ async def _delete_simple_helper(
                     registry_result = await client.send_websocket_message(
                         registry_msg
                     )
-                    if registry_result.get("success"):
-                        entity_entry = registry_result.get("result", {})
+                    if (registry_result or {}).get("success"):
+                        entity_entry = (registry_result or {}).get("result") or {}
                         unique_id = entity_entry.get("unique_id")
                         if unique_id:
                             logger.info(
@@ -1075,29 +1076,82 @@ async def _delete_simple_helper(
                             )
                     return response
 
-                # Fallback strategy 2: already-deleted check
+                # Fallback strategy 2: already-deleted check. Confirm via the
+                # registry too — a disabled entity is missing from the state
+                # machine but still registry-resident, so state-absence alone
+                # is not enough to declare success.
                 try:
                     final_state_check = await client.get_entity_state(entity_id)
                     if not final_state_check:
-                        logger.info(
-                            f"Entity {entity_id} no longer exists; "
-                            "treating as already deleted"
+                        registry_still_has_entry = False
+                        try:
+                            verify_result = await client.send_websocket_message(
+                                {
+                                    "type": "config/entity_registry/get",
+                                    "entity_id": entity_id,
+                                }
+                            )
+                            if (verify_result or {}).get("success"):
+                                verify_entry = (verify_result or {}).get("result") or {}
+                                if verify_entry.get("entity_id"):
+                                    registry_still_has_entry = True
+                        except HomeAssistantAPIError as verify_err:
+                            # On verify failure, conservatively assume the
+                            # entry is still there rather than silently
+                            # short-circuit to already_deleted.
+                            logger.debug(
+                                f"Registry verify for {entity_id} failed: "
+                                f"{verify_err}"
+                            )
+                            registry_still_has_entry = True
+
+                        if not registry_still_has_entry:
+                            logger.info(
+                                f"Entity {entity_id} absent from state and "
+                                "registry; treating as already deleted"
+                            )
+                            return {
+                                "success": True,
+                                "action": "delete",
+                                "target": target,
+                                "helper_type": helper_type,
+                                "method": "websocket_delete",
+                                "entry_id": None,
+                                "entity_ids": [entity_id],
+                                "require_restart": False,
+                                "message": (
+                                    f"Helper {target} was already deleted or "
+                                    "never properly registered."
+                                ),
+                                "fallback_used": "already_deleted",
+                            }
+
+                        logger.warning(
+                            f"Entity {entity_id} absent from state but still "
+                            "in registry; not already_deleted"
+                        )
+                        raise_tool_error(
+                            create_error_response(
+                                ErrorCode.SERVICE_CALL_FAILED,
+                                (
+                                    f"Helper {target} could not be deleted: "
+                                    "registry entry exists but unique_id was "
+                                    "absent and the direct-id fallback "
+                                    "delete failed."
+                                ),
+                                suggestions=[
+                                    "Re-enable the entity via "
+                                    "ha_set_entity(enabled=True), then retry "
+                                    "deletion.",
+                                    "Or inspect the entity registry entry "
+                                    "directly to confirm unique_id presence.",
+                                ],
+                                context={
+                                    "target": target,
+                                    "entity_id": entity_id,
+                                },
+                            )
                         )
-                        return {
-                            "success": True,
-                            "action": "delete",
-                            "target": target,
-                            "helper_type": helper_type,
-                            "method": "websocket_delete",
-                            "entry_id": None,
-                            "entity_ids": [entity_id],
-                            "require_restart": False,
-                            "message": (
-                                f"Helper {target} was already deleted or "
-                                "never properly registered."
-                            ),
-                            "fallback_used": "already_deleted",
-                        }
                 except HomeAssistantAPIError as e:
                     # 404 here means the state-check itself confirmed the
                     # entity is gone — treat as a soft signal and continue

tests/src/e2e/workflows/config/test_helper_crud.py

@@ -620,6 +620,104 @@ async def test_input_button_full_lifecycle(self, mcp_client, cleanup_tracker):
         )
         logger.info("Input button cleanup complete")
 
+    async def test_disabled_input_button_deletion_resolves_via_registry(
+        self, mcp_client, cleanup_tracker
+    ):
+        """Issue #1057 regression: a disabled helper (registered but absent
+        from the state machine) must be resolved via the entity registry,
+        not silently treated as already-deleted.
+
+        End-to-end mirror of the unit test
+        ``test_simple_path_disabled_entity_resolves_via_registry``: creates a
+        helper, disables its entity via ``ha_set_entity(enabled=False)``,
+        deletes it, and asserts the deletion took the standard
+        ``websocket_delete`` path (not the ``already_deleted`` fallback that
+        masked the bug pre-fix).
+        """
+        helper_name = "E2E Disabled Button"
+
+        # CREATE input_button
+        create_result = await mcp_client.call_tool(
+            "ha_config_set_helper",
+            {
+                "helper_type": "input_button",
+                "name": helper_name,
+                "icon": "mdi:gesture-tap-button",
+            },
+        )
+        create_data = assert_mcp_success(create_result, "Create input_button")
+        entity_id = get_entity_id_from_response(create_data, "input_button")
+        assert entity_id, f"Missing entity_id: {create_data}"
+        cleanup_tracker.track("input_button", entity_id)
+        logger.info(f"Created input_button: {entity_id}")
+
+        # Wait until entity is queryable
+        state_reached = await wait_for_entity_state(
+            mcp_client, entity_id, "unknown", timeout=10
+        )
+        assert state_reached, f"Entity {entity_id} not registered within timeout"
+
+        # DISABLE entity at registry level — this is what reproduces the bug
+        disable_result = await mcp_client.call_tool(
+            "ha_set_entity",
+            {"entity_id": entity_id, "enabled": False},
+        )
+        disable_data = assert_mcp_success(disable_result, "Disable entity")
+        assert disable_data.get("entity_entry", {}).get("disabled_by") == "user", (
+            f"Entity not registry-disabled: {disable_data}"
+        )
+        logger.info(f"Disabled entity {entity_id} (disabled_by=user)")
+
+        # DELETE — pre-fix this fell through to the ``already_deleted``
+        # short-circuit, leaving the registry entry in place. Post-fix the
+        # registry lookup runs every iteration and finds the unique_id.
+        delete_result = await mcp_client.call_tool(
+            "ha_delete_helpers_integrations",
+            {
+                "helper_type": "input_button",
+                "target": entity_id,
+                "confirm": True,
+            },
+        )
+        delete_data = assert_mcp_success(delete_result, "Delete disabled helper")
+
+        # Standard registry-driven delete path ran — unique_id was resolved
+        # and no fallback fired. Tighter than `!= "already_deleted"`: also
+        # rejects `direct_id` and any future fallback variant.
+        assert delete_data.get("method") == "websocket_delete", (
+            f"Expected websocket_delete via unique_id; got "
+            f"method={delete_data.get('method')}, data={delete_data}"
+        )
+        assert "unique_id" in delete_data, (
+            f"Standard path not taken (no unique_id in response): {delete_data}"
+        )
+        assert delete_data.get("fallback_used") is None, (
+            f"Expected no fallback; got fallback_used="
+            f"{delete_data.get('fallback_used')!r}, data={delete_data}"
+        )
+        logger.info(
+            f"Disabled helper deleted via "
+            f"{delete_data.get('method')} (unique_id={delete_data.get('unique_id')})"
+        )
+
+        # Verify entity is gone — error code varies (ENTITY_NOT_FOUND vs
+        # SERVICE_CALL_FAILED), so the assertion targets the message.
+        get_data = await safe_call_tool(
+            mcp_client, "ha_get_entity", {"entity_id": entity_id}
+        )
+        assert get_data.get("success", True) is False, (
+            f"Entity still present in registry after delete: {get_data}"
+        )
+        err_msg = (get_data.get("error", {}).get("message") or "").lower()
+        assert "not found" in err_msg, (
+            f"Expected 'not found' in error message, got: {get_data}"
+        )
+
+        logger.info(
+            f"Issue #1057 regression test passed: disabled "
+            f"{entity_id} cleanly resolved via registry"
+        )
+
 
 @pytest.mark.asyncio
 @pytest.mark.config