Release 3.0.7a3 - #885
Open
github-actions[bot] wants to merge 173 commits into
Open
Conversation
Updates the requirements on [ovos-workshop](https://github.com/OpenVoiceOS/OVOS-workshop) to permit the latest version. - [Release notes](https://github.com/OpenVoiceOS/OVOS-workshop/releases) - [Changelog](https://github.com/OpenVoiceOS/ovos-workshop/blob/dev/CHANGELOG.md) - [Commits](OpenVoiceOS/ovos-workshop@7.0.6...8.0.0) --- updated-dependencies: - dependency-name: ovos-workshop dependency-version: 8.0.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Adina Vladu <adina.vladu@usc.es>
* Refine French stop intents * Refine French stop voice intents
* Prevent duplicate skill loads during rescans * chore: modernize GitHub workflows to use shared gh-automations reusables - coverage.yml: replace py-cov-action custom workflow with coverage.yml@dev reusable (system deps, extras, deploy_pages: true) - gh_pages_coverage.yml: deleted — superseded by deploy_pages: true - pipaudit.yml: replace custom multi-version inline job with pip-audit.yml@dev reusable (preserves ignore list) - release_workflow.yml: fix broken YAML (misplaced publish_pypi/ notify_matrix keys); move them into publish_alpha with: block - build_tests.yml: replace inline matrix job with build-tests.yml@dev reusable; add system_deps and install_extras All workflows now reference @dev consistently. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Defer connectivity-triggered skill loads until intents are ready * Guard deferred startup loads with a lock * Make runtime requirements gating optional behind config flag - Add skills.use_deferred_loading config flag (default: false) - When disabled (default): all skills load unconditionally at startup - When enabled: skills with network/internet requirements defer until those conditions are met - Wrap connectivity event handler registration with flag check - Branch run() method based on flag setting - Preserves PR #749's deferred load bug fixes when flag is enabled This builds on PR #749's improvements to deferred loading (thread safety, prevents duplicate loads) while making the feature opt-in so the simpler unconditional loading is the default behavior. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Add documentation for optional deferred loading config - FAQ.md: document default unconditional loading and optional deferred loading - SUGGESTIONS.md: mark S-001 as PARTIALLY ADDRESSED, explain opt-in config flag - MAINTENANCE_REPORT.md: document change and integration with PR #749 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Add test coverage for deferred loading config flag Add TestDeferredLoadingConfigFlag test class with tests for: - Config flag defaults to false (deferred loading disabled) - Config flag can be enabled via use_deferred_loading config - Connectivity handlers NOT registered when deferred loading disabled - Connectivity handlers ARE registered when deferred loading enabled - load_plugin_skills does NOT gate on network/internet when disabled - load_plugin_skills DOES gate on network/internet when enabled - run() calls _load_new_skills directly when deferred loading disabled - run() uses deferred loading flow when flag is enabled Ensures both code paths (flag enabled and disabled) are properly tested. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Add required project configuration and documentation files - pyproject.toml: Python project configuration (required for build) - AUDIT.md: Known issues and technical debt documentation - QUICK_FACTS.md: Machine-readable project reference - docs/: Architecture and feature documentation - .env: Environment configuration These files were merged but not committed. Required for CI builds to succeed. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * update * Address CodeRabbit PR review comments for PR #750 ## Changes 1. **Fix _load_new_skills to bypass gating when deferred_loading disabled** (Major) - When deferred_loading is disabled, pass network=True, internet=True to load_plugin_skills() - This ensures skills with runtime requirements still load unconditionally - Reconciles test expectation with implementation 2. **Replace sequential atomicity test with concurrent thread test** (Major) - test_mark_startup_complete_and_consume_deferred_is_atomic now uses 2 threads - Verifies exactly one thread sees True (winner of race) - Removed redundant test_mark_startup_complete_concurrent_calls_race_safe 3. **Remove unused skill_manager variable bindings** (Minor) - test_connectivity_handlers_not_registered_when_deferred_loading_disabled: Line 472 - test_connectivity_handlers_registered_when_deferred_loading_enabled: Line 497 - Both tests only need side effects of instantiation, not the object itself 4. **Add assert_not_called for opposite branches** (Minor) - test_run_calls_load_new_skills_when_deferred_loading_disabled: Assert deferred loading methods NOT called - test_run_uses_deferred_loading_when_enabled: Assert _load_new_skills NOT called in startup phase - Ensures config flag acts as mutually exclusive switch 5. **Test improvement for load_plugin_skills_no_gating** - Changed to call _load_new_skills() instead of load_plugin_skills() directly - Properly tests end-to-end behavior of unconditional loading when disabled ## Notes - test_instantiate was already fixed in prior work (no connectivity handlers by default) - CI build failure (Install step) is pre-existing issue with pyproject.toml requires-python constraint Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Fix CI build failure by updating requires-python to >=3.10 The ovoscope dependency requires Python >=3.10, but pyproject.toml specified >=3.9. This caused dependency resolution to fail in CI install step for Python 3.9. Updated to match the minimum version required by test dependencies. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Fix GitHub Actions build_tests.yml install_extras syntax The install_extras parameter was incorrectly specified with brackets: install_extras: '[mycroft,plugins,skills-essential,lgpl,test]' This caused pip install to receive double brackets: pip install "wheel[[extras]]" Changed to use correct syntax (gh-automations adds the brackets): install_extras: 'mycroft,plugins,skills-essential,lgpl,test' This fixes the install step failure in the build tests workflow. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Fix test config patching by using context managers The @patch.dict decorator applied to test methods may not correctly apply during setUp() execution. Changed to use with patch.dict() context managers inside each test method to ensure the patch is active when SkillManager is created. This fixes: - test_deferred_loading_enabled_via_config - test_connectivity_handlers_registered_when_deferred_loading_enabled - test_load_plugin_skills_gating_when_deferred_loading_enabled - test_run_uses_deferred_loading_when_enabled Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Continue test fixes for config patching issues - Convert test_deferred_loading_disabled_by_default to use context manager - Convert test_connectivity_handlers_not_registered_when_deferred_loading_disabled to use context manager - Convert test_load_plugin_skills_no_gating_when_deferred_loading_disabled to use context manager - Convert test_run_calls_load_new_skills_when_deferred_loading_disabled to use context manager - Fix test_load_plugin_skills_no_gating_when_deferred_loading_disabled to call load_plugin_skills directly with network=True, internet=True instead of calling _load_new_skills This ensures all tests have config patches properly applied during SkillManager instantiation. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Explicitly set use_deferred_loading to False in all disabled-path tests The patch.dict may not properly set None values. Explicitly set use_deferred_loading to False in mock_config() calls for all tests that expect the disabled-by-default behavior. This ensures the config patch is correctly applied and the flag is definitively set to False. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Fix missing initialization and logic in skill loading - Add missing `_plugin_skills_lock` and `_loading_plugin_skills` attributes to __init__ - Restore reserve/release logic in `_load_plugin_skill` to prevent duplicate loads - Fix `load_plugin_skills` to properly track loading state using `_is_plugin_skill_tracked` - Add thread-safe locking when storing loaded skills - Update test assertion to match the `reserved=True` parameter now passed This fixes CI test failures where `_loading_plugin_skills` was accessed but not initialized, and ensures concurrent skill loads are properly serialized. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * Fix test_instantiate flakiness due to config isolation Ensure test_instantiate explicitly sets use_deferred_loading=False with proper config isolation to prevent state leakage from other test classes when pytest randomizes test order. This fixes intermittent test failures where connectivity handlers were incorrectly registered due to config pollution from TestDeferredLoadingConfigFlag tests. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Gaëtan Trellu <gaetan.trellu@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add required project configuration and documentation files
- pyproject.toml: Python project configuration (required for build)
- AUDIT.md: Known issues and technical debt documentation
- QUICK_FACTS.md: Machine-readable project reference
- docs/: Architecture and feature documentation
- .env: Environment configuration
These files were merged but not committed. Required for CI builds to succeed.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* update
* Performance optimizations: fix race conditions, reduce per-utterance overhead
**Priority 1 — Race Conditions (correctness + perf)**
- Add lock to _unload_plugin_skill to prevent concurrent dict mutation (skill_manager.py:585)
- Snapshot plugin_skills dict inside lock for safe iteration in 4 methods:
send_skill_list, deactivate_skill, activate_skill, deactivate_except
Prevents RuntimeError: dictionary changed size during iteration
- Replace busy-wait in _collect_fallback_skills with threading.Event signaling
(fallback_service.py:122-125) — reduces CPU usage on utterances reaching fallback
**Priority 2 — Per-Utterance Work (latency)**
- Replace threading.Event().wait(1) with self._stop_event.wait(1)
(skill_manager.py:462) — reuses event, correctly respects stop signal
- Move migration_map dict and re.compile regex to module-level constants
(service.py) — 15 utterances/second → rebuilding these on every pipeline stage
- Guard create_daemon calls with config check before spawning thread
(service.py:322, 352) — skip thread creation when open_data.intent_urls not configured
**Priority 3 — Minor Overhead**
- Change _logged_skill_warnings from list to set (O(1) lookup vs O(n))
(skill_manager.py:111)
- Cache sorted plugins in all 3 transformer services (transformers.py)
Invalidate cache on load_plugins()
- Read blacklist once before plugin scan loop instead of per-skill
(skill_manager.py:361)
All 65 unit tests pass. Coverage maintained at 60% for ovos_core.skill_manager.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* Update docs: performance optimizations, race condition fixes, audit report
- FAQ.md: Add comprehensive Performance section documenting all optimizations (thread-safe loading, event signaling, caching, etc.)
- MAINTENANCE_REPORT.md: Add detailed entry for 2026-03-11 performance optimization work
- AUDIT.md: Document fixed race conditions (plugin_skills dict, busy-wait, temporary events)
- SUGGESTIONS.md: Add S-007 marking all performance improvements as ADDRESSED
All changes cross-referenced with code locations and commit SHA.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* Fix documented TODOs: skill uninstall + minor clarifications
**S-002: Implement skill uninstall (VALID FIX)**
- Implement handle_uninstall_skill() to call pip_uninstall() for skill packages
- Replace 'not implemented' error with actual uninstall logic
- Convert skill_id to package name (dots → hyphens)
- Validate skill parameter before attempting uninstall
**Minor TODO clarifications**
- Docker detection warning in launch_standalone() for container environments
- Clarified voc_match() TODO: explain why StopService reimplements instead of using ovos_workshop
(StopService is not a skill; voc_match is service-specific)
**Test updates**
- Updated test_handle_uninstall_skill to expect 'no packages to install' instead of 'not implemented'
**S-006 (DEFERRED)**
- Reverted external skills registry implementation
- These TODOs are architectural limitations, not missing features
- External skills run in separate processes and only communicate via messagebus
- They cannot be listed, activated, or deactivated by ovos-core
All 65 unit tests pass.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* Update docs: clarify S-002 implementation and S-006 architectural limitation
- SUGGESTIONS.md: Mark S-002 as ADDRESSED with implementation details
- SUGGESTIONS.md: Document S-006 as architectural limitation (external skills run in separate processes)
- SUGGESTIONS.md: Explain correct pattern for external skills (self-advertise + respond to bus messages)
- MAINTENANCE_REPORT.md: Add entry for S-002 implementation with rationale on S-006
- All references include commit SHAs and line numbers
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* update
* fix: bus listener leaks, None log crash, and intent service startup timeout
- converse_service: wrap bus.on/event.wait/bus.remove in try/finally so
the skill.converse.pong listener is always removed even if handle_ack
raises; add skill_id guard against malformed pong; change can_handle
default True→False (non-responding skill should not converse)
- stop_service: same try/finally + skill_id guard + can_handle default
True→False for skill.stop.pong listener
- service.py: fix LOG.info string concat crash when cancel_word is None
(use f-string instead of + operator)
- skill_manager: add configurable max_wait to wait_for_intent_service
(default 300 s via skills.intent_service_timeout); raises descriptive
RuntimeError with instructions instead of looping forever
Note: sound config caching was not applied — Configuration() is a live
object in OVOS that reflects runtime changes without restart.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor: StopService inherits OVOSAbstractApplication for voc_match
StopService now follows the same pattern as CommonQAService and
OCPPipelineMatcher: double-inheriting ConfidenceMatcherPipeline and
OVOSAbstractApplication so that vocabulary loading and voc_match/voc_list
are provided by the shared ovos-workshop infrastructure instead of a
hand-rolled reimplementation.
Changes:
- Add OVOSAbstractApplication to base classes; call both __init__s with
skill_id="stop.openvoiceos" and resources_dir=dirname(__file__)
- Remove load_resource_files(), _voc_cache dict, _get_closest_lang(),
and the custom voc_match() override (~60 lines deleted)
- Replace self._voc_cache[lang]['stop'] in match_low with self.voc_list()
- Replace _get_closest_lang() guards in match_* with voc_list() emptiness
check (voc_list returns [] for unknown langs — no crash, no None sentinel)
- Rename all locale/*.intent files to *.voc so OVOSSkill resource loading
finds them via the standard ResourceType("vocab", ".voc", ...) path
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: add unit tests for StopService — 96% coverage
27 tests covering:
- _collect_stop_skills: no active skills, can_handle True/False, timeout
cleanup (try/finally), exception cleanup, malformed pong guard (no
skill_id), blacklisted skills excluded from ping
- handle_stop_confirmation: error branch, response-mode abort_question,
converse force_timeout, TTS stop when speaking, skill_id fallback from
msg_type
- match_high: no vocab → None, stop+no active skills → global stop,
stop+active skills → skill stop, global_stop voc → global stop
- match_medium: no voc → None, stop/global_stop voc delegates to match_low
- match_low: empty voc_list → None, below threshold → None, active skill
confidence boost, above threshold → skill stop
- handle_global_stop / handle_skill_stop bus message forwarding
- get_active_skills session delegation
- shutdown removes both bus listeners
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(e2e): add end2end tests for StopService OVOSAbstractApplication refactor
Fix existing stop e2e tests:
- Add 'stop.openvoiceos.stop.response' to all ignore_messages lists in
test_stop.py — StopService now subclasses OVOSAbstractApplication so it
responds to the mycroft.stop broadcast like other pipeline-plugin skills
(common_query, ocp, persona already filtered there)
New test file test_stop_refactor.py — 5 tests across 4 classes:
TestGlobalStopVocabulary (no skills loaded):
- test_global_stop_voc_no_active_skills: 'stop everything' matches
global_stop.voc and emits stop:global (regression: .voc rename works)
- test_stop_voc_exact_still_works: bare 'stop' still matches stop.voc
(regression: .voc rename did not break the stop vocabulary)
TestGlobalStopVocWithActiveSkill (count skill loaded):
- test_global_stop_voc_with_active_skill: 'stop everything now' emits
stop:global even when a skill is in the active list — verifying that
global_stop.voc takes priority over the stop:skill path
TestStopSkillCanHandleFalse (count skill loaded):
- test_stop_with_active_skill_ping_pong: full stop ping-pong sequence
with a running skill — verifies stop.ping → skill.stop.pong(can_handle=True)
→ stop:skill → {skill}.stop → {skill}.stop.response chain
TestStopServiceAsSkill (no skills loaded):
- test_stop_service_emits_activate_and_stop_response: explicitly asserts
that stop.openvoiceos.activate and stop.openvoiceos.stop.response appear
in the message sequence, confirming StopService participates in the
OVOSSkill stop lifecycle
Also installed missing test dependencies:
ovos-skill-count, ovos-skill-parrot, ovos-skill-hello-world,
ovos-skill-fallback-unknown, ovos-padatious-pipeline-plugin (from local
workspace), ovos-adapt-pipeline-plugin (from local workspace),
ovos-utterance-plugin-cancel (from local workspace)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(S-003): strengthen validate_skill with GitHub API validation
- Parse owner/repo from URL; call api.github.com/repos/{owner}/{repo}/contents/
- Reject repos that don't exist (HTTP 404)
- Reject bare setup.py-only repos (legacy Mycroft packaging)
- Fetch pyproject.toml/setup.cfg and reject if MycroftSkill or CommonPlaySkill found
- Fail-open on network errors and unexpected API status codes (3 s timeout)
- Fix 3 existing tests that assumed no network call (now mock requests.get/validate_skill)
- Add 10 new unit tests covering all validation branches
- Add test_converse_service.py (43 tests, 81% coverage)
- Update FAQ.md and MAINTENANCE_REPORT.md
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: add unit tests for fallback_service, transformers, and intent service
- test_fallback_service.py: 34 tests, 93% coverage
- handle_register/deregister_fallback, _fallback_allowed (ACCEPT_ALL/BLACKLIST/WHITELIST),
_collect_fallback_skills (ping-pong, timeouts, blacklisted sessions),
_fallback_range, match_high/medium/low delegation, shutdown
- test_transformers.py: 40 tests, 66% coverage
- All three transformer services (Utterance/Metadata/Intent)
- Plugin loading, priority ordering + caching, transform chaining,
exception swallowing, context merging, session key stripping
- test_intent_service_extended.py: 37 tests, raises service.py from 0% to 49%
- _handle_transformers, disambiguate_lang, get_pipeline_matcher (migration map),
get_pipeline, context handlers, send_cancel_event/send_complete_intent_failure,
_emit_match_message, handle_utterance (cancel/no-match), handle_get_intent, shutdown
Total: 247 unit tests passing
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(e2e): add intent pipeline routing end-to-end tests
4 tests covering basic pipeline routing with ovos-skill-count:
- Padatious intent matched end-to-end (full handler lifecycle)
- High-priority pipeline stage handles before lower-priority stages
- Unrecognized utterance produces complete_intent_failure + error sound
- Blacklisted skill falls through to complete_intent_failure
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(locale): sync .voc files from translations/ (source of truth)
Regenerate all stop.voc and global_stop.voc files directly from
translations/{lang}/intents.json to ensure they stay in sync.
Changes:
- Preserve phrase order from translations (previously sorted alphabetically)
- Add missing phrases that existed in translations but not locale
- Remove phrases in locale that were not in translations
- Normalize fa-IR -> fa-ir (locale dir is always lowercase)
- nl-NL and nl-nl both exist in translations; nl-nl (canonical) wins
- nl-be has no global_stop translation — global_stop.voc intentionally absent
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix pyproject.toml
* chore: Add ovoscope end-to-end tests with bus coverage report to CI
- Created .github/workflows/ovoscope.yml using gh-automations@dev reusable workflow
- Enables bus coverage tracking for behavioural test metrics
- Posts 🔌 Skill Tests (ovoscope) and 🚌 Bus Coverage sections to PR comments
- Requires Adapt and Padatious pipelines for comprehensive intent testing
- Updated FAQ.md with CI/Testing section explaining bus coverage
- Updated QUICK_FACTS.md with testing workflow reference
- Updated MAINTENANCE_REPORT.md with session log
Bus coverage complements code coverage by showing which bus message types
are exercised during tests, helping identify gaps in skill interaction testing.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix pyproject.toml
* fix
* Delete .coverage
* .
* .
* .
* fix: thread names in bus coverage report
* more workflows
* coderabbit
---------
Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Automated rename submitted by @JarbasAl via OVOS Localize. Co-authored-by: ovos-localize[bot] <ovos-localize[bot]@users.noreply.github.com>
* refactor: migrate to ovos-spec-tools for language matching Replace langcodes.closest_match with ovos_spec_tools.closest_lang and ovos_utils.lang.standardize_lang_tag with ovos_spec_tools.standardize_lang across the intent services. closest_lang already applies the distance<10 threshold and returns None when no candidate is close enough, so the call site in disambiguate_lang is adapted accordingly. Drops the direct langcodes dependency in favour of ovos-spec-tools, the conformant reference implementation of the OVOS specs. Part of Wave 2 of the OVOS migration (OpenVoiceOS/architecture#7). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: depend on ovos-spec-tools[langcodes] `langcodes` is optional for ovos-spec-tools itself, but the OVOS ecosystem (e.g. ovos_utils.lang.standardize_lang_tag, used by get_message_lang) silently degrades without it — it strips the region from a tag. Dropping the direct langcodes dependency left it absent in CI, so language tags came back bare (`de` instead of `de-DE`). Pull it back in via the ovos-spec-tools langcodes extra. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: normalize BCP-47 region in get_message_lang; exclude phoonnx from license check Add get_message_lang() wrapper in ovos_core.intent_services.service that reads the raw lang from message data/context and normalizes it through ovos_spec_tools.standardize_lang, preserving the region subtag (de-de -> de-DE). The ovos_bus_client implementation uses the deprecated standardize_lang_tag with macro=True which strips the region. Update test_intent_service.py to import get_message_lang from ovos_core.intent_services.service so the test exercises the corrected normalization path. Exclude phoonnx from license_tests; the package has no detectable license on PyPI (transitive dep, pre-existing). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: restore files accidentally deleted during rebase Restores everything outside the spec-tools migration scope: - ovos_core/intent_services/locale/ (62 .voc/.intent files across 19 langs) - scripts/prepare_translations.py, scripts/sync_translations.py - the translations job in .github/workflows/release_workflow.yml Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: drop gitlocalize translation scripts (deprecated service) The previous restore commit brought these back along with the locale folder, but the gitlocalize bot is deprecated; the scripts and the release_workflow translations job stay removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: bump ovos_bus_client to >=2.1.0a1 and drop duplicate get_message_lang The prior commit on this branch rewrote get_message_lang locally in ovos_core to work around bus-client 1.x using the region-stripping `standardize_lang_tag` from ovos_utils.lang. bus-client 2.1.0a1 (the spec-tools migration release) already uses `ovos_spec_tools.standardize_lang` internally and preserves the region subtag. Bump the pin and re-import the canonical `get_message_lang`. Also tighten the spec-tools pin to >=0.5.1a1 (the empty-msg_type accept). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: revert bus-client bump + restore local get_message_lang workaround Bumping ovos_bus_client to >=2.1.0a1 broke install because every other core repo (ovos-audio, ovos-dinkum-listener, ovos-PHAL, …) still pins `ovos_bus_client<2.0.0`. Resolving that cascade is its own Wave, not this PR. Restore the <2.0.0 floor and the local get_message_lang workaround that wraps the call in spec-tools' `standardize_lang` to preserve the region subtag (bus-client 1.x uses the region-stripping `standardize_lang_tag` from ovos_utils.lang). CodeRabbit feedback addressed: - pyproject + requirements: add `<1.0.0` upper bound on the ovos-spec-tools pin to match the project's other entries. - service.py disambiguate_lang: rename loop variable `l` → `lang` in the comprehension (Ruff E741). Skipped (with reason): CodeRabbit suggested disambiguate_lang return `best_lang` (the resolved match) instead of `v` (the input tag). The original code on dev returned the input `v` too; the `closest_lang` check is a gate only. Changing the return value is a behaviour change beyond the spec-tools migration scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: gate the local get_message_lang on ovos_bus_client.version Read `VERSION_MAJOR` / `VERSION_MINOR` from `ovos_bus_client.version` at import time. When bus-client is >=2.1, import the canonical `get_message_lang` directly (it already routes through spec-tools' region-preserving `standardize_lang`). When bus-client is older, fall back to the local patch. The workaround branch self-removes the moment ovos-audio + ovos-dinkum-listener + ovos-PHAL release versions that allow `ovos_bus_client>=2.1.0a1` — no further code change needed in ovos-core. Verified both branches: 4 tests pass against the workspace bus-client (2.1.1a1) AND against the PyPI-pinned 1.x install. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: bump ovos-utils + cancel-plugin floors past the alpha fixes CI was pulling `ovos-utils-0.8.5` (stable) and `ovos-utterance-plugin-cancel-0.2.8` (stable) because the floors `>=0.8.2a1` and `>=0.2.3` were satisfied by those older stables, leaving the fixes from the new alpha releases unreachable: - ovos-utils 0.11.1a1 — `standardize_lang_tag(macro=True)` no longer strips the region (`en-US` round-trips). Without this fix, `SessionManager.session.lang` was being normalized to the bare language and the cancel-plugin's region-sensitive locale matching missed. - ovos-utterance-plugin-cancel 0.3.0a1 — entry point now under `opm.transformer.text` (OPM scans), reads lang from the session carrier with a top-level fallback, ships per-locale `cancel.blacklist` veto for issue #7. Without this release the plugin doesn't even load in CI. Bumping both floors restores the ovoscope `test_cancel_match` case (was emitting `snd/error.mp3` instead of `snd/cancel.mp3` because the cancel transformer wasn't firing). Also drops the local `get_message_lang` workaround in service.py: with ovos-utils 0.11.1a1 in the chain, even `ovos_bus_client<2.0` returns region-preserved tags from its own `get_message_lang`. The bus-client-version gate becomes unnecessary; re-import the canonical helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): patch utterance_transformers config to enable cancel plugin by its new entry-point name ovos-utterance-plugin-cancel 0.3.0a1 registers the transformer under the new entry-point name `ovos-utterance-cancel-plugin` (#32 in that repo). The OVOS default `mycroft.conf` still references the historic `ovos-utterance-plugin-cancel` key under `utterance_transformers`, and `UtteranceTransformersService.load_plugins` silently skips any installed plugin whose entry-point name is not a key in that config mapping (`ovos_core/transformers.py`). Patch the config explicitly in the test: - write a temp xdg-conf with the new key enabled, - prepend it to `Configuration.xdg_configs`, - boot MiniCroft with `isolate_config=False` so the boot-time `Configuration.reload()` does not wipe the override. Mirrors the test infrastructure on the cancel-plugin side. The default-config mismatch is a separate ovos-config fix — restoring the key under the new name belongs there, not here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2.x only removed the bundled hivemind agent protocol + messagebus solver (ovos-bus-client#207); no API break (#215). Widening keeps 1.x as default while permitting 2.x, unblocking the HiveMind stack's 2.x adoption.
Raise the upper version cap so this repo accepts the new major(s), matching the semver-major caps used across the OVOS ecosystem (bus-client <3.0.0, plugin-manager <3.0.0). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 'translations' job runs 'python scripts/sync_translations.py', but scripts/ was removed with gitlocalize. publish_alpha is 'needs: translations', so every alpha release fails before publishing — which is why merged fixes (e.g. the ovos-bus-client<3.0.0 cap) never reach PyPI. Removed the job (matching ovos-audio's release workflow) and deleted the stale sync_translations.yml. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ions and self-identifies entries (#856) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ntract (#871) test_legacy_dispatch_topic_fires_handler assumed ovos-workshop dual-binds a skill handler to both the canonical and .intent-suffixed topic (workshop#497). Current workshop registers canonical-only; a legacy-suffixed dispatch now reaches a handler through the bus's own receive-side bridge (RULE 2), gated by the modernize namespace flag, which ovos-utils >=0.13.10a1 carries in FakeBus. Rewrites the test and module docstring to assert that contract on both namespaces instead of the stale dual-bind story. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…873) A quality audit found comments across the intent-context/stop pipelines narrating the review campaign that produced them ("Round 5 (C1)", "Wave-3 CONFIRMED (round 4)", "attack5.py", "regression guard (commit eec4ae0)") instead of stating the constraint being protected. Rewrite each site to say what must hold, not the story of how it was found. Consolidates the session-fold invariant into one canonical statement in _registry_session_for_context_write's docstring (SESSION-2 §2.6), with every other call site shrunk to a one-line pointer back to it. Folds _was_active_pre_drain and _utt_state_pre_drain (StopService) into a single PreDrainSnapshot NamedTuple dict, keyed and popped together, as the one mechanical refactor in scope; no other behavior changes. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
SessionManager.get(message) is never a pure read: it always folds the incoming message's session snapshot onto the live registry entry (full-replace via update_from, for every session id including "default") before returning it. Whether that fold is correct depends on the call site's fold-order contract, not on a read/write split: - True lifecycle entry (match()'s own top-level get(message)) and any site that stamps the resolved session back onto the wire (activate_skill / deactivate_skill, via message.context["session"] = session.serialize()) MUST keep the real fold: SESSION-2 last-writer-wins requires the client's declared fields (lang, blacklist, client-side (de)activations) to apply, and bypassing the fold there would silently discard them from the outgoing message. - An incidental write with no wire echo (get_response.enable/disable) must bypass the fold: a stale message arriving after registry state was written earlier in the same session's lifecycle would otherwise wipe that state via the full-replace before the handler's own write lands - this is the actual named-session bug, mirroring #857's IntentService._registry_session_for_context_write exactly. - A single synchronous call chain sharing one message (match() -> _collect_converse_skills / get_active_skills) must fold ONCE at the top and THREAD that resolved session through the rest of the chain; re-resolving via message at each step re-folds the same stale message and undoes whatever the previous step just wrote. _check_converse_timeout does NOT need this treatment - verified by mutation testing, it sits between two folds of the identical message with no intervening write, so a fresh fold there is idempotent; kept plain (YAGNI). Adds ConverseService._registry_session_for_write() for the incidental case, applies it at handle_get_response_enable/disable, keeps plain SessionManager.get(message) at activate_skill/deactivate_skill, match()'s lifecycle-entry fold, and _check_converse_timeout, and threads the resolved session through match()'s _collect_converse_skills / get_active_skills calls instead of re-folding. Also drops two now-redundant trailing get(message) calls in handle_activate_skill_request/handle_deactivate_skill_request by reusing the session activate_skill/deactivate_skill already resolved (None on the reject/no-op path, where there is nothing to sync). The helper's docstring documents a known residual: activate_skill/ deactivate_skill's full-replace fold means an incidental write this helper protects (e.g. get_response.enable) only survives until the next stale activate/deactivate call for that session - pre-existing on dev, unaffected by this PR, tracked as a known gap rather than fixed here. Went through two rounds of adversarial review before merge-readiness: round 1 caught a regression from an earlier revision that wrongly bypassed the fold at activate_skill/deactivate_skill (would have let a client-blacklisted skill get resurrected), a no-op fix at _check_converse_timeout, two missed call sites, and coverage gaps - all fixed. Round 2 caught that the chain-threading kept at _check_converse_timeout was itself inert (mutation-tested, removed), asked for the residual to be documented, and caught a test (test_deactivate_skill_folds_client_blacklist_and_applies_write) whose assertions passed off the test's own setup rather than the write path under test - fixed by making the driving snapshot declare the skill active so the write branch actually executes. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…p ping/pong (PIPELINE-1 §9.1.1) (#862) Applies the same round-correlation guard added to converse_service's handle_ack in #859 to the remaining poll rounds: fallback_service's ovos.skills.fallback.pong collector and stop_service's skill.stop.pong collector now discard pongs whose utterance_id or session mismatches the open round, standing down when the round carries no utterance_id (V0 back-compat). This closes the same late-answer-wins-wrong-round class of bug for fallback and stop, mirroring converse's fix. Note: common_query.py's phrase-string correlation (also flagged in the originating task) does not live in ovos-core -- that logic is in the separate ovos-common-query-pipeline-plugin repo and is out of scope here; left untouched. > 🤖 Auto-generated by Claude Sonnet 5 (claude-sonnet-5) via Claude Code — NOT human-reviewed. Verify before acting. Verified: new tests exercise the actual FallbackService/StopService ping-pong collectors against a FakeBus; red-before confirmed by reverting the source guard (test files kept) and re-running the new test classes, which failed exactly as expected; green after reapplying. Full unit suite (350 tests) passes on top of the fix. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(converse): broadcast contest poll (OVOS-CONVERSE-1 §4.2) Emit one broadcast `ovos.converse.ping` per round on the static spec topic, derived by `reply`, carrying no candidate identity — the session already names the candidates. The legacy per-skill pings are emitted alongside it for the compat window: no released ovos-workshop vintage binds the broadcast topic, so dropping the legacy leg would silence every skill in the field. The collector binds both pong topics and reads the claim from `result` (spec) or `can_handle` (legacy). Both legs carry the same payload — the inbound data minus skill_id — so a skill that binds both decides the round from identical input whichever ping reaches it first. Feeding the legs different data made the verdict a thread race. Three defects the dual emit makes live are fixed: - a candidate answering both legs was counted twice, and its second answer could override its first. §4.2: the first valid pong per candidate wins. - claimers were returned in pong-arrival order. §4.1 step 3: selection is by recency order and never by response-arrival order. - the two legs fed can_converse different data. The pre-flight mechanism itself is unchanged: same decision point, same single bounded collection window, same early close when every candidate has answered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(e2e): assert the OVOS-CONVERSE-1 §4.2 broadcast ping in converse goldens PR #863 adds a single broadcast `ovos.converse.ping` per converse round, emitted before the legacy per-skill pings (dual-emit compat window). The end2end goldens counted messages exactly and had not been updated, so CI failed with off-by-one message-count mismatches in every scenario that runs a converse round. Insert the broadcast ping as an explicit expected message (topic, data, position) in test_converse.py::test_parrot_mode (both namespaces, all three rounds) and test_activate.py::test_deactivate_inside_converse, matching the real emitted shape rather than loosening the count check. test_intent_alias_backcompat.py::test_legacy_dispatch_topic_fires_handler was also failing in CI, but with a capture timeout, not a count mismatch — its scenario is a raw direct dispatch that never enters the converse pipeline. It reproduces identically on the PR's parent commit (dc0adff), so it is a pre-existing, unrelated defect (installed ovos-workshop 9.3.12a1 only binds the canonical intent topic; the legacy suffixed topic depends on bus-client-side modernize, which a raw same-process message injection bypasses) and is left untouched here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…empotency (#865) * docs+test: mark add_context/remove_context as legacy-compat, prove idempotency (architecture#161) CONTEXT-1 §5.0 (architecture#161, merged) states the session is the only context write path and there is no context-mutation topic. add_context/ remove_context predate that section and are not spec write paths; document them as legacy-compat input for pre-§5.0 emitters, and add regression tests proving a repeated identical add_context/remove_context (as happens when a modern emitter has already written the session directly and the legacy message also arrives during the compat window) is idempotent - a refresh, never a double-decay or double-write artifact. 🤖 Auto-generated by Claude Fable 5 (claude-fable-5) via Claude Code — NOT human-reviewed. Verify before acting. * test: tighten idempotency assertions for add/remove context handlers - test_handle_add_context_idempotent_on_repeated_identical_write: replace assertGreaterEqual on expires_at (which accepts a compounded/stacked expiry) with a tolerance-window assertAlmostEqual around now + timeout_s, so a compounding refresh fails. - test_handle_remove_context_idempotent_on_repeated_identical_removal: assert the whole intent_context map is empty after both removals, not just that the two known keys are absent, so a leaked/accumulated key fails. Both tightened assertions verified red against mutated handler logic (compounded expires_at, leaked removal-count key) and green against the current handlers.
handle_add_context/handle_remove_context/handle_clear_context did a copy-modify-assign on Session.intent_context (dict(sess.intent_context), mutate, sess.intent_context = ctx) outside ovos_bus_client.session's _CONTEXT_LOCK, while Session.set_intent_context/remove_intent_context mutate the same map under that lock. A concurrent skill-side registry write (ovos-workshop's registry-first set_context/remove_context, >=9.3.13a1) landing between the snapshot read and the write-back was silently lost. Wrap each handler's read-modify-write in the same lock. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…cher threads (#882) SkillManager.__init__ starts a recursive watchdog Observer (FileWatcher) on the real XDG skills settings directory by default (enable_file_watcher=True). test/unittests/test_skill_manager.py instantiates SkillManager ~11 times and test/unittests/test_manager.py once, but only one call site ever calls shutdown() (which stops the observer via SkillManager.shutdown():720-722). Every other instance leaks its Observer/Emitter/Buffer daemon threads for the remainder of the test process. These threads keep firing (e.g. when an unrelated test writes a settings.json under the watched directory) and log after stderr is torn down at interpreter shutdown, which aborts CPython (exit 134) - reliably on 3.14, intermittently on 3.11/3.12. Fix: every test-created SkillManager is now shut down - via tearDown (setUp-owned instance), addCleanup (method-local instances), or try/finally (one instance built and asserted on within the same block). One test that does not exercise the watcher passes enable_file_watcher=False instead, matching an existing sibling test in the same class. test_manager.py gains a shutdown() call in its existing tearDown. Fail-before evidence: running test_skill_manager.py + test_manager.py and diffing threading.enumerate() before/after showed 38 leaked InotifyObserver threads (114 total new threads counting their Buffer/Emitter helpers) before the fix; 0 after (verified with a brief settle to let shutdown()'s async observer.join() complete). Full test/unittests suite: 456 passed, 6 xfailed, unchanged. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…ions (#880) ovos-workshop#534 added the OVOS-CONVERSE-1 §4.2 broadcast answer (ovos.converse.pong), emitted once per converse round alongside the existing broadcast poll, before the legacy per-skill ping/pong. The two end2end tests exercising converse rounds (test_parrot_mode, test_deactivate_inside_converse) had stale message-count expectations from before this message existed. Updated both namespaces (spec and legacy) with the extra message and its result value, and added ovos.converse.pong to keep_original_src since it is emitted on the entry source/destination pair rather than the flipped response direction used by other round messages. Verified locally: full end2end suite and full unittest suite both green. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
… in hot path (#879) > 🤖 Auto-generated by Claude Fable 5 (claude-fable-5) via Claude Code — NOT human-reviewed. Verify before acting. converse_service.py and stop_service.py read the legacy bus-client Session.active_skills and Session.utterance_states views on every converse/stop pass, which now log a deprecation warning on each access. Both are write-through shims over the canonical active_handlers and response_mode fields (verified against the installed ovos-bus-client 2.8.2a1 session.py), so every read site is swapped for the equivalent canonical-field expression with identical output. The ConverseService.active_skills property/setter is left untouched: it is not on the hot path (unreferenced elsewhere in the repo) and rewriting it would change its own public return shape rather than just kill a warning. Full unittest suite (test/unittests) is unchanged: 456 passed, 6 xfailed, 4 warnings before and after. A direct probe against the installed bus-client confirms the swapped expressions trigger zero deprecation log calls where the old accessors triggered two.
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
3 participants
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.
Human review requested!