Auto-load the native runtime for SDK/embedded serving on startup - #1018
Auto-load the native runtime for SDK/embedded serving on startup#1018michaelneale wants to merge 1 commit into
Conversation
Embedded/SDK serving (mesh-llm-sdk "serving" feature, start_embedded_node/ start_embedded_serve) went through run_embedded_runtime, which never ran the native-runtime bootstrap that the shipped binary runs before serving. As a result, an application that upgraded its mesh-llm-sdk dependency linked a new Skippy ABI / release version but never loaded or auto-installed the matching native runtime, so serving failed later at the Skippy FFI load unless the app manually called install_native_runtime() first. run_embedded_runtime now performs the same initialize_host_runtime_with_config bootstrap as the binary, honoring the embedded config so a configured [runtime.native_runtime] override still applies. The call is routed through a small injectable seam (bootstrap_embedded_native_runtime) so the config-path and failure-propagation contract is covered by unit tests. Fixes #1016
📝 WalkthroughWalkthroughThe embedded serving path now bootstraps the native runtime before invoking ChangesEmbedded runtime startup
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/mesh-llm-host-runtime/src/runtime/mod.rs`:
- Around line 3585-3601: Extract bootstrap_embedded_native_runtime and its
associated embedded-runtime tests from the runtime module into a dedicated
embedded module, such as embedded.rs, and expose or import the symbols needed by
the existing callers. Preserve the current initializer injection, config_path
propagation, behavior, and test coverage while reducing the oversized runtime
file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 47c72b48-d30b-4379-90c5-9e193a914017
📒 Files selected for processing (1)
crates/mesh-llm-host-runtime/src/runtime/mod.rs
| /// Run the native-runtime bootstrap for the embedded/SDK serving path, deriving | ||
| /// the config path from the embedded options so a configured | ||
| /// `[runtime.native_runtime]` override is honored. | ||
| /// | ||
| /// The initializer is injected so the ordering/config-path contract can be | ||
| /// exercised in tests without standing up a full runtime. | ||
| async fn bootstrap_embedded_native_runtime<Init, Fut>( | ||
| options: &EmbeddedRuntimeOptions, | ||
| initialize: Init, | ||
| ) -> Result<()> | ||
| where | ||
| Init: FnOnce(Option<PathBuf>) -> Fut, | ||
| Fut: std::future::Future<Output = Result<()>>, | ||
| { | ||
| initialize(options.config_path.clone()).await | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Extract this logic into a new module to comply with file size limits.
As per coding guidelines, "Do not add Rust source files over 2,000 lines. If a file is approaching that size, split it by responsibility into an owning module instead of adding more code to the oversized file." This file is currently over 3,800 lines.
Please extract this embedded runtime logic and its associated tests into a separate module (e.g., crates/mesh-llm-host-runtime/src/runtime/embedded.rs) to prevent this file from growing further.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/mesh-llm-host-runtime/src/runtime/mod.rs` around lines 3585 - 3601,
Extract bootstrap_embedded_native_runtime and its associated embedded-runtime
tests from the runtime module into a dedicated embedded module, such as
embedded.rs, and expose or import the symbols needed by the existing callers.
Preserve the current initializer injection, config_path propagation, behavior,
and test coverage while reducing the oversized runtime file.
Source: Coding guidelines
|
Closing. On review this isn't needed: real SDK consumers (e.g. buzz) already handle upgrades correctly by checking the cache for a runtime matching CURRENT_MESH_VERSION and failing with a clear message + explicit install step. The original auto-download-on-startup approach was wrong (a library shouldn't do implicit network I/O on startup), and the revised deny-by-default variant is only a minor DX nicety, not a fix for any real breakage. Not worth the config-schema addition and cross-crate plumbing. |
1 participant
What you can now do
If you embed MeshLLM as a library (
mesh-llm-sdkwith theservingfeature —start_embedded_node/start_embedded_serve/MeshNode), the correct nativeruntime is now loaded — and auto-installed if missing — automatically when the
embedded node starts. When you bump your
mesh-llm-sdkdependency to a newversion, the matching upgraded native runtime is fetched and loaded on the next
start, just like it already is for the shipped
mesh-llmbinary. You no longerhave to call
install_native_runtime()yourself before serving.The problem
Native runtimes are release artifacts selected by exact Skippy ABI +
platform/backend, and the compiled-in release version drives the versioned
release-manifest URL and cache layout. The bootstrap that makes upgrades "just
work" (
initialize_host_runtime_with_config) loads a compatible cached runtimeor does a one-shot auto-install of the right one.
The shipped binary runs that bootstrap before serving. The SDK/embedded serving
path (
start_embedded_node→run_embedded_runtime→run_runtime_cli) neverdid — it only reset tracing and the shutdown flag. So an app that upgraded its
mesh-llm-sdkdependency linked a new Skippy ABI / release version but neverloaded or auto-installed the matching runtime, and serving failed later at the
Skippy FFI load unless the app manually installed the runtime first (as the SDK
README documented).
Fixes #1016.
Change
run_embedded_runtimenow runs the sameinitialize_host_runtime_with_configbootstrap as the binary before serving, deriving the config path from the
embedded options so a configured
[runtime.native_runtime]override stillapplies. The call is routed through a small injectable seam
(
bootstrap_embedded_native_runtime) so behavior can be unit-tested withoutstanding up a full runtime.
Validation
cargo fmt --all --check— cleancargo clippy -p mesh-llm-host-runtime --all-targets -- -D warnings— cleancargo clippy -p mesh-llm --all-targets -- -D warnings— cleancargo test -p mesh-llm-host-runtime --lib embedded_native_runtime_bootstrap— 3 new tests passSummary by CodeRabbit