From c2c98bd04c8c3cfc8ae948ec5c2a2be3ce117e26 Mon Sep 17 00:00:00 2001
From: Michael Hansen <mike@rhasspy.org>
Date: Thu, 1 Feb 2024 13:40:29 -0600
Subject: [PATCH] Move default response out of sentence trigger registration
 and into agent (#109317)

* Move default response out of trigger and into agent

* Add test
---
 .../components/conversation/default_agent.py  |  7 ++-
 .../components/conversation/trigger.py        |  7 ++-
 tests/components/conversation/test_trigger.py | 58 +++++++++++++++++++
 3 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/homeassistant/components/conversation/default_agent.py b/homeassistant/components/conversation/default_agent.py
index c9119935213..123dc7aba1d 100644
--- a/homeassistant/components/conversation/default_agent.py
+++ b/homeassistant/components/conversation/default_agent.py
@@ -238,7 +238,10 @@ class DefaultAgent(AbstractConversationAgent):
                 )
             )
 
-            # Use last non-empty result as response
+            # Use last non-empty result as response.
+            #
+            # There may be multiple copies of a trigger running when editing in
+            # the UI, so it's critical that we filter out empty responses here.
             response_text: str | None = None
             for trigger_response in trigger_responses:
                 response_text = response_text or trigger_response
@@ -246,7 +249,7 @@ class DefaultAgent(AbstractConversationAgent):
             # Convert to conversation result
             response = intent.IntentResponse(language=language)
             response.response_type = intent.IntentResponseType.ACTION_DONE
-            response.async_set_speech(response_text or "")
+            response.async_set_speech(response_text or "Done")
 
             return ConversationResult(response=response)
 
diff --git a/homeassistant/components/conversation/trigger.py b/homeassistant/components/conversation/trigger.py
index d38bb69f3e1..4600135c1e5 100644
--- a/homeassistant/components/conversation/trigger.py
+++ b/homeassistant/components/conversation/trigger.py
@@ -98,7 +98,12 @@ async def async_attach_trigger(
                 # mypy does not understand the type narrowing, unclear why
                 return automation_result.conversation_response  # type: ignore[return-value]
 
-        return "Done"
+        # It's important to return None here instead of a string.
+        #
+        # When editing in the UI, a copy of this trigger is registered.
+        # If we return a string from here, there is a race condition between the
+        # two trigger copies for who will provide a response.
+        return None
 
     default_agent = await _get_agent_manager(hass).async_get_agent(HOME_ASSISTANT_AGENT)
     assert isinstance(default_agent, DefaultAgent)
diff --git a/tests/components/conversation/test_trigger.py b/tests/components/conversation/test_trigger.py
index e40c7554fdd..26626a04079 100644
--- a/tests/components/conversation/test_trigger.py
+++ b/tests/components/conversation/test_trigger.py
@@ -7,6 +7,7 @@ from homeassistant.helpers import trigger
 from homeassistant.setup import async_setup_component
 
 from tests.common import async_mock_service
+from tests.typing import WebSocketGenerator
 
 
 @pytest.fixture
@@ -99,6 +100,63 @@ async def test_response(hass: HomeAssistant, setup_comp) -> None:
     assert service_response["response"]["speech"]["plain"]["speech"] == response
 
 
+async def test_subscribe_trigger_does_not_interfere_with_responses(
+    hass: HomeAssistant, setup_comp, hass_ws_client: WebSocketGenerator
+) -> None:
+    """Test that subscribing to a trigger from the websocket API does not interfere with responses."""
+    websocket_client = await hass_ws_client()
+    await websocket_client.send_json(
+        {
+            "id": 5,
+            "type": "subscribe_trigger",
+            "trigger": {"platform": "conversation", "command": ["test sentence"]},
+        }
+    )
+
+    service_response = await hass.services.async_call(
+        "conversation",
+        "process",
+        {
+            "text": "test sentence",
+        },
+        blocking=True,
+        return_response=True,
+    )
+
+    # Default response, since no automations with responses are registered
+    assert service_response["response"]["speech"]["plain"]["speech"] == "Done"
+
+    # Now register a trigger with a response
+    assert await async_setup_component(
+        hass,
+        "automation",
+        {
+            "automation test1": {
+                "trigger": {
+                    "platform": "conversation",
+                    "command": ["test sentence"],
+                },
+                "action": {
+                    "set_conversation_response": "test response",
+                },
+            }
+        },
+    )
+
+    service_response = await hass.services.async_call(
+        "conversation",
+        "process",
+        {
+            "text": "test sentence",
+        },
+        blocking=True,
+        return_response=True,
+    )
+
+    # Response will now come through
+    assert service_response["response"]["speech"]["plain"]["speech"] == "test response"
+
+
 async def test_same_trigger_multiple_sentences(
     hass: HomeAssistant, calls, setup_comp
 ) -> None: