fix release build version stamping and smoke runtime install - #868
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughFour platform build scripts ( ChangesRelease Profile Version Stamping
CI Native Runtime Installer Replacement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
i386
changed the title
i386
marked this pull request as ready for review
June 18, 2026 01:39
ndizazzo
approved these changes
Jun 18, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/ci-install-native-runtime.sh`:
- Line 42: The mesh_version assignment on line 42 currently defaults to
"unknown" when the value is missing from the runtime dictionary, which masks
manifest contract breaks and causes incorrect cache directory usage. Remove the
fallback to "unknown" and instead raise an error or exception immediately when
mesh_version is not found in the runtime object, so the CI fails at the root
cause rather than silently continuing with an invalid version identifier.
- Around line 51-54: The validation loop for libraries in the native runtime
bundle currently only checks if a library file exists, but does not verify that
the resolved path stays within the intended bundle root directory. This allows
absolute paths or parent directory traversal (like `..`) to potentially
reference files outside the bundle. Modify the validation in the for loop to
first resolve the library_path to its absolute canonical form, then verify that
the resolved path is within the source directory before checking if the file
exists. If the resolved path escapes the bundle root, raise a SystemExit error
indicating that the library path is outside the allowed bundle directory.
🪄 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: 0b849935-72da-4caa-802b-f6832c8339e3
📒 Files selected for processing (5)
scripts/build-linux.shscripts/build-mac.shscripts/build-release.shscripts/build-windows.ps1scripts/ci-install-native-runtime.sh
|
|
||
| runtime = manifest["runtime"] | ||
| runtime_id = runtime["id"] | ||
| mesh_version = runtime.get("mesh_version") or "unknown" |
Contributor
There was a problem hiding this comment.
Fail fast when mesh_version is missing instead of defaulting to "unknown"
On Line 42, falling back to "unknown" hides manifest contract breaks and can install into the wrong cache subtree (<cache>/unknown/<runtime_id>). This should error immediately so CI fails at root cause.
Proposed fix
-mesh_version = runtime.get("mesh_version") or "unknown"
+mesh_version = runtime.get("mesh_version")
+if mesh_version is None:
+ raise SystemExit(f"native runtime mesh_version is missing in {manifest_path}")🤖 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 `@scripts/ci-install-native-runtime.sh` at line 42, The mesh_version assignment
on line 42 currently defaults to "unknown" when the value is missing from the
runtime dictionary, which masks manifest contract breaks and causes incorrect
cache directory usage. Remove the fallback to "unknown" and instead raise an
error or exception immediately when mesh_version is not found in the runtime
object, so the CI fails at the root cause rather than silently continuing with
an invalid version identifier.
Comment on lines
+51
to
+54
| for library in libraries: | ||
| library_path = source / library | ||
| if not library_path.is_file(): | ||
| raise SystemExit(f"native runtime library is missing: {library_path}") |
Contributor
There was a problem hiding this comment.
Constrain library paths to stay within the runtime bundle root
On Lines 51-54, source / library allows absolute paths and parent traversal (..) to pass validation if those external files exist. That weakens the manifest integrity check for the bundle itself.
Proposed fix
for library in libraries:
- library_path = source / library
- if not library_path.is_file():
+ library_path = (source / library).resolve()
+ if source.resolve() not in library_path.parents:
+ raise SystemExit(f"native runtime library escapes bundle root: {library}")
+ if not library_path.is_file():
raise SystemExit(f"native runtime library is missing: {library_path}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for library in libraries: | |
| library_path = source / library | |
| if not library_path.is_file(): | |
| raise SystemExit(f"native runtime library is missing: {library_path}") | |
| for library in libraries: | |
| library_path = (source / library).resolve() | |
| if source.resolve() not in library_path.parents: | |
| raise SystemExit(f"native runtime library escapes bundle root: {library}") | |
| if not library_path.is_file(): | |
| raise SystemExit(f"native runtime library is missing: {library_path}") |
🤖 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 `@scripts/ci-install-native-runtime.sh` around lines 51 - 54, The validation
loop for libraries in the native runtime bundle currently only checks if a
library file exists, but does not verify that the resolved path stays within the
intended bundle root directory. This allows absolute paths or parent directory
traversal (like `..`) to potentially reference files outside the bundle. Modify
the validation in the for loop to first resolve the library_path to its absolute
canonical form, then verify that the resolved path is within the source
directory before checking if the file exists. If the resolved path escapes the
bundle root, raise a SystemExit error indicating that the library path is
outside the allowed bundle directory.
This was referenced Aug 10, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix the release failures exposed by the workflow-dispatch release run:
MESH_LLM_BUILD_VERSIONto the plain Cargo package version instead of deriving+g<sha>[.dirty]metadatamesh-llm runtime installRoot Cause
PR #831 introduced separate build/display identity and release identity, but the release build scripts still derived SHA-bearing build versions. In
workflow_dispatchrelease jobs,scripts/release-version.shmutates tracked manifests before the build, so Windows release binaries reported versions like0.72.0-rc2+g7E9D61.dirty;package-release.ps1correctly rejected those artifacts because release bundles must report0.72.0-rc2exactly.The smoke failure was separate:
scripts/ci-install-native-runtime.shused the just-builtmesh-llmbinary to runruntime install. That command runs inside the binary's Tokio runtime and tripped the nested-runtime guard before inference could start.Validation
bash -n scripts/build-release.sh scripts/build-linux.sh scripts/ci-install-native-runtime.shzsh -n scripts/build-mac.shgit diff --checkcargo run -p xtask -- repo-consistency release-targetscache/<mesh_version>/<runtime_id>/manifest.jsonand the runtime librarySummary by CodeRabbit