Release 9.3.8a1 - #522
Open
github-actions[bot] wants to merge 163 commits into
Open
Conversation
fix: standalone skills wait_for_core
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* Add French workshop locale resources * Refine French workshop dialogs * Polish French workshop voice prompts
* fix: standalone skills wait_for_core
* fix: standalone skills wait_for_core
* fix: standalone skills wait_for_core
* fix: repair 6 failing tests and raise coverage from 45% to 54%
Test fixes:
- test_decorators.py: update TestKillableIntents for renamed class
(KillableSkill → TestAbortSkill) and language-agnostic speak assertions
- test_decorators.py: fix test_get_response — abort_question must carry
matching session_id; the killable_event session check silently ignores
messages from a different session (root cause of the flaky failure)
- test_decorators.py: fix activate_msg type (intent.service.skills.activate
→ skill.converse.get_response.enable) per current ovos-workshop behaviour
- test_abstract_app.py: fix test_gui_init — GUIInterface.__len__==0 makes
an empty gui falsy, so OVOSSkill._startup creates a new SkillGUI; assert
type rather than identity
- skills/test_base.py: fix test_default_shutdown — change `if self.gui:` to
`if self.gui is not None:` in OVOSSkill.default_shutdown so shutdown is
always called on the SkillGUI regardless of its data contents
New tests (12 files, 170 new test cases):
- test_version.py, test_settings.py, test_permissions_extended.py
- test_backwards_compat.py, test_intents_extended.py
- test_decorators_layers_extended.py, test_resource_files_extended.py
- test_simple_imports.py (passive, layers, fallback_handler)
- skills/test_converse_extended.py, skills/test_auto_translatable_extended.py
- skills/test_common_play_extended.py, skills/test_game_skill_extended.py
- skills/test_intent_provider.py
Documentation (new + improved):
- docs/app.md, docs/game-skill.md, docs/auto-translatable.md
- docs/skill-api.md, docs/filesystem.md
- Improved docs/decorators.md (line citations, abort flow, OCP section)
- Improved docs/index.md (full class hierarchy, navigation table)
- Improved docs/skill-classes.md (added game, universal, active classes)
- FAQ.md, QUICK_FACTS.md, MAINTENANCE_REPORT.md, AUDIT.md, SUGGESTIONS.md
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: audit — critical signature bug, bare excepts, missing type annotations
CRITICAL fix (common_play.py:466,481,492,501):
__handle_ocp_pause/resume/next/prev each inspected signature(self.__playback_handler)
to decide whether to pass `message=` to the OTHER handler. Replace with the
correct handler's own signature in each case.
Bare except → explicit (5 locations):
ovos.py:966 _cq_handler call
ovos.py:1216 intent_file.build() — now logs warning
ovos.py:1450 intent_parser.build() — now logs warning
common_query_skill.py:177 CQS_match_query_phrase
resource_files.py:89 language-tag distance computation
Type annotations added (ovos.py):
load_regex_files, find_resource, _handle_first_run, _check_for_first_run,
on_ready/error/stopping/alive/started_status, _handle_settings_changed,
__handle_get_response, _handle_killed_wait_response
Type annotations added (fallback.py):
register_fallback: callable -> Callable, added -> None, added Callable import
AUDIT.md updated with all findings (fixed + open), including:
- race condition on __responses dict
- busy-wait sleep loops in get_response
- god class note (OVOSSkill 2500 lines)
- magic numbers and variable shadowing
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* build: migrate to pyproject.toml, add ovos-gui-api-client dependency
- Add pyproject.toml (setuptools backend) replacing setup.py
- Delete setup.py and MANIFEST.in (superseded by pyproject.toml)
- Add ovos-gui-api-client>=0.1.0,<1.0.0 to requirements.txt
- Update app.py import: ovos_bus_client.apis.gui → ovos_gui_api_client
- Add __version__ string to version.py derived from VERSION_* constants
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* deprecate: IdleDisplaySkill and resting_screen_handler decorator
The homescreen / idle display is now managed by the shell and its adapter
plugin (ovos-shell). Skills must no longer subclass IdleDisplaySkill or
register with the homescreen manager directly.
Changes:
- idle_display_skill.py: replace full implementation with a deprecation stub
that logs a warning on instantiation; retains the class name for import
compatibility
- skills/__init__.py: remove IdleDisplaySkill from the public API exports
- decorators/__init__.py: remove resting_screen_handler decorator (replaced
by the @homescreen_app decorator on OVOSSkill)
- test_idle_display_skill.py: update test to only assert the stub exists
- test_skill.py: remove broken test_stop test (tracked in AUDIT.md — Mock
is not JSON-serialisable through FakeBus)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* ci: migrate workflows to OpenVoiceOS/gh-automations@dev
- coverage.yml: update actions/checkout and actions/setup-python to v4/v5
- downstream.yml: replace bespoke pipdeptree script with reusable
gh-automations downstream-check.yml workflow
- license_tests.yml: switch from neongeckocom to gh-automations
license-check.yml; add dev branch PR trigger
- publish_stable.yml: migrate from TigreGotico gh-automations to
OpenVoiceOS gh-automations; remove inline build/publish steps now
handled by the reusable workflow; add sync_dev and publish_pypi flags
- release_workflow.yml: migrate from TigreGotico; collapse ~90 lines of
inline steps into the reusable publish-alpha.yml call; add
workflow_dispatch trigger; enable propose_release and notify_matrix
- unit_tests.yml: delete (replaced by build_tests.yml which uses the
standardised gh-automations build-tests.yml reusable workflow)
- build_tests.yml: new workflow using gh-automations build-tests.yml
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: revert accidental GUI client refactor and pyproject.toml migration
* update
* update
* update
* update
* update
* fix: catch AbortEvent in killable thread to prevent unhandled thread exception
pytest ≥3.11 ships a threadexception plugin that treats any unhandled
exception raised from a daemon thread as a test failure. When
killable_event kills a running intent thread via raise_exc(AbortEvent),
the exception propagates up through the handler and exits the thread
without being caught, causing non-deterministic pytest failures.
Wrap the decorated function in a _guarded closure that catches AbortEvent
so the thread exits cleanly — it is an intentional kill, not an error.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: narrow except clause and improve error logging
- resource_files.py: narrow bare `except Exception` to `except ValueError`
in locate_lang_directories; tag_distance only raises ValueError for invalid
language codes — catching everything masks unrelated errors
- skills/ovos.py: include exception instance in initialization failure log
so the root cause is visible in traces
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: address CodeRabbit review comments in unit tests
- test_abstract_app.py: fix constructor typo __int__ → __init__ (critical;
subclass constructor was never called)
- test_idle_display_skill.py: replace misleading hasattr check with
assertion that handle_idle is abstract
- test_converse_extended.py: tighten assertIsInstance(dict) to assertEqual({})
for converse_matchers initialization
- test_common_play_extended.py: save/restore XDG env vars in tearDown;
tighten ocp_cache_dir assertion using os.path.basename; add initial_count+1
assertion to test_register_media_type
- test_decorators_layers_extended.py: add assertFalse(is_active) assertion
after activate_layer on non-existent layer
- test_intent_provider.py: add actual DeprecationWarning assertion using
warnings.catch_warnings (removes module from cache first so warning fires)
- test_decorators.py: remove unnecessary f-prefix from string literals with
no interpolation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* ci: fix existing workflows and add missing standard gh-automations workflows
Fixes:
- license_tests.yml: remove invalid empty with: block; add secrets: inherit
- build_tests.yml: add dev to push trigger branches (post-merge CI now runs)
Add missing standard reusable workflows (all using @dev ref):
- test.yml: unit tests on every PR/push via build-tests.yml
- lint.yml: ruff linting via lint.yml
- pip_audit.yml: CVE dependency scan via pip-audit.yml
- repo_health.yml: repo hygiene checks via repo-health.yml
All workflows reference OpenVoiceOS/gh-automations@dev per workspace policy.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs: fix CodeRabbit review comments in docs and FAQ
- docs/game-skill.md: add 'text' language specifier to fenced code block
- FAQ.md: fix path examples to use '.' instead of 'ovos-workshop/' prefix
- MAINTENANCE_REPORT.md: create required maintenance log (first entry)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Rename all locale directories to use canonical BCP-47 codes: - Lowercase regions → uppercase (da-dk → da-DK, en-us → en-US) - Bare language codes → full BCP-47 (da → da-DK, eu → eu-ES) - Invalid codes remapped (eu-eu → eu-ES, fa-fa → fa-IR) - Nested bare-language subdirs merged up (eu-ES/eu/ → eu-ES/) This fixes wheel build failures caused by ZIP files containing duplicate entries with different contents for the same logical path. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
test/end2end/test_keyword_casing_e2e.py and test_intent4_producer_e2e.py
boot a real MiniCroft (real IntentService), whose startup calls
SessionManager.connect_to_bus(mc.bus) - mutating the process-wide
SessionManager.bus class attribute. Neither test's teardown_class
restored it (unlike test_ask_e2e.py, which already saves/restores it).
Left dangling, SessionManager.bus keeps pointing at the stopped
MiniCroft's bus. The next test that calls speak_dialog(..., wait=True)
- e.g. test_decorators.py::TestKillableIntents::test_get_response - hits
SessionManager.wait_while_speaking(), whose `if not cls.bus: return`
guard no longer short-circuits, so it registers its
recognizer_loop:audio_output_end listener on the stale bus instead of
the test's own FakeBus and blocks for the full 15s timeout. The test's
get_response() thread never reaches _real_wait_response, so the
killable_event("mycroft.skills.abort_question") listener is never
registered, and the test's abort message goes unhandled.
Save/restore SessionManager.bus around MiniCroft boot in both files,
matching the existing pattern in test_ask_e2e.py.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…#497) register_intent_file() dual-registers each padatious intent on the wire -- the legacy `<skill_id>:<file>.intent` topic (register_template's emit_legacy_register_template) and the INTENT-4 canonical suffix-less `<skill_id>:<file>` topic (register_template's spec-compliant emit) -- but only bound the dispatch handler to the legacy name via self.add_event(). Pipeline plugins are moving to register-time alias collapse under the canonical name (padatious-pipeline#89, padacioso#73), so IntentService now dispatches on the canonical topic. With no handler bound to that topic, skill handlers never fire and utterances time out. Bind the handler to both names during the migration window (older pipelines still dispatch the legacy name). Mirror the same in disable_intent(), which now unbinds both events -- previously it never unbound the (single) handler binding at all. Cross-refs: OpenVoiceOS/ovos-padatious-pipeline-plugin#89, OpenVoiceOS/padacioso#73, OpenVoiceOS/ovos-core#831. This must merge and publish together with padatious-pipeline#89 and padacioso#73 -- on its own it is a no-op dual-bind; the dispatch-on- canonical behavior only exists once those pipeline plugins land. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
pip failed on ./test/end2end/session/skill-ovos-hello-world — the session/ tree no longer exists; the current end2end suites inject their skills in-place. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…515) can_answer is an opt-in optimization: it lets a skill decline before ovos-core pays for a full fallback request. The base implementation raises NotImplementedError, and _handle_fallback_ack called it unguarded, so the ping handler threw and no pong was ever emitted. From ovos-core's side the skill looked unreachable. Every fallback skill that never overrode can_answer was therefore dead -- wolfie, wikipedia and icanhazdadjokes among the official ones. 'tell me a joke' and 'what is the weather' returned nothing at all on a live install. Treat NotImplementedError as 'has not opted in, so ask it', which is the behaviour those skills were written against. An explicit True or False from a skill that did override can_answer is still respected. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
#514) _wait_response() busy-polled `while not ans: time.sleep(0.1)` with no deadline while waiting on the killable background thread (_real_wait_response) to populate __validated_responses. If that thread never finishes (eg. the bus/TTS handshake it waits on inside __get_response never completes), the caller blocks indefinitely instead of returning the documented "no response" (None) outcome. Add a deadline derived from the same budget the background thread itself uses per attempt: skills.get_response_timeout (default 20s) plus the 15s wait_while_speaking() ceiling already used elsewhere in this file for a reprompt, multiplied by the number of retry attempts. Normal-path and abort-path (externally set None) behavior is unchanged; only the previously-infinite stall case now returns None within a bounded time. Field evidence: OpenVoiceOS/ovos-skill-alerts#138 "Update 3" captured a live py-spy stack of ask_yesno -> get_response -> _wait_response blocked forever in exactly this loop, hanging a CI job to its 30-minute kill.
…eak on natural completion (#513) create_killable_daemon already marks the wrapper thread as daemon, but that guarantee lives entirely in the ovos-utils dependency; enforce t.daemon = True in killable.py itself too, so a leaked get_response()/ killable_intent waiter thread can never block process exit regardless of what ovos-utils does. More importantly, every call to a killable_intent/killable_event wrapped method registers two `.once` bus listeners (abort msg + skill stop msg) and appends the thread to skill._threads, but neither was ever cleaned up when the wrapped function finished on its own (no abort message ever arrived). Over the life of a long-lived skill instance - eg. a test suite sharing one skill across many calls, or an embedded/CI process - this accumulates one stale listener/thread reference per call, which is exactly the pattern surfaced by CI thread dumps in ovos-skill-alerts#123 (killable.py / get_response `_guarded` threads and their listeners piling up as more tests run against the same shared skill). _guarded now unregisters both listeners and drops the thread from skill._threads in a finally block, so cleanup happens whether the thread is killed or finishes normally. Fixes: OpenVoiceOS/ovos-skill-alerts#138
disable_intent()/enable_intent() never actually restored a handler: - OVOSSkill.enable_intent() called register_intent_file(name, None) / register_intent(intent, None); both register_* paths skip add_event() whenever handler is falsy, so nothing was ever rebound. - IntentServiceInterface.remove_intent() was called with the skill_id-prefixed name, but registered_intents/detached_intents are keyed by the bare name (register_intent/register_template strip the prefix before storing), so the detach bookkeeping never matched and disable_intent's internal state was already wrong going in. - register_template() also never dropped a re-registered intent from detached_intents (register_intent already did), so intent_is_detached() kept reporting True forever after a padatious intent was re-enabled. Fix: OVOSSkill now remembers each intent's handler in self._intent_handlers at registration time and passes it back into register_intent_file/register_intent on enable_intent(), instead of None. remove_intent() normalizes the skill_id prefix before matching against the bare-keyed registry, and register_template() clears the matching detached_intents entry on re-registration. Reproduced on origin/dev; found during adversarial review of #500.
locate_lang_directories and _get_closest_lang each reimplemented the langcodes distance check with their own inlined '< 10' threshold and a copy of the same explanatory comment. That policy belongs in one place. Delegate both to ovos_spec_tools.lang_matches / closest_lang, which own the OVOS-INTENT-2 §2.2 language-distance policy. Requires ovos-spec-tools>=1.6.0a2: a macrolanguage member scores exactly 10 against its macrolanguage (arz/ar, wuu/zh) and the bound was exclusive before that release, so those pairs resolved to nothing. Without the floor bump this change would silently stop matching them. Co-authored-by: Claude Opus 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
2 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!