fix(plugin-wasm): reorder namespaced_kind checks and disable relaxed-simd when simd off#513
Conversation
…simd when simd off
- Reorder checks in namespaced_kind so starts_with("core::") runs
before contains("::"), making the core-prefix error reachable.
- Disable relaxed-simd when enable_simd=false to prevent wasmtime
config conflict.
- Update pinning tests to assert the corrected behavior.
Closes #470
Closes #469
Signed-off-by: streamkit-devin <devin@streamkit.dev>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![staging-devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/870844?s=60&v=4){kind=small}
| if kind.starts_with(RESERVED_PREFIX) { | ||
| return Err(format!( | ||
| "Plugin kind '{kind}' contains '::' which is reserved for namespace prefixes. \ | ||
| Plugin kinds must be simple names like 'gain', 'reverb', etc." | ||
| "Plugin kind '{kind}' uses reserved prefix '{RESERVED_PREFIX}'. \ | ||
| This prefix is reserved for built-in core nodes." | ||
| )); | ||
| } | ||
|
|
||
| // Validate: reject if uses reserved prefix | ||
| if kind.starts_with(RESERVED_PREFIX) { | ||
| if kind.contains("::") { | ||
| return Err(format!( | ||
| "Plugin kind '{kind}' uses reserved prefix '{RESERVED_PREFIX}'. \ | ||
| This prefix is reserved for built-in core nodes." | ||
| "Plugin kind '{kind}' contains '::' which is reserved for namespace prefixes. \ | ||
| Plugin kinds must be simple names like 'gain', 'reverb', etc." | ||
| )); | ||
| } |
There was a problem hiding this comment.
📝 Info: core:: kinds now fail with the reserved-prefix error before the generic namespace check
The guard order in namespaced_kind now checks kind.starts_with("core::") before the broader kind.contains("::"), so core::audio remains rejected but receives the more specific reserved-prefix message. This is a behavior change only for diagnostics/logging from register_plugins; current registration behavior still skips the plugin either way, so I did not classify it as a bug.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| engine_config.wasm_simd(config.enable_simd); | ||
| if !config.enable_simd { | ||
| engine_config.wasm_relaxed_simd(false); | ||
| } | ||
| engine_config.wasm_threads(config.enable_threads); |
There was a problem hiding this comment.
📝 Info: Disabled SIMD now also disables relaxed SIMD while preserving the default feature set
The Wasmtime config still leaves relaxed SIMD at Wasmtime's default when enable_simd=true, but explicitly disables relaxed SIMD when base SIMD is disabled. That matches the exposed PluginRuntimeConfig::enable_simd contract and avoids changing default plugin compatibility for runtimes that still allow SIMD.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #513 +/- ##
=======================================
Coverage 79.39% 79.39%
=======================================
Files 232 232
Lines 66904 66906 +2
Branches 1909 1909
=======================================
+ Hits 53117 53123 +6
+ Misses 13781 13777 -4
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
2 participants
Summary
core::reserved-prefix branch is unreachable #470:namespaced_kindcheckedcontains("::")beforestarts_with("core::"), making the core-prefix error path dead code. Reordered so the more specificcore::check runs first, returning a distinct error.enable_simd=falsefailed because wasmtime enables relaxed-simd by default, which requires base SIMD. Now explicitly disableswasm_relaxed_simdwhenenable_simdis false.Closes #470
Closes #469
Review & Validation
namespaced_kind("core::audio")now returns the reserved-prefix error, not the generic namespace-separator error.PluginRuntime::newsucceeds withenable_simd: falsefor bothenable_threadsvalues.Link to Devin session: https://staging.itsdev.in/sessions/1a4ac010110d4e81bc6d1d4e21a96321
Requested by: @streamer45
Devin Review
64cd26e