ci: share final gate checks across Core and REST - #4742
Merged
Conversation
Contributor
Summary by CodeRabbit
WalkthroughThe CI gate checker now supports Core CI and REST CI policies. Both workflows use the shared checker for inventory and result validation. Tests cover policy selection, REST inventory rules, command errors, and result handling. ChangesCI gate policy validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant REST CI workflow
participant check_ci_gate.py
participant GitHub Actions job results
REST CI workflow->>check_ci_gate.py: inventory --policy rest
check_ci_gate.py-->>REST CI workflow: inventory validation result
REST CI workflow->>GitHub Actions job results: collect NEEDS_JSON
REST CI workflow->>check_ci_gate.py: results with serialized job results
check_ci_gate.py-->>REST CI workflow: result validation status
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
@coderabbitai full_review, thanks! |
Contributor
|
🐇 ✅ Action performedFull review finished. |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-08-08 01:49:19 UTC | Commit: 71c3119 |
As it stood, Core checked its final-gate inventory in source, while REST still had a separate `jq` block and no guard when a top-level job was added without updating `rest-ci-pass`. That meant the two required checks could drift even though they enforce the same current rule. So, this turns `check_core_ci_gate.py` into `check_ci_gate.py`, keeps the lane-specific gate names and exemptions in explicit policies, and runs the same inventory and result checks for both workflows. Primary callouts are: - **Expected green-run effect:** No speedup is expected; `rest-ci-pass` adds one small hosted checkout before it evaluates the job results. - **What it really buys us:** New REST jobs cannot quietly sit outside `rest-ci-pass`, and malformed result data now fails closed instead of passing through REST's separate `jq` check. - Core keeps its existing 52-job contract, while REST starts with all nine top-level jobs accounted for. - `success` and `skipped` remain the only passing results; NVIDIA#4586 still owns deciding whether an individual skip was actually valid. Tests updated! This supports NVIDIA#4741 Signed-off-by: Chet Nichols III <chetn@nvidia.com>
10 tasks
This was referenced Aug 10, 2026
chet
added a commit
that referenced
this pull request
Aug 11, 2026
## TLDR This fixes how Core and REST tell TruffleHog which commits to scan. On a PR's first CI run, GitHub has no previous commit, which caused REST to scan the entire repository history. After a force-push, the next run can have the opposite problem: `copy-pr-bot` replaces its internal PR branch, but GitHub still identifies that branch's now-missing old commit as the starting point. TruffleHog then scans zero bytes, and the surrounding action still reports success -- so yes, we would fail open -- and this was caught during the CICD work that we've been doing. Both workflows now: 1. Ignore that [unreliable] previous commit for PR runs. 2. Look up the PR's current base. 3. Use the commit that started the workflow as the current head. 4. Calculate the real range from the fetched Git history before starting TruffleHog. If that range cannot be calculated, the workflow fails before the scan; this keeps the first run from scanning the whole repository and prevents this missing-commit case from going green after scanning nothing. ## Expected green-run effect No reliable steady-state speedup. The first synthetic REST scan on this PR processed 11,464 bytes instead of the roughly 446 MB full-history scan we observed on #4742 -- about 39,000x less scan input. This is still primarily a correctness fix; normal update scans should take about the same time. ## What it really buys us One latest green Core or REST secret check now proves that the repository-side range was resolved from the current PR and passed to the scanner. Rebases and force-pushes can no longer make that run depend on a stale or missing synthetic-branch commit. ## Testing Ran a bunch of commands to test it out: - `bash -n .github/ci/resolve-pr-scan-range.sh .github/ci/test-resolve-pr-scan-range.sh` - `bash .github/ci/test-resolve-pr-scan-range.sh` - `python3 -B .github/ci/test_check_ci_gate.py` - Core and REST final-gate inventory checks - CI token-permission checker fixtures, plus the Core and stale-workflow permission checks - Core and REST concurrency-policy checks - `actionlint` for both edited workflows, ignoring only the repository's known custom-runner labels - `cargo make check-format-nightly` - `cargo make clippy` - cached Carbide-lints workflow (`--all-targets --all-features`) - `git diff --check` Acceptance exercised the initial synthetic push and two rewritten heads. On the initial head, Core and REST both resolved `00ece37...b8acd55`, scanned 6 chunks / 11,464 bytes, and reported no source errors. On the first rewrite, the push event contained stale `before=b8acd55`, but both workflows resolved `00ece37...eb5f72f` and scanned the same real range. After the final simplification and rebase, the event contained stale `before=eb5f72f`; both workflows instead resolved `7d9b87e...d79119f`, scanned 6 chunks / 9,129 bytes, and passed. Final-head Core CI completed 52 jobs with zero failures, and REST's final gate also passed. This supports #4786 Signed-off-by: Chet Nichols III <chetn@nvidia.com>
Sinck
pushed a commit
to Sinck/infra-controller
that referenced
this pull request
Aug 11, 2026
## TLDR This fixes how Core and REST tell TruffleHog which commits to scan. On a PR's first CI run, GitHub has no previous commit, which caused REST to scan the entire repository history. After a force-push, the next run can have the opposite problem: `copy-pr-bot` replaces its internal PR branch, but GitHub still identifies that branch's now-missing old commit as the starting point. TruffleHog then scans zero bytes, and the surrounding action still reports success -- so yes, we would fail open -- and this was caught during the CICD work that we've been doing. Both workflows now: 1. Ignore that [unreliable] previous commit for PR runs. 2. Look up the PR's current base. 3. Use the commit that started the workflow as the current head. 4. Calculate the real range from the fetched Git history before starting TruffleHog. If that range cannot be calculated, the workflow fails before the scan; this keeps the first run from scanning the whole repository and prevents this missing-commit case from going green after scanning nothing. ## Expected green-run effect No reliable steady-state speedup. The first synthetic REST scan on this PR processed 11,464 bytes instead of the roughly 446 MB full-history scan we observed on NVIDIA#4742 -- about 39,000x less scan input. This is still primarily a correctness fix; normal update scans should take about the same time. ## What it really buys us One latest green Core or REST secret check now proves that the repository-side range was resolved from the current PR and passed to the scanner. Rebases and force-pushes can no longer make that run depend on a stale or missing synthetic-branch commit. ## Testing Ran a bunch of commands to test it out: - `bash -n .github/ci/resolve-pr-scan-range.sh .github/ci/test-resolve-pr-scan-range.sh` - `bash .github/ci/test-resolve-pr-scan-range.sh` - `python3 -B .github/ci/test_check_ci_gate.py` - Core and REST final-gate inventory checks - CI token-permission checker fixtures, plus the Core and stale-workflow permission checks - Core and REST concurrency-policy checks - `actionlint` for both edited workflows, ignoring only the repository's known custom-runner labels - `cargo make check-format-nightly` - `cargo make clippy` - cached Carbide-lints workflow (`--all-targets --all-features`) - `git diff --check` Acceptance exercised the initial synthetic push and two rewritten heads. On the initial head, Core and REST both resolved `00ece37...b8acd55`, scanned 6 chunks / 11,464 bytes, and reported no source errors. On the first rewrite, the push event contained stale `before=b8acd55`, but both workflows resolved `00ece37...eb5f72f` and scanned the same real range. After the final simplification and rebase, the event contained stale `before=eb5f72f`; both workflows instead resolved `7d9b87e...d79119f`, scanned 6 chunks / 9,129 bytes, and passed. Final-head Core CI completed 52 jobs with zero failures, and REST's final gate also passed. This supports NVIDIA#4786 Signed-off-by: Chet Nichols III <chetn@nvidia.com>
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.
This is a follow-up to #4655, where we made Core verify that every top-level CI job is covered by
core-ci-pass, but left REST on its ownjqresult check. Nothing catches a new REST job added without also updatingrest-ci-pass, so the two required checks can drift even though they enforce the same rule.So, this turns
check_core_ci_gate.pyintocheck_ci_gate.py, keeps the lane-specific gate names and exemptions in explicit policies, and runs the same inventory and result checks for both workflows.rest-ci-passadds one small hosted checkout before it evaluates the job results.rest-ci-pass, and malformed result data now fails closed instead of passing through REST's separatejqcheck.Core keeps its existing 52-job contract, while REST starts with all nine top-level jobs accounted for.
successandskippedremain the only passing results; #4586 still owns deciding whether an individual skip was actually valid.Related issues
This supports #4741
Type of Change
Breaking Changes
Testing
Validated both live workflow inventories, Actionlint,
cargo make format-nightly,cargo make clippy, and the cached full Carbide-lints gate.Additional Notes
This deliberately preserves the current blanket acceptance of
skipped. #4586 still owns classifier-aware skip validation, Actions API pagination, job-name mapping, cross-workflow correlation, and timing reports.Closes #4741