From 79b78d6006e20a60d416e2374dd7f588c68d4a86 Mon Sep 17 00:00:00 2001 From: JarbasAi Date: Wed, 9 Sep 2026 01:40:43 +0100 Subject: [PATCH] fix: the legacy register_vocab names the registering skill emit_legacy_register_vocab dug the ambient message and stamped the producer only when the context had none, so a skill registering while handling another component's message shipped that component as the owner of its vocabulary while the intent referencing it went out attributed correctly. It now takes the caller's stamped copy, as its register_intent sibling already does. --- ovos_workshop/intents.py | 15 +++++--- test/unittests/test_intent4_producer.py | 47 +++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/ovos_workshop/intents.py b/ovos_workshop/intents.py index 6453e5de..2c5a5b23 100644 --- a/ovos_workshop/intents.py +++ b/ovos_workshop/intents.py @@ -145,17 +145,21 @@ def munge_intent_parser(intent_parser, name, skill_id): # dual-emit (see IntentServiceInterface.register_keyword/register_intent) # ------------------------------------------------------------------ - def emit_legacy_register_vocab(self, vocab_type: str, entity: str, + def emit_legacy_register_vocab(self, msg: Message, vocab_type: str, + entity: str, aliases: Optional[List[str]] = None, lang: str = None): """Emit the legacy adapt ``register_vocab`` topic (entity + aliases). + `msg` is the caller's stamped copy, carrying this skill as the + producer. The adapt engine reads the legacy producer from the + context, so digging the ambient message here would attribute the + vocabulary to whichever component the skill happens to be handling + while the intent it belongs to went out attributed correctly. + TODO: drop once the adapt pipeline consumes ovos.intent.register.keyword (INTENT-4 ยง5) directly. """ aliases = aliases or [] - msg = dig_for_message() or Message("") - if "skill_id" not in msg.context: - msg.context["skill_id"] = self.skill_id entity_data = {'entity_value': entity, 'entity_type': vocab_type, 'lang': lang} @@ -480,7 +484,8 @@ def register_keyword(self, vocab_type: str, entity: str, samples.append(value) # TODO: drop once _AdaptIntentApi.emit_legacy_register_vocab is removed. - self._adapt.emit_legacy_register_vocab(vocab_type, entity, aliases, lang) + self._adapt.emit_legacy_register_vocab(msg, vocab_type, entity, + aliases, lang) def _unmunge_vocab_name(self, vocab_type: str) -> str: prefix = _AdaptIntentApi.to_alnum(self.skill_id) diff --git a/test/unittests/test_intent4_producer.py b/test/unittests/test_intent4_producer.py index 281df772..6bde9736 100644 --- a/test/unittests/test_intent4_producer.py +++ b/test/unittests/test_intent4_producer.py @@ -471,6 +471,53 @@ def test_munge_and_register_intent_still_spec_emits_keyword(self): self.assertEqual(data["required"], [{"name": "kw", "samples": ["hello"]}]) +class LegacyVocabProducerIdentityTest(unittest.TestCase): + """The legacy register_vocab names the registering skill as its producer. + + The adapt engine reads the legacy producer from the context, so a skill + that registers while handling another component's message must not ship + that component as the owner of its vocabulary. + """ + + def setUp(self): + self.bus = CapturingBus() + self.iface = IntentServiceInterface(self.bus) + self.iface.set_id("test.skill") + self.foreign = Message("some.other.event", {}, + {"skill_id": "other.component"}) + + def test_legacy_vocab_is_attributed_to_the_registering_skill(self): + with patch("ovos_workshop.intents.dig_for_message", + return_value=self.foreign): + self.iface.register_adapt_keyword("setKW", "set", + aliases=["change"], + lang="en-US") + legacy = self.bus.of_type("register_vocab") + self.assertEqual(len(legacy), 2) # entity + one alias + for _, context in legacy: + self.assertEqual(context["skill_id"], "test.skill") + + def test_legacy_and_spec_agree_on_the_producer(self): + # The vocabulary and the intent that references it are one + # registration; disagreeing between them splits it across two owners. + with patch("ovos_workshop.intents.dig_for_message", + return_value=self.foreign): + self.iface.register_adapt_keyword("setKW", "set", lang="en-US") + parser = IntentBuilder("set_it").require("setKW").build() + self.iface.register_adapt_intent("set_it", parser) + spec = self.bus.of_type(SpecMessage.INTENT_REGISTER_KEYWORD) + legacy = self.bus.of_type("register_vocab") + self.assertTrue(spec and legacy) + owners = {c["skill_id"] for _, c in spec + legacy} + self.assertEqual(owners, {"test.skill"}) + + def test_the_dug_message_is_not_mutated(self): + with patch("ovos_workshop.intents.dig_for_message", + return_value=self.foreign): + self.iface.register_adapt_keyword("setKW", "set", lang="en-US") + self.assertEqual(self.foreign.context["skill_id"], "other.component") + + class RegexRegistrationTest(unittest.TestCase): """Regex intents are adapt-engine only; the surviving registration name is register_adapt_regex."""