Skip to content

ci: fix Check job status to detect skipped results (fixes #2208) - #2210

Merged
rwgk merged 5 commits into
NVIDIA:mainfrom
leofang:leofang/fix-issue-2208-gating-check
Jun 16, 2026
Merged

ci: fix Check job status to detect skipped results (fixes #2208)#2210
rwgk merged 5 commits into
NVIDIA:mainfrom
leofang:leofang/fix-issue-2208-gating-check

Conversation

@leofang

@leofangleofang commented Jun 13, 2026

Copy link
Copy Markdown
Member

Fixes#2208.

Replaces the cancelled || failure predicate in .github/workflows/ci.yml's checks: job with CCCL's check_result pattern: per-dep expected="success", with expected="skipped" for legitimate [doc-only] skips, and an early short-circuit for [no-ci].

Reference: https://github.com/NVIDIA/cccl/blob/8c0e6cb1b6412d7bd0070f9ac3f55fa80231961a/.github/workflows/ci-workflow-pull-request.yml#L463-L526

End-to-end validation (pre-fix vs post-fix reproducer): #2210 (comment)

…esult
FixesNVIDIA#2208. The previous `Check job status` body only treated a dep's
`result == 'cancelled' || == 'failure'` as failure, letting `'skipped'`
slip through silently. When a `build-*` job fails, the dependent
`test-*` job is set to `'skipped'` by default needs-failure
propagation, and the aggregator passes -- exactly the case demonstrated
by NVIDIA#2209.
Adopt CCCL's `check_result` pattern: explicit `expected="success"` per
dependency, with `expected="skipped"` for legitimate `[doc-only]` skips,
and an early short-circuit for `[no-ci]`. Now any deviation from the
expected status (including `'skipped'` from a failed upstream) fails
the aggregator.
Reference: NVIDIA/cccl ci-workflow-pull-request.yml L463-L526.
@copy-pr-bot

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actionsgithub-actionsBot added the CI/CD CI/CD infrastructure label Jun 13, 2026
@leofang

Copy link
Copy Markdown
MemberAuthor

/ok to test 170c799

@github-actions

This comment has been minimized.

@leofang

Copy link
Copy Markdown
MemberAuthor

/ok to test 739c499

@leofangleofang added bug Something isn't working P0 High priority - Must do! labels Jun 15, 2026
@leofangleofang added this to the cuda.core v1.1.0 milestone Jun 15, 2026
Mirrors the NVIDIA#2209 reproducer on this branch's new aggregator. Expected
outcome:
- All Build * matrix entries fail at the env-vars build step.
- Downstream Test * jobs are skipped (cascaded needs-failure).
- Check job status now reports failure -- the symmetric, post-fix
counterpart to NVIDIA#2209 (where Check job status reported success despite
the same set of failures).
DO NOT MERGE while this commit is present. Remove before merge.
Refs NVIDIA#2208, NVIDIA#2209.
@leofang

Copy link
Copy Markdown
MemberAuthor

/ok to test 811388c

@leofang

leofang commented Jun 16, 2026

Copy link
Copy Markdown
MemberAuthor

Gating-check fix validated end-to-end

The new aggregator catches the exact failure mode that motivated #2208. Demonstrated by running the #2209 reproducer (intentional exit 7 in ci/tools/env-vars) against this branch:

Same reproducer, opposite verdict. The skipped-as-success hole is closed.

The demo commit 811388c99e will be reverted next so this PR returns to a mergeable state.

@leofang

Copy link
Copy Markdown
MemberAuthor

/ok to test 7d31a04

@leofangleofang self-assigned this Jun 16, 2026
@leofang
leofang marked this pull request as ready for review June 16, 2026 02:14

@rwgkrwgk 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.

Looks good to me and codex gpt-5.5:

The new check_result approach is a clear improvement over the previous cancelled || failure checks: it closes the skipped-as-success hole while keeping the intentional [doc-only] and [no-ci] cases explicit.

The one subtle behavior change I noticed is that fork or non-NVIDIA repo runs may now report a red final aggregator where jobs are skipped by owner-gated conditions. I agree that is not a concern for this PR, and it is preferable to avoid weakening the aggregator in a way that could reintroduce false-green CI.

@rwgk
rwgk merged commit 671af69 into NVIDIA:mainJun 16, 2026
203 of 206 checks passed
@github-actions

Copy link
Copy Markdown
Doc Preview CI
Preview removed because the pull request was closed or merged.

@leofang
leofang deleted the leofang/fix-issue-2208-gating-check branch June 16, 2026 12:20
u7k4rs6

This comment was marked as duplicate.

u7k4rs6

This comment was marked as outdated.

@leofang

Copy link
Copy Markdown
MemberAuthor

@u7k4rs6 please do not fire up nonsense agents against our repo and add noise. This is not a welcomed behavior.

@u7k4rs6

Copy link
Copy Markdown

@leofang Apologies, that was an automated agent I should not have run against your repo, especially on an already-merged PR. It added noise and got the core point wrong. I've turned it off. Thanks for the patience.

atmnp added a commit to NVIDIA/numba-cuda-mlir that referenced this pull request Jun 23, 2026
* ci: adopt CCCL check_result aggregator and add [doc-only] PR mode
Mirrors the gating-check fix in NVIDIA/cuda-python#2210 (root-cause
analysis: NVIDIA/cuda-python#2208) and bundles in cuda-python's
`[doc-only]` PR-mode feature, which had not been propagated to this
repo.
Changes:
* should-skip: emit both `skip` (existing `[no-ci]` gate) and a new
`doc_only` output, computed from PR title containing `[doc-only]`.
Single `gh pr view` call now drives both, matching cuda-python's
pattern.
* `[doc-only]` gating on non-doc-chain jobs: build-llvm-linux-aarch64,
build-llvm-windows, build-linux-aarch64, build-windows, and all 11
test-* jobs now have
`&& \!fromJSON(needs.should-skip.outputs.doc-only)` in their `if:`.
The linux-64 chain (build-llvm-linux-64, build-linux-64) and
build-docs are intentionally NOT `[doc-only]`-gated because
build-docs needs the linux-64 wheel.
* checks: aggregator rewritten to CCCL's `check_result` pattern:
explicit `expected="success"` per dependency, anything else
(including `'skipped'` from a failed upstream) fails the gate.
`[no-ci]` short-circuits at the top; `[doc-only]` flips `expected`
to `"skipped"` for every test-* dep. `should-skip` itself is now in
`needs:` and checked, so a crash in the gating job can't masquerade
as a green CI run.
Verified the aggregator script with a local simulation across 8
scenarios: normal-green, build-fail-cascade, `[no-ci]`,
`[doc-only]`-OK, `[doc-only]`-doc-fails, build-windows-fail-cascade,
single-test-fail, should-skip-itself-fails. All behave correctly.
Reference: CCCL gate,
https://github.com/NVIDIA/cccl/blob/8c0e6cb1b6412d7bd0070f9ac3f55fa80231961a/.github/workflows/ci-workflow-pull-request.yml#L463-L526
Refs: NVIDIA/cuda-python#2208, NVIDIA/cuda-python#2210
* Bump the actions-monthly group across 1 directory with 5 updates (#125)
Bumps the actions-monthly group with 5 updates in the / directory:
| Package | From | To |
| --- | --- | --- |
| [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) | `8.1.0` | `8.2.0` |
| [github/codeql-action](https://github.com/github/codeql-action) | `4.35.3` | `4.36.2` |
| [pypa/cibuildwheel](https://github.com/pypa/cibuildwheel) | `3.4.1` | `4.1.0` |
| [prefix-dev/setup-pixi](https://github.com/prefix-dev/setup-pixi) | `0.9.5` | `0.9.6` |
| [test-summary/action](https://github.com/test-summary/action) | `2.4` | `2.6` |
Updates `astral-sh/setup-uv` from 8.1.0 to 8.2.0
- [Release notes](https://github.com/astral-sh/setup-uv/releases)
- [Commits](astral-sh/setup-uv@0880764...fac544c)
Updates `github/codeql-action` from 4.35.3 to 4.36.2
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@e46ed2c...8aad20d)
Updates `pypa/cibuildwheel` from 3.4.1 to 4.1.0
- [Release notes](https://github.com/pypa/cibuildwheel/releases)
- [Changelog](https://github.com/pypa/cibuildwheel/blob/main/docs/changelog.md)
- [Commits](pypa/cibuildwheel@8d2b08b...2947353)
Updates `prefix-dev/setup-pixi` from 0.9.5 to 0.9.6
- [Release notes](https://github.com/prefix-dev/setup-pixi/releases)
- [Commits](prefix-dev/setup-pixi@1b2de7f...5185adf)
Updates `test-summary/action` from 2.4 to 2.6
- [Release notes](https://github.com/test-summary/action/releases)
- [Commits](test-summary/action@v2.4...v2.6)
---
updated-dependencies:
- dependency-name: astral-sh/setup-uv
dependency-version: 8.2.0
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: actions-monthly
- dependency-name: github/codeql-action
dependency-version: 4.36.2
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: actions-monthly
- dependency-name: prefix-dev/setup-pixi
dependency-version: 0.9.6
dependency-type: direct:production
update-type: version-update:semver-patch
dependency-group: actions-monthly
- dependency-name: pypa/cibuildwheel
dependency-version: 4.0.0
dependency-type: direct:production
update-type: version-update:semver-major
dependency-group: actions-monthly
- dependency-name: test-summary/action
dependency-version: '2.6'
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: actions-monthly
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* ci: opt out of delvewheel repair on Windows (cibuildwheel v4 default)
cibuildwheel v4 made `delvewheel repair -w {dest_dir} -v {wheel}` the
default for `CIBW_REPAIR_WHEEL_COMMAND_WINDOWS`; v3.x had no default. We
intentionally do not need wheel repair on Windows because the wheel is
built with `/MT` (static CRT) per #127 -- it is self-contained and has
no DLL dependencies for delvewheel to bundle.
Without this opt-out, the default delvewheel invocation discovers an
unresolvable reference to the project's own internal nanobind module
and fails wheel build for every win-64 matrix entry. This regression
re-surfaced when the cibuildwheel v3.4.1 -> v4.1.0 bump from #125 was
cherry-picked into this branch to validate the new aggregator catches
it. With the opt-out, the build succeeds again.
Refs: #127 (static-CRT Windows wheel design), #125 / #134 (original
cibuildwheel bump + revert).
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Atmn Patel <atmnp@nvidia.com>
leofang added a commit to NVIDIA/numba-cuda that referenced this pull request Jul 5, 2026
…) (#912)
## Summary
Ports the gating-check fix from NVIDIA/cuda-python#2210 to this repo.
Closes#900.
Root cause and design discussion live in NVIDIA/cuda-python#2208 — not
repeating here. Bit us in this repo on 2026-07-03 when
[`5f530b96`](5f530b96)
auto-merged with a silent Windows failure and had to be reverted in
[`441882b2`](441882b2).
## Change
One file, `.github/workflows/ci.yaml`, `checks:` job only:
- Replace the `cancelled || failure` predicate with CCCL's
`check_result` pattern: per-dep `expected="success"`, `[no-ci]`
short-circuit at the top.
- Add `should-skip` to `needs:` and check its result explicitly — a
crash in the gating job can no longer masquerade as a green CI run.
- No `[doc-only]` machinery — this repo doesn't have that PR mode. If
added later, mirror NVIDIA/numba-cuda-mlir#142.
## Verification
Local simulation of the new aggregator across 6 scenarios (normal-green,
build-windows-fail-cascade, build-linux-64-fail-cascade, `[no-ci]`,
pre-commit-fail, should-skip-itself-fails) — all behave as expected.
## Reference
- Tracking issue on this repo: #900
- Root cause: NVIDIA/cuda-python#2208
- Source-of-truth fix: NVIDIA/cuda-python#2210
- Sibling port to numba-cuda-mlir: NVIDIA/numba-cuda-mlir#142
- CCCL gate pattern:
https://github.com/NVIDIA/cccl/blob/8c0e6cb1b6412d7bd0070f9ac3f55fa80231961a/.github/workflows/ci-workflow-pull-request.yml#L463-L526
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugSomething isn't workingCI/CDCI/CD infrastructureP0High priority - Must do!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: "Check job status" aggregator passes when build jobs fail (skipped-as-success regression)

3 participants

@leofang@u7k4rs6@rwgk