fix: return actionable not-found for ha_config_set_label with an unknown label_id (#1926)

ha_config_set_label routed create vs update purely on whether label_id was
passed (action = "update" if label_id else "create"). Passing a label_id for a
not-yet-existing label therefore dispatched config/label_registry/update, which
HA rejects with an opaque "Command failed: Unknown error" — the tool's not-found
branch keys on "not found"/"doesn't exist" substrings and never matched HA's
actual error text, so callers saw a generic SERVICE_CALL_FAILED (the report has
an agent retrying 23 times before it found the create path).

Verify the label_id exists in the registry before routing to update, and raise
RESOURCE_NOT_FOUND with an "omit label_id to create" hint when it does not.
config/label_registry/create cannot honor a caller-supplied id anyway (HA's
create schema takes only name/color/icon/description and derives the id from the
name), so the contract stays strictly update-only rather than upserting to a
differently-derived id.

Extract the shared registry list into _list_labels() (used by get and set); it
raises on a non-list envelope rather than collapsing to an empty registry, so a
degraded fetch is never misreported as "label missing".

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: Patch76

src/ha_mcp/tools/tools_labels.py

@@ -31,6 +31,73 @@ class LabelTools:
     def __init__(self, client: Any) -> None:
         self._client = client
 
+    async def _list_labels(
+        self, context: dict[str, Any] | None = None
+    ) -> list[dict[str, Any]]:
+        """Return all labels from the registry (shared by get/set).
+
+        Raises ToolError (SERVICE_CALL_FAILED) if the list call fails or returns
+        an unexpected non-list envelope — a degraded response must not collapse
+        to an empty registry, or callers would confidently report a real label
+        as missing (mirrors ``backup_manager._require_list``).
+        """
+        result = await self._client.send_websocket_message(
+            {"type": "config/label_registry/list"}
+        )
+        if not result.get("success"):
+            raise_tool_error(
+                create_error_response(
+                    ErrorCode.SERVICE_CALL_FAILED,
+                    result.get("error", "Failed to get labels"),
+                    context=context,
+                )
+            )
+        # No ``[]`` default: a success envelope that omits ``result`` (or sends
+        # a non-list) is a degraded response, not an empty registry — defaulting
+        # would let callers confidently report a real label as missing.
+        labels = result.get("result")
+        if not isinstance(labels, list):
+            raise_tool_error(
+                create_error_response(
+                    ErrorCode.SERVICE_CALL_FAILED,
+                    "Label registry returned a missing or non-list result",
+                    context=context,
+                )
+            )
+        return labels
+
+    async def _require_existing_label(self, label_id: str, name: str) -> None:
+        """Raise RESOURCE_NOT_FOUND if ``label_id`` is not in the registry.
+
+        Enforces the strict update-only contract (issue #1860): update of an
+        unknown id yields an opaque "Unknown error", and create cannot honor a
+        caller-supplied id (HA derives it from the name), so an unknown id is a
+        clear error with a create hint rather than a silently divergent upsert.
+        """
+        existing = await self._list_labels(context={"name": name, "label_id": label_id})
+        if any(lbl.get("label_id") == label_id for lbl in existing):
+            return
+        raise_tool_error(
+            create_error_response(
+                ErrorCode.RESOURCE_NOT_FOUND,
+                f"Label not found: {label_id}",
+                context={
+                    "name": name,
+                    "label_id": label_id,
+                    "available_label_ids": [
+                        lbl.get("label_id") for lbl in existing[:10]
+                    ],
+                },
+                suggestions=[
+                    "To create a new label, omit label_id — Home Assistant "
+                    + "derives the id from the name (e.g. 'vendor:tapo' becomes "
+                    + "'vendor_tapo')",
+                    "To update an existing label, pass its exact current "
+                    + "label_id (list all with ha_config_get_label())",
+                ],
+            )
+        )
+
     @tool(
         name="ha_config_get_label",
         tags={"Labels & Categories"},
@@ -82,22 +149,7 @@ async def ha_config_get_label(
                     ],
                     context={"action": "get"},
                 )
-            message: dict[str, Any] = {
-                "type": "config/label_registry/list",
-            }
-
-            result = await self._client.send_websocket_message(message)
-
-            if not result.get("success"):
-                raise_tool_error(
-                    create_error_response(
-                        ErrorCode.SERVICE_CALL_FAILED,
-                        result.get("error", "Failed to get labels"),
-                        context={"label_id": label_id},
-                    )
-                )
-
-            labels = result.get("result", [])
+            labels = await self._list_labels(context={"label_id": label_id})
 
             if label_id is None:
                 return {
@@ -223,6 +275,13 @@ async def ha_config_set_label(
                     ],
                     context={"action": "set", "name": name},
                 )
+                # Strict update-only contract (issue #1860): routing to
+                # label_registry/update with an unknown id returns an opaque
+                # "Unknown error", and label_registry/create cannot honor a
+                # caller-supplied id (HA derives it from the name). Verify the
+                # id exists up front and return actionable guidance instead of
+                # dispatching an update that fails cryptically.
+                await self._require_existing_label(label_id, name)
             action = "update" if label_id else "create"
 
             message: dict[str, Any] = {
@@ -254,6 +313,13 @@ async def ha_config_set_label(
                     "message": f"Successfully {action_past} label: {name}",
                 }
             else:
+                # The unknown-id case is caught up front by
+                # _require_existing_label. This substring match only catches HA
+                # error texts containing "not found"/"doesn't exist"; HA's label
+                # update does NOT emit those for an unknown/deleted id (it
+                # surfaces "Unknown error"), so it is a best-effort guard for
+                # other/future phrasings only — the check-then-update delete race
+                # is not handled here and still surfaces as SERVICE_CALL_FAILED.
                 error_str = str(result.get("error", "")).lower()
                 if "not found" in error_str or "doesn't exist" in error_str:
                     raise_tool_error(

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

@@ -240,6 +240,42 @@ async def test_get_nonexistent_label(self, mcp_client):
         )
         logger.info("Non-existent label properly returned error")
 
+    async def test_set_label_with_unknown_id_returns_not_found_1860(self, mcp_client):
+        """Regression for #1860: ha_config_set_label with a label_id that does
+        not exist must return a structured RESOURCE_NOT_FOUND that points at the
+        create path — NOT the opaque "Failed to update label: Command failed:
+        Unknown error" HA emits when label_registry/update receives an unknown
+        id (an agent in the report retried 23 times before finding the create
+        path).
+
+        Source path: tools_labels.py — set_label now verifies the id exists via
+        _list_labels() before routing to update, and raises RESOURCE_NOT_FOUND
+        with an "omit label_id" suggestion when it does not.
+        """
+        logger.info("Testing set_label with unknown label_id (#1860)")
+
+        data = await safe_call_tool(
+            mcp_client,
+            "ha_config_set_label",
+            {"name": "vendor:tapo", "label_id": "vendor_nonexistent_1860_xyz"},
+        )
+
+        assert data.get("success") is False, f"Should fail for unknown label_id: {data}"
+        assert data["error"]["code"] == "RESOURCE_NOT_FOUND", (
+            f"Expected RESOURCE_NOT_FOUND (not the opaque update failure), "
+            f"got: {data['error']}"
+        )
+        error_msg = data["error"]["message"].lower()
+        assert "unknown error" not in error_msg, (
+            f"The opaque HA update failure must not leak through: {data['error']}"
+        )
+        # Actionable guidance must steer the caller to the create path.
+        suggestion = str(data["error"].get("suggestion", "")).lower()
+        assert "omit label_id" in suggestion, (
+            f"Expected an 'omit label_id' create hint, got: {data['error']}"
+        )
+        logger.info("Unknown label_id returned actionable RESOURCE_NOT_FOUND (#1860)")
+
     async def test_delete_nonexistent_label(self, mcp_client):
         """Test deleting a non-existent label."""
         logger.info("Testing delete non-existent label")

tests/src/unit/test_error_code_consistency_1297.py

@@ -263,9 +263,12 @@ def tools(self, mock_ws_client):
         return LabelTools(mock_ws_client)
 
     async def test_set_update_with_missing_label_id(self, tools, mock_ws_client):
+        # set_label lists the registry first (issue #1860); "missing" is absent,
+        # so the existence pre-check short-circuits to RESOURCE_NOT_FOUND before
+        # any label_registry/update call is dispatched.
         mock_ws_client.send_websocket_message.return_value = {
-            "success": False,
-            "error": "Label not found",
+            "success": True,
+            "result": [{"label_id": "other", "name": "Other"}],
         }
 
         with pytest.raises(ToolError) as exc_info: