Skip to content

ci: add an aggregation layer to CI to allow blocking merge more dynamically#239

Merged
crookedstorm merged 8 commits into
mainfrom
complete-ci/crookedstorm
Jun 9, 2026
Merged

ci: add an aggregation layer to CI to allow blocking merge more dynamically#239
crookedstorm merged 8 commits into
mainfrom
complete-ci/crookedstorm

Conversation

@crookedstorm

@crookedstorm crookedstorm commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Besides the aggregation layer for the workflow, this also adds an actionlint check to ensure github actions stay high quality.

Summary by CodeRabbit

  • Chores
    • Enhanced CI/CD pipeline security by pinning GitHub Actions to specific versions
    • Improved wheel building and testing process with stricter validation to ensure proper artifact handling
    • Added automated linting rule to enforce consistent naming conventions across configuration files
    • Introduced CI status aggregator for improved workflow visibility and job dependency management

@crookedstorm
crookedstorm requested a review from a team as a code owner June 9, 2026 18:15
@crookedstorm
crookedstorm force-pushed the complete-ci/crookedstorm branch from a7f6f55 to c86d1d7 Compare June 9, 2026 18:19
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

CI workflow hardens GitHub Actions through commit SHA pinning across all jobs, adds actionlint validation for kebab-case action inputs, replaces shell globbing with deterministic array-based wheel discovery, and introduces a ci-status aggregator job that validates downstream results and gates notify-ci-consumer.

Changes

CI Hardening: Action Pinning, Wheel Determinism, and Status Aggregation

Layer / File(s) Summary
Actionlint kebab-case rule
.github/actionlint.yaml
Custom rule enforces action input names match ^inputs\.[a-z0-9-]+$ with kebab-case validation message.
Workflow permissions and action pinning
.github/workflows/ci.yaml
Adds actions: read to workflow permissions. Pins actions/checkout, actions/setup-node, actions/upload-artifact, actions/download-artifact, and astral-sh/setup-uv to specific commit SHAs across changes, actionlint, lint, policy-wasm, python-unit-test-tools, python-unit-test, python-integration-test, wheel-build, wheel-test, python-e2e-test, benchmark-guardrails, coverage-comment, and opa-policy-test jobs.
Wheel discovery determinism
.github/workflows/ci.yaml
Changes wheel file matching in wheel-test from ls globbing to shopt -s nullglob backed arrays; hard-fails if exactly one wheel is not found for both nemo-platform and nemo-platform-plugin package discovery.
CI status aggregator and gating
.github/workflows/ci.yaml
Introduces ci-status job running if: always() that validates all dependent job needs results (fails if any is not success or skipped), renders status table to GITHUB_STEP_SUMMARY. Updates notify-ci-consumer to depend on ci-status for downstream gating.

Suggested reviewers

  • svvarom
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title accurately describes the main changes: adds CI aggregation layer via new ci-status job and updates workflow permissions/actions pinning.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch complete-ci/crookedstorm

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/actions/test-status/action.yaml (1)

57-64: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Missing trailing newline.

YAML files should end with a newline.

Proposed fix
         retention-days: 1
         if-no-files-found: error
+
🤖 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/actions/test-status/action.yaml around lines 57 - 64, The YAML file
is missing a trailing newline at EOF; open the action step block containing the
"Upload status" job (the step with name: "Upload status" and uses:
actions/upload-artifact@v6, including the with keys name and path) and add a
single newline character at the end of the file so the file ends with a newline.
🤖 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.yaml:
- Around line 54-57: Add a SHA-256 integrity check for the downloaded actionlint
tarball before extracting it: after downloading
"${RUNNER_TEMP}/actionlint.tar.gz" (actionlint_1.7.12_linux_amd64.tar.gz)
compute its SHA-256 (e.g., with sha256sum) and compare it to the expected hash
"8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8"; if the
checksum does not match, fail the workflow (exit non‑zero) and do not run tar
-xzf; if it matches, proceed to extract into "${RUNNER_TEMP}/actionlint".

---

Outside diff comments:
In @.github/actions/test-status/action.yaml:
- Around line 57-64: The YAML file is missing a trailing newline at EOF; open
the action step block containing the "Upload status" job (the step with name:
"Upload status" and uses: actions/upload-artifact@v6, including the with keys
name and path) and add a single newline character at the end of the file so the
file ends with a newline.
🪄 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: CHILL

Plan: Enterprise

Run ID: 14684629-4350-4ffa-97ef-6f357545d471

📥 Commits

Reviewing files that changed from the base of the PR and between a3e68a6 and 784b30a.

📒 Files selected for processing (2)
  • .github/actions/test-status/action.yaml
  • .github/workflows/ci.yaml

Comment thread .github/workflows/ci.yaml
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 19067/25122 75.9% 62.5%
Integration Tests 12033/23810 50.5% 25.8%

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 @.github/workflows/ci.yaml:
- Around line 627-718: Modify the "Wait for contributing jobs" and "Check test
statuses" steps so we fail when an expected contributing job has no status
artifact: fetch the completed job list from the GH API (same endpoint used in
the waiting loop) and build the set of contributor job names (exclude
non-contributors via the existing excluded_jobs array / is_excluded function),
then when processing artifacts in the "Check test statuses" step compare that
contributor set against the names extracted from the downloaded
test-status-*.json files (the jq '.name' values); if any contributor is missing,
emit an error and set failure (exit non-zero) instead of silently passing. Keep
the existing artifact parsing, summary table, and existing
excluded_jobs/is_excluded logic but add the missing-check before the existing
total==0 check so timeouts/canceled jobs cause CI to fail.
- Around line 48-49: Replace all floating action refs in the CI workflow (e.g.,
actions/checkout@v6, actions/download-artifact@v8 and other `@v`* refs such as
docker/login-action, setup-uv, upload-artifact) with their pinned commit SHAs;
locate occurrences by searching for those action names in the file and update
each uses: entry to the corresponding commit SHA. Additionally, in the ci-status
aggregation job (named ci-status) and the coverage-comment flow, ensure every
job you wait for actually produced a test-status artifact by adding a validation
step after the wait that checks for the existence of each expected
test-status-...-*.json artifact (or alternatively call the GitHub Actions API to
derive job conclusions) and fail/mark missing artifacts so the gate decision
reflects missing or failed jobs rather than silently ignoring them.
🪄 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: CHILL

Plan: Enterprise

Run ID: 792072f2-163c-44b6-888c-8e561aed197e

📥 Commits

Reviewing files that changed from the base of the PR and between 784b30a and c86d1d7.

📒 Files selected for processing (3)
  • .github/actionlint.yaml
  • .github/actions/test-status/action.yaml
  • .github/workflows/ci.yaml
✅ Files skipped from review due to trivial changes (1)
  • .github/actionlint.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/actions/test-status/action.yaml

Comment thread .github/workflows/ci.yaml Outdated
Comment thread .github/workflows/ci.yaml Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/actions/test-status/action.yaml (1)

83-89: ⚠️ Potential issue | 🟠 Major

Pin actions/upload-artifact to a commit SHA (immutable ref).

Upload status in .github/actions/test-status/action.yaml uses the floating ref actions/upload-artifact@v6, leaving a supply-chain gap.

Snippet
    - name: Upload status
      uses: actions/upload-artifact@v6
      with:
        name: ${{ steps.write.outputs.artifact-name }}
        path: ${{ steps.write.outputs.status-file }}
        retention-days: 1
        if-no-files-found: error
🤖 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/actions/test-status/action.yaml around lines 83 - 89, The workflow
step named "Upload status" currently uses the floating ref
actions/upload-artifact@v6; replace that with an immutable commit SHA for
actions/upload-artifact (e.g., actions/upload-artifact@<commit-sha>) to pin the
dependency and remove the supply-chain risk, keeping the rest of the step (name,
with: name: ${{ steps.write.outputs.artifact-name }}, path: ${{
steps.write.outputs.status-file }}, retention-days: 1, if-no-files-found: error)
unchanged; update the uses value for the "Upload status" step to the specific
commit SHA from the official repository.
🤖 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.yaml:
- Around line 35-39: The "Record test start" step using
./.github/actions/test-status (name: Detect Path Changes, phase: start) runs
after actions/checkout so failures during checkout never emit the start marker;
move or add an inline pre-checkout step that emits the same start marker (same
name and phase) before any actions/checkout occurs for this job (or update the
ci-status reconciler to query the Actions API for jobs with missing markers) so
the job always opts into CI status even if checkout fails.

---

Outside diff comments:
In @.github/actions/test-status/action.yaml:
- Around line 83-89: The workflow step named "Upload status" currently uses the
floating ref actions/upload-artifact@v6; replace that with an immutable commit
SHA for actions/upload-artifact (e.g., actions/upload-artifact@<commit-sha>) to
pin the dependency and remove the supply-chain risk, keeping the rest of the
step (name, with: name: ${{ steps.write.outputs.artifact-name }}, path: ${{
steps.write.outputs.status-file }}, retention-days: 1, if-no-files-found: error)
unchanged; update the uses value for the "Upload status" step to the specific
commit SHA from the official repository.
🪄 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: CHILL

Plan: Enterprise

Run ID: 19fb22bc-26b4-4462-9923-4aff9f825c94

📥 Commits

Reviewing files that changed from the base of the PR and between 0fc4173 and cea8af4.

📒 Files selected for processing (2)
  • .github/actions/test-status/action.yaml
  • .github/workflows/ci.yaml

Comment thread .github/workflows/ci.yaml Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.yaml:
- Line 34: The checkout steps using
actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 leave GITHUB_TOKEN in
git config; update every actions/checkout step in the workflow (all uses of
actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10) to include a with:
persist-credentials: false block so the token is not persisted to git config;
ensure you add the setting to each occurrence (including any checkout used
before pushes or commits) and verify no other checkout steps omit this option.
🪄 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: CHILL

Plan: Enterprise

Run ID: 4f2bf4e4-94a1-42a3-8a5b-71992deff69c

📥 Commits

Reviewing files that changed from the base of the PR and between cea8af4 and b86ecd5.

📒 Files selected for processing (1)
  • .github/workflows/ci.yaml

Comment thread .github/workflows/ci.yaml
@svvarom
svvarom self-requested a review June 9, 2026 20:33

@svvarom svvarom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Comment thread .github/workflows/ci.yaml
Signed-off-by: Brooke Storm <brookes@nvidia.com>
Signed-off-by: Brooke Storm <brookes@nvidia.com>
Signed-off-by: Brooke Storm <brookes@nvidia.com>
Signed-off-by: Brooke Storm <brookes@nvidia.com>
Signed-off-by: Brooke Storm <brookes@nvidia.com>
Signed-off-by: Brooke Storm <brookes@nvidia.com>
Signed-off-by: Brooke Storm <brookes@nvidia.com>
…hat to not block

Signed-off-by: Brooke Storm <brookes@nvidia.com>
@crookedstorm
crookedstorm force-pushed the complete-ci/crookedstorm branch from 16988cd to 5ef6046 Compare June 9, 2026 21:43
@crookedstorm
crookedstorm enabled auto-merge June 9, 2026 21:45
@crookedstorm
crookedstorm added this pull request to the merge queue Jun 9, 2026
Merged via the queue into main with commit 3502dd0 Jun 9, 2026
33 checks passed
@crookedstorm
crookedstorm deleted the complete-ci/crookedstorm branch June 24, 2026 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants