Fix kit-less container bugs and ship OVPhysX plus all Newton viewers

[hujc7]

1. Summary

• Fixes 4 QA-reported kit-less container bugs (nvbug 6569001 / 6569031 / 6569217 / 6569864), each verified against QA's verbatim command.
• container.py no longer reports success on a failed Docker command — it exited 0 after printing Finished building, which is why 6569864 arrived as a screenshot with no error text.
• Kit-less image gains OVPhysX and the Viser/Rerun viewers: physics=ovphysx and --viz newton,viser,rerun now work out of the box. Image 14.3 GB → 15.4 GB (+7.7%).
• libxrender1 added: --viz newton crashed with AttributeError: 'NoneType' object has no attribute 'XRenderFindVisualFormat'.

2. Why OVPhysX was excluded, and why it no longer needs to be

ovphysx caps packaging<24; ray (via rl[all]) needs >=24.2. Installing OVPhysX downgraded packaging to 23.2 and broke ray — a real conflict, so the exclusion was correct.

The cap is over-strict: ovphysx's entire use of the library is one from packaging.version import Version in ovphysx/api.py. A packaging>=26.0 override reconciles it, as the neighbouring overrides already do for other over-strict pins. The in-file comment claimed an OmniClient pin instead; omniclient appears nowhere in uv.lock.

3. Validation

Run against the rebuilt image with QA's verbatim commands:

nvbug	Command	Result
6569001	train … physics=ovphysx renderer=ovrtx	rc 0, 0 tracebacks, Training time: 155.44 s
6569031	play …, empty log dir	ValueError: No runs present in the directory (was a bare FileNotFoundError)
6569217	play … --viz newton,viser,rerun	rc 124 (full 300 s window), 0 tracebacks; viser :8080, rerun :9090 serving
6569864	forced Docker failure	exits 1, no Finished building (was exit 0)

Newton regression: presets=newton_mjwarp → Training time: 2.06 s. Build gated by in-image assertions on ovphysx, ovrtx, viser, rerun-sdk.

4. Notes

• nvbug 6569864's reported symptom is a host docker.sock permission error, not an Isaac Lab defect — but it surfaced the silent-success bug fixed here.
• uv pip check now reports ovphysx's declared packaging<24 as violated. That is inherent to any override; no CI job runs uv pip check.
• Not addressed here: Dockerfile.* hard-codes useradd --uid 1000, which breaks bind-mounted source reads on hosts with a different uid and leaves the X cookie unreadable. Separate change.

5. Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Documentation update

6. Checklist

• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added a changelog fragment for the touched Isaac Lab packages

[isaaclab-review-bot]

Isaac Lab Review Bot

The Docker command wrapper correctly centralizes failure propagation, but its error-reporting path breaks for commands containing Path arguments, preventing the intended warning or RuntimeError.

• Design and architecture: Centralizing Docker invocation in _run_docker_command is appropriate, and the check=False option preserves non-fatal artifact-copy behavior. The helper must support the argument types accepted by its call sites.
• API: The intended behavior changes are coherent: Docker build/start/stop/config failures become non-zero exits, artifact-copy failures remain warnings, and get_checkpoint_path now raises its documented ValueError for a missing log directory. However, the Docker failure contract is not reliably met when command arguments include paths.
• Implementation: At line 140, ' '.join(cmd) assumes every command element is a string. Both copy() and config() can include Path objects, so a failed command raises TypeError while constructing the message. This particularly defeats copy(check=False), where missing artifacts are expected to produce a warning. Convert each element when formatting, such as ' '.join(str(part) for part in cmd).

Minor fixes needed. Posted 1 actionable finding inline.

The full PR diff was reviewed; some supplemental surrounding file context was omitted.

Automated review; human maintainers own approval decisions.

[isaaclab-review-bot]

🟡 Warning · Implementation — Failure message joins non-string command parts

' '.join(cmd) requires every element to be str, but copy() appends host_path (a Path) and config() appends output_yaml (a Path). On a non-zero exit these paths raise TypeError instead of emitting the intended warning or RuntimeError. This is most damaging in copy(), where a missing artifact directory returns non-zero and is explicitly meant to be tolerated via check=False. Format with ' '.join(str(part) for part in cmd).

[greptile-apps]

Greptile Summary

The PR improves the kit-less container by shipping OVPhysX and all Newton visualizers, adds the required XRender runtime library, surfaces failed Docker commands, and normalizes missing checkpoint-log errors.

• Expands the kit-less install to ov[all] and visualizer[all], with in-image package assertions.
• Adds libxrender1 for the Newton GL viewer.
• Centralizes Docker subprocess handling so required command failures abort.
• Converts absent checkpoint log directories into the documented ValueError.
• Regenerates the dependency lock with a packaging>=26.0 override.

Confidence Score: 5/5

The PR appears safe to merge based on the reviewed changed paths, with no concrete actionable defect established.

The container command wrapper consistently propagates required Docker failures, optional artifact copying remains explicitly non-fatal, the checkpoint error translation matches the documented contract, and the expanded kit-less dependencies are validated during image construction.

Important Files Changed

Filename	Overview
docker/Dockerfile.kitless	Expands the kit-less environment with OVPhysX and all visualizers, validates their presence, and adds the XRender runtime dependency.
docker/utils/container_interface.py	Introduces centralized Docker command execution that raises on failures while retaining best-effort artifact copying.
source/isaaclab_tasks/isaaclab_tasks/utils/parse_cfg.py	Maps an absent checkpoint log directory to the same documented no-runs ValueError used for an empty directory.
pyproject.toml	Overrides the OVPhysX packaging cap with packaging 26 or newer while preserving unvalidated extra conflicts.
uv.lock	Regenerates the multi-platform dependency resolution for the new override and expanded kit-less extras.
docker/test/test_container_profiles.py	Updates Docker subprocess mocks to represent successful commands under the new return-code checks.
docker/test/test_dockerfile_nonroot.py	Updates static Dockerfile assertions for OVPhysX, visualizers, and libxrender1.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    CLI[container.py operation] --> Helper[_run_docker_command]
    Helper --> Docker[Docker Compose or docker cp]
    Docker -->|exit 0| Success[Continue and report success]
    Docker -->|nonzero, required| Error[Raise RuntimeError]
    Docker -->|nonzero, optional artifact| Warning[Warn and continue]
    Build[Kit-less image build] --> Install[Install Newton, RL, OV, and visualizer extras]
    Install --> Assert[Assert OVPhysX, OVRTX, Viser, and Rerun packages]
    Assert --> Runtime[Kit-less runtime image]

Loading

_{Reviews (1): Last reviewed commit: "Merge branch 'develop' into jichuanh/kit..." | Re-trigger Greptile}