Skip to content

chore(customizer): fix setuptools CVE in the RL image + docs cleanup - #1187

Merged
anubhutivyas merged 1 commit into
mainfrom
anubhutiv/more-rl-cves
Aug 11, 2026
Merged

chore(customizer): fix setuptools CVE in the RL image + docs cleanup#1187
anubhutivyas merged 1 commit into
mainfrom
anubhutiv/more-rl-cves

Conversation

@anubhutivyas

@anubhutivyas anubhutivyas commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix setuptools CVE in RL image
  • docs/dead-code cleanup.

Changes

1. setuptools CVEs

  • The findings are on jaraco.context 5.3.0 and wheel 0.45.1, vendored inside a setuptools 79.0.1 that no venv uses. It comes from Megatron-Bridge, which pins setuptools<80.0.0 in build-system.requires; uv resolves that in an isolated PEP 517 build environment and leaves the result in /opt/uv_cache, which ships.
  • RL's setuptools>=80.10.2 override can't reach it, overrides govern the projectresolution, not build environments and a build constraint can't either
  • The publish stage now drops cached setuptools archives that no venv symlinks into.

2. Cleanup

  • removes the unused transformer-engine-wheel target and its bake wiring. It was not being consumed, and it can't be. uv.lock pins TE as a git source, so uv sync --frozen builds from git regardless of any wheel on disk
  • trims Dockerfile.nmp-rl-base comments and now the detail lives in docker/rl/README.md

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with documentation updates
  • Documentation only
  • Contributor tooling or automation
  • CI, build, or test infrastructure

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Documentation updated for user-visible behavior
  • Documentation not applicable — justification:

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:

Summary by CodeRabbit

  • Documentation

    • Clarified training image support for DPO, with GRPO planned for a future release.
    • Updated backend and GPU guidance, including Transformer Engine build behavior.
    • Documented dependency exclusions, cache handling, and optional Gym environment setup.
  • Chores

    • Removed prebuilt Transformer Engine wheel support from the image build process.
    • Improved published image cleanup while preserving required cached dependencies.

@anubhutivyas
anubhutivyas requested review from a team as code owners August 7, 2026 21:37
@github-actions github-actions Bot added the chore label Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The RL image documentation now identifies DPO as supported and GRPO as planned. It documents dependency prefetching, runtime cache behavior, and publish cleanup. The standalone Transformer Engine wheel build and related Bake configuration are removed.

Changes

RL image build cleanup

Layer / File(s) Summary
Transformer Engine wheel removal
docker-bake.hcl, docker/base/Dockerfile.python-wheels
The standalone Transformer Engine wheel target, builder, context, and related helpers are removed.
RL image dependency and cache flow
docker/rl/Dockerfile.nmp-rl-base
The documentation describes excluded backends, dependency synchronization, worker and Gym environment materialization, publish cleanup, and read-only uv cache behavior.
RL documentation alignment
docker/rl/README.md
The README identifies DPO as supported, marks GRPO as planned, documents Transformer Engine source compilation, and describes writable runtime cache behavior.

Suggested reviewers: a2bondar

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies the main security fix and documentation cleanup in the pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch anubhutiv/more-rl-cves

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

@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 `@docker/rl/Dockerfile.nmp-rl-base`:
- Around line 269-285: Update the packaging documentation in docker/rl/README.md
to reflect the exclusions in the Dockerfile’s nemo-rl COPY step: state that the
Megatron-Bridge/mcore source tree is absent from /opt/nemo-rl, and document that
enabling the mcore extra requires restoring both the source checkout and
corresponding copy rules.
- Around line 209-211: Make both nemo-rl COPY steps fail when their excluded
TensorRT-LLM-workspace or Megatron-Bridge-workspace paths are present in the
destination. Add immediate post-copy verification commands for each exclusion,
using checks that return a nonzero status on any match, so later uv steps cannot
proceed after an ineffective exclusion.
🪄 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: CHILL

Plan: Enterprise

Run ID: 9f6ec822-4e31-4bc8-ad35-a9835ad41e77

📥 Commits

Reviewing files that changed from the base of the PR and between e32287b and 382e21b.

📒 Files selected for processing (1)
  • docker/rl/Dockerfile.nmp-rl-base

Comment thread docker/rl/Dockerfile.nmp-rl-base
Comment thread docker/rl/Dockerfile.nmp-rl-base Outdated
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 32006/40616 78.8% 63.6%
Integration Tests 18559/38542 48.1% 20.8%

@anubhutivyas
anubhutivyas force-pushed the anubhutiv/more-rl-cves branch from 382e21b to 77b628a Compare August 10, 2026 22:44
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@anubhutivyas anubhutivyas changed the title chore(customizer): fix setuptools CVE in RL image chore(customizer): fix setuptools CVE + add automodel backend + cleanup Aug 10, 2026

@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 `@docker/rl/README.md`:
- Around line 122-138: Update the later worker-to-venv mapping to assign GRPO
policy training to DTensorPolicyWorkerV2 with the automodel extra, matching the
Dockerfile behavior. Revise the prefetch documentation to state seven filters
and eight venvs, and remove the claim that automodel is not prefetched or builds
on the node by default.
🪄 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: CHILL

Plan: Enterprise

Run ID: b1a44363-5c42-4af4-a40d-a7b317fc4d25

📥 Commits

Reviewing files that changed from the base of the PR and between 33cecaf and 77b628a.

📒 Files selected for processing (4)
  • docker-bake.hcl
  • docker/base/Dockerfile.python-wheels
  • docker/rl/Dockerfile.nmp-rl-base
  • docker/rl/README.md
💤 Files with no reviewable changes (2)
  • docker-bake.hcl
  • docker/base/Dockerfile.python-wheels

Comment thread docker/rl/README.md Outdated
@gabwow

gabwow commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

For the main/upcoming release that doesn't have GRPO merged yet, can we remove the GRPO references in the README or just add (future) by the references?

Signed-off-by: anubhutiv <anubhutiv@nvidia.com>
@anubhutivyas
anubhutivyas force-pushed the anubhutiv/more-rl-cves branch from 77b628a to 340f79a Compare August 10, 2026 23:39
@anubhutivyas anubhutivyas changed the title chore(customizer): fix setuptools CVE + add automodel backend + cleanup chore(customizer): fix setuptools CVE in the RL image + docs cleanup Aug 10, 2026
@anubhutivyas
anubhutivyas added this pull request to the merge queue Aug 11, 2026
Merged via the queue into main with commit 60ad37b Aug 11, 2026
60 checks passed
@anubhutivyas
anubhutivyas deleted the anubhutiv/more-rl-cves branch August 11, 2026 00:43
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.

2 participants