From 42bc1375748f915b1607fdaad72a01fea5d0e770 Mon Sep 17 00:00:00 2001 From: Mike Date: Sun, 16 Aug 2026 22:15:31 -0500 Subject: [PATCH 1/3] feat: add shared node.invoke_native helper Capability-gate, emit, await-response, and settings (confirm_on_success, per_action_timeouts) logic for Node native actions, shared across every consuming skill's `if node:` branch. --- neon_utils/native_actions.py | 224 ++++++++++++++++++++ requirements/requirements.txt | 1 + tests/native_actions_tests.py | 386 ++++++++++++++++++++++++++++++++++ 3 files changed, 611 insertions(+) create mode 100644 neon_utils/native_actions.py create mode 100644 tests/native_actions_tests.py diff --git a/neon_utils/native_actions.py b/neon_utils/native_actions.py new file mode 100644 index 00000000..65c81bd8 --- /dev/null +++ b/neon_utils/native_actions.py @@ -0,0 +1,224 @@ +# NEON AI (TM) SOFTWARE, Software Development Kit & Application Framework +# All trademark and other rights reserved by their respective owners +# Copyright 2008-2026 Neongecko.com Inc. +# Contributors: Daniel McKnight, Guy Daniels, Elon Gasper, Richard Leeds, +# Regina Bloomstine, Casimiro Ferreira, Andrii Pernatii, Kirill Hrymailo +# BSD-3 License +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# 3. Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from this +# software without specific prior written permission. +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR +# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +# EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, +# OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +# LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +# NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import time + +from dataclasses import dataclass +from enum import Enum +from typing import Optional + +from ovos_bus_client import Message +from ovos_utils.log import LOG + +from neon_data_models.enum import NodeNativeAction, NativeActionErrorCode + +# Default response timeout in seconds, per action. +_DEFAULT_TIMEOUTS = { + NodeNativeAction.LAUNCH_CAMERA_APP.value: 10, + NodeNativeAction.LAUNCH_VOICE_RECORDER_APP.value: 10, + NodeNativeAction.LAUNCH_REMINDERS_APP.value: 5, + NodeNativeAction.LAUNCH_CLOCK_APP.value: 5, + NodeNativeAction.LAUNCH_SMS_APP.value: 5, + NodeNativeAction.LAUNCH_EMAIL_APP.value: 5, +} + + +class NativeActionOutcome(Enum): + SUCCESS = "success" + NOT_SUPPORTED = "not_supported" + ERROR = "error" + TIMEOUT = "timeout" + + +@dataclass +class NativeActionResult: + outcome: NativeActionOutcome + error_code: Optional[NativeActionErrorCode] = None + error_message: str = "" + + +def invoke_native_action(skill, message: Message, action: NodeNativeAction, + params: Optional[dict] = None) -> NativeActionResult: + """ + Capability-gate, dispatch, and await a `node.invoke_native` request. + Shared by every skill's `if node:` branch so the flow (gate, emit, + await, settings) lives in one place. + :param skill: the calling OVOSSkill/NeonSkill instance + :param message: the triggering Message (must carry `context.node`) + :param action: `NodeNativeAction` to invoke + :param params: optional payload (only meaningful for sms/email actions) + :returns: `NativeActionResult` describing what happened + """ + action_value = action.value if isinstance(action, NodeNativeAction) \ + else action + node_ctx = message.context.get("node") or {} + capabilities = node_ctx.get("capabilities") or {} + + if not capabilities.get(action_value): + LOG.debug(f"Node does not support {action_value}; " + f"capabilities={capabilities}") + _speak_not_supported(skill, message, action_value) + return NativeActionResult(NativeActionOutcome.NOT_SUPPORTED) + + session_id = message.context.get("session", {}).get("session_id") or \ + node_ctx.get("node_id") + + invoke_msg = message.reply("node.invoke_native", + {"action": action_value, + "params": params or {}}) + skill.bus.emit(invoke_msg) + + timeout = _resolve_timeout(skill, action_value) + response = _await_response(skill, session_id, action_value, timeout) + + if response is None: + LOG.warning(f"Timed out awaiting node.invoke_native.response for " + f"action={action_value} session={session_id}") + _speak_timeout(skill, message, action_value) + return NativeActionResult(NativeActionOutcome.TIMEOUT) + + return _interpret_response(skill, message, action_value, response) + + +def _interpret_response(skill, message: Message, action_value: str, + response: Message) -> NativeActionResult: + if response.data.get("status") == "success": + if skill.settings.get("confirm_on_success", False): + _speak_success(skill, message, action_value) + return NativeActionResult(NativeActionOutcome.SUCCESS) + + error = response.data.get("error") or {} + try: + error_code = NativeActionErrorCode(error.get("code")) + except ValueError: + error_code = NativeActionErrorCode.INTERNAL_ERROR + error_message = error.get("message", "") + _speak_error(skill, message, action_value, error_code, error_message) + return NativeActionResult(NativeActionOutcome.ERROR, error_code, + error_message) + + +def _resolve_timeout(skill, action_value: str) -> int: + overrides = skill.settings.get("per_action_timeouts") or {} + if action_value in overrides: + try: + return int(overrides[action_value]) + except (TypeError, ValueError): + LOG.warning(f"Invalid per_action_timeouts override for " + f"{action_value}: {overrides[action_value]!r}") + return _DEFAULT_TIMEOUTS.get(action_value, 5) + + +def _await_response(skill, session_id: Optional[str], action_value: str, + timeout: float) -> Optional[Message]: + """ + `wait_for_message` filters only by `msg_type`, not content. Loop with + one shared deadline and drop any response for a different session or + action. + """ + deadline = time.monotonic() + timeout + while True: + remaining = deadline - time.monotonic() + if remaining <= 0: + return None + response = skill.bus.wait_for_message("node.invoke_native.response", + timeout=remaining) + if response is None: + return None + resp_action = response.data.get("action") + resp_session = response.context.get("session", {}).get("session_id") + if resp_action == action_value and ( + session_id is None or resp_session == session_id): + return response + LOG.debug(f"Discarding non-matching node.invoke_native.response: " + f"action={resp_action} session={resp_session}") + + +def _speak_not_supported(skill, message: Message, action_value: str): + _speak(skill, message, "native_action_not_supported", + {"action": action_value, "description": _describe(action_value)}) + + +def _speak_success(skill, message: Message, action_value: str): + _speak(skill, message, "native_action_success", + {"action": action_value, "description": _describe(action_value)}) + + +def _speak_timeout(skill, message: Message, action_value: str): + _speak(skill, message, "native_action_timeout", + {"action": action_value, "description": _describe(action_value)}) + + +def _speak_error(skill, message: Message, action_value: str, + error_code: NativeActionErrorCode, error_message: str): + _speak(skill, message, "native_action_error", + {"action": action_value, "code": error_code.value, + "message": error_message, "description": _describe(action_value)}) + + +def _speak(skill, message: Message, dialog_key: str, data: dict): + """ + Speak the skill's own dialog file when it exists, so a skill can still + override the default text through the normal OVOS dialog convention. + Otherwise speak the canonical fallback text. Checks `templates` + directly rather than calling `speak_dialog` blind: a missing dialog + file is the expected case here, not an error to log. + """ + try: + if skill.dialog_renderer and \ + dialog_key in skill.dialog_renderer.templates: + skill.speak_dialog(dialog_key, data, message=message) + return + except Exception as e: + LOG.warning(f"Failed checking dialog_renderer for {dialog_key}: {e}") + skill.speak(_fallback_text(dialog_key, data), message=message) + + +def _fallback_text(dialog_key: str, data: dict) -> str: + """ + Canonical plain-English text for a skill with no matching dialog file. + A skill can still override any of these keys with its own dialog file; + this text is the default, not a template for one. + """ + action = data.get("action", "") + if dialog_key == "native_action_not_supported": + return f"This device cannot open {_describe(action)}." + if dialog_key == "native_action_success": + return "Done." + if dialog_key == "native_action_timeout": + return "I did not reach your device." + if dialog_key == "native_action_error": + message = data.get("message") + return message or f"I could not open {_describe(action)}." + return "" + + +def _describe(action_value: str) -> str: + name = action_value.replace("launch_", "").replace("_app", "") \ + .replace("_", " ") + return f"the {name} app" if name else "that app" diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 6cd64011..f0a994fe 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -9,5 +9,6 @@ ovos-utils~=0.8,>=0.8.5 geopy~=2.1 ovos-config~=0.1 ovos-workshop~=0.0,>=0.0.15 +neon-data-models~=0.0,>=0.0.2 # TODO: Refactor to remove `pkg_resources` imports setuptools<82.0.0 diff --git a/tests/native_actions_tests.py b/tests/native_actions_tests.py new file mode 100644 index 00000000..6a42d78f --- /dev/null +++ b/tests/native_actions_tests.py @@ -0,0 +1,386 @@ +# NEON AI (TM) SOFTWARE, Software Development Kit & Application Framework +# All trademark and other rights reserved by their respective owners +# Copyright 2008-2026 Neongecko.com Inc. +# Contributors: Daniel McKnight, Guy Daniels, Elon Gasper, Richard Leeds, +# Regina Bloomstine, Casimiro Ferreira, Andrii Pernatii, Kirill Hrymailo +# BSD-3 License +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# 3. Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from this +# software without specific prior written permission. +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR +# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +# EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, +# OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +# LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +# NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import sys +import os +import threading +import time +import unittest + +from unittest.mock import Mock, call, patch +from ovos_bus_client import Message +from ovos_utils.fakebus import FakeBus + +sys.path.append(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))) +from neon_utils.native_actions import (invoke_native_action, + NativeActionOutcome) +from neon_data_models.enum import NodeNativeAction, NativeActionErrorCode + + +def _node_message(action_supported: bool = True, action="launch_camera_app", + settings=None, session_id="node-abc123"): + context = { + "node": { + "node_id": "node-abc123", + "node_name": "Kitchen Phone", + "capabilities": {action: action_supported} + }, + "session": {"session_id": session_id} + } + return Message("recognizer_loop:utterance", {}, context) + + +def _response(action="launch_camera_app", status="success", error=None, + session_id="node-abc123"): + data = {"action": action, "status": status} + if error: + data["error"] = error + return Message("node.invoke_native.response", data, + {"session": {"session_id": session_id}}) + + +def _make_skill(settings=None, wait_for_message_return=None): + skill = Mock() + skill.settings = settings or {} + skill.bus = Mock() + skill.bus.wait_for_message = Mock(return_value=wait_for_message_return) + skill.dialog_renderer = None + return skill + + +class NativeActionsTests(unittest.TestCase): + def test_not_supported_speaks_and_emits_nothing(self): + skill = _make_skill() + message = _node_message(action_supported=False) + + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(result.outcome, NativeActionOutcome.NOT_SUPPORTED) + skill.bus.emit.assert_not_called() + skill.speak.assert_called_once() + + def test_capabilities_entirely_absent_treated_as_not_supported(self): + skill = _make_skill() + message = Message("recognizer_loop:utterance", {}, + {"node": {"node_id": "node-abc123"}, + "session": {"session_id": "node-abc123"}}) + + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(result.outcome, NativeActionOutcome.NOT_SUPPORTED) + skill.bus.emit.assert_not_called() + + def test_absent_capability_key_treated_as_not_supported(self): + skill = _make_skill() + message = _node_message(action_supported=True, + action="launch_sms_app") + # Request an action that has no key in capabilities at all + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(result.outcome, NativeActionOutcome.NOT_SUPPORTED) + skill.bus.emit.assert_not_called() + + def test_supported_emits_invoke_native_with_action_and_params(self): + response = _response(status="success") + skill = _make_skill(wait_for_message_return=response) + message = _node_message(action_supported=True) + + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(result.outcome, NativeActionOutcome.SUCCESS) + emitted = skill.bus.emit.call_args[0][0] + self.assertEqual(emitted.msg_type, "node.invoke_native") + self.assertEqual(emitted.data["action"], "launch_camera_app") + self.assertEqual(emitted.data["params"], {}) + + def test_params_forwarded_for_sms(self): + response = _response(action="launch_sms_app", status="success") + skill = _make_skill(wait_for_message_return=response) + message = _node_message(action_supported=True, action="launch_sms_app") + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_SMS_APP, + params={"to": "5551234567", "body": "On my way"}) + + emitted = skill.bus.emit.call_args[0][0] + self.assertEqual(emitted.data["params"], + {"to": "5551234567", "body": "On my way"}) + + def test_success_silent_by_default(self): + response = _response(status="success") + skill = _make_skill(wait_for_message_return=response) + message = _node_message(action_supported=True) + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) + + skill.speak.assert_not_called() + skill.speak_dialog.assert_not_called() + + def test_success_confirms_when_setting_enabled(self): + response = _response(status="success") + skill = _make_skill(settings={"confirm_on_success": True}, + wait_for_message_return=response) + message = _node_message(action_supported=True) + + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(result.outcome, NativeActionOutcome.SUCCESS) + skill.speak.assert_called_once() + + def test_error_response_speaks_failure_with_code(self): + response = _response(status="error", + error={"code": "unavailable", + "message": "No camera app."}) + skill = _make_skill(wait_for_message_return=response) + message = _node_message(action_supported=True) + + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(result.outcome, NativeActionOutcome.ERROR) + self.assertEqual(result.error_code, NativeActionErrorCode.UNAVAILABLE) + skill.speak.assert_called_once() + + def test_unknown_error_code_maps_to_internal_error(self): + response = _response(status="error", + error={"code": "made_up_code", "message": "?"}) + skill = _make_skill(wait_for_message_return=response) + message = _node_message(action_supported=True) + + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(result.error_code, + NativeActionErrorCode.INTERNAL_ERROR) + + def test_timeout_speaks_failure(self): + # wait_for_message always returns None -> immediate timeout + skill = _make_skill(wait_for_message_return=None) + message = _node_message(action_supported=True) + + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(result.outcome, NativeActionOutcome.TIMEOUT) + skill.speak.assert_called_once() + + def test_real_bus_times_out_after_deadline_elapses(self): + # Exercises the actual deadline loop in _await_response, not a + # mocked wait_for_message returning None immediately. + # per_action_timeouts is int()-cast, so use a whole-second value — + # 0.2 truncates to 0 and would pass this test for the wrong reason. + skill = Mock() + skill.settings = {"per_action_timeouts": {"launch_camera_app": 1}} + skill.bus = FakeBus() + skill.dialog_renderer = None + message = _node_message(action_supported=True) + + start = time.monotonic() + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + elapsed = time.monotonic() - start + + self.assertEqual(result.outcome, NativeActionOutcome.TIMEOUT) + self.assertLess(elapsed, 2.0) + self.assertGreaterEqual(elapsed, 1.0) + + def test_real_bus_discards_wrong_action_then_matches(self): + # Proves the discard-and-keep-waiting loop can still succeed on a + # later, matching response within the same deadline. + skill = Mock() + skill.settings = {} + skill.bus = FakeBus() + skill.dialog_renderer = None + message = _node_message(action_supported=True) + + # Space the two emits out: `_await_response` only resubscribes to + # the bus AFTER the wrong-action message is received and discarded, + # so firing both immediately can drop the second before anything + # is listening for it again. + def _emit_wrong(): + skill.bus.emit(_response(action="launch_clock_app", + status="success")) + + def _emit_right(): + skill.bus.emit(_response(action="launch_camera_app", + status="success")) + + threading.Timer(0.05, _emit_wrong).start() + threading.Timer(0.2, _emit_right).start() + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(result.outcome, NativeActionOutcome.SUCCESS) + + def test_mismatched_response_is_discarded_then_timeout(self): + wrong_action = _response(action="launch_clock_app", status="success") + skill = _make_skill() + # First call returns a response for the wrong action, second + # call (after filtering) returns None because timeout elapsed + skill.bus.wait_for_message = Mock( + side_effect=[wrong_action, None]) + message = _node_message(action_supported=True) + + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(result.outcome, NativeActionOutcome.TIMEOUT) + self.assertEqual(skill.bus.wait_for_message.call_count, 2) + + def test_mismatched_session_is_discarded(self): + wrong_session = _response(status="success", session_id="node-other") + skill = Mock() + skill.settings = {} + skill.dialog_renderer = None + skill.bus = Mock() + skill.bus.wait_for_message = Mock(side_effect=[wrong_session, None]) + message = _node_message(action_supported=True) + + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(result.outcome, NativeActionOutcome.TIMEOUT) + + def test_per_action_timeout_override_used(self): + skill = _make_skill(settings={"per_action_timeouts": + {"launch_camera_app": 1}}) + message = _node_message(action_supported=True) + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) + + _, kwargs = skill.bus.wait_for_message.call_args + self.assertLessEqual(kwargs["timeout"], 1) + + def test_default_timeout_used_when_no_override(self): + skill = _make_skill() + message = _node_message(action_supported=True, + action="launch_sms_app") + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_SMS_APP) + + _, kwargs = skill.bus.wait_for_message.call_args + self.assertLessEqual(kwargs["timeout"], 5) + + def test_speaks_dialog_key_when_dialog_renderer_has_it(self): + response = _response(status="error", + error={"code": "unavailable", "message": "x"}) + skill = _make_skill(wait_for_message_return=response) + skill.dialog_renderer = Mock() + skill.dialog_renderer.templates = {"native_action_error": []} + message = _node_message(action_supported=True) + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) + + skill.speak_dialog.assert_called_once() + skill.speak.assert_not_called() + + def test_falls_back_to_speak_when_no_dialog_file(self): + response = _response(status="error", + error={"code": "unavailable", "message": "x"}) + skill = _make_skill(wait_for_message_return=response) + skill.dialog_renderer = Mock() + skill.dialog_renderer.templates = {} + message = _node_message(action_supported=True) + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) + + skill.speak.assert_called_once() + skill.speak_dialog.assert_not_called() + + def test_no_dialog_file_does_not_log_error_or_warning(self): + # A missing native_action_*.dialog file is the expected, common + # case post-change, not an exceptional one worth logging. + skill = _make_skill() + message = _node_message(action_supported=False) + + with patch("neon_utils.native_actions.LOG") as mock_log: + invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + + mock_log.error.assert_not_called() + mock_log.warning.assert_not_called() + + def test_not_supported_fallback_text(self): + skill = _make_skill() + message = _node_message(action_supported=False) + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(skill.speak.call_args[0][0], + "This device cannot open the camera app.") + + def test_timeout_fallback_text(self): + skill = _make_skill(wait_for_message_return=None) + message = _node_message(action_supported=True) + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(skill.speak.call_args[0][0], + "I did not reach your device.") + + def test_success_fallback_text(self): + response = _response(status="success") + skill = _make_skill(settings={"confirm_on_success": True}, + wait_for_message_return=response) + message = _node_message(action_supported=True) + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(skill.speak.call_args[0][0], "Done.") + + def test_error_fallback_uses_node_message_when_present(self): + response = _response(status="error", + error={"code": "unavailable", + "message": "No camera app is available."}) + skill = _make_skill(wait_for_message_return=response) + message = _node_message(action_supported=True) + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(skill.speak.call_args[0][0], + "No camera app is available.") + + def test_error_fallback_generic_when_node_gives_no_message(self): + response = _response(action="launch_sms_app", status="error", + error={"code": "unavailable", "message": ""}) + skill = _make_skill(wait_for_message_return=response) + message = _node_message(action_supported=True, + action="launch_sms_app") + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_SMS_APP) + + self.assertEqual(skill.speak.call_args[0][0], + "I could not open the sms app.") + + +if __name__ == '__main__': + unittest.main() From 6eefb4615acd6929008f3ebc4c986b35582e8237 Mon Sep 17 00:00:00 2001 From: Mike Date: Fri, 4 Sep 2026 16:14:13 -0500 Subject: [PATCH 2/3] fix: subscribe before emitting in invoke_native_action; localize action names Address review feedback on the node.invoke_native helper: - Register the response handler before emitting the request, so a reply that lands before emit() returns is not missed. The waiter filters on action and session and unregisters itself once the wait ends. - Annotate `skill` as OVOSSkill. - Look up the spoken name of an action from a skill dialog file named after the NodeNativeAction value (e.g. launch_camera_app.dialog) so the `description` passed to dialogs is localizable. English remains the fallback when no such file exists. - Accept fractional per_action_timeouts values. Tests now drive a FakeBus that replies synchronously from inside the invoke emit, which the previous implementation timed out on. --- neon_utils/native_actions.py | 182 +++++++++++++---------- tests/native_actions_tests.py | 264 ++++++++++++++++++++-------------- 2 files changed, 269 insertions(+), 177 deletions(-) diff --git a/neon_utils/native_actions.py b/neon_utils/native_actions.py index 65c81bd8..96b2266d 100644 --- a/neon_utils/native_actions.py +++ b/neon_utils/native_actions.py @@ -27,6 +27,7 @@ # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import time +import threading from dataclasses import dataclass from enum import Enum @@ -34,9 +35,13 @@ from ovos_bus_client import Message from ovos_utils.log import LOG +from ovos_workshop.skills.ovos import OVOSSkill from neon_data_models.enum import NodeNativeAction, NativeActionErrorCode +INVOKE_TYPE = "node.invoke_native" +RESPONSE_TYPE = "node.invoke_native.response" + # Default response timeout in seconds, per action. _DEFAULT_TIMEOUTS = { NodeNativeAction.LAUNCH_CAMERA_APP.value: 10, @@ -46,6 +51,7 @@ NodeNativeAction.LAUNCH_SMS_APP.value: 5, NodeNativeAction.LAUNCH_EMAIL_APP.value: 5, } +_FALLBACK_TIMEOUT = 5 class NativeActionOutcome(Enum): @@ -62,7 +68,8 @@ class NativeActionResult: error_message: str = "" -def invoke_native_action(skill, message: Message, action: NodeNativeAction, +def invoke_native_action(skill: OVOSSkill, message: Message, + action: NodeNativeAction, params: Optional[dict] = None) -> NativeActionResult: """ Capability-gate, dispatch, and await a `node.invoke_native` request. @@ -85,19 +92,17 @@ def invoke_native_action(skill, message: Message, action: NodeNativeAction, _speak_not_supported(skill, message, action_value) return NativeActionResult(NativeActionOutcome.NOT_SUPPORTED) - session_id = message.context.get("session", {}).get("session_id") or \ - node_ctx.get("node_id") + session_id = _session_of(message) or node_ctx.get("node_id") - invoke_msg = message.reply("node.invoke_native", + invoke_msg = message.reply(INVOKE_TYPE, {"action": action_value, "params": params or {}}) + waiter = _ResponseWaiter(skill.bus, session_id, action_value) skill.bus.emit(invoke_msg) - - timeout = _resolve_timeout(skill, action_value) - response = _await_response(skill, session_id, action_value, timeout) + response = waiter.wait(_resolve_timeout(skill, action_value)) if response is None: - LOG.warning(f"Timed out awaiting node.invoke_native.response for " + LOG.warning(f"Timed out awaiting {RESPONSE_TYPE} for " f"action={action_value} session={session_id}") _speak_timeout(skill, message, action_value) return NativeActionResult(NativeActionOutcome.TIMEOUT) @@ -105,7 +110,51 @@ def invoke_native_action(skill, message: Message, action: NodeNativeAction, return _interpret_response(skill, message, action_value, response) -def _interpret_response(skill, message: Message, action_value: str, +class _ResponseWaiter: + """ + Subscribes before the request is emitted, so a reply that lands before + `emit()` returns is still caught. Filters on action and session so + concurrent requests on a shared bus cannot take each other's reply. + """ + + def __init__(self, bus, session_id: Optional[str], action_value: str): + self._bus = bus + self._session_id = session_id + self._action_value = action_value + self._received = threading.Event() + self._handler = self._on_response + self.response: Optional[Message] = None + bus.on(RESPONSE_TYPE, self._handler) + + def wait(self, timeout: float) -> Optional[Message]: + try: + self._received.wait(timeout) + finally: + self._bus.remove(RESPONSE_TYPE, self._handler) + return self.response + + def _on_response(self, response: Message): + if not self._matches(response): + LOG.debug(f"Ignoring {RESPONSE_TYPE} for " + f"action={response.data.get('action')} " + f"session={_session_of(response)}") + return + self.response = response + self._received.set() + + def _matches(self, response: Message) -> bool: + if response.data.get("action") != self._action_value: + return False + return self._session_id is None or \ + _session_of(response) == self._session_id + + +def _session_of(message: Message) -> Optional[str]: + return message.context.get("session", {}).get("session_id") + + +def _interpret_response(skill: OVOSSkill, message: Message, + action_value: str, response: Message) -> NativeActionResult: if response.data.get("status") == "success": if skill.settings.get("confirm_on_success", False): @@ -123,102 +172,91 @@ def _interpret_response(skill, message: Message, action_value: str, error_message) -def _resolve_timeout(skill, action_value: str) -> int: +def _resolve_timeout(skill: OVOSSkill, action_value: str) -> float: overrides = skill.settings.get("per_action_timeouts") or {} if action_value in overrides: try: - return int(overrides[action_value]) + return float(overrides[action_value]) except (TypeError, ValueError): LOG.warning(f"Invalid per_action_timeouts override for " - f"{action_value}: {overrides[action_value]!r}") - return _DEFAULT_TIMEOUTS.get(action_value, 5) + f"{action_value}: {overrides[action_value]!r}") + return _DEFAULT_TIMEOUTS.get(action_value, _FALLBACK_TIMEOUT) -def _await_response(skill, session_id: Optional[str], action_value: str, - timeout: float) -> Optional[Message]: - """ - `wait_for_message` filters only by `msg_type`, not content. Loop with - one shared deadline and drop any response for a different session or - action. - """ - deadline = time.monotonic() + timeout - while True: - remaining = deadline - time.monotonic() - if remaining <= 0: - return None - response = skill.bus.wait_for_message("node.invoke_native.response", - timeout=remaining) - if response is None: - return None - resp_action = response.data.get("action") - resp_session = response.context.get("session", {}).get("session_id") - if resp_action == action_value and ( - session_id is None or resp_session == session_id): - return response - LOG.debug(f"Discarding non-matching node.invoke_native.response: " - f"action={resp_action} session={resp_session}") - - -def _speak_not_supported(skill, message: Message, action_value: str): +def _speak_not_supported(skill: OVOSSkill, message: Message, + action_value: str): _speak(skill, message, "native_action_not_supported", - {"action": action_value, "description": _describe(action_value)}) + _dialog_data(skill, action_value)) -def _speak_success(skill, message: Message, action_value: str): +def _speak_success(skill: OVOSSkill, message: Message, action_value: str): _speak(skill, message, "native_action_success", - {"action": action_value, "description": _describe(action_value)}) + _dialog_data(skill, action_value)) -def _speak_timeout(skill, message: Message, action_value: str): +def _speak_timeout(skill: OVOSSkill, message: Message, action_value: str): _speak(skill, message, "native_action_timeout", - {"action": action_value, "description": _describe(action_value)}) + _dialog_data(skill, action_value)) -def _speak_error(skill, message: Message, action_value: str, +def _speak_error(skill: OVOSSkill, message: Message, action_value: str, error_code: NativeActionErrorCode, error_message: str): _speak(skill, message, "native_action_error", - {"action": action_value, "code": error_code.value, - "message": error_message, "description": _describe(action_value)}) + _dialog_data(skill, action_value, code=error_code.value, + message=error_message)) + + +def _dialog_data(skill: OVOSSkill, action_value: str, **extra) -> dict: + return {"action": action_value, + "description": _describe(skill, action_value), **extra} -def _speak(skill, message: Message, dialog_key: str, data: dict): +def _speak(skill: OVOSSkill, message: Message, dialog_key: str, data: dict): + """Prefer the skill's own dialog file; fall back to built-in text.""" + if _has_dialog(skill, dialog_key): + skill.speak_dialog(dialog_key, data, message=message) + else: + skill.speak(_fallback_text(dialog_key, data), message=message) + + +def _has_dialog(skill: OVOSSkill, dialog_key: str) -> bool: """ - Speak the skill's own dialog file when it exists, so a skill can still - override the default text through the normal OVOS dialog convention. - Otherwise speak the canonical fallback text. Checks `templates` - directly rather than calling `speak_dialog` blind: a missing dialog - file is the expected case here, not an error to log. + Checks `templates` directly rather than calling `speak_dialog` blind: + a missing dialog file is the expected case here, not an error to log. """ try: - if skill.dialog_renderer and \ - dialog_key in skill.dialog_renderer.templates: - skill.speak_dialog(dialog_key, data, message=message) - return + renderer = skill.dialog_renderer + return bool(renderer) and dialog_key in renderer.templates except Exception as e: LOG.warning(f"Failed checking dialog_renderer for {dialog_key}: {e}") - skill.speak(_fallback_text(dialog_key, data), message=message) + return False -def _fallback_text(dialog_key: str, data: dict) -> str: +def _describe(skill: OVOSSkill, action_value: str) -> str: """ - Canonical plain-English text for a skill with no matching dialog file. - A skill can still override any of these keys with its own dialog file; - this text is the default, not a template for one. + A skill localizes the spoken name of an action with a dialog file named + after the `NodeNativeAction` value, e.g. `launch_camera_app.dialog`. """ - action = data.get("action", "") + if _has_dialog(skill, action_value): + return skill.dialog_renderer.render(action_value) + return _fallback_description(action_value) + + +def _fallback_description(action_value: str) -> str: + name = action_value.replace("launch_", "").replace("_app", "") \ + .replace("_", " ") + return f"the {name} app" if name else "that app" + + +def _fallback_text(dialog_key: str, data: dict) -> str: + """Built-in English for a skill with no matching dialog file.""" + description = data.get("description") or "that app" if dialog_key == "native_action_not_supported": - return f"This device cannot open {_describe(action)}." + return f"This device cannot open {description}." if dialog_key == "native_action_success": return "Done." if dialog_key == "native_action_timeout": return "I did not reach your device." if dialog_key == "native_action_error": - message = data.get("message") - return message or f"I could not open {_describe(action)}." + return data.get("message") or f"I could not open {description}." return "" - - -def _describe(action_value: str) -> str: - name = action_value.replace("launch_", "").replace("_app", "") \ - .replace("_", " ") - return f"the {name} app" if name else "that app" diff --git a/tests/native_actions_tests.py b/tests/native_actions_tests.py index 6a42d78f..ce896b94 100644 --- a/tests/native_actions_tests.py +++ b/tests/native_actions_tests.py @@ -32,18 +32,24 @@ import time import unittest -from unittest.mock import Mock, call, patch +from unittest.mock import Mock, patch from ovos_bus_client import Message +from ovos_utils.dialog import MustacheDialogRenderer from ovos_utils.fakebus import FakeBus sys.path.append(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))) from neon_utils.native_actions import (invoke_native_action, - NativeActionOutcome) + NativeActionOutcome, + INVOKE_TYPE, RESPONSE_TYPE, + _resolve_timeout) from neon_data_models.enum import NodeNativeAction, NativeActionErrorCode +QUICK_TIMEOUTS = {"per_action_timeouts": {"launch_camera_app": 0.2, + "launch_sms_app": 0.2}} + def _node_message(action_supported: bool = True, action="launch_camera_app", - settings=None, session_id="node-abc123"): + session_id="node-abc123"): context = { "node": { "node_id": "node-abc123", @@ -56,37 +62,57 @@ def _node_message(action_supported: bool = True, action="launch_camera_app", def _response(action="launch_camera_app", status="success", error=None, - session_id="node-abc123"): + session_id="node-abc123"): data = {"action": action, "status": status} if error: data["error"] = error - return Message("node.invoke_native.response", data, - {"session": {"session_id": session_id}}) + return Message(RESPONSE_TYPE, data, {"session": {"session_id": session_id}}) -def _make_skill(settings=None, wait_for_message_return=None): +def _make_skill(settings=None, responses=()): + """ + Each response is emitted synchronously from inside the invoke emit, so + it lands before `emit()` returns. A helper that subscribed only after + emitting would miss every one of them. + """ skill = Mock() skill.settings = settings or {} - skill.bus = Mock() - skill.bus.wait_for_message = Mock(return_value=wait_for_message_return) + skill.bus = FakeBus() skill.dialog_renderer = None + if responses: + skill.bus.on(INVOKE_TYPE, + lambda _: [skill.bus.emit(r) for r in responses]) return skill +def _capture_invokes(skill) -> list: + invokes = [] + skill.bus.on(INVOKE_TYPE, invokes.append) + return invokes + + +def _renderer(templates: dict) -> MustacheDialogRenderer: + renderer = MustacheDialogRenderer() + renderer.templates = templates + return renderer + + class NativeActionsTests(unittest.TestCase): def test_not_supported_speaks_and_emits_nothing(self): skill = _make_skill() + invokes = _capture_invokes(skill) message = _node_message(action_supported=False) result = invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) self.assertEqual(result.outcome, NativeActionOutcome.NOT_SUPPORTED) - skill.bus.emit.assert_not_called() + self.assertEqual(invokes, []) skill.speak.assert_called_once() def test_capabilities_entirely_absent_treated_as_not_supported(self): skill = _make_skill() + invokes = _capture_invokes(skill) message = Message("recognizer_loop:utterance", {}, {"node": {"node_id": "node-abc123"}, "session": {"session_id": "node-abc123"}}) @@ -95,10 +121,11 @@ def test_capabilities_entirely_absent_treated_as_not_supported(self): NodeNativeAction.LAUNCH_CAMERA_APP) self.assertEqual(result.outcome, NativeActionOutcome.NOT_SUPPORTED) - skill.bus.emit.assert_not_called() + self.assertEqual(invokes, []) def test_absent_capability_key_treated_as_not_supported(self): skill = _make_skill() + invokes = _capture_invokes(skill) message = _node_message(action_supported=True, action="launch_sms_app") # Request an action that has no key in capabilities at all @@ -106,37 +133,35 @@ def test_absent_capability_key_treated_as_not_supported(self): NodeNativeAction.LAUNCH_CAMERA_APP) self.assertEqual(result.outcome, NativeActionOutcome.NOT_SUPPORTED) - skill.bus.emit.assert_not_called() + self.assertEqual(invokes, []) def test_supported_emits_invoke_native_with_action_and_params(self): - response = _response(status="success") - skill = _make_skill(wait_for_message_return=response) + skill = _make_skill(responses=[_response(status="success")]) + invokes = _capture_invokes(skill) message = _node_message(action_supported=True) result = invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) self.assertEqual(result.outcome, NativeActionOutcome.SUCCESS) - emitted = skill.bus.emit.call_args[0][0] - self.assertEqual(emitted.msg_type, "node.invoke_native") - self.assertEqual(emitted.data["action"], "launch_camera_app") - self.assertEqual(emitted.data["params"], {}) + self.assertEqual(len(invokes), 1) + self.assertEqual(invokes[0].msg_type, INVOKE_TYPE) + self.assertEqual(invokes[0].data["action"], "launch_camera_app") + self.assertEqual(invokes[0].data["params"], {}) def test_params_forwarded_for_sms(self): - response = _response(action="launch_sms_app", status="success") - skill = _make_skill(wait_for_message_return=response) + skill = _make_skill(responses=[_response(action="launch_sms_app")]) + invokes = _capture_invokes(skill) message = _node_message(action_supported=True, action="launch_sms_app") invoke_native_action(skill, message, NodeNativeAction.LAUNCH_SMS_APP, - params={"to": "5551234567", "body": "On my way"}) + params={"to": "5551234567", "body": "On my way"}) - emitted = skill.bus.emit.call_args[0][0] - self.assertEqual(emitted.data["params"], + self.assertEqual(invokes[0].data["params"], {"to": "5551234567", "body": "On my way"}) def test_success_silent_by_default(self): - response = _response(status="success") - skill = _make_skill(wait_for_message_return=response) + skill = _make_skill(responses=[_response(status="success")]) message = _node_message(action_supported=True) invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) @@ -145,9 +170,8 @@ def test_success_silent_by_default(self): skill.speak_dialog.assert_not_called() def test_success_confirms_when_setting_enabled(self): - response = _response(status="success") skill = _make_skill(settings={"confirm_on_success": True}, - wait_for_message_return=response) + responses=[_response(status="success")]) message = _node_message(action_supported=True) result = invoke_native_action(skill, message, @@ -160,7 +184,7 @@ def test_error_response_speaks_failure_with_code(self): response = _response(status="error", error={"code": "unavailable", "message": "No camera app."}) - skill = _make_skill(wait_for_message_return=response) + skill = _make_skill(responses=[response]) message = _node_message(action_supported=True) result = invoke_native_action(skill, message, @@ -173,7 +197,7 @@ def test_error_response_speaks_failure_with_code(self): def test_unknown_error_code_maps_to_internal_error(self): response = _response(status="error", error={"code": "made_up_code", "message": "?"}) - skill = _make_skill(wait_for_message_return=response) + skill = _make_skill(responses=[response]) message = _node_message(action_supported=True) result = invoke_native_action(skill, message, @@ -182,87 +206,74 @@ def test_unknown_error_code_maps_to_internal_error(self): self.assertEqual(result.error_code, NativeActionErrorCode.INTERNAL_ERROR) - def test_timeout_speaks_failure(self): - # wait_for_message always returns None -> immediate timeout - skill = _make_skill(wait_for_message_return=None) + # --- response waiting ------------------------------------------------- + + def test_response_landing_before_emit_returns_is_caught(self): + # The default 10s camera timeout stays in place: if the synchronous + # reply were missed, this test would take 10s and report TIMEOUT. + skill = _make_skill(responses=[_response(status="success")]) message = _node_message(action_supported=True) + start = time.monotonic() result = invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) - self.assertEqual(result.outcome, NativeActionOutcome.TIMEOUT) - skill.speak.assert_called_once() + self.assertEqual(result.outcome, NativeActionOutcome.SUCCESS) + self.assertLess(time.monotonic() - start, 1.0) - def test_real_bus_times_out_after_deadline_elapses(self): - # Exercises the actual deadline loop in _await_response, not a - # mocked wait_for_message returning None immediately. - # per_action_timeouts is int()-cast, so use a whole-second value — - # 0.2 truncates to 0 and would pass this test for the wrong reason. - skill = Mock() - skill.settings = {"per_action_timeouts": {"launch_camera_app": 1}} - skill.bus = FakeBus() - skill.dialog_renderer = None + def test_late_response_within_deadline_is_caught(self): + skill = _make_skill(settings={"per_action_timeouts": + {"launch_camera_app": 1}}) message = _node_message(action_supported=True) + threading.Timer(0.1, lambda: skill.bus.emit(_response())).start() start = time.monotonic() result = invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) - elapsed = time.monotonic() - start - self.assertEqual(result.outcome, NativeActionOutcome.TIMEOUT) - self.assertLess(elapsed, 2.0) - self.assertGreaterEqual(elapsed, 1.0) - - def test_real_bus_discards_wrong_action_then_matches(self): - # Proves the discard-and-keep-waiting loop can still succeed on a - # later, matching response within the same deadline. - skill = Mock() - skill.settings = {} - skill.bus = FakeBus() - skill.dialog_renderer = None + self.assertEqual(result.outcome, NativeActionOutcome.SUCCESS) + self.assertLess(time.monotonic() - start, 1.0) + + def test_timeout_waits_for_full_deadline_then_speaks(self): + skill = _make_skill(settings={"per_action_timeouts": + {"launch_camera_app": 0.5}}) message = _node_message(action_supported=True) - # Space the two emits out: `_await_response` only resubscribes to - # the bus AFTER the wrong-action message is received and discarded, - # so firing both immediately can drop the second before anything - # is listening for it again. - def _emit_wrong(): - skill.bus.emit(_response(action="launch_clock_app", - status="success")) + start = time.monotonic() + result = invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) + elapsed = time.monotonic() - start - def _emit_right(): - skill.bus.emit(_response(action="launch_camera_app", - status="success")) + self.assertEqual(result.outcome, NativeActionOutcome.TIMEOUT) + self.assertGreaterEqual(elapsed, 0.5) + self.assertLess(elapsed, 1.5) + skill.speak.assert_called_once() + + def test_wrong_action_then_right_action_back_to_back(self): + # Both replies land synchronously with no gap; the waiter must keep + # listening through the first without re-subscribing. + skill = _make_skill(responses=[_response(action="launch_clock_app"), + _response(action="launch_camera_app")]) + message = _node_message(action_supported=True) - threading.Timer(0.05, _emit_wrong).start() - threading.Timer(0.2, _emit_right).start() result = invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) self.assertEqual(result.outcome, NativeActionOutcome.SUCCESS) - def test_mismatched_response_is_discarded_then_timeout(self): - wrong_action = _response(action="launch_clock_app", status="success") - skill = _make_skill() - # First call returns a response for the wrong action, second - # call (after filtering) returns None because timeout elapsed - skill.bus.wait_for_message = Mock( - side_effect=[wrong_action, None]) + def test_mismatched_action_only_times_out(self): + skill = _make_skill(settings=QUICK_TIMEOUTS, + responses=[_response(action="launch_clock_app")]) message = _node_message(action_supported=True) result = invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) self.assertEqual(result.outcome, NativeActionOutcome.TIMEOUT) - self.assertEqual(skill.bus.wait_for_message.call_count, 2) - - def test_mismatched_session_is_discarded(self): - wrong_session = _response(status="success", session_id="node-other") - skill = Mock() - skill.settings = {} - skill.dialog_renderer = None - skill.bus = Mock() - skill.bus.wait_for_message = Mock(side_effect=[wrong_session, None]) + + def test_mismatched_session_only_times_out(self): + skill = _make_skill(settings=QUICK_TIMEOUTS, + responses=[_response(session_id="node-other")]) message = _node_message(action_supported=True) result = invoke_native_action(skill, message, @@ -270,32 +281,53 @@ def test_mismatched_session_is_discarded(self): self.assertEqual(result.outcome, NativeActionOutcome.TIMEOUT) - def test_per_action_timeout_override_used(self): - skill = _make_skill(settings={"per_action_timeouts": - {"launch_camera_app": 1}}) + def test_response_handler_removed_after_success(self): + skill = _make_skill(responses=[_response(status="success")]) message = _node_message(action_supported=True) invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) - _, kwargs = skill.bus.wait_for_message.call_args - self.assertLessEqual(kwargs["timeout"], 1) + self.assertEqual(skill.bus.ee.listeners(RESPONSE_TYPE), []) + + def test_response_handler_removed_after_timeout(self): + skill = _make_skill(settings=QUICK_TIMEOUTS) + message = _node_message(action_supported=True) + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) + + self.assertEqual(skill.bus.ee.listeners(RESPONSE_TYPE), []) + + # --- timeouts --------------------------------------------------------- + + def test_per_action_timeout_override_used(self): + skill = _make_skill(settings={"per_action_timeouts": + {"launch_camera_app": 1.5}}) + + self.assertEqual(_resolve_timeout(skill, "launch_camera_app"), 1.5) def test_default_timeout_used_when_no_override(self): skill = _make_skill() - message = _node_message(action_supported=True, - action="launch_sms_app") - invoke_native_action(skill, message, NodeNativeAction.LAUNCH_SMS_APP) + self.assertEqual(_resolve_timeout(skill, "launch_camera_app"), 10) + self.assertEqual(_resolve_timeout(skill, "launch_sms_app"), 5) + + def test_invalid_timeout_override_falls_back_with_warning(self): + skill = _make_skill(settings={"per_action_timeouts": + {"launch_camera_app": "soon"}}) - _, kwargs = skill.bus.wait_for_message.call_args - self.assertLessEqual(kwargs["timeout"], 5) + with patch("neon_utils.native_actions.LOG") as mock_log: + timeout = _resolve_timeout(skill, "launch_camera_app") + + self.assertEqual(timeout, 10) + mock_log.warning.assert_called_once() + + # --- dialog ----------------------------------------------------------- def test_speaks_dialog_key_when_dialog_renderer_has_it(self): response = _response(status="error", error={"code": "unavailable", "message": "x"}) - skill = _make_skill(wait_for_message_return=response) - skill.dialog_renderer = Mock() - skill.dialog_renderer.templates = {"native_action_error": []} + skill = _make_skill(responses=[response]) + skill.dialog_renderer = _renderer({"native_action_error": ["{code}"]}) message = _node_message(action_supported=True) invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) @@ -306,9 +338,8 @@ def test_speaks_dialog_key_when_dialog_renderer_has_it(self): def test_falls_back_to_speak_when_no_dialog_file(self): response = _response(status="error", error={"code": "unavailable", "message": "x"}) - skill = _make_skill(wait_for_message_return=response) - skill.dialog_renderer = Mock() - skill.dialog_renderer.templates = {} + skill = _make_skill(responses=[response]) + skill.dialog_renderer = _renderer({}) message = _node_message(action_supported=True) invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) @@ -318,7 +349,7 @@ def test_falls_back_to_speak_when_no_dialog_file(self): def test_no_dialog_file_does_not_log_error_or_warning(self): # A missing native_action_*.dialog file is the expected, common - # case post-change, not an exceptional one worth logging. + # case, not an exceptional one worth logging. skill = _make_skill() message = _node_message(action_supported=False) @@ -329,6 +360,30 @@ def test_no_dialog_file_does_not_log_error_or_warning(self): mock_log.error.assert_not_called() mock_log.warning.assert_not_called() + def test_action_dialog_localizes_description(self): + skill = _make_skill() + skill.dialog_renderer = _renderer( + {"native_action_not_supported": ["No puedo abrir {description}."], + "launch_camera_app": ["la cámara"]}) + message = _node_message(action_supported=False) + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) + + key, data = skill.speak_dialog.call_args[0] + self.assertEqual(key, "native_action_not_supported") + self.assertEqual(data["description"], "la cámara") + + def test_description_falls_back_to_english_without_action_dialog(self): + skill = _make_skill() + skill.dialog_renderer = _renderer( + {"native_action_not_supported": ["Cannot open {description}."]}) + message = _node_message(action_supported=False) + + invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) + + _, data = skill.speak_dialog.call_args[0] + self.assertEqual(data["description"], "the camera app") + def test_not_supported_fallback_text(self): skill = _make_skill() message = _node_message(action_supported=False) @@ -339,7 +394,7 @@ def test_not_supported_fallback_text(self): "This device cannot open the camera app.") def test_timeout_fallback_text(self): - skill = _make_skill(wait_for_message_return=None) + skill = _make_skill(settings=QUICK_TIMEOUTS) message = _node_message(action_supported=True) invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) @@ -348,9 +403,8 @@ def test_timeout_fallback_text(self): "I did not reach your device.") def test_success_fallback_text(self): - response = _response(status="success") skill = _make_skill(settings={"confirm_on_success": True}, - wait_for_message_return=response) + responses=[_response(status="success")]) message = _node_message(action_supported=True) invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) @@ -361,7 +415,7 @@ def test_error_fallback_uses_node_message_when_present(self): response = _response(status="error", error={"code": "unavailable", "message": "No camera app is available."}) - skill = _make_skill(wait_for_message_return=response) + skill = _make_skill(responses=[response]) message = _node_message(action_supported=True) invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) @@ -372,7 +426,7 @@ def test_error_fallback_uses_node_message_when_present(self): def test_error_fallback_generic_when_node_gives_no_message(self): response = _response(action="launch_sms_app", status="error", error={"code": "unavailable", "message": ""}) - skill = _make_skill(wait_for_message_return=response) + skill = _make_skill(responses=[response]) message = _node_message(action_supported=True, action="launch_sms_app") From 2781d5cc3f3467441dc1a1e040cb199e1ff6a842 Mon Sep 17 00:00:00 2001 From: Mike Date: Fri, 4 Sep 2026 16:19:55 -0500 Subject: [PATCH 3/3] refactor: require skill dialog files for native action speech Drop the built-in English fallback text. Skills that call invoke_native_action are NeonGecko-controlled and must ship the native_action_* dialogs plus one .dialog per action they invoke; the module docstring lists them. A missing file gets the standard OVOS behavior of speaking the key. Annotate `skill` as NeonSkill rather than OVOSSkill: the helper passes `message=` to speak_dialog, which only NeonSkill accepts. --- neon_utils/native_actions.py | 118 +++++++++++--------------- tests/native_actions_tests.py | 150 ++++++++++++---------------------- 2 files changed, 100 insertions(+), 168 deletions(-) diff --git a/neon_utils/native_actions.py b/neon_utils/native_actions.py index 96b2266d..2b66e904 100644 --- a/neon_utils/native_actions.py +++ b/neon_utils/native_actions.py @@ -27,18 +27,37 @@ # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import time +""" +Shared helper for Node native actions (`node.invoke_native`). + +A skill that calls `invoke_native_action` must ship these dialog files: + +- `native_action_not_supported.dialog` +- `native_action_success.dialog` (spoken only when `confirm_on_success` is set) +- `native_action_timeout.dialog` +- `native_action_error.dialog` +- one `.dialog` per action the skill invokes, e.g. + `launch_camera_app.dialog`, holding the spoken name of that action + +Every `native_action_*` dialog receives `{action}` and `{description}`; +`native_action_error` also receives `{code}` and `{message}`. A missing +dialog file gets the standard OVOS behavior: the key itself is spoken. +""" + import threading from dataclasses import dataclass from enum import Enum -from typing import Optional +from typing import Optional, TYPE_CHECKING from ovos_bus_client import Message from ovos_utils.log import LOG -from ovos_workshop.skills.ovos import OVOSSkill from neon_data_models.enum import NodeNativeAction, NativeActionErrorCode +if TYPE_CHECKING: + from neon_utils.skills.neon_skill import NeonSkill + INVOKE_TYPE = "node.invoke_native" RESPONSE_TYPE = "node.invoke_native.response" @@ -68,14 +87,15 @@ class NativeActionResult: error_message: str = "" -def invoke_native_action(skill: OVOSSkill, message: Message, +def invoke_native_action(skill: "NeonSkill", message: Message, action: NodeNativeAction, params: Optional[dict] = None) -> NativeActionResult: """ Capability-gate, dispatch, and await a `node.invoke_native` request. Shared by every skill's `if node:` branch so the flow (gate, emit, await, settings) lives in one place. - :param skill: the calling OVOSSkill/NeonSkill instance + :param skill: the calling NeonSkill instance (its `speak_dialog` must + accept `message=`, which the OVOSSkill base does not) :param message: the triggering Message (must carry `context.node`) :param action: `NodeNativeAction` to invoke :param params: optional payload (only meaningful for sms/email actions) @@ -153,7 +173,7 @@ def _session_of(message: Message) -> Optional[str]: return message.context.get("session", {}).get("session_id") -def _interpret_response(skill: OVOSSkill, message: Message, +def _interpret_response(skill: "NeonSkill", message: Message, action_value: str, response: Message) -> NativeActionResult: if response.data.get("status") == "success": @@ -172,7 +192,7 @@ def _interpret_response(skill: OVOSSkill, message: Message, error_message) -def _resolve_timeout(skill: OVOSSkill, action_value: str) -> float: +def _resolve_timeout(skill: "NeonSkill", action_value: str) -> float: overrides = skill.settings.get("per_action_timeouts") or {} if action_value in overrides: try: @@ -183,80 +203,40 @@ def _resolve_timeout(skill: OVOSSkill, action_value: str) -> float: return _DEFAULT_TIMEOUTS.get(action_value, _FALLBACK_TIMEOUT) -def _speak_not_supported(skill: OVOSSkill, message: Message, +def _speak_not_supported(skill: "NeonSkill", message: Message, action_value: str): - _speak(skill, message, "native_action_not_supported", - _dialog_data(skill, action_value)) + skill.speak_dialog("native_action_not_supported", + _dialog_data(skill, action_value), message=message) -def _speak_success(skill: OVOSSkill, message: Message, action_value: str): - _speak(skill, message, "native_action_success", - _dialog_data(skill, action_value)) +def _speak_success(skill: "NeonSkill", message: Message, action_value: str): + skill.speak_dialog("native_action_success", + _dialog_data(skill, action_value), message=message) -def _speak_timeout(skill: OVOSSkill, message: Message, action_value: str): - _speak(skill, message, "native_action_timeout", - _dialog_data(skill, action_value)) +def _speak_timeout(skill: "NeonSkill", message: Message, action_value: str): + skill.speak_dialog("native_action_timeout", + _dialog_data(skill, action_value), message=message) -def _speak_error(skill: OVOSSkill, message: Message, action_value: str, +def _speak_error(skill: "NeonSkill", message: Message, action_value: str, error_code: NativeActionErrorCode, error_message: str): - _speak(skill, message, "native_action_error", - _dialog_data(skill, action_value, code=error_code.value, - message=error_message)) + skill.speak_dialog("native_action_error", + _dialog_data(skill, action_value, + code=error_code.value, + message=error_message), + message=message) -def _dialog_data(skill: OVOSSkill, action_value: str, **extra) -> dict: +def _dialog_data(skill: "NeonSkill", action_value: str, **extra) -> dict: return {"action": action_value, "description": _describe(skill, action_value), **extra} -def _speak(skill: OVOSSkill, message: Message, dialog_key: str, data: dict): - """Prefer the skill's own dialog file; fall back to built-in text.""" - if _has_dialog(skill, dialog_key): - skill.speak_dialog(dialog_key, data, message=message) - else: - skill.speak(_fallback_text(dialog_key, data), message=message) - - -def _has_dialog(skill: OVOSSkill, dialog_key: str) -> bool: - """ - Checks `templates` directly rather than calling `speak_dialog` blind: - a missing dialog file is the expected case here, not an error to log. - """ - try: - renderer = skill.dialog_renderer - return bool(renderer) and dialog_key in renderer.templates - except Exception as e: - LOG.warning(f"Failed checking dialog_renderer for {dialog_key}: {e}") - return False - - -def _describe(skill: OVOSSkill, action_value: str) -> str: - """ - A skill localizes the spoken name of an action with a dialog file named - after the `NodeNativeAction` value, e.g. `launch_camera_app.dialog`. - """ - if _has_dialog(skill, action_value): - return skill.dialog_renderer.render(action_value) - return _fallback_description(action_value) - - -def _fallback_description(action_value: str) -> str: - name = action_value.replace("launch_", "").replace("_app", "") \ - .replace("_", " ") - return f"the {name} app" if name else "that app" - - -def _fallback_text(dialog_key: str, data: dict) -> str: - """Built-in English for a skill with no matching dialog file.""" - description = data.get("description") or "that app" - if dialog_key == "native_action_not_supported": - return f"This device cannot open {description}." - if dialog_key == "native_action_success": - return "Done." - if dialog_key == "native_action_timeout": - return "I did not reach your device." - if dialog_key == "native_action_error": - return data.get("message") or f"I could not open {description}." - return "" +def _describe(skill: "NeonSkill", action_value: str) -> str: + """Render `.dialog`; the raw key if the file is missing.""" + renderer = skill.dialog_renderer + if not renderer: + LOG.warning(f"No dialog_renderer; speaking raw key {action_value}") + return action_value + return renderer.render(action_value) diff --git a/tests/native_actions_tests.py b/tests/native_actions_tests.py index ce896b94..46479156 100644 --- a/tests/native_actions_tests.py +++ b/tests/native_actions_tests.py @@ -46,6 +46,8 @@ QUICK_TIMEOUTS = {"per_action_timeouts": {"launch_camera_app": 0.2, "launch_sms_app": 0.2}} +ACTION_DIALOGS = {"launch_camera_app": ["the camera app"], + "launch_sms_app": ["the messages app"]} def _node_message(action_supported: bool = True, action="launch_camera_app", @@ -69,6 +71,12 @@ def _response(action="launch_camera_app", status="success", error=None, return Message(RESPONSE_TYPE, data, {"session": {"session_id": session_id}}) +def _renderer(templates: dict) -> MustacheDialogRenderer: + renderer = MustacheDialogRenderer() + renderer.templates = templates + return renderer + + def _make_skill(settings=None, responses=()): """ Each response is emitted synchronously from inside the invoke emit, so @@ -78,7 +86,7 @@ def _make_skill(settings=None, responses=()): skill = Mock() skill.settings = settings or {} skill.bus = FakeBus() - skill.dialog_renderer = None + skill.dialog_renderer = _renderer(dict(ACTION_DIALOGS)) if responses: skill.bus.on(INVOKE_TYPE, lambda _: [skill.bus.emit(r) for r in responses]) @@ -91,10 +99,11 @@ def _capture_invokes(skill) -> list: return invokes -def _renderer(templates: dict) -> MustacheDialogRenderer: - renderer = MustacheDialogRenderer() - renderer.templates = templates - return renderer +def _spoken(skill): + """(dialog key, data) of the single speak_dialog call.""" + skill.speak_dialog.assert_called_once() + key, data = skill.speak_dialog.call_args[0] + return key, data class NativeActionsTests(unittest.TestCase): @@ -108,7 +117,10 @@ def test_not_supported_speaks_and_emits_nothing(self): self.assertEqual(result.outcome, NativeActionOutcome.NOT_SUPPORTED) self.assertEqual(invokes, []) - skill.speak.assert_called_once() + key, data = _spoken(skill) + self.assertEqual(key, "native_action_not_supported") + self.assertEqual(data, {"action": "launch_camera_app", + "description": "the camera app"}) def test_capabilities_entirely_absent_treated_as_not_supported(self): skill = _make_skill() @@ -178,9 +190,10 @@ def test_success_confirms_when_setting_enabled(self): NodeNativeAction.LAUNCH_CAMERA_APP) self.assertEqual(result.outcome, NativeActionOutcome.SUCCESS) - skill.speak.assert_called_once() + key, _ = _spoken(skill) + self.assertEqual(key, "native_action_success") - def test_error_response_speaks_failure_with_code(self): + def test_error_response_speaks_failure_with_code_and_message(self): response = _response(status="error", error={"code": "unavailable", "message": "No camera app."}) @@ -192,7 +205,12 @@ def test_error_response_speaks_failure_with_code(self): self.assertEqual(result.outcome, NativeActionOutcome.ERROR) self.assertEqual(result.error_code, NativeActionErrorCode.UNAVAILABLE) - skill.speak.assert_called_once() + key, data = _spoken(skill) + self.assertEqual(key, "native_action_error") + self.assertEqual(data, {"action": "launch_camera_app", + "description": "the camera app", + "code": "unavailable", + "message": "No camera app."}) def test_unknown_error_code_maps_to_internal_error(self): response = _response(status="error", @@ -205,6 +223,8 @@ def test_unknown_error_code_maps_to_internal_error(self): self.assertEqual(result.error_code, NativeActionErrorCode.INTERNAL_ERROR) + _, data = _spoken(skill) + self.assertEqual(data["code"], "internal_error") # --- response waiting ------------------------------------------------- @@ -247,7 +267,8 @@ def test_timeout_waits_for_full_deadline_then_speaks(self): self.assertEqual(result.outcome, NativeActionOutcome.TIMEOUT) self.assertGreaterEqual(elapsed, 0.5) self.assertLess(elapsed, 1.5) - skill.speak.assert_called_once() + key, _ = _spoken(skill) + self.assertEqual(key, "native_action_timeout") def test_wrong_action_then_right_action_back_to_back(self): # Both replies land synchronously with no gap; the waiter must keep @@ -323,117 +344,48 @@ def test_invalid_timeout_override_falls_back_with_warning(self): # --- dialog ----------------------------------------------------------- - def test_speaks_dialog_key_when_dialog_renderer_has_it(self): - response = _response(status="error", - error={"code": "unavailable", "message": "x"}) - skill = _make_skill(responses=[response]) - skill.dialog_renderer = _renderer({"native_action_error": ["{code}"]}) - message = _node_message(action_supported=True) - - invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) - - skill.speak_dialog.assert_called_once() - skill.speak.assert_not_called() - - def test_falls_back_to_speak_when_no_dialog_file(self): - response = _response(status="error", - error={"code": "unavailable", "message": "x"}) - skill = _make_skill(responses=[response]) - skill.dialog_renderer = _renderer({}) - message = _node_message(action_supported=True) - - invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) - - skill.speak.assert_called_once() - skill.speak_dialog.assert_not_called() - - def test_no_dialog_file_does_not_log_error_or_warning(self): - # A missing native_action_*.dialog file is the expected, common - # case, not an exceptional one worth logging. - skill = _make_skill() - message = _node_message(action_supported=False) - - with patch("neon_utils.native_actions.LOG") as mock_log: - invoke_native_action(skill, message, - NodeNativeAction.LAUNCH_CAMERA_APP) - - mock_log.error.assert_not_called() - mock_log.warning.assert_not_called() - def test_action_dialog_localizes_description(self): skill = _make_skill() - skill.dialog_renderer = _renderer( - {"native_action_not_supported": ["No puedo abrir {description}."], - "launch_camera_app": ["la cámara"]}) + skill.dialog_renderer = _renderer({"launch_camera_app": ["la cámara"]}) message = _node_message(action_supported=False) invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) - key, data = skill.speak_dialog.call_args[0] - self.assertEqual(key, "native_action_not_supported") + _, data = _spoken(skill) self.assertEqual(data["description"], "la cámara") - def test_description_falls_back_to_english_without_action_dialog(self): + def test_missing_action_dialog_speaks_raw_key(self): + # Standard OVOS behavior for a missing dialog file, and the cue a + # skill author gets that `launch_camera_app.dialog` is absent. skill = _make_skill() - skill.dialog_renderer = _renderer( - {"native_action_not_supported": ["Cannot open {description}."]}) + skill.dialog_renderer = _renderer({}) message = _node_message(action_supported=False) invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) - _, data = skill.speak_dialog.call_args[0] - self.assertEqual(data["description"], "the camera app") + _, data = _spoken(skill) + self.assertEqual(data["description"], "launch_camera_app") - def test_not_supported_fallback_text(self): + def test_no_dialog_renderer_speaks_raw_key_with_warning(self): skill = _make_skill() + skill.dialog_renderer = None message = _node_message(action_supported=False) - invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) - - self.assertEqual(skill.speak.call_args[0][0], - "This device cannot open the camera app.") - - def test_timeout_fallback_text(self): - skill = _make_skill(settings=QUICK_TIMEOUTS) - message = _node_message(action_supported=True) - - invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) - - self.assertEqual(skill.speak.call_args[0][0], - "I did not reach your device.") - - def test_success_fallback_text(self): - skill = _make_skill(settings={"confirm_on_success": True}, - responses=[_response(status="success")]) - message = _node_message(action_supported=True) - - invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) + with patch("neon_utils.native_actions.LOG") as mock_log: + invoke_native_action(skill, message, + NodeNativeAction.LAUNCH_CAMERA_APP) - self.assertEqual(skill.speak.call_args[0][0], "Done.") + _, data = _spoken(skill) + self.assertEqual(data["description"], "launch_camera_app") + mock_log.warning.assert_called_once() - def test_error_fallback_uses_node_message_when_present(self): - response = _response(status="error", - error={"code": "unavailable", - "message": "No camera app is available."}) - skill = _make_skill(responses=[response]) - message = _node_message(action_supported=True) + def test_speaks_with_triggering_message(self): + skill = _make_skill() + message = _node_message(action_supported=False) invoke_native_action(skill, message, NodeNativeAction.LAUNCH_CAMERA_APP) - self.assertEqual(skill.speak.call_args[0][0], - "No camera app is available.") - - def test_error_fallback_generic_when_node_gives_no_message(self): - response = _response(action="launch_sms_app", status="error", - error={"code": "unavailable", "message": ""}) - skill = _make_skill(responses=[response]) - message = _node_message(action_supported=True, - action="launch_sms_app") - - invoke_native_action(skill, message, NodeNativeAction.LAUNCH_SMS_APP) - - self.assertEqual(skill.speak.call_args[0][0], - "I could not open the sms app.") + self.assertIs(skill.speak_dialog.call_args[1]["message"], message) if __name__ == '__main__':