feat!: OVOS-STOP-1 reserved-intent_name dispatch + separable legacy bridge - #802
Open
JarbasAl wants to merge 4 commits into
Open
feat!: OVOS-STOP-1 reserved-intent_name dispatch + separable legacy bridge#802JarbasAl wants to merge 4 commits into
JarbasAl wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughStopService now implements OVOS-STOP-1 global and targeted routing. Match-level activation suppression prevents stopped skills from activating. ChangesStop pipeline migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant StopService
participant IntentService
participant Session
participant _LegacyStopBridge
participant LegacySkill
StopService->>Session: create targeted or global stop match
StopService->>IntentService: dispatch match with suppress_activation
IntentService->>Session: skip activation and active-handler registration
StopService->>_LegacyStopBridge: publish STOP-1 intent match
_LegacyStopBridge->>LegacySkill: emit legacy stop event
LegacySkill-->>_LegacyStopBridge: emit HandlerLifecycle events
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
Tada! The results of the latest automation run are here. 🎉I've aggregated the results of the automated checks for this PR below. 📚 DocsProcessing complete! Details follow. 📬 ✅ All required documentation files present. ✅ 🔎 Type CheckChecking the alignment of your contribution. 📏 ❌ mypy: 276 error(s) found
Errors (showing first 10/276)🌍 Locale BuildChecking if there's anything else we need to do. 📋 ✅ Locale properly configured (64 files, 17 languages) Locale directories found:
Localization coverage:
pyproject.toml: ✅
Build manifest: ✅ 31 locale files included in package 🏷️ Release PreviewSetting the stage for the upcoming deployment. 🎭 Caution Breaking change — this PR will bump the MAJOR version ( Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
📋 Repo HealthA detailed health report for the project. 📝 ✅ All required files present. Latest Version: ✅ 📊 CoverageMeasuring the breadth of our automated checks. 📏 Files below 80% coverage (8 files)
Full report: download the 🔌 Plugin DetectionI've checked the plugin's 'error messages' for clarity. 🗣️ ❌ Plugin Status: ERRORS (1) Plugin Info:
OPM Detection:
Entry Point Validation:
⊘ No Issues:
🔨 Build TestsChecking the plumbing of your data flows. 🚰 ✅ All versions pass
⚖️ License CheckVerifying the SPDX identifiers for correctness. 🆔 ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔌 Skill Tests (ovoscope)I've checked the skill's 'response time' to rapid fire questions. ⏱️ ✅ 8/32 passed ❌ **TestAdaptIntent** — 0/4
❌ **TestCancelIntentMidSentence** — 0/1
❌ **TestConverse** — 0/1
❌ **TestDeactivate** — 2/3
❌ **TestFallback** — 0/1
❌ **TestIntentPipelineRouting** — 0/4
❌ **TestLangDisambiguation** — 0/4
❌ **TestLegacyIntentIdBackCompat** — 0/2
❌ **TestNoSkills** — 0/2
❌ **TestPadatiousIntent** — 0/4
🚌 Bus CoverageThe bus coverage audit is now available for inspection. 📋 🔴 Coverage Summary
📊 Per-Skill Breakdown
🔍 Detailed Message Type Breakdown
|
JarbasAl
marked this pull request as ready for review
July 16, 2026 07:21
Add test_stop_spec_e2e.py asserting the spec dispatch (<skill_id>:stop with
Match.skill_id==skill_id, <pipeline_id>:global_stop with skill_id==pipeline_id,
suppress_activation suppressing {skill_id}.activate, ovos.stop broadcast, and the
§5.2/§6 session drain) and test_stop_legacy_e2e.py asserting the pre-spec
stop:global/stop:skill dispatch re-emit onto mycroft.stop / <skill_id>.stop.
Supersede the prior test_stop.py / test_stop_refactor.py suites.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Return the spec dispatch shape from the stop pipeline: a targeted stop on
<skill_id>:stop with Match.skill_id==skill_id (§2/§3.1) and a global stop on
<pipeline_id>:global_stop with skill_id==pipeline_id (§5). Both set
IntentHandlerMatch.suppress_activation, and the orchestrator honours it by
registering no activation (no active_handlers push, no {skill_id}.activate) for
such a dispatch (§6.2/§7.3). The §5.2/§6 session drain (active_handlers,
converse_handlers, response_mode) is committed via Match.updated_session before
dispatch. handle_global_stop broadcasts ovos.stop (§5.3).
BREAKING CHANGE: the stop pipeline no longer dispatches stop:global/stop:skill
with skill_id=stop.openvoiceos; it dispatches the reserved intent_names
<skill_id>:stop and <pipeline_id>:global_stop. Requires ovos-plugin-manager
>=2.9.0a1 for IntentHandlerMatch.suppress_activation.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Compose a self-contained shim that observes the §9.2 ovos.intent.matched notification and re-emits the pre-spec stop:global/stop:skill dispatch, and owns the legacy stop:global/stop:skill handlers that fan out to mycroft.stop and <skill_id>.stop. Un-migrated skills still consuming <skill_id>.stop keep working when the ovos-spec-tools namespace translator is inactive. The unit lives in its own module and is wired via three lines in StopService, so it is removed in one move once every skill consumes <skill_id>:stop and ovos.stop directly. A one-time deprecation warning is logged while active. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JarbasAl
force-pushed
the
feat/stop-1-spec
branch
from
July 23, 2026 22:32
28d8037 to
2021d7e
Compare
JarbasAl
marked this pull request as draft
July 24, 2026 11:14
Resolve test_stop.py conflict by keeping the STOP-1 spec deletion (replaced by test_stop_spec_e2e.py + test_stop_legacy_e2e.py). Carry dev's INTENT-4 alias-collapse fix into test_stop_spec_e2e.py (:count_to_n, not the legacy .intent-suffixed id). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JarbasAl
marked this pull request as ready for review
August 1, 2026 18:44
github-actions
Bot
added
breaking
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
ovos_core/intent_services/stop_service.py (1)
258-263: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace both single-iteration loops with a first-item selection.
_collect_stop_skillsreturns a candidate list, and both call sites return inside the first loop iteration, so the loop hides the "take the first candidate" intent.
ovos_core/intent_services/stop_service.py#L258-L263: inmatch_high, assignskill_id = next(iter(self._collect_stop_skills(message)), None)and returnself._targeted_stop(...)when it is set.ovos_core/intent_services/stop_service.py#L337-L342: apply the same change inmatch_low, keeping the_global_stopescalation when no candidate exists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ovos_core/intent_services/stop_service.py` around lines 258 - 263, Replace the single-iteration skill loops in match_high and match_low with first-item selection using next(iter(self._collect_stop_skills(message)), None); call _targeted_stop when a skill_id is found, and preserve _global_stop escalation in match_low when no candidate exists. Apply this in ovos_core/intent_services/stop_service.py at lines 258-263 and 337-342.ovos_core/intent_services/stop_service_legacy.py (1)
56-67: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLog the deprecation warning only when the bridge re-emits.
Lines 61-67 set
_warnedand log before the branch test on lines 68-74. Any match from the stop pipeline whoseintent_nameis neither:global_stopnor:stoptriggers the deprecation warning without any legacy re-emission. Move the warning into the two branches, or gate it on the branch result.♻️ Proposed reordering
intent_name = message.data.get("intent_name") or "" - if not self._warned: - self._warned = True - LOG.warning(...) if intent_name.endswith(":global_stop"): + self._warn_once() self.bus.emit(self._forward_legacy(message, f"{self.LEGACY_SKILL_ID}.activate")) self.bus.emit(self._forward_legacy(message, "stop:global")) elif intent_name.endswith(":stop"): + self._warn_once() skill_id = message.data.get("skill_id")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ovos_core/intent_services/stop_service_legacy.py` around lines 56 - 67, The _on_intent_matched method currently logs the deprecation warning before confirming a legacy dispatch will be re-emitted. Move the _warned update and LOG.warning call into the :global_stop and :stop handling branches, or otherwise gate them on a successful re-emission, so unrelated intent names never trigger the warning.test/end2end/test_stop_spec_e2e.py (2)
174-179: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse iterable unpacking for the ignore list.
Ruff reports RUF005 on the
_IGNORE + [...]concatenation.♻️ Proposed fix
- ignore_messages=_IGNORE + [ - f"{self.skill_id}.stop.response", - f"{self.skill_id}.stop.ping", - "skill.stop.pong", - f"{self.skill_id}:count_to_n", - ], + ignore_messages=[ + *_IGNORE, + f"{self.skill_id}.stop.response", + f"{self.skill_id}.stop.ping", + "skill.stop.pong", + f"{self.skill_id}:count_to_n", + ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/end2end/test_stop_spec_e2e.py` around lines 174 - 179, Update the ignore_messages construction in the test to use iterable unpacking for _IGNORE and the additional message entries instead of list concatenation, while preserving the current ignored-message contents and order.Source: Linters/SAST tools
69-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShare the duplicated end-to-end stop helpers. Both new end-to-end modules copy the same
_wait_for_active_skillpolling helper and a largely identical_IGNOREtopic list. One shared test helper module keeps the two suites in sync when the ignored topic set changes.
test/end2end/test_stop_spec_e2e.py#L69-L76: import_wait_for_active_skilland the common ignore entries from a shared helper instead of defining them locally.test/end2end/test_stop_legacy_e2e.py#L53-L60: import the same shared helper and extend the ignore list with the legacy-only entries.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/end2end/test_stop_spec_e2e.py` around lines 69 - 76, The duplicated end-to-end stop helpers should be centralized. In test/end2end/test_stop_spec_e2e.py lines 69-76, remove the local _wait_for_active_skill and common _IGNORE entries and import them from a shared helper module; in test/end2end/test_stop_legacy_e2e.py lines 53-60, import the same shared helper and retain only the legacy-specific additions to _IGNORE.test/unittests/test_stop_service.py (1)
531-539: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the bridge listener registration and removal.
_make_bridgebypasses__init__, so no test asserts that_LegacyStopBridgeregistersovos.intent.matched,stop:global, andstop:skill, or thatshutdown()removes exactly those three. The bridge is the removable compatibility unit, so a test that pins its bus surface makes the future removal safe and verifiable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/unittests/test_stop_service.py` around lines 531 - 539, Add tests for _LegacyStopBridge listener lifecycle without using _make_bridge: instantiate the bridge normally and assert registration of exactly ovos.intent.matched, stop:global, and stop:skill, then call shutdown() and assert those same three listeners are removed from the bus.test/unittests/test_intent_service_extended.py (1)
691-698: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlso assert that no
{skill_id}.activateevent is emitted.The new gate in
service.pysuppresses two things: theactive_handlerspush and the{skill_id}.activatecallback. This test covers only the push. Add an assertion on the bus emissions so a regression that keeps emitting the activate event is caught.♻️ Suggested extra assertion
def test_suppress_activation_match_suppresses_push(self): # OVOS-STOP-1 §6.2/§7.3: a Match.suppress_activation dispatch (a stop) # must NOT push onto active_handlers regardless of its pipeline_id. - sess = self._dispatch("ovos-adapt-pipeline-plugin-high", - suppress_activation=True) + emitted = [] + with patch.object(FakeBus, "emit", lambda _self, m: emitted.append(m.msg_type)): + sess = self._dispatch("ovos-adapt-pipeline-plugin-high", + suppress_activation=True) ids = [h.get("skill_id") if isinstance(h, dict) else getattr(h, "skill_id", h) for h in sess.active_handlers] self.assertNotIn("test.skill", ids) + self.assertNotIn("test.skill.activate", emitted)Adapt the patch target to the bus object
_make_serviceuses.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/unittests/test_intent_service_extended.py` around lines 691 - 698, Extend test_suppress_activation_match_suppresses_push to inspect emissions from the bus object configured by _make_service, and assert that no test.skill.activate event is emitted when dispatching with suppress_activation=True. Keep the existing active_handlers assertion unchanged and target the actual bus instance used by the service.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ovos_core/intent_services/stop_service_legacy.py`:
- Around line 83-89: Move the skill_id lookup inside the HandlerLifecycle
context in handle_skill_stop, and handle a missing key through the lifecycle’s
existing error path so the handler emits its terminal error event instead of
raising before lifecycle setup. Preserve the current reply emission for messages
containing a valid skill_id.
In `@ovos_core/intent_services/stop_service.py`:
- Around line 185-205: The _targeted_stop method mutates the live Session and
registers an unbounded once listener before dispatch is confirmed. Build the
stopped state on a copied session or apply it only from the successful dispatch
path, and move confirmation-listener registration into the relevant dispatch
handler; ensure the listener is removed or expires using the same 0.5-second
timeout as _collect_stop_skills, including dispatch rejection or failure paths.
---
Nitpick comments:
In `@ovos_core/intent_services/stop_service_legacy.py`:
- Around line 56-67: The _on_intent_matched method currently logs the
deprecation warning before confirming a legacy dispatch will be re-emitted. Move
the _warned update and LOG.warning call into the :global_stop and :stop handling
branches, or otherwise gate them on a successful re-emission, so unrelated
intent names never trigger the warning.
In `@ovos_core/intent_services/stop_service.py`:
- Around line 258-263: Replace the single-iteration skill loops in match_high
and match_low with first-item selection using
next(iter(self._collect_stop_skills(message)), None); call _targeted_stop when a
skill_id is found, and preserve _global_stop escalation in match_low when no
candidate exists. Apply this in ovos_core/intent_services/stop_service.py at
lines 258-263 and 337-342.
In `@test/end2end/test_stop_spec_e2e.py`:
- Around line 174-179: Update the ignore_messages construction in the test to
use iterable unpacking for _IGNORE and the additional message entries instead of
list concatenation, while preserving the current ignored-message contents and
order.
- Around line 69-76: The duplicated end-to-end stop helpers should be
centralized. In test/end2end/test_stop_spec_e2e.py lines 69-76, remove the local
_wait_for_active_skill and common _IGNORE entries and import them from a shared
helper module; in test/end2end/test_stop_legacy_e2e.py lines 53-60, import the
same shared helper and retain only the legacy-specific additions to _IGNORE.
In `@test/unittests/test_intent_service_extended.py`:
- Around line 691-698: Extend test_suppress_activation_match_suppresses_push to
inspect emissions from the bus object configured by _make_service, and assert
that no test.skill.activate event is emitted when dispatching with
suppress_activation=True. Keep the existing active_handlers assertion unchanged
and target the actual bus instance used by the service.
In `@test/unittests/test_stop_service.py`:
- Around line 531-539: Add tests for _LegacyStopBridge listener lifecycle
without using _make_bridge: instantiate the bridge normally and assert
registration of exactly ovos.intent.matched, stop:global, and stop:skill, then
call shutdown() and assert those same three listeners are removed from the bus.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 86ad7934-05b2-43d9-8fee-f6bb702ec9fe
📒 Files selected for processing (9)
ovos_core/intent_services/service.pyovos_core/intent_services/stop_service.pyovos_core/intent_services/stop_service_legacy.pytest/end2end/test_stop.pytest/end2end/test_stop_legacy_e2e.pytest/end2end/test_stop_refactor.pytest/end2end/test_stop_spec_e2e.pytest/unittests/test_intent_service_extended.pytest/unittests/test_stop_service.py
💤 Files with no reviewable changes (2)
- test/end2end/test_stop_refactor.py
- test/end2end/test_stop.py
Comment on lines
+83
to
+89
| def handle_skill_stop(self, message: Message) -> None: | ||
| """Legacy ``stop:skill`` handler — re-emit the skill-directed ``<skill_id>.stop``.""" | ||
| skill_id = message.data["skill_id"] | ||
| with HandlerLifecycle(self.bus, message, | ||
| skill_id=self.LEGACY_SKILL_ID, | ||
| data={"name": "StopService.handle_skill_stop"}): | ||
| self.bus.emit(message.reply(f"{skill_id}.stop")) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard the missing skill_id and keep the failure inside HandlerLifecycle.
stop:skill is a public legacy topic that any component can emit. Line 85 indexes message.data["skill_id"] outside the HandlerLifecycle block. A message without that key raises KeyError before the lifecycle opens, so no mycroft.skill.handler.error terminal reaches the bus and the caller waits without a result.
🐛 Proposed fix
def handle_skill_stop(self, message: Message) -> None:
"""Legacy ``stop:skill`` handler — re-emit the skill-directed ``<skill_id>.stop``."""
- skill_id = message.data["skill_id"]
with HandlerLifecycle(self.bus, message,
skill_id=self.LEGACY_SKILL_ID,
data={"name": "StopService.handle_skill_stop"}):
+ skill_id = message.data.get("skill_id")
+ if not skill_id:
+ raise ValueError("'stop:skill' requires a 'skill_id' in message.data")
self.bus.emit(message.reply(f"{skill_id}.stop"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def handle_skill_stop(self, message: Message) -> None: | |
| """Legacy ``stop:skill`` handler — re-emit the skill-directed ``<skill_id>.stop``.""" | |
| skill_id = message.data["skill_id"] | |
| with HandlerLifecycle(self.bus, message, | |
| skill_id=self.LEGACY_SKILL_ID, | |
| data={"name": "StopService.handle_skill_stop"}): | |
| self.bus.emit(message.reply(f"{skill_id}.stop")) | |
| def handle_skill_stop(self, message: Message) -> None: | |
| """Legacy ``stop:skill`` handler — re-emit the skill-directed ``<skill_id>.stop``.""" | |
| with HandlerLifecycle(self.bus, message, | |
| skill_id=self.LEGACY_SKILL_ID, | |
| data={"name": "StopService.handle_skill_stop"}): | |
| skill_id = message.data.get("skill_id") | |
| if not skill_id: | |
| raise ValueError("'stop:skill' requires a 'skill_id' in message.data") | |
| self.bus.emit(message.reply(f"{skill_id}.stop")) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ovos_core/intent_services/stop_service_legacy.py` around lines 83 - 89, Move
the skill_id lookup inside the HandlerLifecycle context in handle_skill_stop,
and handle a missing key through the lifecycle’s existing error path so the
handler emits its terminal error event instead of raising before lifecycle
setup. Preserve the current reply emission for messages containing a valid
skill_id.
Comment on lines
+185
to
+205
| def _targeted_stop(self, skill_id: str, conf: float, utterance: str, | ||
| sess: Session) -> IntentHandlerMatch: | ||
| """Build the OVOS-STOP-1 §2 targeted ``<skill_id>:stop`` Match. | ||
|
|
||
| Drains the dispatch target from ``active_handlers`` and clears its | ||
| ``response_mode`` entry (§6.1/§6.2) via ``Match.updated_session``. The | ||
| §7.1 stamping push is suppressed (``suppress_activation``, §7.3), so the | ||
| removal is the final state. | ||
| """ | ||
| LOG.debug(f"Telling skill to stop: {skill_id}") | ||
| sess.disable_response_mode(skill_id) | ||
| sess.deactivate_skill(skill_id) | ||
| self.bus.once(f"{skill_id}.stop.response", self.handle_stop_confirmation) | ||
| return IntentHandlerMatch( | ||
| match_type=f"{skill_id}:stop", | ||
| match_data={"conf": conf, "skill_id": skill_id}, | ||
| updated_session=sess, | ||
| utterance=utterance, | ||
| skill_id=skill_id, | ||
| suppress_activation=self.suppress_activation, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
_targeted_stop mutates the live session and registers a once handler before dispatch is certain.
sess comes from SessionManager.get(message), so lines 195-196 drain the live session during the match phase. Line 197 registers a one-shot listener for <skill_id>.stop.response.
Two consequences:
- The orchestrator can discard the match after
match()returns.IntentService.handle_utteranceskips a match whosematch_typeis insession.blacklisted_intents, and it also skips on a_dispatch_matchexception. In those paths the skill is already deactivated andresponse_modeis already cleared, but no stop was dispatched. bus.onceremoves the handler only when the event arrives. A skill that never emits<skill_id>.stop.responseleaves the handler registered. Repeated stops then accumulate handlers on the bus for the process lifetime.
Consider building the drained state on a copy or applying it from a dispatch-time hook, and bounding the confirmation listener (register in the global/targeted dispatch handler, or remove it after the same 0.5 s window that _collect_stop_skills uses).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ovos_core/intent_services/stop_service.py` around lines 185 - 205, The
_targeted_stop method mutates the live Session and registers an unbounded once
listener before dispatch is confirmed. Build the stopped state on a copied
session or apply it only from the successful dispatch path, and move
confirmation-listener registration into the relevant dispatch handler; ensure
the listener is removed or expires using the same 0.5-second timeout as
_collect_stop_skills, including dispatch rejection or failure paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OVOS-STOP-1 conformance: reserved-intent_name dispatch + separable legacy bridge
Reworks the stop pipeline to implement OVOS-STOP-1 cleanly as a single spec
path, with backward compatibility isolated as one deletable unit.
The single spec design (
ovos_core/intent_services/stop_service.py)IntentHandlerMatch(match_type="<skill_id>:stop", skill_id=<skill_id>, suppress_activation=True, updated_session=<§6-drained>)(§2/§3.1). The skill is reached on<skill_id>:stop; the ovos-spec-toolsNamespaceTranslatorbridges it to the legacy<skill_id>.stopfor un-migrated skills.IntentHandlerMatch(match_type="<pipeline_id>:global_stop", skill_id=<pipeline_id>, suppress_activation=True, updated_session=<§5.2-drained>);pipeline_id="ovos-stop-pipeline-plugin"is shared across confidence tiers so exactly oneovos.stopbroadcast is emitted (§3.1).handle_global_stopemitsovos.stop(§5.3).IntentHandlerMatch.suppress_activationfield directly (no getattr): a suppress-activation dispatch registers noactive_handlerspush and no{skill_id}.activate(§6.2/§7.3). Requiresovos-plugin-manager>=2.9.0a1.Separable back-compat (
ovos_core/intent_services/stop_service_legacy.py)_LegacyStopBridgeobservesovos.intent.matched(§9.2) and re-emits thepre-spec
stop:global/stop:skilldispatch, and owns the legacy handlers thatfan out to
mycroft.stopand<skill_id>.stop. It holds no place in the specpath — the whole module plus three wiring lines in
StopServiceare removed inone move once skills consume
<skill_id>:stopandovos.stopdirectly (removalversion derived from
version.py). A one-time deprecation warning is logged.TDD (commit order)
test:— both e2e suites (test_stop_spec_e2e.py,test_stop_legacy_e2e.py).feat!:— spec core (breaking: reserved-intent_name dispatch).feat:— the droppable_LegacyStopBridge.Verified transitions: spec-core-only (no bridge) → spec 2 pass / legacy 2 fail; with the bridge → all 4 pass (xdist
-n4). Unit: 302 pass.Summary by CodeRabbit
New Features
Bug Fixes
Compatibility