refactor: delegate handler-lifecycle done-signal to HandlerLifecycle util (#440)

The three event-wrapper callbacks (_on_event_start/_on_event_end/
_on_event_error) emitted the internal workshop->core done-signal trio
(mycroft.skill.handler.{start,complete,error}) by hand. Delegate that
emission to the shared ovos_bus_client.handler.HandlerLifecycle util
(#246) so the wire format lives in one place.

Pure DRY: same topics, same payloads ({'name': <handler>} on
start/complete, + {'exception': repr(error)} on error), same
context['skill_id'] stamping and preserved session. The util stamps
skill_id on the forwarded copy rather than mutating the caller's
message, and builds a fresh payload per emission instead of mutating
the shared skill_data dict.

Preserved (NOT part of the trio): _on_event_end still emits
ovos.utterance.handled when is_intent and still runs settings.store();
_on_event_error still speaks the skill.error dialog and logs the
exception (the util deliberately does not speak).

Bumps the ovos-bus-client floor to >=2.6.0a1 (carries the util).
Fills the previously-stubbed test_on_event_{start,end,error} tests,
asserting the emitted topic/payload/context is unchanged.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: JarbasAl

ovos_workshop/skills/ovos.py

@@ -33,6 +33,7 @@
 from ovos_bus_client.apis.events import EventSchedulerInterface
 from ovos_bus_client.apis.gui import GUIInterface
 from ovos_bus_client.apis.ocp import OCPInterface
+from ovos_bus_client.handler import HandlerLifecycle
 from ovos_bus_client.message import Message, dig_for_message
 from ovos_bus_client.session import SessionManager, Session
 from ovos_bus_client.util import get_message_lang
@@ -1454,10 +1455,11 @@ def _on_event_start(self, message: Message, handler_info: str,
         """
         if handler_info:
             # internal workshop->core done-signal (see docstring); NOT a spec
-            # topic -> emits mycroft.skill.handler.start
-            msg_type = handler_info + '.start'
-            message.context["skill_id"] = self.skill_id
-            self.bus.emit(message.forward(msg_type, skill_data))
+            # topic -> emits mycroft.skill.handler.start. Delegated to the
+            # shared ovos-bus-client HandlerLifecycle util (DRY; same topic,
+            # payload and context["skill_id"] as before).
+            HandlerLifecycle(self.bus, message, skill_id=self.skill_id,
+                             data=skill_data, handler_info=handler_info).start()
 
     def _on_event_end(self, message: Message, handler_info: str,
                       skill_data: dict, is_intent: bool = False):
@@ -1467,10 +1469,10 @@ def _on_event_end(self, message: Message, handler_info: str,
         """
         if handler_info:
             # internal workshop->core done-signal (see _on_event_start); NOT a
-            # spec topic -> emits mycroft.skill.handler.complete
-            msg_type = handler_info + '.complete'
-            message.context["skill_id"] = self.skill_id
-            self.bus.emit(message.forward(msg_type, skill_data))
+            # spec topic -> emits mycroft.skill.handler.complete. Delegated to
+            # the shared HandlerLifecycle util (same topic/payload/context).
+            HandlerLifecycle(self.bus, message, skill_id=self.skill_id,
+                             data=skill_data, handler_info=handler_info).complete()
         if is_intent:
             self.bus.emit(message.forward(SpecMessage.UTTERANCE_HANDLED, skill_data))
 
@@ -1492,15 +1494,15 @@ def _on_event_error(self, error: str, message: Message, handler_info: str,
         if speak_errors:
             self.speak(speech)
         self.log.exception(error)
-        # append exception information in message
-        skill_data['exception'] = repr(error)
         if handler_info:
             # internal workshop->core done-signal (see _on_event_start); NOT a
-            # spec topic -> emits mycroft.skill.handler.error
-            msg_type = handler_info + '.error'
+            # spec topic -> emits mycroft.skill.handler.error. Delegated to the
+            # shared HandlerLifecycle util, which merges {"exception": repr(...)}
+            # into the payload (same topic/payload/context as before). The util
+            # deliberately does NOT speak; the spoken-error UX above stays here.
             message = message or Message("")
-            message.context["skill_id"] = self.skill_id
-            self.bus.emit(message.forward(msg_type, skill_data))
+            HandlerLifecycle(self.bus, message, skill_id=self.skill_id,
+                             data=skill_data, handler_info=handler_info).error(error)
 
     def _register_adapt_intent(self,
                                intent_parser: Union[IntentBuilder, Intent, str],

pyproject.toml

@@ -12,7 +12,7 @@ authors = [{name = "jarbasAi", email = "jarbasai@mailfence.com"}]
 requires-python = ">=3.9"
 dependencies = [
     "ovos-utils>= 0.7.0,<1.0.0",
-    "ovos_bus_client>=2.2.0a1,<3.0.0",
+    "ovos_bus_client>=2.6.0a1,<3.0.0",  # HandlerLifecycle done-signal util (#246)
     "ovos-config>=0.0.12,<3.0.0",
     "ovos-spec-tools>=0.16.1a2",  # carries the adapt-free intent-definition primitives (OVOS-INTENT-4)
     "ovos-yes-no-plugin>=0.3.0,<1.0.0",

test/unittests/skills/test_base.py

@@ -297,17 +297,97 @@ def test_find_resource(self):
         # TODO
         pass
 
+    def _capture(self, fn, *args, **kwargs):
+        """Run an _on_event_* callback against a private FakeBus and return the
+        list of (type, data, context) tuples it emitted on the topics of
+        interest (the handler trio + ovos.utterance.handled)."""
+        from ovos_spec_tools import SpecMessage
+        skill = OVOSSkill(bus=FakeBus(), skill_id=self.skill_id)
+        captured = []
+        topics = ["mycroft.skill.handler.start",
+                  "mycroft.skill.handler.complete",
+                  "mycroft.skill.handler.error",
+                  SpecMessage.UTTERANCE_HANDLED.value]
+        for t in topics:
+            skill.bus.on(t, lambda m: captured.append(
+                (m.msg_type, m.data, dict(m.context))))
+        fn(skill, *args, **kwargs)
+        return captured
+
     def test_on_event_start(self):
-        # TODO
-        pass
+        from ovos_bus_client.message import Message
+        msg = Message("trigger", {}, {"session": {"session_id": "sess1"}})
+        skill_data = {"name": "TestSkill.handle_test"}
+        captured = self._capture(OVOSSkill._on_event_start, msg,
+                                 "mycroft.skill.handler", dict(skill_data))
+        # exactly one emission: the .start done-signal
+        starts = [c for c in captured
+                  if c[0] == "mycroft.skill.handler.start"]
+        self.assertEqual(len(starts), 1)
+        mtype, data, context = starts[0]
+        # byte-identical topic + payload
+        self.assertEqual(mtype, "mycroft.skill.handler.start")
+        self.assertEqual(data, {"name": "TestSkill.handle_test"})
+        # context carries skill_id + preserves originating session
+        self.assertEqual(context["skill_id"], self.skill_id)
+        self.assertEqual(context["session"]["session_id"], "sess1")
+        # original message context not mutated by the util
+        self.assertNotIn("skill_id", msg.context)
+        # empty/false handler_info disables emission entirely
+        none_captured = self._capture(OVOSSkill._on_event_start, msg, "",
+                                      dict(skill_data))
+        self.assertEqual(
+            [c for c in none_captured
+             if c[0].startswith("mycroft.skill.handler")], [])
 
     def test_on_event_end(self):
-        # TODO
-        pass
+        from ovos_bus_client.message import Message
+        msg = Message("trigger", {}, {"session": {"session_id": "sess1"}})
+        skill_data = {"name": "TestSkill.handle_test"}
+        captured = self._capture(OVOSSkill._on_event_end, msg,
+                                 "mycroft.skill.handler", dict(skill_data))
+        completes = [c for c in captured
+                     if c[0] == "mycroft.skill.handler.complete"]
+        self.assertEqual(len(completes), 1)
+        mtype, data, context = completes[0]
+        self.assertEqual(mtype, "mycroft.skill.handler.complete")
+        self.assertEqual(data, {"name": "TestSkill.handle_test"})
+        self.assertEqual(context["skill_id"], self.skill_id)
+        self.assertEqual(context["session"]["session_id"], "sess1")
+
+    def test_on_event_end_is_intent_still_emits_utterance_handled(self):
+        # the ovos.utterance.handled emission must be PRESERVED alongside the
+        # delegated .complete done-signal when is_intent=True
+        from ovos_bus_client.message import Message
+        from ovos_spec_tools import SpecMessage
+        msg = Message("trigger", {}, {})
+        skill_data = {"name": "TestSkill.handle_test"}
+        captured = self._capture(OVOSSkill._on_event_end, msg,
+                                 "mycroft.skill.handler", dict(skill_data),
+                                 True)
+        types = [c[0] for c in captured]
+        self.assertIn("mycroft.skill.handler.complete", types)
+        self.assertIn(SpecMessage.UTTERANCE_HANDLED.value, types)
 
     def test_on_event_error(self):
-        # TODO
-        pass
+        from ovos_bus_client.message import Message
+        msg = Message("trigger", {}, {"session": {"session_id": "sess1"}})
+        skill_data = {"name": "TestSkill.handle_test"}
+        err = "boom"  # workshop passes str(error)
+        captured = self._capture(OVOSSkill._on_event_error, err, msg,
+                                 "mycroft.skill.handler", dict(skill_data),
+                                 False)
+        errors = [c for c in captured
+                  if c[0] == "mycroft.skill.handler.error"]
+        self.assertEqual(len(errors), 1)
+        mtype, data, context = errors[0]
+        self.assertEqual(mtype, "mycroft.skill.handler.error")
+        # payload = original {name} + repr(error) under "exception" (identical
+        # to the pre-refactor skill_data['exception'] = repr(error))
+        self.assertEqual(data, {"name": "TestSkill.handle_test",
+                                "exception": repr(err)})
+        self.assertEqual(context["skill_id"], self.skill_id)
+        self.assertEqual(context["session"]["session_id"], "sess1")
 
     def test_add_event(self):
         # TODO