fix: normalize pipeline ids when matching session blacklists - #854
fix: normalize pipeline ids when matching session blacklists#854JarbasAl wants to merge 1 commit into
Conversation
session.blacklisted_pipelines is compared against session.pipeline as literal strings, but the deployment blacklist is naturally expressed in installed plugin IDs (e.g. "ovos-adapt-pipeline-plugin") while a session pipeline carries confidence-suffixed matcher IDs (e.g. "ovos-adapt-pipeline-plugin-high"). A base plugin policy ID silently failed to block its suffixed matcher variants. Normalize both sides through _PIPELINE_MIGRATION_MAP and strip the confidence suffix (_PIPELINE_RE) before comparing, so a blacklisted base plugin ID blocks all of its matcher variants, filtered before matcher lookup (no invocation, no bus event for the skip). Split out of ovos-core#808 per review -- unrelated to that PR's fallback ping/pong migration-window fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Hello! The automated checks have been performed. 👋I've aggregated the results of the automated checks for this PR below. 📚 DocsA detailed summary of the latest automation run. 📝 ✅ All required documentation files present. ✅ 📋 Repo HealthEnsuring the project's documentation is healthy. 📚 ✅ All required files present. Latest Version: ✅ 🔎 Type CheckThe automated pipeline is running smoothly. 🚂 ❌ mypy: 272 error(s) found
Errors (showing first 10/272)🏷️ Release PreviewEnsuring the release is as shiny as a new penny. ✨ Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
🌍 Locale BuildChecking if we've met all our check criteria. ✅ ✅ Locale properly configured (64 files, 17 languages) Locale directories found:
Localization coverage:
pyproject.toml: ✅
Build manifest: ✅ 31 locale files included in package 📊 CoverageA bird's eye view of your test coverage landscape. 🦅 Files below 80% coverage (9 files)
Full report: download the 🔌 Plugin DetectionScanning for any 'non-standard' entry points. 🚪 ❌ Plugin Status: ERRORS (1) Plugin Info:
OPM Detection:
Entry Point Validation:
⊘ No Issues:
🔒 Security (pip-audit)Checking for any potential security breaches. 🔓 ✅ No known vulnerabilities found (113 packages scanned). 🔨 Build TestsDid the code survive the compilation gauntlet? Let's see. 🛡️ ✅ All versions pass
🔌 Skill Tests (ovoscope)I've checked the skill's 'response time' to rapid fire questions. ⏱️ ✅ 4/40 passed ❌ **TestAdaptIntent** — 0/4
❌ **TestCancelIntentMidSentence** — 0/1
❌ **TestConverse** — 0/1
❌ **TestCountSkills** — 0/4
❌ **TestDeactivate** — 2/3
❌ **TestFallback** — 0/1
❌ **TestGlobalStopVocWithActiveSkill** — 0/1
❌ **TestGlobalStopVocabulary** — 0/2
❌ **TestIntentPipelineRouting** — 0/4
❌ **TestLangDisambiguation** — 0/4
❌ **TestLegacyIntentIdBackCompat** — 0/2
❌ **TestNoSkills** — 0/2
❌ **TestPadatiousIntent** — 0/4
❌ **TestStopNoSkills** — 0/3
❌ **TestStopServiceNotASkill** — 0/1
❌ **TestStopSkillCanHandleFalse** — 0/1
🚌 Bus CoverageQuantifying the invisible connections in your code. 💪 🔴 Coverage Summary
📊 Per-Skill Breakdown
🔍 Detailed Message Type Breakdown
|
…egration/full-compat rebuild v3
Summary
Split out of #808 per review. That PR's diff carried an unrelated change
to
ovos_core/intent_services/service.py(pipeline-blacklist matching)mixed in with its fallback ping/pong migration-window fix. This PR
contains only the blacklist-normalization change plus its test.
session.blacklisted_pipelineswas compared againstsession.pipelineas literal strings, but the deployment blacklist is naturally expressed
in installed plugin IDs (e.g.
ovos-adapt-pipeline-plugin) while asession pipeline carries confidence-suffixed matcher IDs (e.g.
ovos-adapt-pipeline-plugin-high). A base plugin policy ID silentlyfailed to block its suffixed matcher variants.
This normalizes both sides through
_PIPELINE_MIGRATION_MAPand stripsthe confidence suffix (
_PIPELINE_RE) before comparing, so ablacklisted base plugin ID blocks all of its matcher variants. Filtering
still happens before matcher lookup (no invocation, no bus event for the
skip) per OVOS-PIPELINE-1 §5.2.
Verified claims
directly against
origin/dev(not copied from the original Implement FALLBACK-1 skill-addressed polling #808commit as-is) and the new test was confirmed red against the
unpatched code and green after the fix (revert-code-keep-test
technique, no
git stash).ovos-coreunit suite (322 tests + 9 subtests) passes with thischange applied on top of
origin/dev.Test plan
test_plugin_blacklist_skips_all_confidence_matchers_before_lookupfails on unpatched
origin/devservice.py, passes after the fix.test/unittestssuite green (322 passed, 9 subtests passed).