fix(pin): hold garage covers by device class and through groups (#773)

CHANGELOG.md

@@ -9,6 +9,7 @@ their own. See [RELEASING.md](RELEASING.md).
 
 ### Fixed
 
+- **Opening a garage door, gate or door cover now asks for the Critical Action PIN when its entity id doesn't say so** ([#773](https://github.com/goruck/home-generative-agent/issues/773)). The recommended rules looked for `door`, `gate` or `garage` in the cover's entity id only. A cover with a plain id (such as `cover.msg100_abc`) opened with no PIN even when its device class was garage, gate or door, or its name was "Garage Door", and so did a cover group like `cover.all_covers` ("All Covers") holding the garage door. That held whether the agent opened it directly, by name through "turn on", in a scene it activated, or in an automation it wrote. A rule's word now also matches a cover whose device class is that word (the one Home Assistant reports, the one set under "Show as", or the integration's own, so a garage door shown as a blind is still held), or whose name, alias or device name has it (or its plural) with no letter next to it ("Garage Door", "Driveway Gates", "Gate1", not "Indoor Blinds"), unless its integration says it is a blind, shade, curtain, awning or window (setting "Show as", or `customize` in YAML, does not turn this off). Any group holding a matching cover (cover group, MQTT or ZHA group, or old-style group, nested to any depth) is held too. Covers and groups that are not set up yet count, since an automation the agent writes runs later: a cover not set up yet that its integration does not class as a blind, shade, curtain, awning or window, a group member Home Assistant does not know yet, and a group helper whose members can't be read are held; a disabled entity is not. Each extra match names one entity exactly, so `cover.a` being a garage door does not hold `cover.awning`. Rules you added yourself get the same reach: one for `msg100` now also holds a group containing that cover, and one written like an id (`garage_door`) matches the name "Garage Door". The security audit's list of public webhook automations that perform a critical action uses the same rules. Group membership and device class are read when the automation is written, so a garage door added to a group later is not checked again. Still not held, so add a rule naming part of the entity id: a cover with no device class whose id and name both lack the words (including a name in another language or a run-together word like "GarageDoor"), a cover whose only link to the words is its area's name, and the switch a garage cover is built on, switched directly ([#794](https://github.com/goruck/home-generative-agent/issues/794)).
 - **Opening or closing a cover by voice no longer fails when other device tools rank higher** ([#795](https://github.com/goruck/home-generative-agent/issues/795)). "Open the study shade" could fail with "I couldn't find a suitable tool": the agent offers the model only the few tools that best match the request, and device tools such as a vacuum's start or a media player's unpause could outscore the general "turn on" tool, which is the only one Home Assistant gives an assistant for opening a cover. The model then guessed a tool that does not exist and the call was refused. A request with an English action word ("turn", "open", "close", ...), or a "yes" to the agent offering one ("Want me to open the Study Shade?"), now always offers the turn on and turn off tools, alongside what the ranking chose. Requests in other languages, and replies such as "the left one", still rely on the ranking. A tool you excluded is still never offered.
 
 ### Changed

custom_components/home_generative_agent/agent/graph.py

@@ -137,6 +137,7 @@
     service_runs_critical_action,
 )
 from .pin_messages import describe_pending_action, pin_msg
+from .rule_entities import expand_critical_rules
 from .token_counter import count_tokens_cross_provider
 
 if TYPE_CHECKING:
@@ -459,7 +460,10 @@ def _critical_action_guard(
     """Enforce critical-action PIN flow when needed."""
     if not ctx.pin_enabled:
         return None
-    if not _is_critical_action(tool_args, ctx.critical_actions, tool_name, ctx.hass):
+    # Extend entity_match rules to device classes, names and groups (#773,
+    # see rule_entities).
+    rules = expand_critical_rules(ctx.hass, ctx.critical_actions)
+    if not _is_critical_action(tool_args, rules, tool_name, ctx.hass):
         return None
     if not ctx.pin_hash or not ctx.pin_salt:
         LOGGER.warning(

custom_components/home_generative_agent/agent/helpers.py

@@ -877,9 +877,12 @@ def resolve_critical_action_policy(options: Mapping[str, Any]) -> CriticalAction
         pin_salt=pin_salt,
         # Copy: the default is a module-level list handed to every config
         # entry, so an in-place edit anywhere would corrupt it process-wide.
-        critical_actions=list(
-            options.get(CONF_CRITICAL_ACTIONS) or RECOMMENDED_CRITICAL_ACTIONS
-        ),
+        # The internal exact-entity marker is never taken from stored rules.
+        critical_actions=[
+            {k: v for k, v in rule.items() if k != CRITICAL_RULE_EXACT_ENTITY}
+            for rule in options.get(CONF_CRITICAL_ACTIONS)
+            or RECOMMENDED_CRITICAL_ACTIONS
+        ],
     )
 
 
@@ -1150,6 +1153,29 @@ def thread_has_pending_action(
     )
 
 
+# Marks a rule copy `rule_entities.expand_critical_rules` makes for one entity:
+# it matches that entity id exactly, not as a substring. Internal: removed from
+# stored rules, so a user rule can never be narrowed by carrying it. The copy
+# keeps the user rule's `entity_match`, so a reader that does not know this
+# key still reads it as the rule it was copied from.
+CRITICAL_RULE_EXACT_ENTITY = "_exact_entity"
+
+
+def rule_entity_matches(rule: Mapping[str, str], entity_ids: Sequence[str]) -> bool:
+    """
+    Return True when a rule's entity filter admits one of these entity ids.
+
+    The ids are lowercased. A rule with no filter admits any. A comma-joined
+    id ("cover.a, cover.b") is read as the ids it lists for an exact match.
+    """
+    if exact := (rule.get(CRITICAL_RULE_EXACT_ENTITY) or "").lower():
+        return any(
+            exact == part.strip() for entity in entity_ids for part in entity.split(",")
+        )
+    entity_match = (rule.get("entity_match") or "").lower()
+    return not entity_match or any(entity_match in e for e in entity_ids)
+
+
 def matches_critical_rule(
     *,
     domain: str,
@@ -1176,17 +1202,12 @@ def matches_critical_rule(
     for rule in critical_actions:
         rule_domain = (rule.get("domain") or "").lower()
         rule_service = (rule.get("service") or "").lower()
-        entity_match = (rule.get("entity_match") or "").lower()
 
         if rule_domain and rule_domain != domain:
             continue
         if rule_service and rule_service != service:
             continue
-        if (
-            entity_match
-            and not any(entity_match in e for e in entities)
-            and not unresolved_target
-        ):
+        if not unresolved_target and not rule_entity_matches(rule, entities):
             continue
         return True
     return False

custom_components/home_generative_agent/agent/indirect_pin.py

@@ -45,7 +45,7 @@
     find_critical_automation_calls,
     step_targets,
 )
-from .helpers import TOOL_NAMESPACE_SEP, matches_critical_rule
+from .helpers import TOOL_NAMESPACE_SEP, matches_critical_rule, rule_entity_matches
 
 if TYPE_CHECKING:
     from collections.abc import Mapping, Sequence
@@ -506,10 +506,9 @@ def _entity_is_guarded(
     """Return True if any rule applies to this entity, whatever the service."""
     for rule in rules:
         rule_domain = (rule.get("domain") or "").lower()
-        entity_match = (rule.get("entity_match") or "").lower()
         if rule_domain and rule_domain != domain:
             continue
-        if entity_match and entity_match not in entity_id:
+        if not rule_entity_matches(rule, [entity_id]):
             continue
         return True
     return False