ci: add cli draft release assets#124
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (3)**/Cargo.{toml,lock}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.toml📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/Cargo.toml📄 CodeRabbit inference engine (.agents/skills/prepare-code-freeze/SKILL.md)
Files:
🔇 Additional comments (1)
WalkthroughPackage job now builds platform-specific Rust CLI binaries using explicit target triples and stages versioned artifacts; Cargo metadata for bininstall added; a new workflow job downloads those ChangesCLI Release Packaging and Distribution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci_rust.yml (1)
156-163: 🧹 Nitpick | 🔵 TrivialRelease-tag validation belongs in the caller workflow.
The
Packagejob is a reusable workflow that builds CLI binaries for all invocations (tag and non-tag refs). Ensure the caller workflow (.github/workflows/ci.yaml) enforces release-tag validation and the non-alpha publish policy before uploading assets to GitHub Releases. As per coding guidelines, tag-triggered release workflows must fail early when a tag violates repo policy.🤖 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 @.github/workflows/ci_rust.yml around lines 156 - 163, The Package job currently runs for both tag and non-tag refs and should not perform release-tag validation; remove any tag-validation logic from the reusable "Package" job (e.g., avoid gating on tag patterns or publishing policy inside the job and keep the condition as uses inputs.run_package and needs.Test.result), and instead implement the release-tag validation and non-alpha publish policy checks in the caller workflow (ci.yaml) before it invokes the Package reusable workflow and before any GitHub Releases upload step; ensure the caller fails early on invalid tags and only calls Package when the tag passes policy.
🤖 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/cli/Cargo.toml`:
- Around line 16-18: Update the pkg-url template to use the correct variable for
direct binaries: replace the use of { archive-suffix } with { binary-ext } in
the package metadata block where pkg-url is defined (the keys pkg-url and
pkg-fmt in Cargo.toml); keep pkg-fmt = "bin" and ensure pkg-url becomes "{ repo
}/releases/download/{ version }/{ name }-{ target }-{ version }{ binary-ext }"
so Windows gets ".exe" and Unix gets an empty extension.
---
Outside diff comments:
In @.github/workflows/ci_rust.yml:
- Around line 156-163: The Package job currently runs for both tag and non-tag
refs and should not perform release-tag validation; remove any tag-validation
logic from the reusable "Package" job (e.g., avoid gating on tag patterns or
publishing policy inside the job and keep the condition as uses
inputs.run_package and needs.Test.result), and instead implement the release-tag
validation and non-alpha publish policy checks in the caller workflow (ci.yaml)
before it invokes the Package reusable workflow and before any GitHub Releases
upload step; ensure the caller fails early on invalid tags and only calls
Package when the tag passes policy.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: a75d0a5b-c4cd-4346-81ab-3309b82e65a3
📒 Files selected for processing (3)
.github/workflows/ci.yaml.github/workflows/ci_rust.ymlcrates/cli/Cargo.toml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check / Run
🧰 Additional context used
📓 Path-based instructions (7)
**/Cargo.{toml,lock}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run
cargo deny checkfor Rust dependency auditing as configured indeny.toml
Files:
crates/cli/Cargo.toml
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license headers in TOML files using TOML comment syntax
Files:
crates/cli/Cargo.toml
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/prepare-code-freeze/SKILL.md)
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>
Files:
crates/cli/Cargo.toml
.github/workflows/**/*.{yml,yaml}
📄 CodeRabbit inference engine (.agents/skills/maintain-ci/SKILL.md)
.github/workflows/**/*.{yml,yaml}: Putpermissions:on each job that needs token access in GitHub Actions workflows
Avoid workflow-level permissions unless the repository intentionally centralizes them and the inheritance tradeoff is documented
Keep third-party actions pinned to full commit SHAs and preserve the readable version comment after the SHA
Prefer action-native or ecosystem-native caching over genericactions/cachein GitHub Actions workflows
Use lockfiles or dependency manifests to drive cache invalidation in GitHub Actions workflows
Keep deploy and publish permissions isolated to the jobs that need them
Read both caller and callee when a workflow usesworkflow_callin GitHub Actions
Put release-tag validation in the earliest practical caller job when the pipeline has tag-based publish behavior
Keep release-tag policy aligned withRELEASING.md: raw SemVer tags only, no leadingv
contents: readis the default minimum for checkout-based build, test, docs, and packaging jobs
pull-requests: readis required for PR metadata lookup jobs in GitHub Actions workflows
pages: writeandid-token: writeshould be limited to Pages deployment jobs and any caller that invokes them through a reusable workflow
For reusable workflows, the caller must grant every permission the called jobs require and the callee cannot elevate beyond what the caller provides
Preferastral-sh/setup-uvcache support withcache-dependency-globanchored touv.lockfor Python dependency caching
PreferSwatinem/rust-cachewith explicitshared-keyandworkspacesinstead of ad hoc target-directory caching
Avoid caching generated outputs that can hide stale behavior unless the repo already relies on them deliberately
Ensure each job has the minimum permissions it needs during GitHub Actions CI review
Ensure reusable workflow callers grant only the scopes their callees require
Ensure every external action is pinned to a full SHA in GitHub Actions workflows
Ensure cache ...
Files:
.github/workflows/ci.yaml.github/workflows/ci_rust.yml
{.github/**/*.{yml,yaml},*.patch,scripts/**/*,*.sh,*.bat,Dockerfile*}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update CI configuration, patch files, and build scripts with new functional identifiers after rename operations
Files:
.github/workflows/ci.yaml.github/workflows/ci_rust.yml
{.github/workflows/*.{yml,yaml},.gitlab-ci.yml}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure CI workflow references match local package names and installation methods
Files:
.github/workflows/ci.yaml.github/workflows/ci_rust.yml
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}
⚙️ CodeRabbit configuration file
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}: Review automation changes for reproducibility, pinned versions where appropriate, secret handling, and consistency with the documented validation matrix.
Pay attention to commands that need generated native artifacts, FFI libraries, or platform-specific environment variables.
Files:
.github/workflows/ci.yaml.github/workflows/ci_rust.yml
🧠 Learnings (1)
📚 Learning: 2026-05-03T04:23:07.497Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Flow PR: 46
File: .github/workflows/ci_rust.yml:31-64
Timestamp: 2026-05-03T04:23:07.497Z
Learning: In GitHub Actions workflow YAML, it’s valid to conditionally disable a service container by setting the service container’s `image` to an empty string (`''`) via a matrix variable (e.g., `redis_service_image: ''`). This intentionally makes the runner skip service initialization for that matrix entry rather than failing the job. When reviewing workflows, don’t flag this as an error if the workflow uses an empty `image` to disable the service on specific matrix entries (e.g., OS-specific setups); verify the `image` is sourced from the matrix variable and that the service is only expected to be available when a non-empty image is provided.
Applied to files:
.github/workflows/ci_rust.yml
🔇 Additional comments (6)
.github/workflows/ci.yaml (1)
324-360: LGTM!.github/workflows/ci_rust.yml (5)
170-182: LGTM!
196-196: LGTM!
217-217: LGTM!
206-211: ⚡ Quick winThe workflow's
musl-toolsinstallation step requires verification against the full job matrix configuration. Web search confirms thatmusl-toolsmust be installed at runtime onubuntu-24.04runners and providesmusl-gccfor native architecture compilation. However,musl-toolsdoes not include cross-compiler support foraarch64targets onx86_64runners (or vice versa). Confirm whether the matrix builds targets natively only (each runner compiles for its own architecture) or includes cross-compilation scenarios. If cross-compilation toaarch64is needed onx86_64runners, additional toolchains beyondmusl-toolswill be required.
223-241: ⚡ Quick winVerify asset filename compatibility with binstall configuration.
The web search confirms that cargo-binstall's
{ archive-suffix }placeholder resolves to an empty string for both Windows and Unix whenpkg-fmt = "bin". However, the staging logic in lines 230–232 appends.exeonly on Windows. If the binstall metadata usespkg-fmt = "bin"with a template like{name}-{target}-{version}{archive-suffix}, the Windows asset would be staged asnemo-flow-cli-{target}-{version}.exebut binstall would seeknemo-flow-cli-{target}-{version}, causing a mismatch. Confirm the actual binstall configuration in the repository (check forpkg-fmtsetting in the workflow and any Cargo.toml binstall metadata) and ensure the asset filenames align with what the template expects on all platforms.
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
/merge |
Overview
Adds cargo-binstall-compatible direct CLI binary assets for non-alpha release tags, with Linux release binaries built against musl for wider compatibility. The draft release upload also publishes SHA-256 checksums for the direct binaries. Documentation changes were left out because they are handled in a separate PR.
Details
[package.metadata.binstall]metadata tonemo-flow-cliusing the direct binary asset URL pattern expected by cargo-binstall.nemo-flow-clifor the five release targets, using musl targets for Linux.nemo-flow-cli-<target>-<version>[.exe]direct binaries.softprops/action-gh-release.SHA256SUMSfrom downloadednemo-flow-cli-*assets, verifies it locally, creates one.sha256sidecar per CLI binary, and uploads the resultingrelease-assets/*set.Validation:
ruby -e 'require "yaml"; [".github/workflows/ci.yaml", ".github/workflows/ci_rust.yml"].each { |f| YAML.load_file(f) }; puts "workflow-yaml-ok"'cargo metadata --no-deps --format-version 1 >/dev/nullgit diff --check -- .github/workflows/ci.yaml .github/workflows/ci_rust.yml crates/cli/Cargo.tomluv run pre-commit run --files .github/workflows/ci.yaml .github/workflows/ci_rust.yml crates/cli/Cargo.tomluv run pre-commit run --files .github/workflows/ci.yamlcargo build --release --target aarch64-apple-darwin -p nemo-flow-cliWhere should the reviewer start?
Start with
.github/workflows/ci_rust.ymlfor the target-specific CLI binary packaging path, then.github/workflows/ci.yamlfor the draft release upload and checksum generation.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit