fix: stale pre-0.75 runtime cache entries no longer break startup for SDK embedders - #1196
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughNative runtime cache discovery now preserves valid runtimes when other entries have invalid manifests or checksums. Startup logs skipped entries, and the runtime state route uses the lenient scan. ChangesNative runtime cache handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Startup
participant NativeRuntimeCache
participant Logger
participant RuntimeControlStateRoute
Startup->>NativeRuntimeCache: installed_lenient()
NativeRuntimeCache-->>Startup: valid runtimes and skipped entries
Startup->>Logger: log skipped entries
RuntimeControlStateRoute->>NativeRuntimeCache: installed_lenient()
NativeRuntimeCache-->>RuntimeControlStateRoute: valid runtimes
Possibly related PRs
Suggested labels: 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-native-runtime/src/cache.rs`:
- Around line 162-163: Update the runtime-directory scanning logic around the
manifest check to record a SkippedNativeRuntime containing the directory and an
explicit reason when the manifest is missing, not a regular file, or cannot be
read, instead of silently continuing. Use metadata/read errors to distinguish
inaccessible manifests from missing ones, and add a regression test covering a
runtime directory without manifest.json.
🪄 Autofix
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: 82da9166-a9bb-4c9a-9b7b-cda29754f900
📒 Files selected for processing (4)
crates/mesh-llm-host-runtime/src/api/routes/runtime_control_state_sources.rscrates/mesh-llm-host-runtime/src/system/native_runtime.rscrates/mesh-llm-native-runtime/src/cache.rscrates/mesh-llm-native-runtime/src/lib.rs
A native runtime cache entry written by a pre-0.75 loader (no per-file checksums, issue #1162) aborted the embedded startup path before the download/install fallback could run. The installed CLI was unaffected because it discovers its binary-adjacent bundle dir first and never reaches the strict full-cache scan; SDK embedders (no bundle dir) hit the strict scan and failed hard. Add NativeRuntimeCache::installed_lenient(), which enumerates the whole cache and collects unreadable entries as skipped instead of failing the scan. Startup runtime resolution now uses it and warns per skipped entry; the runtime-control backend options view uses it too so one stale entry no longer hides all installed backends.
5868579 to
bc4fb94
Compare
Address review feedback: a runtime directory whose manifest.json is missing, not a regular file, or unreadable is now recorded as a skipped entry with an explicit reason instead of being silently ignored, so the startup warning surfaces every unusable cache entry. Adds a regression test for a runtime directory without manifest.json.
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-native-runtime/src/cache.rs`:
- Around line 149-159: Update the directory traversal in installed_lenient to
skip and record cache-version directories whose read_dir fails, then continue
scanning other versions. Apply the same lenient handling to per-entry
enumeration and file_type() failures when the entry path is available,
preserving valid runtime discovery and fallback resolution.
🪄 Autofix
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: 05c66293-a892-4b23-bb38-d7bf32d4d703
📒 Files selected for processing (1)
crates/mesh-llm-native-runtime/src/cache.rs
i386
left a comment
i386
left a comment
There was a problem hiding this comment.
Reviewed and fixed the remaining cache-scan robustness issue. The lenient startup scan now skips unreadable cache-version directories, runtime-entry metadata failures, and iterator item failures where no path is available, while continuing to discover valid runtimes. The preceding PR commit already handles missing/non-regular manifests as skipped entries. Validated with cargo fmt --all -- --check, cargo check -p mesh-llm-native-runtime, cargo clippy -p mesh-llm-native-runtime --all-targets -- -D warnings, and cargo test -p mesh-llm-native-runtime --lib (28 passed). mesh-llm-host-runtime check is green; its broad Clippy run is currently blocked by pre-existing unrelated unfulfilled_lint_expectations warnings. Approving.
|
Desk-checked this from Buzz (SDK embedder) — it fixes the startup abort for us. Setup: pinned Buzz's mesh-llm deps to this branch's head ( Before (pinned to the Buzz has no executable-adjacent bundle dir (the desktop app doesn't ship one), so it After this branch: no abort, runtime resolved, and our end-to-end One extra step needed to test from a hash, worth noting for anyone else doing this: Setting What I did not verify: the new |
v0.75.1 carries the upstream fix (Mesh-LLM/mesh-llm#1196): startup now scans the native-runtime cache leniently (`installed_lenient`), skipping unusable entries instead of aborting the whole scan on the first one. That removes the blocker this branch has carried since the v0.75.0 bump. On a machine with a pre-0.75 cache, v0.75.0 failed with: native runtime artifact meshllm-native-runtime-darwin-aarch64-metal does not declare file checksums because caches written by older loaders have no per-file checksums, and the strict enumeration covered every version under the cache root. Verified on a machine that still has the offending entries in place (0.73.1, 0.74.0, 0.74.0-rc8 all with `runtime.files = 0`), with no bundle directory and no MESH_LLM_BUILD_VERSION override: - no checksum abort - the v0.75.1 runtime installed itself alongside the stale entries - crates/buzz-relay/examples/mesh_agent_e2e passed 4/4 (explicit-model chat, auto-model chat, oversized-budget rejection, agentic tool use) Pins move in both manifests (six mesh-llm crates in the desktop Tauri crate, two in buzz-relay) with both lockfiles refreshed. The v0.75.1 release publishes native-runtimes.json with 13 artifacts including darwin-aarch64-metal at Skippy ABI 0.1.35, which matches what the crates compile against. Signed-off-by: Michael Neale <michael.neale@gmail.com>
3 participants
Summary
Upgrading to 0.75.x with an existing native runtime cache no longer aborts startup for SDK embedders. A cache entry written by a pre-0.75 loader (manifest without per-file checksums) is now skipped with a warning during startup runtime resolution, instead of failing the whole cache scan before the download/install fallback can run.
This is the startup-path completion of #1170 (issue #1162). #1170 fixed the resolver's cache enumeration, but the startup fast-path in
mesh-llm-host-runtimestill used the strict full-cacheNativeRuntimeCache::installed(), where one stale manifest?-aborts everything.Who hits this
Anyone embedding the host runtime via the SDK (
initialize_host_runtime()) on a machine with a pre-0.75 runtime cache — observed during the Buzz desktop upgrade to v0.75.0:The installed CLI was unaffected by accident of packaging:
install.shships anative-runtimes/directory beside the binary, so startup takes the bundle-discovery branch (which reads manifests leniently) and never reaches the strict cache scan. SDK embedders have no adjacent bundle dir, fall intoresolve_installed_native_runtime_plan→cache.installed(), and failed hard — beforeallow_downloadcould rescue them.What changed
NativeRuntimeCache::installed_lenient()enumerates the whole cache and collects unreadable entries (missing/malformed/checksum-failing manifests) asskippedinstead of failing the scan.resolve_installed_native_runtime_plan) uses the lenient scan and emits atracing::warn!per skipped entry with path and reason.Validation
stale_pre_checksum_cache_entry_does_not_block_startup_plan(host runtime): a valid current-version runtime plus a pre-checksum 0.74.0 cache entry resolves to the valid runtime.installed_lenient_skips_legacy_manifest_and_keeps_valid_runtime(native-runtime crate): lenient scan returns the valid runtime, reports the stale one as skipped, and asserts the strict scan still errors on the same cache (documenting why startup must not use it).cargo test -p mesh-llm-host-runtime --lib— 1924 passed.cargo test -p mesh-llm-native-runtime --lib— 27 passed.cargo fmt --all --check,cargo clippy -p mesh-llm-native-runtime --all-targets -- -D warnings— clean. (-p mesh-llm-host-runtimeclippy has 28 pre-existing unfulfilled-lint-expectation errors on main, unchanged by this PR.)Protocol
No wire or protocol impact. Cache-directory compatibility only: pre-0.75 cache entries are now skipped (and pruned by existing maintenance) rather than fatal. No manifest format change.
Summary by CodeRabbit