Skip to content

ci: remove the NVSkills gate, which can no longer pass - #1363

Closed
SandyChapman wants to merge 1 commit into
mainfrom
remove-nvskills-gate/schapman
Closed

ci: remove the NVSkills gate, which can no longer pass#1363
SandyChapman wants to merge 1 commit into
mainfrom
remove-nvskills-gate/schapman

Conversation

@SandyChapman

@SandyChapman SandyChapman commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

require-nvskills currently blocks every PR that touches top-level skills/, and it cannot be made to pass.

NVIDIA/skills#447 ("refactor: remove nemo-platform", merged 2026-08-13) removed components.d/nemo-platform.yml from the NVSkills catalog. The upstream validation pipeline no longer knows about this repo, so it cannot produce the signature commit the gate waits for.

The gate itself is local to this repo and never consults the catalog. It fails any PR touching skills/ unless HEAD is authored by svc-nvskills-signing with the signature commit title. With no pipeline to dispatch to, /nvskills-ci cannot produce that commit — so the check fails closed with no path to green, and since it sits in ci-status's needs, it takes the required aggregate check down with it.

Before #447 the gate was already blocked by the tier-3 --env-mode local bubblewrap failure on the nvcarps runners; pyproject.toml still carries a TEMPORARY ty override added specifically because "editing any file under skills/ puts the PR behind the NVSkills gate, and that gate currently cannot pass". #447 turned a temporary block into a permanent one.

Changes

  • Remove the require-nvskills job from .github/workflows/ci.yaml
  • Remove its entry from ci-status's needs: list
  • Delete .github/workflows/request-nvskills-ci.yml (the /nvskills-ci dispatch)
  • Remove the corresponding .github/CI_README.md section

Type of Change

  • CI, build, or test infrastructure

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification: deletion of CI workflow configuration; there is no test surface for a removed job, and the effect is observable only in CI itself
  • Documentation updated for user-visible behavior — CI_README.md section removed alongside the workflows it documented

Verification

  • Pull request title follows the repository's Conventional Commit format
  • Every commit includes an appropriate Signed-off-by: trailer
  • uv run pre-commit run -a passes, or any blocked checks are identified below
  • Targeted tests pass, or tests are marked not applicable above
  • No secrets, API keys, or credentials are included

Targeted validation:

Check Result
ci.yaml parses as YAML 40 jobs, require-nvskills absent
ci-status needs: 30 entries, no dangling references to removed jobs
Repo-wide grep for require-nvskills / request-nvskills / /nvskills-ci no matches outside skills/*/BENCHMARK.md prose
tools/lint/lint-merge-conflict.sh pass
tools/lint/lint-copyright-headers.sh pass

This PR deliberately touches no files under skills/, so it does not gate itself.

pre-commit run -a was not run to completion here: three hooks fail on this machine for missing local toolchain (helm-docs, pinned uv version, pnpm shim), none of which governs a file in this diff.

Notes for reviewers

  • Branch protection. If require-nvskills is configured as a required check in branch protection or a ruleset, it must also be removed there, or PRs will block on a check that no longer runs. ci-status remains the aggregate pin.
  • If nemo-platform returns to the catalog, this is a straight revert plus re-onboarding upstream. Worth confirming with whoever drove feat(evaluator): AgentEvalTaskset and filesystem evidence handle #447 whether the removal is permanent — if it is temporary, disabling the job rather than deleting it may be preferable.
  • Unblocks the pyproject.toml override. The TEMPORARY ty override for skills/nemo-evaluator-plugin/assets/examples/plugin_sdk_examples.py exists only because skills/ edits could not land; it is left in place here since its removal belongs with the fix in docs(evaluator): align the evaluator skill with main #1237.

Summary by CodeRabbit

  • Chores

    • Removed the NVSkills validation check from pull request CI.
    • Removed automatic NVSkills CI requests triggered by comments and signed pushes.
    • Updated overall CI status reporting to exclude the retired validation check.
  • Documentation

    • Removed documentation describing the retired NVSkills workflows and merge-blocking behavior.

NVIDIA/skills#447 removed nemo-platform from the NVSkills catalog, so the
validation pipeline no longer knows about this repo and can never produce
the signature commit `require-nvskills` waits for.

The gate is local: it fails any PR touching top-level `skills/` unless HEAD
is authored by the signing bot. With no pipeline to sign, `/nvskills-ci` has
nothing to dispatch to, so the check fails closed with no path to green — and
because it sits in `ci-status`'s `needs`, it takes the required aggregate down
with it. Any PR touching `skills/` is currently unmergeable.

Removes the job, its `ci-status` entry, the `/nvskills-ci` dispatch workflow,
and the corresponding CI_README section.

Signed-off-by: Sandy Chapman <schapman@nvidia.com>
@github-actions github-actions Bot added the ci label Aug 18, 2026
@SandyChapman
SandyChapman marked this pull request as ready for review August 18, 2026 12:32
@SandyChapman
SandyChapman requested a review from a team as a code owner August 18, 2026 12:32
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8f71eaef-49be-4387-a9ac-fa506fa201d6

📥 Commits

Reviewing files that changed from the base of the PR and between 137a5e5 and d7be357.

📒 Files selected for processing (3)
  • .github/CI_README.md
  • .github/workflows/ci.yaml
  • .github/workflows/request-nvskills-ci.yml
💤 Files with no reviewable changes (3)
  • .github/CI_README.md
  • .github/workflows/request-nvskills-ci.yml
  • .github/workflows/ci.yaml

Included review availability: Your plan includes up to 12 reviews per rolling hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The change removes NVSkills CI request and required-check workflows. It also removes their aggregate CI dependency and documentation entries.

Changes

NVSkills CI removal

Layer / File(s) Summary
Remove NVSkills CI workflow integration
.github/workflows/ci.yaml, .github/workflows/request-nvskills-ci.yml, .github/CI_README.md
The required NVSkills signature check and its ci-status dependency were removed. The NVSkills CI request workflow was deleted. The documentation entries were removed.

Merge Risk: ⚪ Minimal · up to d7be3

This PR removes an obsolete CI gate and its dispatch documentation without changing product runtime behavior; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 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 The title clearly and concisely describes the removal of the non-passing NVSkills CI gate.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch remove-nvskills-gate/schapman

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

@github-actions

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 34295/43312 79.2% 64.0%
Integration Tests 20248/41111 49.2% 22.0%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant