Release 3.0.6a1 - #878
Open
github-actions[bot] wants to merge 164 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>
…cer names it (#857) * fix: mirror add_context under the resolved private key when the producer names it handle_add_context stores add_context's munged legacy key as-is; the OVOS-CONTEXT-1 gate (ovos_spec_tools.context.resolve_key) resolves a private declaration to <skill_id>:<key> instead, so gating was never reachable from OVOSSkill.set_context. When data['key'] (the original, unmunged key) is present and the message context carries a skill_id, also write the same entry under resolve_key(key, "private", skill_id) alongside the legacy munged write. handle_remove_context mirrors both spellings out symmetrically. handle_clear_context already clears the whole map, nothing to change there. Companion fix in ovos-workshop (fix/set-context-original-key) adds the data['key'] field set_context/remove_context now emit - merge in the required order documented in both PR bodies (workshop release must land on PyPI before this branch's floor pin can be bumped). Round 2 (pair review): - the resolved twin's fallback value used the munged legacy context string instead of the original key - fixed to `word or key`, so the ADAPT wire spelling never leaks into a OVOS-CONTEXT-1 consumer. - the resolved twin write replaced the whole entry, clobbering any expires_at/turns_remaining a prior write had set for that key - fixed to a setdefault-style merge (update only "value"); the pre-existing munged-key write keeps today's dev behavior (full overwrite) unchanged, deliberately - see PR body for the rationale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: skill-API context entries carry their legacy expiry on both dialect keys sess.context.inject_context() (ovos-bus-client's legacy _IntentContextView) already stamps expires_at = now + context.timeout (the adapt-engine timeout convention, Configuration()['context'] ['timeout'], minutes, default 2) on its own write into session.intent_context. The plain-dict overwrite that immediately followed in handle_add_context (ctx[context] = {"value": ...}) clobbered that stamp two lines later - the pre-existing dev "immortal context entries" bug: ovos_spec_tools.context.is_live() treats a missing expires_at as never-expiring, so prune() could never reap these entries. OVOS-CONTEXT-1 sides against that for legacy-sourced entries; immortality-by-omission is reserved for deliberate writers, which the skill API is not. Fixed by reading back whatever inject_context() already stamped for the munged key and preserving it (falling back to the same timeout computation only if nothing was stamped) instead of clobbering. The resolved private key gets the same default decay stamp for fresh writes, while keeping Round 2's setdefault-style preservation of whatever expires_at/turns_remaining a prior write already established for that exact key. This deliberately changes the Round 2 "dev parity" choice for the munged key: dev's full-overwrite-every-time behavior there WAS the immortality bug, and the spec sides against reproducing it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: context handlers operate on the live registry session - folding belongs at lifecycle entry Wave-3 CONFIRMED (round 4): SessionManager.get(message) always folds the incoming message's session onto the live registry entry, and for NAMED sessions that fold is full-replace (update_from). handle_add_context / handle_remove_context / handle_clear_context called it on every invocation, so an in-lifecycle set_context on a NAMED session first got wiped by its own call's fold, then every subsequent stale message re-wiped it again - no client could ever accumulate context on a named session. SESSION-2 §2.6: folding a message's session onto the working session belongs at lifecycle entry only; incidental messages must never mutate it. Fix (this handler's scope only): resolve the session_id off the message and, when the registry already holds a live entry for it, mutate that object directly - no fold. Fall back to SessionManager.get(message) only when no registry entry exists (out-of-registry/test callers keep today's behavior). The general fold-discipline fix (every get(message) call site, including the message-bus's own inbound fold) is a tracked follow-up design item, out of scope here. Tests: two unit tests exercise the real SessionManager.sessions registry directly (no mocking) and are red against the pre-fix every-call fold. test_context1_reachability.py gains a third scenario that fixes its own known weakness (it previously mocked SessionManager.get, which is exactly why this bug escaped to wave 3): it drives the real workshop producer over a real NAMED session, then reproduces the wave-3 mechanism with a second, genuinely stale externally-arriving message dispatched straight to the real consumer - Message.forward()'s local session-refresh self-heals same-process traffic, so only a message shaped like a real remote client's stale echo can observe the defect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: unify context decay policy across both keys (round 5) > 🤖 Auto-generated by Claude Sonnet 4.5 via Claude Code — NOT human-reviewed. Verify before acting. Round-5 adversarial re-review found three confirmed issues in fix/mirror-context-resolved-key (head a8a266c): C1 (behavioral, blocking): handle_add_context refreshed expires_at on the munged legacy key on every re-set (inject_context()'s plain dict.update) but preserved the FIRST write's expiry forever on the resolved <skill_id>:<key> key (setdefault-style merge). A skill re-calling set_context 100s later left the two keys decaying on different schedules, so the declarative OVOS-CONTEXT-1 gate could close (resolved key expired) while the legacy adapt context was still alive, or vice versa. Per CONTEXT-1 §5 (a re-set replaces wholesale) and §5.3 (no read-back API), the fix computes expires_at ONCE per write and stamps it unconditionally on both keys - dropping the setdefault preservation and the now-dead `if munged_expires_at is None and timeout_s > 0` fallback. Root cause: two independent decay computations for what is logically one write. Fail-before evidence: with the fix reverted (git apply -R on the source-only diff), both test_handle_add_context_refreshes_resolved_expiry_on_reset and test_handle_add_context_reset_refreshes_both_keys_in_lockstep fail (999999999.0 == 999999999.0; second_resolved not greater than first_resolved). Both pass after the fix, and prune(now=t0+150) reaps neither key. C2 (docs): test_intent_service_extended.py cited a nonexistent test name (test_handle_add_context_preserves_custom_munged_expiry) and an end2end file cited a stale pre-rename name (test_named_session_context_survives_in_lifecycle_and_terminal_event, renamed to ..._survives_a_second_stale_client_message). Fixed both citations and removed the false "the munged key is now ALSO merge-preserving" docstring claim (superseded by the C1 fix, which removes merge-preservation entirely). C3 (docs + missing test): the _registry_session_for_context_write docstring falsely claimed "default sessions happen to survive today only because their fold preserves omitted fields" - Session.update_from is a full serialize/deserialize replace for EVERY session id, including "default", so the registry-first fix is load-bearing for the device-local default session too, not only named ones. Corrected the docstring and added test_add_context_survives_stale_default_session_snapshot_fold. Root cause: the original round-4 fix reasoned only about named sessions and never checked update_from's default-id behavior. Fail-before evidence: with the three `_registry_session_for_context_write` call sites patch-reverted to `SessionManager.get(message)` (git apply -R limited to those three lines via sed, then git checkout -- to restore), the new test fails ('Existing' not found - the stale snapshot's fold wipes the default session's pre-existing context); passes after restoring the registry-first resolution. Verification: full test/unittests suite 334 passed (baseline 332 + 2 net-new regression tests), test/end2end/test_context1_reachability.py 3 passed. test_converse_service.py's test_malformed_pong_no_skill_id_is_ignored failed once under -x in the full run but passes in isolation and every other run - a pre-existing timing flake unrelated to this change, not touched by this commit. * build: require ovos-workshop>=9.3.11a1 (data['key'] producer for the context mirror) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ridge (#802) * test: e2e suites for OVOS-STOP-1 spec + legacy dispatch surfaces 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> * feat!: dispatch stop on the reserved OVOS-STOP-1 intent_names 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> * feat: add droppable _LegacyStopBridge for the pre-STOP-1 dispatch 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> * fix: single-delivery legacy stop, side-effect-free match, and reachable force_timeout Adversarial audit found 4 confirmed defects on OVOS-STOP-1's dispatch path: - CONFIRMED-1 (double stop): _LegacyStopBridge unconditionally re-emitted mycroft.stop / <skill_id>.stop on top of the NamespaceTranslator's own receive-side mirror of ovos.stop / <skill_id>:stop, double-firing every legacy skill's stop() when the translator is active (the default, executed proof: ['mycroft.stop', 'mycroft.stop']). The bridge now detects whether the translator already covers the mirror (NamespaceTranslator.counterpart_topics) and only re-emits when it does not (off-translator deployments still need it). - CONFIRMED-2 (force_timeout unreachable): _targeted_stop drained the session before dispatch, so by the time .stop.response arrived the session was already deactivated and handle_stop_confirmation's sess.is_active() check was always False, silently skipping ovos.skills.converse.force_timeout. Now records the pre-drain active state and consults it instead. - CONFIRMED-3 (session drained on discarded match): _targeted_stop / _global_stop mutated the LIVE SessionManager session inside match(), so a Match later discarded by the orchestrator (blacklist, missing required slots, dispatch exception) still left the session drained with nothing dispatched. Both now operate on a Session copy and carry the drain via Match.updated_session, applied only on the dispatch path. - CONFIRMED-4 (spec constants): "skill.stop.pong" / "mycroft.audio.speech.stop" hardcoded strings replaced with SpecMessage.STOP_PONG / SpecMessage.AUDIO_STOP, verified against the translator's payload-compatible mirror so behaviour is unchanged for both translator-on and translator-off deployments. Also: manifest.py now warns when a skill registers a real intent literally named "stop" (collides with the reserved <skill_id>:stop dispatch topic), and _LegacyStopBridge.handle_skill_stop no longer KeyErrors on a stop:skill message missing skill_id. Regression tests added for all four defects, proven red on the unfixed source and green after the fix (patch-revert verified). * fix: a response-mode holder is a stop candidate — stop releases a blocked get_response Live-confirmed: a user's "stop" was silently ignored for up to ~35s when the session's only activity was an outstanding get_response. enable_response_mode does not push an active_handlers entry, so §4.1 candidate selection saw an empty list and fell through to a global stop; the killable-event abort listens only on <skill_id>.stop, which global stop never emits per-skill. - StopService._stop_candidates ranks the session's response_mode holder first (most recent by definition), even when active_handlers is empty, so a generic stop targets it directly instead of escalating to global. - _global_stop now carries the holder through match_data as response_mode_holder; handle_global_stop emits <skill_id>.stop for it before the ovos.stop broadcast, so an explicit "stop everything" also releases a blocked get_response. - handle_stop_confirmation's abort_question check read utterance_states off the already-drained session carried by the .stop.response dispatch (disable_response_mode runs before dispatch in _targeted_stop), making the RESPONSE-state branch unreachable dead code. Mirrors the existing _was_active_pre_drain pattern with _utt_state_pre_drain so abort_question actually fires for a skill genuinely blocked in get_response. Regression tests proven red on the unfixed source (patch-revert, no git stash), green after the fix. * fix: session-scoped pre-drain snapshots, lifecycle for malformed legacy stop, honest reserved-name warning * fix(stop): resolve dispatcher lifecycle synchronously and rank stop candidates by recency > 🤖 Auto-generated by Claude Sonnet 4.5 via Claude Code — NOT human-reviewed. Verify before acting. Two live-confirmed defects in the STOP-1 rework (PR #802): L1 — dispatcher lifecycle never resolves for stop dispatches IntentDispatcher tracks every <skill_id>:<intent> dispatch and resolves it on the framework done-signal mycroft.skill.handler.complete/.error. The targeted stop dispatch goes out on the spec colon-topic <skill_id>:stop, which ovos-workshop has no direct listener for -- only _LegacyStopBridge mirrors it onto the dot-topic <skill_id>.stop skills actually bind, via add_event with handler_info=None, which deliberately disables that dot-topic handler's own HandlerLifecycle/complete emission. Root cause is therefore cross-repo: workshop's dot-topic binding never produces the done-signal the dispatcher listens for. Since workshop is a separate repo/release, the emission cannot be added there for this fix. Fix: StopService.handle_stop_confirmation -- the real completion signal of a stop round-trip, since it only fires once the skill has actually finished stop() -- now also emits mycroft.skill.handler.complete with context.skill_id set, via the new _resolve_dispatch_lifecycle helper. This resolves the dispatcher's in-flight entry through the bus, the same way a normal handler completion does, keeping StopService decoupled from IntentDispatcher's internals. Before the fix every stop left its ovos.utterance.handled end-marker parked on the dispatcher's 5-minute §8.3 timeout instead of firing synchronously. Fail-before evidence: test_stop_round_trip_resolves_dispatcher_entry_synchronously fails against the unfixed code (asserts the dispatcher's in-flight entry list is empty immediately after the round-trip; unfixed it still holds one unresolved entry with a live timer). Passes after the fix. L2 — response_mode holder "ranks first" was a race, not a guarantee _stop_candidates orders the response_mode holder first (most-recent interaction by definition), but _collect_stop_skills pings all candidates in parallel and picked want_stop[0] = the first PONG TO ARRIVE, not the first candidate in recency order. Live-reproduced: with a holder + an older active_handlers skill both answering can_stop=True, the older skill won 2/7 runs. Fix: _collect_stop_skills keeps the parallel broadcast-contest collection unchanged (mechanism untouched) but now re-sorts the collected want_stop set by the candidate list's recency order (`sorted(want_stop, key=active_skills.index)`) before returning, so the winner is always the most-recent stoppable candidate regardless of arrival order. Fail-before evidence: test_selection_deterministic_by_recency_not_arrival_order fails against the unfixed code (asserts the recency-first candidate wins even when it answers second; unfixed the first-arriving/older skill wins). Passes after the fix. Verification: both new tests confirmed red-before (git apply -R of this commit's stop_service.py hunk, patch-revert technique, no git stash) and green-after. Full test/unittests suite: 325 passed. Stop-specific e2e suite (test_stop_legacy_e2e.py, test_stop_response_mode_e2e.py, test_stop_spec_e2e.py): 6 passed. * fix(stop): gate synthetic dispatch-lifecycle resolution to the actual stop and preserve the §8.2 error terminal > 🤖 Auto-generated by Claude Sonnet 4.5 via Claude Code — NOT human-reviewed. Verify before acting. Adversarial re-review of f275050 (verdict: SHIP-WITH-FIXES) found two confirmed hazards introduced by that commit's synthetic `mycroft.skill.handler.complete`/`.error` emission in `StopService._resolve_dispatch_lifecycle`. Both independently reproduced against the unfixed code with the reviewer-supplied attack scripts before writing the fix, per this repo's verify-before-filing gate. C1 — stale bus.once() resolves an unrelated, still-running intent `_targeted_stop` registers `bus.once(f"{skill_id}.stop.response", handle_stop_confirmation)` at MATCH-BUILD time — a pre-existing side effect that survives even when the orchestrator later discards the Match (blacklisted intent, missing slots, a dispatch exception) and never actually calls IntentDispatcher.dispatch() for it. Before this fix, a LATER `.stop.response` for that skill_id (e.g. from an unrelated global stop's own ping-pong round trip) fired the stale listener, and the synthetic `mycroft.skill.handler.complete` it emitted popped whatever in-flight dispatcher entry existed for that skill_id — `IntentDispatcher._pop` matches on skill_id alone, ignoring intent_name — including a completely unrelated, still-running ordinary intent handler. That produced a premature `ovos.utterance.handled` end-marker for work that was still in progress. Root cause is two-layered, so the fix has two parts: 1. `handle_stop_confirmation` now only calls `_resolve_dispatch_lifecycle` when `(session_id, skill_id)` is present in the existing `_was_active_pre_drain`/`_utt_state_pre_drain` pre-drain snapshots (no new state — these dicts already key exactly this pair and are consumed by the same handler). This narrows *when* a synthetic terminal can fire at all. 2. `IntentDispatcher._pop` (and its callers `_on_skill_complete`/ `_on_skill_error`) gained an optional `intent_name` filter, used only when the caller supplies `context["intent_name"]` — the normal framework done-signal from a real skill still matches on skill_id alone (unchanged, backward compatible: a skill has one handler running at a time), but `_resolve_dispatch_lifecycle` now stamps `context["intent_name"] = "stop"` on its synthetic signal, so it can only ever resolve the skill's `stop` entry, never an unrelated one. This narrows *what* it can resolve. Layer 1 alone was verified (empirically, against the reviewer's attack.py::test_B_stale_once_pops_wrong_entry) to be INSUFFICIENT on its own — the pre-drain snapshot is set at match-build time regardless of whether the match is later discarded, so presence alone cannot distinguish a genuinely-dispatched stop from a discarded one; layer 2 is the piece that actually stops the wrong-entry pop. Regression test: TestStaleStopOnceDoesNotResolveUnrelatedEntry, mirroring attack.py::test_B_stale_once_pops_wrong_entry — a discarded targeted-stop match, an unrelated running intent for the same skill, then a foreign .stop.response; asserts the running intent's dispatcher entry survives. Confirmed red (entries == []) against the code before this commit (git apply -R of this commit's dispatcher.py/stop_service.py hunks), green after. C2 — a failed stop() resolved as `complete`, not `error` `_resolve_dispatch_lifecycle` unconditionally emitted `mycroft.skill.handler.complete` even when the `.stop.response` carried `error` (the skill's `stop()` raised) — §8.2 requires the `error` terminal so a failed stop is distinguishable from a successful one on the handler- lifecycle trio. Fix: `_resolve_dispatch_lifecycle` now emits `mycroft.skill.handler.error` (with `exception` set from `message.data["error"]`) when `'error' in message.data`, `.complete` otherwise. Regression test: TestFailedStopYieldsErrorTerminal, mirroring attack3.py — a stop() that reports an error; asserts the terminal is `ovos.intent.handler.error`, not `.complete`. Confirmed red (terminal was `.complete`) against the code before this commit, green after. Verification: both new tests confirmed red-before (git apply -R / git apply patch-revert technique, no git stash) and green-after, alongside the reviewer's own attack.py/attack2.py/attack3.py scripts run directly. Full test/unittests suite: 327 passed (325 baseline + 2 new). Stop-specific e2e suite (test_stop_legacy_e2e.py, test_stop_response_mode_e2e.py, test_stop_spec_e2e.py): 6 passed. Known residual (out of scope for this re-review): attack.py::test_A and test_C_pong_from_unknown_skill probe a SEPARATE, pre-existing gap — the global-stop dispatch's OWN in-flight dispatcher entry (skill_id=pipeline_id, intent_name=global_stop) is never resolved by individual skills' .stop.response round trips, and a stray pong from an unlisted skill_id is silently accepted into want_stop. Neither was flagged as a confirmed finding by this re-review round; not addressed here. * fix(stop): source the intent_name filter from data not context, and stop leaking pre-drain snapshots on failed stops > 🤖 Auto-generated by Claude Sonnet 4.5 via Claude Code — NOT human-reviewed. Verify before acting. Round-3 adversarial re-review of 6e8c816 (verdict: SHIP-WITH-FIXES) found two confirmed hazards and one untested mechanism in that commit's `_resolve_dispatch_lifecycle`/`handle_stop_confirmation` code. All three independently reproduced against the pre-fix code with the reviewer-supplied attack4.py/attack5.py before writing the fix, per this repo's verify-before-filing gate. (The reviewer also definitively refuted round-2's attack.py::test_A/test_C as invalid probes — no action needed on those; the "known residual" note in 6e8c816's message referred to them and no longer applies. That commit is already pushed/shared, so it is not rewritten here — this note supersedes it.) F1 — the intent_name filter was sourced from client-inherited context `IntentDispatcher._pop`'s optional `intent_name` filter (added in 6e8c816 to fix C1) read `message.context.get("intent_name")`. Context is CLIENT-INHERITED: `Message.forward` deep-copies the context of the message it's called on, and for a dispatch chain that traces back to the ORIGINATING client utterance. Any client that sets `context["intent_name"]` on its own utterance would have that value survive every `forward()` down the dispatch chain and land on an unrelated skill's REAL `mycroft.skill.handler.complete` too — mismatching the filter and silently parking that skill's genuinely-completed intent on the dispatcher's 5-minute §8.3 timeout instead of resolving it. Confirmed with attack4.py test_2 before the fix (leftover in-flight entry after a real completion). Fix: `_resolve_dispatch_lifecycle` now stamps `data["intent_name"] = "stop"` instead of `context["intent_name"]`, and `_on_skill_complete`/ `_on_skill_error` read `message.data.get("intent_name")`. `data` is passed fresh by `forward()`'s second argument on every hop — never inherited from the client — so only StopService's own synthetic emission ever carries this key; the normal single-handler-at-a-time framework signal (no `data` `intent_name`) is unaffected, matching attack4.py test_1/test_3. F2 — pre-drain snapshots leaked forever on any non-success stop The `(session_id, skill_id)` pre-drain snapshots (`_was_active_pre_drain`/`_utt_state_pre_drain`) were popped ONLY inside the `result: True` branch of `handle_stop_confirmation`. Every `error`, `result: False`, or never-actually-dispatched stop left both dicts holding that key forever — an unbounded memory leak (confirmed with attack5.py: 50 failed stops -> 50 leaked keys in each dict) — which ALSO kept the C1 presence-gate permanently open for that pair, since the gate is presence-based: any later, unrelated `.stop.response` reusing the same (session_id, skill_id) would pass it. Fix: both dicts are now popped UNCONDITIONALLY at the top of `handle_stop_confirmation`, right after `sess_id`/`skill_id` are computed, and the popped values are threaded through the rest of the method (the gate check and the existing RESPONSE-state/converse-active fallbacks below). F3 — the presence-gate mechanism itself was untested Round-2's C1 fix (the presence-gate in `handle_stop_confirmation`) had no test that exercises the gate in isolation — the full suite stayed green even with the gate deleted entirely (verified: reverting it to unconditional `True` locally still passed every existing test). Added TestPreDrainGateBlocksUnknownPair: a `.stop.response` for a `(session, skill)` pair with NO pre-drain snapshot must emit no `mycroft.skill.handler.complete`/`.error` at all. Confirmed red with the gate forced open (`if True:`), green with it restored. Regression tests added to test/unittests/test_stop_service.py: - TestIntentNameFilterIsDataNotContext (mirrors attack4.py test_1/2/3) - TestPreDrainSnapshotsDoNotLeakOnFailedStop (mirrors attack5.py, all 3 cases) - TestPreDrainGateBlocksUnknownPair (F3, new coverage) Verification: all new tests confirmed red-before (git apply -R / git apply patch-revert technique against this commit's own diff, no git stash) and green-after, alongside the reviewer's own attack4.py/attack5.py run directly. Full test/unittests suite: 333 passed (327 baseline + 6 new). Stop-specific e2e suite (test_stop_legacy_e2e.py, test_stop_response_mode_e2e.py, test_stop_spec_e2e.py): 6 passed. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sion.sync merge, slot-fill) (#786) * feat: OVOS-CONTEXT-1 orchestrator-resident intent context Implement the core-resident half of OVOS-CONTEXT-1: the flat, decaying session.intent_context key/value store, alongside the legacy frame-based IntentContextManager. New module ovos_core.intent_services.intent_context provides: - §2 entry shape + liveness predicate (value/flag/null, turns/wallclock) - §3.1 scope resolution (private <skill_id>:<key> vs shared bare key) - §6/§6.1 gate_satisfied predicate (requires/excludes, post-decay) - §7 context_supplied_slots fill rule (utterance value wins) - IntentContextStore: §4 prune-then-decrement decay, §4.1 mid-dispatch exemption, §5.3 ovos.session.sync entry-by-entry merge (set+null-delete), §2 max-entry cap eviction Wire into IntentService (the orchestrator): - handle_session_sync merges ovos.session.sync intent_context payloads - handle_utterance adopts inbound snapshot, prunes pre-match, decrements post-match over the pre-match key set (so mid-dispatch syncs survive) - _emit_match_message applies §5.1 promotion + §7 slot fill and stamps the working map onto the emitted session (legacy Session drops the field) - intent_context exposed as a lazily-backed property for safe partial construction Engine-side §6/§6.1 gating *inside* matchers (adapt/padacioso) is out of scope and deferred; core exposes the shared gating vocabulary they consult. SESSION_SYNC is a literal here; its SpecMessage registration is a SESSION-2/spec-tools follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor!: delegate ovos.session.sync merge to SessionManager (OVOS-CONTEXT-1 §5.3) The session.intent_context map and the §5.3 ovos.session.sync entry-by-entry merge (set + null-delete) are now owned by the SessionManager singleton (bus-client #239): it carries intent_context as a first-class round-tripping Session field and applies the merge in SessionManager.handle_session_sync / merge_intent_context. The orchestrator no longer subscribes to ovos.session.sync and holds no parallel session_id-keyed store: - service.py: drop the SESSION_SYNC subscription, the handle_session_sync handler, the bus.remove on shutdown, the _intent_context store, the intent_context property/setter, and _stamp_intent_context. The §4 decay now operates on the session's own intent_context map (prune-then- decrement around the match round), written back via SessionManager.update so the singleton stays authoritative; §4.1 mid-dispatch sync keys are skipped from the decrement. §5.1 promotion merges via SessionManager.merge_intent_context; §7 slot fill reads sess.intent_context. - intent_context.py: IntentContextStore (the {session_id: map} store + its merge_sync) is gone. The decay/liveness/scope/gate/fill logic is kept as stateless module helpers (prune, decrement, enforce_cap + the existing pure predicates) that operate on a passed-in intent_context dict. - pyproject: floor-pin ovos_bus_client>=2.5.0a1 (the alpha carrying #239). - tests: §5.3 merge tests re-homed to bus-client #239; core keeps/expands the decay/liveness/scope/gate/fill tests plus a live check driving the REAL SessionManager merge and asserting core sees merged+decayed context. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat: OVOS-CONTEXT-1 orchestrator gate + manifest-sourced require/exclude context (#801) * feat: OVOS-CONTEXT-1 orchestrator-resident intent context Implement the core-resident half of OVOS-CONTEXT-1: the flat, decaying session.intent_context key/value store, alongside the legacy frame-based IntentContextManager. New module ovos_core.intent_services.intent_context provides: - §2 entry shape + liveness predicate (value/flag/null, turns/wallclock) - §3.1 scope resolution (private <skill_id>:<key> vs shared bare key) - §6/§6.1 gate_satisfied predicate (requires/excludes, post-decay) - §7 context_supplied_slots fill rule (utterance value wins) - IntentContextStore: §4 prune-then-decrement decay, §4.1 mid-dispatch exemption, §5.3 ovos.session.sync entry-by-entry merge (set+null-delete), §2 max-entry cap eviction Wire into IntentService (the orchestrator): - handle_session_sync merges ovos.session.sync intent_context payloads - handle_utterance adopts inbound snapshot, prunes pre-match, decrements post-match over the pre-match key set (so mid-dispatch syncs survive) - _emit_match_message applies §5.1 promotion + §7 slot fill and stamps the working map onto the emitted session (legacy Session drops the field) - intent_context exposed as a lazily-backed property for safe partial construction Engine-side §6/§6.1 gating *inside* matchers (adapt/padacioso) is out of scope and deferred; core exposes the shared gating vocabulary they consult. SESSION_SYNC is a literal here; its SpecMessage registration is a SESSION-2/spec-tools follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor!: delegate ovos.session.sync merge to SessionManager (OVOS-CONTEXT-1 §5.3) The session.intent_context map and the §5.3 ovos.session.sync entry-by-entry merge (set + null-delete) are now owned by the SessionManager singleton (bus-client #239): it carries intent_context as a first-class round-tripping Session field and applies the merge in SessionManager.handle_session_sync / merge_intent_context. The orchestrator no longer subscribes to ovos.session.sync and holds no parallel session_id-keyed store: - service.py: drop the SESSION_SYNC subscription, the handle_session_sync handler, the bus.remove on shutdown, the _intent_context store, the intent_context property/setter, and _stamp_intent_context. The §4 decay now operates on the session's own intent_context map (prune-then- decrement around the match round), written back via SessionManager.update so the singleton stays authoritative; §4.1 mid-dispatch sync keys are skipped from the decrement. §5.1 promotion merges via SessionManager.merge_intent_context; §7 slot fill reads sess.intent_context. - intent_context.py: IntentContextStore (the {session_id: map} store + its merge_sync) is gone. The decay/liveness/scope/gate/fill logic is kept as stateless module helpers (prune, decrement, enforce_cap + the existing pure predicates) that operate on a passed-in intent_context dict. - pyproject: floor-pin ovos_bus_client>=2.5.0a1 (the alpha carrying #239). - tests: §5.3 merge tests re-homed to bus-client #239; core keeps/expands the decay/liveness/scope/gate/fill tests plus a live check driving the REAL SessionManager merge and asserting core sees merged+decayed context. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat: OVOS-CONTEXT-1 §6/§6.1 orchestrator gate + manifest-sourced declarations The orchestrator drops a matched candidate whose requires_context is unmet or whose excludes_context is present, re-checking the gate so a misbehaving matcher cannot dispatch a context-gated intent. The declared gates and slot names are read from the passive INTENT-4 §10 manifest (the single source of an intent's declaration) — never off the Match. - IntentManifest.get_context_requirements / get_slot_names: union an intent's requires_context / excludes_context / slot names across its registration definitions. - match loop: gate backstop via gate_satisfied against session.intent_context. - _apply_context_slots (§7): sources requires_context + slot_names from the manifest instead of the Match. Additive: an intent that declares no gates is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * chore: timeless docstrings + drop shadowing slot-fill duplicate; §7 orchestrator coverage Strip PR/issue numbers and replacement narration from the CONTEXT-1 store/decay comments, docstrings and intent-service docs. Remove the duplicate Match-reading _apply_context_slots that shadowed the manifest-sourced one, so the orchestrator's §7 slot fill sources requires_context/slot_names from the passive INTENT-4 §10 manifest only. Mirror intent_manifest in the dispatcher test fixture and add orchestrator-level §7 slot-fill coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: correct docs ownership + same-dispatch decrement bug in intent context docs/intent-service.md: ovos.session.sync §5.3 merge is owned and handled by SessionManager.handle_session_sync (bus-client), not IntentService; correct the bus-events table and §5.3 description to match. service.py: pre_match_keys tracked only key presence, so a mid-dispatch ovos.session.sync that REFRESHES an existing key (not just adds a new one) left the key in the snapshot and its fresh entry got decremented in the same dispatch, violating the §4.1 same-dispatch exemption. Track pre-match entry values and only decrement keys whose value is unchanged. service.py: context_supplied_slots was fed reply.data as filled_slots, which carries framework/echo fields (utterance, lang, ...) alongside matched slots, so a declared slot name colliding with one of those was wrongly treated as utterance-filled. Feed match.match_data instead. Adds regression tests for the mid-dispatch refresh exemption and the match_data vs framework-field slot-fill distinction. * fix: propagate decayed intent_context onto terminal emissions (§4.2) The OVOS-CONTEXT-1 post-match decrement ran, but ovos_bus_client's Message.forward()/reply() always re-stamp an outbound context['session'] from the live SessionManager registry at emission time, and SessionManager.get() unconditionally folds any inbound session snapshot back onto that same registry (SESSION-1 wholesale-replace). Since the decrement previously ran *after* the dispatch Message was already handed to the IntentDispatcher (i.e. after it was actually put on the bus for the skill to receive), the skill always received the pre-decrement snapshot -- and its own routine SessionManager.get(message) call folded that stale snapshot back onto the registry the moment it started running, silently undoing the decrement before any §8/§9.5 terminal fired. A remote client that echoes back exactly what it was handed (per SESSION-1 value-passing semantics) therefore never saw turns_remaining progress. Move the §4.2 decrement into _dispatch_match, before the dispatch Message is handed to the IntentDispatcher, so the dispatch a skill actually receives -- and everything it echoes back -- already carries the decayed map. Also stop _dispatch_match's initial session lookup from folding the *original* pre-match utterance Message (SessionManager.get(message)) onto the registry, which was clobbering a legitimate same-round mid-dispatch sync from an earlier matcher; read the live registry entry instead. Verified live on a running core + hello-world skill: turns_remaining decays 3 -> 2 -> 1 across two real turns, visible on both ovos.utterance.handled and ovos.intent.handler.complete. * refactor!: move intent_context decay/cap primitives to the session layer OVOS-CONTEXT-1 §4/§4.1's prune-then-decrement decay lifecycle and §2's live-entry cap eviction are map-mutation mechanics over session.intent_context, not orchestrator decisions — they belong next to SessionManager.merge_intent_context (§5.3) in the session layer, not duplicated locally. ovos_spec_tools.context already carries them byte-identical (ovos-spec-tools#77); intent_context.py now re-exports prune/decrement/enforce_cap/DEFAULT_MAX_ENTRIES from there instead of defining its own copies. The orchestrator keeps gate_satisfied/context_supplied_slots (matcher vocabulary) and _apply_post_match_decay's same-dispatch-refresh exemption (value-compared, §4.1) unchanged — only the mechanics moved. Bumps the ovos-spec-tools floor pin comment to document the ovos-core #77 dependency (the floor version itself, 1.5.0a1, already covers it). Primitive-level TestDecay/TestCapEviction cases move to ovos-spec-tools#86; orchestrator-level tests (terminal-emission decay, two-turn 3->2->1, mid-dispatch exemption) stay here, unchanged. Depends on OpenVoiceOS/ovos-spec-tools#86 * refactor: consume intent-context helpers from ovos-spec-tools (drop core-resident module) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: trim spec commentary to short §-pointers Comments and docstrings re-explained OVOS-CONTEXT-1 semantics at length; the spec is the source of truth, code keeps only short §-pointers and constraints code cannot express. * fix: align session-sync tests to the SESSION-2 data carrier and harden context decay - test: ovos.session.sync now rides Message.data["session"] (SESSION-2 §2.7); the paths that need ovos-bus-client#278 are xfail(strict=True), plus new explicit cases for the context-only fallback and data-wins-over-context. - drop the dead `getattr(match, "intent_context")` promotion block — IntentHandlerMatch has no such field; entries reach the session via updated_session + the §5.3 sync. _dispatch_match returns None again. - _apply_post_match_decay no longer falls back to the DEFAULT session for an unregistered session_id (cross-session context corruption); it warns and no-ops. - the §6.2 missing-required-slots backstop now consults live intent_context, so a slot §7 would have filled no longer kills the match. - every intent_context write mutates the existing dict in place under the bus-client _CONTEXT_LOCK; an empty context is {} never None. - pre_match_entries is a deepcopy, so in-place mutation of a nested entry cannot defeat the §4.1 mid-round exemption check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…struction order (#855)
…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>
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!