feat!: drop legacy wire compat (kill-switch — merge when fleet upgraded) - #412
feat!: drop legacy wire compat (kill-switch — merge when fleet upgraded)#412JarbasAl wants to merge 12 commits into
Conversation
Old ovos-workshop built the per-intent dispatch topic from the padatious resource filename, so the `.intent` extension leaked onto the wire. The real MessageBusClient now mirrors an intent dispatch onto that suffixed twin when emit_legacy is on. A test double that skipped the mirror would hide the compat path from every harness built on it. FakeBus and AsyncFakeBus now share a `_LegacyIntentBridge` that calls `ovos_spec_tools.intent_topics.legacy_reemit_targets` after the namespace counterpart dispatch, with an `IntentAliasRegistry` filled from the bus's own on() / once() calls. Blanket mode (`intent_reemit_blanket`, off by default) mirrors every intent dispatch for pure-bus listeners that never register. The spec-tools import is guarded: against a release without the intent-topic helpers both fake buses behave exactly as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
FakeBus and AsyncFakeBus mirrored the two migration bridges that MessageBusClient ran. Both are removed there, so both are removed here. Gone from the fake bus: * the OVOS-MSG-1 namespace bridge and its payload reshaping; * the handler mirror-guard that made dual-namespace subscriptions safe; * the intent-topic twin, which mirrored a canonical <skill_id>:<intent> dispatch onto the old <skill_id>:<intent>.intent spelling. A test double that kept the bridge would be worse than useless: every harness built on it would pass against behaviour the fleet no longer has. The emit_legacy, modernize and intent_reemit_blanket flags are removed in both spellings the fake bus accepted - constructor kwarg and env/config - and raise RuntimeError when explicitly enabled. Passing them as False is still accepted, so callers that already turned the bridge off need no edit. test_fakebus_namespace_migration.py and test_fakebus_intent_legacy_reemit.py proved the bridge worked; test_fakebus_no_legacy_compat.py proves it is absent, sweeping the whole MIGRATION_MAP in both directions. The AsyncFakeBus namespace class is inverted in place. BREAKING CHANGE: FakeBus no longer bridges legacy bus topics. A test that emitted a legacy topic and listened on the spec one (or the reverse) must pick one namespace. emit_legacy / modernize / intent_reemit_blanket raise RuntimeError when enabled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Analyzing your contribution... results ready! 🧪I've aggregated the results of the automated checks for this PR below. 📋 Repo HealthChecking for any potential maintenance bottlenecks. 🚧 ✅ All required files present. Latest Version: ✅ 🔒 Security (pip-audit)Checking for any insecure data transmissions. 📡 ✅ No known vulnerabilities found (47 packages scanned). ⚖️ License CheckVerifying the legal status of all dependencies. 📜 ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔍 LintEvaluating the overall quality of your PR. ✨ ❌ ruff: issues found — see job log 🏷️ Release PreviewThe release banner is being designed! 🎨 Caution Breaking change — this PR will bump the MAJOR version ( Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
📊 CoverageTesting the resilience of our codebase. 🧱 ✅ 85.1% total coverage Files below 80% coverage (5 files)
Full report: download the 🔨 Build TestsThe build engine is firing on all cylinders. 🏎️ ✅ All versions pass
Helping you push code with confidence 🚀 |
The fake buses bridge the canonical `<skill_id>:<intent_name>` dispatch topic and the legacy `.intent`-suffixed spelling with the same two rules the real `MessageBusClient` now uses. The client splits them over a wire send and a wire receive; a fake bus is one process, so both land in `emit`: * a canonical dispatch also fires its suffixed twin, marked in `context` with `_intent_compat_twin`; * a suffixed dispatch that is not already such a twin also fires its canonical spelling. The two cases are mutually exclusive and neither cascades, so one emit reaches each handler exactly once. The alias registry, the per-bus alias table, the `on()` / `remove()` bookkeeping, and the blanket flag are all gone: which handlers exist is not something the bridge needs to know. Only the two pure helpers are imported from ovos-spec-tools, so the existing >=1.6.0a1 floor stays sufficient.
github-actions
Bot
added
breaking
…t' into feat/drop-fakebus-legacy-compat
spec-tools#92 (registry removal) merged and released as 1.6.0a2; pin to the simplified intent_topics surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t' into feat/drop-fakebus-legacy-compat # Conflicts: # ovos_utils/fakebus.py # test/unittests/log_test/configured.log # test/unittests/log_test/rotate.log # test/unittests/log_test/rotate.log.1 # test/unittests/test_fakebus_intent_legacy_reemit.py
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Message.forward()/reply() deep-copy the whole context, so the twin marker in message.context rode onto every descendant frame. A handler that forwarded a received twin's context to emit an unrelated suffixed intent branded that frame a twin, and _bridge_intent_topics skipped its canonical spelling -- silent loss for the old-emitter -> new-core population the rule serves. emit now pops the marker into a local decision before local dispatch, so descendants start clean, and _bridge_intent_topics trusts that decision instead of reading context. RULE 1's twin still carries the marker on the wire (its serialized form goes on the "message" firehose), while local handlers receive an unmarked copy -- mirroring the real client, whose local delivery happens on the receive side after the pop. Regression tests build the follow-up via forward()/reply() off a received twin and assert the unrelated canonical topic is still modernized; wire-survival keeps the second-receiver skip property. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…us-legacy-compat # Conflicts: # ovos_utils/fakebus.py # test/unittests/log_test/configured.log # test/unittests/log_test/rotate.log # test/unittests/log_test/rotate.log.1 # test/unittests/test_fakebus_intent_legacy_reemit.py
_bridge_intent_topics() ran unguarded in both FakeBus.emit and AsyncFakeBus.emit, unlike the counterpart-topics loop right above it, which isolates per-topic errors with try/except + LOG.exception. A raising bridge helper would propagate out of emit() and take down local handler dispatch with it. Wrap both call sites in the same guard. Addresses CodeRabbit review on ovos-utils#411.
…t' into feat/drop-fakebus-legacy-compat # Conflicts: # ovos_utils/fakebus.py # test/unittests/test_fakebus_intent_legacy_reemit.py
1 participant
The kill-switch
This is a prepared trigger, not a change to merge today. It is the
FakeBushalf of the stack-wide legacy wire-compat drop.
Do not merge until the fleet no longer runs legacy-namespace consumers. It
depends on #411 landing first, and it must go in together with its sibling
drop PRs.
What is removed
FakeBusandAsyncFakeBusmirrored the two migration bridges thatMessageBusClientran. Both are removed there, so both are removed here.emit)on,remove)_LegacyIntentBridge)<skill_id>:<intent>dispatch onto the old<skill_id>:<intent>.intentspelling — the hook #411 adds_resolve_bus_flags,INTENT_REEMIT_CONTEXT_KEYA test double that kept the bridge would be worse than useless: every harness
built on it would pass against behaviour the fleet no longer has.
The two flag spellings get different treatment
This is deliberate, and it is the one judgement call in this PR.
websocketconfig →RuntimeError, the same as the realclient. That is an operator asking a live deployment to keep the legacy
topics on the wire; the belief is now wrong and silence would cost them
messages.
harness, not a deployment. Harnesses across the ecosystem pass
emit_legacy=Trueunconditionally —ovoscope'sMiniCroftdoes — andraising there would break every one of them at construction without telling
anybody anything useful.
Follow-up for
ovoscope:MiniCroftshould stop passing the flagsentirely. Until it does it takes a deprecation warning per boot, which is
noisy but harmless.
Tests
test_fakebus_namespace_migration.pyandtest_fakebus_intent_legacy_reemit.pyproved the bridge worked. They arereplaced by
test_fakebus_no_legacy_compat.py, which proves it is absent andsweeps the whole
MIGRATION_MAPin both directions. TheAsyncFakeBusnamespace class in
test_async_fakebus.pyis inverted in place.To test the post-compat world:
Result on this branch: 935 passed, 1 skipped, 1 failed. The one failure is
test_events.py::TestEventSchedulerInterface::test_00_init, which fails thesame way on the base branch — pre-existing, unrelated.
Order
FakeBusparity, the branch this PR is based onMerging any drop PR alone breaks the stack. They go in as one flip.
Implemented by Claude (opus), orchestrated by Claude Fable.
🤖 Generated with Claude Code
The four drop PRs
They flip together, or the stack breaks.
devThe compat train they undo: OpenVoiceOS/ovos-spec-tools#88 (merged),
OpenVoiceOS/ovos-bus-client#271, #411,
OpenVoiceOS/ovos-workshop#500.
ovos-spec-tools keeps its helpers.
MIGRATION_MAP,SPEC_TO_LEGACY,migration_counterpartandovos_spec_tools.intent_topicsare pure functionsthe spec linter and migration tooling use. Nothing is dropped there.