ci: parallelize and strengthen package validation - #721
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
github-actions
Bot
added
size:L
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe CI workflows now use direct change outputs, fail-fast platform matrices, pinned musllinux testing, locked builds, and dedicated smoke-test jobs for Node.js, Python, and Rust artifacts. ChangesCI orchestration
Node.js packaging
Python packaging
Rust packaging
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PackageJobs as Package Jobs
participant Artifacts as GitHub Artifacts
participant PackageSmoke as Package Smoke Jobs
participant Runtime as Platform Runtime
PackageJobs->>Artifacts: upload platform artifacts
PackageSmoke->>Artifacts: download matching artifacts
PackageSmoke->>Runtime: install or execute artifacts
Runtime-->>PackageSmoke: return import or CLI validation results
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 @.github/workflows/ci_rust.yml:
- Around line 120-125: Add the musl exclusion condition to the
Swatinem/rust-cache step in the Test job at
.github/workflows/ci_rust.yml:120-125, .github/workflows/ci_node.yml:89-95, and
.github/workflows/ci_python.yml:89-94, matching the neighboring Rust and uv
steps so the cache runs only when matrix.platform is not musl.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: a5255897-ac2e-4478-805e-316f4548bc82
📒 Files selected for processing (8)
.github/workflows/ci.yaml.github/workflows/ci_check.yml.github/workflows/ci_go.yml.github/workflows/ci_node.yml.github/workflows/ci_python.yml.github/workflows/ci_rust.ymlscripts/package_node_musllinux.mjsscripts/package_python_musllinux.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
**/*: Use release tags in raw Rust-compatible SemVer without a leadingv; tags such asv0.1.0are prohibited.
Use branch prefixesfeat/,fix/,docs/,test/, orrefactor/according to the change purpose.
Every commit in a pull request must include a DCOSigned-off-by:sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latestmainare complete.
Use commit messages in the formtype: short description, with a valid type and a first line under 72 characters.
Files:
scripts/package_python_musllinux.pyscripts/package_node_musllinux.mjs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
scripts/package_python_musllinux.py
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
scripts/package_python_musllinux.py
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
scripts/package_python_musllinux.py
**/*.{rs,py,js,jsx,ts,tsx,go,c,h,cc,cpp,md,toml,yml,yaml,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, documentation, scripts, and configuration files; the project is Apache-2.0.
Files:
scripts/package_python_musllinux.py
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Use
snake_casenaming in Rust and Python.
Files:
scripts/package_python_musllinux.py
**/*.{rs,py,js,mjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Preserve the existing Tokio-based asynchronous model and callback/future lifetimes; do not unexpectedly block or hide async work in bindings.
Files:
scripts/package_python_musllinux.pyscripts/package_node_musllinux.mjs
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: Lint Python with Ruff using rule setsE,F,W, andI.
Format Python with the Ruff formatter, using a 120-character line length and double quotes.
Runtyfor Python type checking.
Use Pythonsnake_casenaming conventions.
Files:
scripts/package_python_musllinux.py
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,py,go,js,jsx,ts,tsx,c,h}: Run tests for every language affected by a change; changes to the core Rust crate require tests across all bindings.
UseSONAR_IGNORE_START/SONAR_IGNORE_ENDonly for documented false positives, keep ignored blocks minimal, explain them with a comment, and obtain reviewer sign-off.
Preserve the layered architecture in which Rust provides the core runtime and C FFI, PyO3, and NAPI provide bindings that mirror the full API surface.
Files:
scripts/package_python_musllinux.py
**/*.{rs,py,go,js,jsx,ts,tsx,c,h,html,md,mdx,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include the appropriate SPDX copyright and Apache-2.0 license header in every source file.
Files:
scripts/package_python_musllinux.py
{.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:
scripts/package_python_musllinux.py.github/workflows/ci_go.yml.github/workflows/ci_check.ymlscripts/package_node_musllinux.mjs.github/workflows/ci.yaml.github/workflows/ci_node.yml.github/workflows/ci_python.yml.github/workflows/ci_rust.yml
.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.
Avoid workflow-levelpermissions: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/cache.
Use lockfiles or dependency manifests to drive cache invalidation.
Keep deploy and publish permissions isolated to the jobs that need them.
Read both caller and callee when a workflow usesworkflow_call.
Put release-tag validation in the earliest practical caller job when the pipeline has tag-based publish behavior.
Keep pure-Python plugin SDK packaging as a single wheel artifact instead of duplicating it across every platform matrix entry.
contents: readis the default minimum for checkout-based build, test, docs, and packaging jobs.
pull-requests: readis required for PR metadata lookup jobs.
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; the callee cannot elevate beyond what the caller provides.
Preferastral-sh/setup-uvcache support withcache-dependency-globanchored touv.lock.
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.Keep CI workflow commands and package references in GitHub Actions workflows aligned with the current install, build, and example commands.
Files:
.github/workflows/ci_go.yml.github/workflows/ci_check.yml.github/workflows/ci.yaml.github/workflows/ci_node.yml.github/workflows/ci_python.yml.github/workflows/ci_rust.yml
{justfile,codecov.yml,codecov.yaml,.github/workflows/**/*.yml,.github/workflows/**/*.yaml}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
justfile, Codecov, and CI package/test workflows must include new plugin crates and packages.
Files:
.github/workflows/ci_go.yml.github/workflows/ci_check.yml.github/workflows/ci.yaml.github/workflows/ci_node.yml.github/workflows/ci_python.yml.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_go.yml.github/workflows/ci_check.yml.github/workflows/ci_node.yml.github/workflows/ci_python.yml.github/workflows/ci_rust.yml
🪛 zizmor (1.29.0)
.github/workflows/ci_node.yml
[warning] 109-109: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
.github/workflows/ci_python.yml
[warning] 150-150: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 199-199: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 524-524: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 538-538: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 574-574: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 574-574: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 575-575: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 622-622: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
.github/workflows/ci_rust.yml
[warning] 345-345: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 361-361: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 474-474: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 499-499: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 520-520: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 520-520: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 521-521: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🔇 Additional comments (10)
.github/workflows/ci.yaml (1)
128-128: LGTM!Also applies to: 144-146, 156-168, 181-191
.github/workflows/ci_check.yml (1)
47-67: LGTM!Also applies to: 78-93, 109-109, 158-158
.github/workflows/ci_go.yml (1)
31-31: LGTM!Also applies to: 64-64
.github/workflows/ci_node.yml (2)
50-73: LGTM!Also applies to: 153-166, 472-479
126-133: 📐 Maintainability & Code QualityKeep the test step unchanged.
npm test --workspace=nemo-relay-noderunspretest, which invokesnpm run build-debugand builds the native addon.> Likely an incorrect or invalid review comment.scripts/package_node_musllinux.mjs (1)
5-16: LGTM!Also applies to: 25-39, 41-71
.github/workflows/ci_python.yml (2)
50-73: LGTM!Also applies to: 217-235, 329-342, 474-545, 546-583, 585-658
182-186: 📐 Maintainability & Code QualityNo issue:
maturin develop --skip-installbuildsnemo_relay._nativeinpython/nemo_relay, andPYTHONPATH=pythonmakes it importable by pytest.> Likely an incorrect or invalid review comment..github/workflows/ci_rust.yml (1)
58-58: LGTM!Also applies to: 91-104, 193-216, 231-281, 345-345, 361-361, 421-537
scripts/package_python_musllinux.py (1)
85-85: 🩺 Stability & AvailabilityRemove this verification request.
copy_sourcedoes not ignoreCargo.lock, which is present at the repository root and is copied intobuild_source;--lockedcan use it.> Likely an incorrect or invalid review comment.
Signed-off-by: Will Killian <wkillian@nvidia.com>
github-actions
Bot
added
size:XL
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 (3)
.github/workflows/ci_python.yml (1)
592-603: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe sdist smoke test resolves the build backend from PyPI without a version pin.
uv pip install "$sdist"uses build isolation, sopip/uvdownloads thematurinbuild backend at run time. The musl wheel path pins maturin throughNEMO_RELAY_MATURIN_VERSIONderived fromuv.lock. Reuse that derivation here to keep the sdist build reproducible and to avoid breakage from a new maturin release.🤖 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_python.yml around lines 592 - 603, Update the “Build, install, and import Python source distribution” step to derive the pinned maturin version from uv.lock using the existing NEMO_RELAY_MATURIN_VERSION mechanism, then pass that version into the sdist build environment so build isolation cannot resolve an unpinned backend from PyPI. Preserve the existing installation and import smoke-test flow..github/workflows/ci_node.yml (1)
111-134: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winRun the native build explicitly before the musl tests.
npm testdoes not runpretestwhen--ignore-scriptsis active. Runnpm run build-debugbeforenpm test.🤖 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_node.yml around lines 111 - 134, Update the musllinux test command block after npm ci to explicitly run npm run build-debug before npm test, since --ignore-scripts prevents the pretest build from running. Keep the existing workspace targeting and test command unchanged..github/workflows/ci_rust.yml (1)
346-346: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPackaging builds do not use
--locked. Both CLI release builds resolve dependencies without honoringCargo.lock, so a drifted lock file can be rewritten during packaging and shipped artifacts can differ from the locked dependency set. The musl test step at line 215 already usescargo test --locked.
.github/workflows/ci_rust.yml#L346-L346: add--lockedto the hostcargo build --release --target "${{ matrix.target }}" -p nemo-relay-cli..github/workflows/ci_rust.yml#L362-L362: add--lockedto the samecargo buildinvocation inside the manylinux container.🤖 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 at line 346, Update both nemo-relay-cli packaging build invocations in .github/workflows/ci_rust.yml at lines 346-346 and 362-362 to include --locked with cargo build --release --target "${{ matrix.target }}" -p nemo-relay-cli, ensuring host and manylinux builds use Cargo.lock.Source: Path instructions
🤖 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 @.github/workflows/ci_node.yml:
- Around line 445-482: Tighten the native tarball lookup in the musl smoke-test
shell block by matching the same nemo-relay-node-npm-*-*-[0-9]*.tgz pattern used
by the non-musl path. Add the corresponding native-package count assertion
before installation, preserving the existing metapackage validation and install
flow.
---
Outside diff comments:
In @.github/workflows/ci_node.yml:
- Around line 111-134: Update the musllinux test command block after npm ci to
explicitly run npm run build-debug before npm test, since --ignore-scripts
prevents the pretest build from running. Keep the existing workspace targeting
and test command unchanged.
In @.github/workflows/ci_python.yml:
- Around line 592-603: Update the “Build, install, and import Python source
distribution” step to derive the pinned maturin version from uv.lock using the
existing NEMO_RELAY_MATURIN_VERSION mechanism, then pass that version into the
sdist build environment so build isolation cannot resolve an unpinned backend
from PyPI. Preserve the existing installation and import smoke-test flow.
In @.github/workflows/ci_rust.yml:
- Line 346: Update both nemo-relay-cli packaging build invocations in
.github/workflows/ci_rust.yml at lines 346-346 and 362-362 to include --locked
with cargo build --release --target "${{ matrix.target }}" -p nemo-relay-cli,
ensuring host and manylinux builds use Cargo.lock.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 06d82347-dfd4-4241-b4a2-31ed8fa0b048
📒 Files selected for processing (3)
.github/workflows/ci_node.yml.github/workflows/ci_python.yml.github/workflows/ci_rust.yml
📜 Review details
⏰ Context from checks skipped due to timeout. (45)
- GitHub Check: Node.js / Package (windows-arm64)
- GitHub Check: Node.js / Package (linux-musl-arm64)
- GitHub Check: Node.js / Test (linux-musl-arm64)
- GitHub Check: Node.js / Package (linux-musl-amd64)
- GitHub Check: Node.js / Package (linux-arm64)
- GitHub Check: Python / Test (macos-arm64)
- GitHub Check: Node.js / Package (macos-arm64)
- GitHub Check: Python / Test (linux-arm64)
- GitHub Check: Node.js / Test (macos-arm64)
- GitHub Check: Python / Test (windows-arm64)
- GitHub Check: Node.js / Test (windows-arm64)
- GitHub Check: Python / Test (linux-musl-amd64)
- GitHub Check: Rust / Package (windows-amd64)
- GitHub Check: Node.js / Test (windows-amd64)
- GitHub Check: Python / Package (macos-arm64)
- GitHub Check: Python / Test (windows-amd64)
- GitHub Check: Node.js / Test (linux-arm64)
- GitHub Check: Python / Test (linux-musl-arm64)
- GitHub Check: Python / Package (windows-arm64)
- GitHub Check: Rust / Package (linux-arm64)
- GitHub Check: Python / Package (linux-amd64)
- GitHub Check: Rust / Test (linux-musl-amd64)
- GitHub Check: Rust / Test (macos-arm64)
- GitHub Check: Rust / Test (windows-arm64)
- GitHub Check: Python / Package (linux-musl-arm64)
- GitHub Check: Rust / Test (linux-musl-arm64)
- GitHub Check: Check / Install script (windows-x86_64)
- GitHub Check: Rust / Package (linux-musl-amd64)
- GitHub Check: Python / Package (windows-amd64)
- GitHub Check: Rust / Package (macos-arm64)
- GitHub Check: Rust / Test (linux-arm64)
- GitHub Check: Rust / Package (linux-musl-arm64)
- GitHub Check: Rust / Test (windows-amd64)
- GitHub Check: Rust / Test (linux-amd64)
- GitHub Check: Rust / Package (linux-amd64)
- GitHub Check: Rust / Package (windows-arm64)
- GitHub Check: Check / Install script (windows-ARM64)
- GitHub Check: Go / Test (linux-arm64)
- GitHub Check: Go / Test (linux-amd64)
- GitHub Check: Node.js / Package OpenClaw plugin
- GitHub Check: Check / Install script (macos-arm64)
- GitHub Check: Go / Test (macos-arm64)
- GitHub Check: Check / Run
- GitHub Check: Go / Test (windows-amd64)
- GitHub Check: Go / Test (windows-arm64)
🧰 Additional context used
📓 Path-based instructions (3)
.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.
Avoid workflow-levelpermissions: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/cache.
Use lockfiles or dependency manifests to drive cache invalidation.
Keep deploy and publish permissions isolated to the jobs that need them.
Read both caller and callee when a workflow usesworkflow_call.
Put release-tag validation in the earliest practical caller job when the pipeline has tag-based publish behavior.
Keep pure-Python plugin SDK packaging as a single wheel artifact instead of duplicating it across every platform matrix entry.
contents: readis the default minimum for checkout-based build, test, docs, and packaging jobs.
pull-requests: readis required for PR metadata lookup jobs.
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; the callee cannot elevate beyond what the caller provides.
Preferastral-sh/setup-uvcache support withcache-dependency-globanchored touv.lock.
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.Keep CI workflow commands and package references in GitHub Actions workflows aligned with the current install, build, and example commands.
Files:
.github/workflows/ci_rust.yml.github/workflows/ci_node.yml.github/workflows/ci_python.yml
{justfile,codecov.yml,codecov.yaml,.github/workflows/**/*.yml,.github/workflows/**/*.yaml}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
justfile, Codecov, and CI package/test workflows must include new plugin crates and packages.
Files:
.github/workflows/ci_rust.yml.github/workflows/ci_node.yml.github/workflows/ci_python.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_rust.yml.github/workflows/ci_node.yml.github/workflows/ci_python.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.github/workflows/ci_node.yml.github/workflows/ci_python.yml
🪛 zizmor (1.29.0)
.github/workflows/ci_rust.yml
[warning] 346-346: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 362-362: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
.github/workflows/ci_node.yml
[warning] 451-451: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 451-451: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 452-452: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 453-453: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 454-454: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 455-455: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
.github/workflows/ci_python.yml
[info] 294-294: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 405-405: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 417-417: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 417-417: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 418-418: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 443-443: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 443-443: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 445-445: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 529-529: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🔇 Additional comments (12)
.github/workflows/ci_node.yml (3)
89-95:Swatinem/rust-cachestill lacks the musl guard in theTestjob.The neighboring Rust,
taiki-e/install-action, andsetup-nodesteps are guarded with!contains(matrix.platform, 'musl'), but this cache step is not. On musl entries the hosttargetdirectory is never populated by a host build, so the step saves an unusablenemo-relay-rust-linux-musl-*cache entry. The same gap exists in.github/workflows/ci_python.yml(lines 89-94) and.github/workflows/ci_rust.yml(lines 120-125).
47-73: LGTM!
154-179: LGTM!Also applies to: 190-211, 221-236, 250-250, 263-270, 281-281, 295-304
.github/workflows/ci_rust.yml (5)
120-125:Swatinem/rust-cachestill lacks the musl guard.Lines 115, 128, 165, and 170 all guard on
!contains(matrix.platform, 'musl'). This cache step does not, so musl entries write a cache keyednemo-relay-rust-linux-musl-*that no host build ever populates.
91-104: LGTM!
193-217: LGTM!
230-282: LGTM!
408-420: LGTM!.github/workflows/ci_python.yml (4)
152-187: LGTM!
407-436: LGTM!Also applies to: 438-447, 449-455
457-478: LGTM!Also applies to: 528-539, 553-576, 578-593, 606-606
383-399: 🩺 Stability & AvailabilityNo change required.
NEMO_RELAY_CI_WORKSPACEis${{ github.workspace }}, souv.lockresolves from the repository root.> Likely an incorrect or invalid review comment.
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci_python.yml (2)
170-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftPin the musllinux bootstrap inputs.
The container images and requested tool versions are pinned, but
apk addresolves current repository versions andhttps://sh.rustup.rsdownloads the current installer. Later runs can therefore use different native build inputs without a repository change. Pin the APK package set and rustup installer, or move these tools into a digest-pinned builder image. The rustup installer supportsRUSTUP_VERSIONfor installer-version pinning. (github.com)
.github/workflows/ci_python.yml#L170-L172: pin the APK packages and rustup installer used by Python musllinux tests..github/workflows/ci_rust.yml#L209-L211: apply the same pins to Rust musllinux tests.🤖 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_python.yml around lines 170 - 172, Pin the musllinux bootstrap dependencies in both .github/workflows/ci_python.yml lines 170-172 and .github/workflows/ci_rust.yml lines 209-211: make every apk package resolve to a fixed version or use an equivalent pinned builder image, and pin the rustup installer version via RUSTUP_VERSION (or an equivalent versioned installer URL) while preserving NEMO_RELAY_RUST_VERSION for the requested Rust toolchain.Source: Path instructions
178-183: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse the committed lockfile for the musllinux Python test.
When project metadata differs,
uv syncrelocks by default. Add--lockedso the step fails instead of resolving a different dependency set.maturin develop --lockeddoes not constrain this Python resolution.🤖 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_python.yml around lines 178 - 183, Update the uv sync invocation in the musllinux Python test step to include the --locked option, ensuring it uses the committed lockfile and fails when project metadata is inconsistent. Do not rely on maturin develop --locked, since it does not constrain this dependency resolution.
🤖 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.
Outside diff comments:
In @.github/workflows/ci_python.yml:
- Around line 170-172: Pin the musllinux bootstrap dependencies in both
.github/workflows/ci_python.yml lines 170-172 and .github/workflows/ci_rust.yml
lines 209-211: make every apk package resolve to a fixed version or use an
equivalent pinned builder image, and pin the rustup installer version via
RUSTUP_VERSION (or an equivalent versioned installer URL) while preserving
NEMO_RELAY_RUST_VERSION for the requested Rust toolchain.
- Around line 178-183: Update the uv sync invocation in the musllinux Python
test step to include the --locked option, ensuring it uses the committed
lockfile and fails when project metadata is inconsistent. Do not rely on maturin
develop --locked, since it does not constrain this dependency resolution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d75add02-a3f3-4123-8b2d-748233be0258
📒 Files selected for processing (2)
.github/workflows/ci_python.yml.github/workflows/ci_rust.yml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
.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.
Avoid workflow-levelpermissions: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/cache.
Use lockfiles or dependency manifests to drive cache invalidation.
Keep deploy and publish permissions isolated to the jobs that need them.
Read both caller and callee when a workflow usesworkflow_call.
Put release-tag validation in the earliest practical caller job when the pipeline has tag-based publish behavior.
Keep pure-Python plugin SDK packaging as a single wheel artifact instead of duplicating it across every platform matrix entry.
contents: readis the default minimum for checkout-based build, test, docs, and packaging jobs.
pull-requests: readis required for PR metadata lookup jobs.
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; the callee cannot elevate beyond what the caller provides.
Preferastral-sh/setup-uvcache support withcache-dependency-globanchored touv.lock.
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.Keep CI workflow commands and package references in GitHub Actions workflows aligned with the current install, build, and example commands.
Files:
.github/workflows/ci_python.yml.github/workflows/ci_rust.yml
{justfile,codecov.yml,codecov.yaml,.github/workflows/**/*.yml,.github/workflows/**/*.yaml}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
justfile, Codecov, and CI package/test workflows must include new plugin crates and packages.
Files:
.github/workflows/ci_python.yml.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_python.yml.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_python.yml.github/workflows/ci_rust.yml
🔇 Additional comments (3)
.github/workflows/ci_rust.yml (2)
120-125: Duplicate: skipSwatinem/rust-cachefor musllinux jobs.The cache step runs before each musllinux job installs Rust inside Docker. The action caches
./targetand uses the current host/toolchain context for its cache key, so it can restore or save artifacts that do not match the container-local musl toolchain. Addif: ${{ !contains(matrix.platform, 'musl') }}to both steps. (raw.githubusercontent.com)
.github/workflows/ci_rust.yml#L120-L125: add the musllinux exclusion to the Rust test cache step..github/workflows/ci_python.yml#L89-L94: add the same exclusion to the Python test cache step.
114-118: LGTM!Also applies to: 127-170, 193-208, 212-229
.github/workflows/ci_python.yml (1)
47-73: LGTM!Also applies to: 83-87, 96-106, 108-145, 147-169, 173-177, 184-188, 199-217
Signed-off-by: Will Killian <wkillian@nvidia.com>
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 @.github/workflows/ci_rust.yml:
- Around line 215-228: Update the CI setup block to verify the rustup installer,
just archive, cargo-nextest archive, and uv installer against pinned checksums
or signatures before executing them. Download each artifact to a temporary file,
validate it, then pass only the verified files to sh or tar; preserve the
existing installation paths and version variables.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 93a7e117-e00d-4cc5-ba84-3ef10e829c43
📒 Files selected for processing (2)
.github/workflows/ci_rust.ymljustfile
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
{justfile,codecov.yml,codecov.yaml,.github/workflows/**/*.yml,.github/workflows/**/*.yaml}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
justfile, Codecov, and CI package/test workflows must include new plugin crates and packages.
Files:
justfile.github/workflows/ci_rust.yml
justfile
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Keep
justfilebuild, test, clean, version, and package recipes for plugin crates and packages aligned with the current packaging layout.
Files:
justfile
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
**/*: Use release tags in raw Rust-compatible SemVer without a leadingv; tags such asv0.1.0are prohibited.
Use branch prefixesfeat/,fix/,docs/,test/, orrefactor/according to the change purpose.
Every commit in a pull request must include a DCOSigned-off-by:sign-off.
Before submitting a pull request, ensure pre-commit hooks, relevant tests, target-specific builds, documentation updates, and a rebase on the latestmainare complete.
Use commit messages in the formtype: short description, with a valid type and a first line under 72 characters.
Files:
justfile
{.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:
justfile.github/workflows/ci_rust.yml
.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.
Avoid workflow-levelpermissions: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/cache.
Use lockfiles or dependency manifests to drive cache invalidation.
Keep deploy and publish permissions isolated to the jobs that need them.
Read both caller and callee when a workflow usesworkflow_call.
Put release-tag validation in the earliest practical caller job when the pipeline has tag-based publish behavior.
Keep pure-Python plugin SDK packaging as a single wheel artifact instead of duplicating it across every platform matrix entry.
contents: readis the default minimum for checkout-based build, test, docs, and packaging jobs.
pull-requests: readis required for PR metadata lookup jobs.
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; the callee cannot elevate beyond what the caller provides.
Preferastral-sh/setup-uvcache support withcache-dependency-globanchored touv.lock.
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.Keep CI workflow commands and package references in GitHub Actions workflows aligned with the current install, build, and example commands.
Files:
.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 (3)
.github/workflows/ci_rust.yml (2)
120-125: Exclude musllinux fromSwatinem/rust-cache.The musllinux entries still run this host cache step while the host Rust setup is skipped. Add the same musl guard used by the adjacent toolchain and uv steps.
34-115: LGTM!Also applies to: 127-191
justfile (1)
875-885: LGTM!Also applies to: 1153-1168
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
github-actions
Bot
added
size:L
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <2007799+willkill07@users.noreply.github.com>
|
/merge |
2 participants
Overview
Improve CI throughput and artifact confidence by parallelizing independent work, unifying matrices, and adding musl and downstream package validation.
Details
just --set ci true test-rustpath as native platforms, including locked nextest execution and JUnit output.uv,just, andcargo-nextestversions inside the musllinux runtime; retain the existing musl coverage exclusion.justhelpers honor an explicitUV_PYTHONinterpreter so PyO3 uses Alpine's shared Python instead of another interpreter bundled in the musllinux image.npm ciand Cargo/Maturin locked dependency resolution in musl validation paths.Validation:
uv run pre-commit run --all-filesjustrecipe dry-run, andgit diff --checkuv,just, andcargo-nextestdownloads for x86_64 and ARM64 muslUV_PYTHONfails immediately and a valid explicit interpreter completesjust build-rust.nemo-relay/plugins.tomland failed their clean-environment assertionsDocker is unavailable locally, so the complete musllinux container paths will run in GitHub Actions.
Where should the reviewer start?
Start with
.github/workflows/ci.yamlfor the top-level dependency graph, then review.github/workflows/ci_rust.ymland the shared Python resolver injustfilefor the unified native/musl Rust test entrypoint.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Bug Fixes
Chores