OMPE-96696: Using ISAAC_LAB_SAVE_STAGES when running tests corrupts http:// asset URLs in saved USDs

[mataylor-nvidia]

Description

Preserve URI asset paths in save_stage

Avoid resolving asset paths with explicit URI schemes as filesystem paths
during stage save path remapping. This keeps remote assets such as
DomeLight texture URLs from being rewritten into invalid relative paths.

Fixes # (OMPE-96696)

Type of change

• Bug fix (non-breaking change which fixes an issue)

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

• I have read and understood the contribution guidelines
• 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 tests that prove my fix is effective or that my feature works
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[mataylor-nvidia]

@greptileai

[greptile-apps]

Greptile Summary

This PR fixes save_stage / resolve_paths incorrectly rewriting URI asset paths (e.g., https://, omniverse://, s3://) into broken relative filesystem paths when saving a USD stage with ISAAC_LAB_SAVE_STAGES. The root cause was that UsdUtils.ModifyAssetPaths would pass URIs to os.path.relpath, corrupting them.

• Adds _is_uri_path() helper (RFC 3986–compliant) and applies it in two places inside _modify_path: once on the raw asset_path before resolution, and once on the result of ComputeAbsolutePath to guard against relative paths on remote layers resolving to full URIs.
• Adds a parametrised test covering http://, https://, omniverse://, and s3:// that verifies the URI is preserved verbatim in the serialised .usda file.

Confidence Score: 5/5

Safe to merge; the change is narrowly scoped to URI detection in path rewriting and does not affect any non-URI asset paths.

The two-guard approach correctly handles both assets already expressed as URIs and relative paths that resolve to URIs after ComputeAbsolutePath. The RFC 3986 scheme checker is well-constructed, the len(scheme) > 1 guard prevents Windows drive-letter false positives, and the new parametrised test covers all practically relevant URI schemes used in Isaac Lab.

No files require special attention.

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/sim/utils/stage.py	Adds _is_uri_path() RFC 3986 scheme detector and two guards in _modify_path; logic is correct and the len(scheme) > 1 guard prevents Windows single-letter drive paths from being misclassified.
source/isaaclab/test/sim/test_utils_stage.py	New parametrised test now covers http, https, omniverse, and s3 schemes, addressing the previously noted gap; assertions check both preservation and absence of mangled relative paths.
source/isaaclab/changelog.d/fix-stage-uri-paths.rst	Changelog entry accurately describes the fix.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["_modify_path(asset_path)"] --> B{"empty or _is_uri_path?"}
    B -- Yes --> C["return asset_path unchanged"]
    B -- No --> D["resolved = ComputeAbsolutePath(asset_path)"]
    D --> E{"_is_uri_path(resolved)?"}
    E -- Yes --> F["return resolved unchanged"]
    E -- No --> G{"store_relative_path?"}
    G -- Yes --> H["os.path.relpath(resolved, dst_dir)"]
    H --> I{"ValueError?"}
    I -- Yes --> J["return resolved"]
    I -- No --> K["return relative path"]
    G -- No --> L["return resolved or asset_path"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["_modify_path(asset_path)"] --> B{"empty or _is_uri_path?"}
    B -- Yes --> C["return asset_path unchanged"]
    B -- No --> D["resolved = ComputeAbsolutePath(asset_path)"]
    D --> E{"_is_uri_path(resolved)?"}
    E -- Yes --> F["return resolved unchanged"]
    E -- No --> G{"store_relative_path?"}
    G -- Yes --> H["os.path.relpath(resolved, dst_dir)"]
    H --> I{"ValueError?"}
    I -- Yes --> J["return resolved"]
    I -- No --> K["return relative path"]
    G -- No --> L["return resolved or asset_path"]

Loading

_{Reviews (3): Last reviewed commit: "format" | Re-trigger Greptile}

[rilei-nvidia]

Does this second _is_uri_path need to be called? If we determine that asset_path is not a URI, would ComputeAbsolutePath ever return a URI?

[mataylor-nvidia]

asset_path being non-URI only tells us about the authored value. src_layer.ComputeAbsolutePath(asset_path) anchors that value relative to the source layer. If the source layer lives at a URI-backed location, for example omniverse://server/project/scene.usd, then a relative authored path like:
textures/albedo.png
can resolve to something like:
omniverse://server/project/textures/albedo.png

The goal is to detect URI-backed resolved assets when this is the case

[rilei-nvidia]

I see - thanks for the explanation. What could be helpful is adding this as code comments or a docstring of what _modify_paths is trying to achieve - it was not immediately clear with all the conditionals

[rilei-nvidia]

urllib.parse.urlparse is a utility from the python standard library that can aid with parsing the scheme from a URI.

[mataylor-nvidia]

Yes I wanted to try something like this.

This helper is doing a stricter policy check: “treat this asset path as already URI-like only if it has an explicit :// scheme.”

That matters because urlparse will report schemes for things that are not remote/URI asset paths in this context, for example:

urlparse("C:/tmp/model.usd").scheme      # "c"
urlparse("anon:0x123:World.usd").scheme  # "anon"

The custom checks avoid those false positives by requiring:

scheme != asset_path   # the string contained ://
len(scheme) > 1        # avoid Windows-drive-ish C:// cases
scheme[0].isalpha()
all(c.isalnum() or c in "+-." for c in scheme[1:])

The reason I ended up not using it in this code is intentionally is because some paths may be URI-style so we should not relativize or resolve it as a filesystem path. Note: IsaacLab support for windows

[rilei-nvidia]

OK great to know you dug deeper into this. Similar to the comment above I think it would be helpful to add a docstring with your findings so that in the future a developer (or AI) doesn't go and refactor this into using urlparse thinking that would handle these specific situations!