fix: resolve skill_id from bus context in spec and legacy detach handlers

[JarbasAl]

🤖 Auto-generated by Claude Fable 5.1 (claude-fable-5-1) via Claude Code — NOT human-reviewed. Verify before acting.

handle_register_template, handle_register_entity, handle_deregister_intent, handle_deregister_entity, handle_deregister_skill and the legacy handle_detach_skill trusted message.data['skill_id'] unconditionally and ignored message.context['skill_id'], so a payload naming another skill acted under that skill's name. Guarding only the spec handler is not enough: the bus client twins ovos.skill.deregister into the legacy detach_skill topic while forwarding the producer's original context. On a FakeBus with ovos-bus-client 2.11.14a2, an ovos.skill.deregister with data.skill_id=victim and context.skill_id=attacker still removed the victim's intents through that twin. This is the same integrity hole OVOS-INTENT-4 §3.2 names, on legacy topics the spec does not cover.

A module-level _skill_id_from_context(message, handler) resolves the skill id: when the context carries a skill_id, that value is used regardless of the payload, and a mismatch is logged at WARNING naming both values and the topic but never acted on. When the context carries no skill_id at all, the payload value is trusted. That no-context branch is where padacioso deliberately diverges from the ovos_adapt and ovos_padatious twins, which return None there; a pre-spec producer keeps its real id instead of having its registration dropped. The helper is wired into the five spec handlers and into handle_detach_skill, matching how ovos_adapt and ovos_padatious route their legacy detach_skill. This branch is based on padacioso#102 and assumes it merges first: handle_detach_skill keeps #102's prefix match and only changes where skill_id comes from.

The legacy handle_detach_intent, twinned from ovos.intent.deregister with the producer's context, compares the <skill_id>: prefix of intent_name against context.skill_id and rejects a mismatch. The guard condition is the same one adapt#75 and padatious#152 use: a truthy context skill_id AND a namespaced intent_name. A bare unnamespaced name or an empty-string context skill_id falls through to the pre-spec detach, so the three engines resolve the same legacy message the same way.

Eleven tests in test/test_pipeline.py assert the roster by value. The ovos.skill.deregister case is emitted through the bus so it reaches the detach_skill twin, and asserts the victim's intents survive and still match. detach_intent is covered by forged prefix rejected, matching prefix detaches, no context keeps legacy behaviour, plus the bare-name and empty-context parity cases. With the tests kept and the source reverted to origin/dev, 4 of the 13 detach/deregister tests fail (the bus-emitted deregister fails with [] != ['victim.skill:play_music']); after the fix all 13 pass.

Full suite: 109 passed, 12 failed. origin/dev in the same venv is 96 passed, 12 failed with the identical failed set: every failure is register_padatious_intent() missing 1 required keyword-only argument: 'skill_id' from the ovoscope e2e files, the kwarg drift padacioso#97 addresses. This branch does not touch those files.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Ping! I've got your results right here. 🛎️

I've aggregated the results of the automated checks for this PR below.

⚖️ License Check

I've checked the license history of this repo. 📜

✅ No license violations found.

Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed.

🏷️ Release Preview

I've checked the release assets for completeness. 💾

Current: 2.3.4a1 → Next: 2.3.5a1

Signal	Value
Label	fix
PR title	fix: resolve skill_id from bus context in spec and legacy detach handlers
Bump	build

✅ PR title follows conventional commit format.

---

🚀 Release Channel Compatibility

Predicted next version: 2.3.5a1

Channel	Status	Note	Current Constraint
Stable	⚪	Not in channel	-
Testing	⚪	Not in channel	-
Alpha	⚪	Not in channel	-

🔌 Skill Tests (ovoscope)

Testing the skill's 'offline' capabilities (if any). 🔌

❌ 8/20 passed, 12 failed

❌ **TestDetach** — 0/2

Test	Result
test_detach_skill_removes_all_its_intents	❌ failed
test_detach_intent_prevents_match	❌ failed

test_detach_skill_removes_all_its_intents failure:

thon/3.11.16/x64/bin/python

self = <test_ovoscope_e2e.TestDetach testMethod=test_detach_skill_removes_all_its_intents>

    def test_detach_skill_removes_all_its_intents(self):
>       self._register_intent(f"{self.SKILL_ID}:hello", _HELLO_SAMPLES)

test/test_ovoscope_e2e.py:92: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_ovoscope_e2e.TestDetach testMethod=test_detach_skill_removes_all_its_intents>
name = 'test_skill_padacioso:hello'
samples = ['hello', 'hi', 'hey', 'greetings', 'good morning']

    def _register_intent(self, name, samples):
>       register_padatious_intent(self.bus, name, samples)
E       TypeError: register_padatious_intent() missing 1 required keyword-only argument: 'skill_id'

test/test_ovoscope_e2e.py:41: TypeError

test_detach_intent_prevents_match failure:

/opt/hostedtoolcache/Python/3.11.16/x64/bin/python

self = <test_ovoscope_e2e.TestDetach testMethod=test_detach_intent_prevents_match>

    def test_detach_intent_prevents_match(self):
>       self._register_intent(f"{self.SKILL_ID}:hello", _HELLO_SAMPLES)

test/test_ovoscope_e2e.py:84: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_ovoscope_e2e.TestDetach testMethod=test_detach_intent_prevents_match>
name = 'test_skill_padacioso:hello'
samples = ['hello', 'hi', 'hey', 'greetings', 'good morning']

    def _register_intent(self, name, samples):
>       register_padatious_intent(self.bus, name, samples)
E       TypeError: register_padatious_intent() missing 1 required keyword-only argument: 'skill_id'

test/test_ovoscope_e2e.py:41: TypeError

❌ **TestEntityExtraction** — 0/1

Test	Result
test_entity_slot_captured_in_match	❌ failed

test_entity_slot_captured_in_match failure:

hon 3.11.16 /opt/hostedtoolcache/Python/3.11.16/x64/bin/python

self = <test_ovoscope_e2e.TestEntityExtraction testMethod=test_entity_slot_captured_in_match>

    def test_entity_slot_captured_in_match(self):
>       self._register_entity("item", ["milk", "bread", "eggs", "cheese"])

test/test_ovoscope_e2e.py:72: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_ovoscope_e2e.TestEntityExtraction testMethod=test_entity_slot_captured_in_match>
name = 'item', samples = ['milk', 'bread', 'eggs', 'cheese']

    def _register_entity(self, name, samples):
>       register_padatious_entity(self.bus, name, samples)
E       TypeError: register_padatious_entity() missing 1 required keyword-only argument: 'skill_id'

test/test_ovoscope_e2e.py:44: TypeError

❌ **TestLegacyStillConsumed** — 0/1

Test	Result
test_legacy_template_registration_still_matches	❌ failed

test_legacy_template_registration_still_matches failure:

[gw3] linux -- Python 3.11.16 /opt/hostedtoolcache/Python/3.11.16/x64/bin/python

self = <test_intent4_consume_e2e.TestLegacyStillConsumed testMethod=test_legacy_template_registration_still_matches>

    def test_legacy_template_registration_still_matches(self):
        from ovoscope import register_padatious_intent
>       register_padatious_intent(self.bus, f"{self.SKILL_ID}:bye", _BYE)
E       TypeError: register_padatious_intent() missing 1 required keyword-only argument: 'skill_id'

test/end2end/test_intent4_consume_e2e.py:108: TypeError

✅ **TestNegativeKeywordTopic** — 1/1

❌ **TestRegisteredIntentMatch** — 1/4

Test	Result
test_best_intent_selected_among_multiple	❌ failed
test_no_match_unrelated_utterance	❌ failed
test_exact_utterance_dispatches_intent	❌ failed
test_no_match_when_no_intents_registered	✅ passed

test_best_intent_selected_among_multiple failure:

self = <test_ovoscope_e2e.TestRegisteredIntentMatch testMethod=test_best_intent_selected_among_multiple>

    def test_best_intent_selected_among_multiple(self):
>       self._register_intent(f"{self.SKILL_ID}:hello", _HELLO_SAMPLES)

test/test_ovoscope_e2e.py:63: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_ovoscope_e2e.TestRegisteredIntentMatch testMethod=test_best_intent_selected_among_multiple>
name = 'test_skill_padacioso:hello'
samples = ['hello', 'hi', 'hey', 'greetings', 'good morning']

    def _register_intent(self, name, samples):
>       register_padatious_intent(self.bus, name, samples)
E       TypeError: register_padatious_intent() missing 1 required keyword-only argument: 'skill_id'

test/test_ovoscope_e2e.py:41: TypeError

test_no_match_unrelated_utterance failure:

.11.16/x64/bin/python

self = <test_ovoscope_e2e.TestRegisteredIntentMatch testMethod=test_no_match_unrelated_utterance>

    def test_no_match_unrelated_utterance(self):
>       self._register_intent(f"{self.SKILL_ID}:hello", _HELLO_SAMPLES)

test/test_ovoscope_e2e.py:59: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_ovoscope_e2e.TestRegisteredIntentMatch testMethod=test_no_match_unrelated_utterance>
name = 'test_skill_padacioso:hello'
samples = ['hello', 'hi', 'hey', 'greetings', 'good morning']

    def _register_intent(self, name, samples):
>       register_padatious_intent(self.bus, name, samples)
E       TypeError: register_padatious_intent() missing 1 required keyword-only argument: 'skill_id'

test/test_ovoscope_e2e.py:41: TypeError

test_exact_utterance_dispatches_intent failure:

python

self = <test_ovoscope_e2e.TestRegisteredIntentMatch testMethod=test_exact_utterance_dispatches_intent>

    def test_exact_utterance_dispatches_intent(self):
>       self._register_intent(f"{self.SKILL_ID}:hello", _HELLO_SAMPLES)

test/test_ovoscope_e2e.py:49: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_ovoscope_e2e.TestRegisteredIntentMatch testMethod=test_exact_utterance_dispatches_intent>
name = 'test_skill_padacioso:hello'
samples = ['hello', 'hi', 'hey', 'greetings', 'good morning']

    def _register_intent(self, name, samples):
>       register_padatious_intent(self.bus, name, samples)
E       TypeError: register_padatious_intent() missing 1 required keyword-only argument: 'skill_id'

test/test_ovoscope_e2e.py:41: TypeError

❌ **TestSessionBlacklist** — 0/2

Test	Result
test_blacklisted_intent_is_skipped	❌ failed
test_blacklisted_skill_is_skipped	❌ failed

test_blacklisted_intent_is_skipped failure:

thon/3.11.16/x64/bin/python

self = <test_ovoscope_e2e.TestSessionBlacklist testMethod=test_blacklisted_intent_is_skipped>

    def test_blacklisted_intent_is_skipped(self):
>       self._register_intent(f"{self.SKILL_ID}:hello", _HELLO_SAMPLES)

test/test_ovoscope_e2e.py:109: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_ovoscope_e2e.TestSessionBlacklist testMethod=test_blacklisted_intent_is_skipped>
name = 'test_skill_padacioso:hello'
samples = ['hello', 'hi', 'hey', 'greetings', 'good morning']

    def _register_intent(self, name, samples):
>       register_padatious_intent(self.bus, name, samples)
E       TypeError: register_padatious_intent() missing 1 required keyword-only argument: 'skill_id'

test/test_ovoscope_e2e.py:41: TypeError

test_blacklisted_skill_is_skipped failure:

/Python/3.11.16/x64/bin/python

self = <test_ovoscope_e2e.TestSessionBlacklist testMethod=test_blacklisted_skill_is_skipped>

    def test_blacklisted_skill_is_skipped(self):
>       self._register_intent(f"{self.SKILL_ID}:hello", _HELLO_SAMPLES)

test/test_ovoscope_e2e.py:117: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_ovoscope_e2e.TestSessionBlacklist testMethod=test_blacklisted_skill_is_skipped>
name = 'test_skill_padacioso:hello'
samples = ['hello', 'hi', 'hey', 'greetings', 'good morning']

    def _register_intent(self, name, samples):
>       register_padatious_intent(self.bus, name, samples)
E       TypeError: register_padatious_intent() missing 1 required keyword-only argument: 'skill_id'

test/test_ovoscope_e2e.py:41: TypeError

❌ **TestSessionBlacklistAlias** — 0/3

Test	Result
test_blacklisting_legacy_id_suppresses_new_alias	❌ failed
test_blacklisting_new_id_suppresses_legacy_alias	❌ failed
test_non_blacklisted_intent_still_matches	❌ failed

test_blacklisting_legacy_id_suppresses_new_alias failure:

gister_both_aliases()

test/test_ovoscope_e2e.py:146: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
test/test_ovoscope_e2e.py:142: in _register_both_aliases
    self._register_intent(self.LEGACY_NAME, _HELLO_SAMPLES)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_ovoscope_e2e.TestSessionBlacklistAlias testMethod=test_blacklisting_legacy_id_suppresses_new_alias>
name = 'test_skill_padacioso:hello.intent'
samples = ['hello', 'hi', 'hey', 'greetings', 'good morning']

    def _register_intent(self, name, samples):
>       register_padatious_intent(self.bus, name, samples)
E       TypeError: register_padatious_intent() missing 1 required keyword-only argument: 'skill_id'

test/test_ovoscope_e2e.py:41: TypeError

test_blacklisting_new_id_suppresses_legacy_alias failure:

gister_both_aliases()

test/test_ovoscope_e2e.py:154: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
test/test_ovoscope_e2e.py:142: in _register_both_aliases
    self._register_intent(self.LEGACY_NAME, _HELLO_SAMPLES)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_ovoscope_e2e.TestSessionBlacklistAlias testMethod=test_blacklisting_new_id_suppresses_legacy_alias>
name = 'test_skill_padacioso:hello.intent'
samples = ['hello', 'hi', 'hey', 'greetings', 'good morning']

    def _register_intent(self, name, samples):
>       register_padatious_intent(self.bus, name, samples)
E       TypeError: register_padatious_intent() missing 1 required keyword-only argument: 'skill_id'

test/test_ovoscope_e2e.py:41: TypeError

test_non_blacklisted_intent_still_matches failure:

elf._register_both_aliases()

test/test_ovoscope_e2e.py:162: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
test/test_ovoscope_e2e.py:142: in _register_both_aliases
    self._register_intent(self.LEGACY_NAME, _HELLO_SAMPLES)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_ovoscope_e2e.TestSessionBlacklistAlias testMethod=test_non_blacklisted_intent_still_matches>
name = 'test_skill_padacioso:hello.intent'
samples = ['hello', 'hi', 'hey', 'greetings', 'good morning']

    def _register_intent(self, name, samples):
>       register_padatious_intent(self.bus, name, samples)
E       TypeError: register_padatious_intent() missing 1 required keyword-only argument: 'skill_id'

test/test_ovoscope_e2e.py:41: TypeError

✅ **TestSpecDeregister** — 2/2 ✅ **TestSpecDisableEnable** — 2/2 ✅ **TestSpecTemplateConsumed** — 2/2

🚌 Bus Coverage

Measuring the reach of our bus handlers. 📏

⚠️ Bus coverage report unavailable — check the job log.

📊 Coverage

Quantifying the quality of our test suite. 🧪

✅ 82.4% total coverage

Per-file coverage (3 files)

File	Coverage	Missing lines
padacioso/version.py	0.0%	5
padacioso/__init__.py	82.7%	61
padacioso/opm.py	83.1%	72

Full report: download the coverage-report artifact.

📋 Repo Health

Checking the repo's cholesterol levels (aka code bloat). 🥩

✅ All required files present.

Latest Version: 2.3.4a1

✅ padacioso/version.py — Version file
✅ README.md — README
✅ LICENSE.md — License file (consider renaming to LICENSE)
✅ pyproject.toml — pyproject.toml
⚠️ setup.py — setup.py
✅ CHANGELOG.md — Changelog
✅ padacioso/version.py has valid version block markers

🔒 Security (pip-audit)

Scanning for any potential denial-of-service vectors. 🚫

✅ No known vulnerabilities found (49 packages scanned).

🔍 Lint

I've finished the heavy lifting on this check. 🏋️‍♂️

❌ ruff: issues found — see job log

🔌 Plugin Detection

I've verified the plugin's 'hot-reload' performance. 🔥

❌ Plugin Status: ERRORS (1)

Plugin Info:

• Name: padacioso
• Description: dead simple intent parser

OPM Detection:

Plugin Type	Wheel	Editable
pipeline	✅	✅

Entry Point Validation:

Entry Point	Type	Import	Interface
ovos-padacioso-pipeline-plugin	pipeline	✅ 739ms	⊘

⊘ No settingsmeta.json
✅ requires-python >=3.8 — running Python 3.11

Issues:

• ❌ Import time for ovos-padacioso-pipeline-plugin exceeds 500ms (739ms)
• ⚠️ No settingsmeta.json found
• ⚠️ No settingsmeta.json found

🔨 Build Tests

Construction of your features is officially finished. 🏠

✅ All versions pass

Python	Build	Install	Tests
3.10	✅	✅	✅
3.11	✅	✅	✅
3.12	✅	✅	✅
3.13	✅	✅	✅
3.14	✅	✅	✅

---

Processing... Done! Have a productive day! ☕