Run teleop_ros2 in all modes in the teleop-ros2-docker test#226
Merged
sgrizan-nv merged 7 commits intomainfrom Apr 6, 2026
Merged
Run teleop_ros2 in all modes in the teleop-ros2-docker test#226sgrizan-nv merged 7 commits intomainfrom
sgrizan-nv merged 7 commits intomainfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughReplaces the CI smoke-test with a matrixed self-hosted GPU job that downloads per-arch build artifacts and runs per-mode Docker verification against CloudXR runtime; adds a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as CI Workflow
participant Runner as Self-hosted Runner
participant Artifact as Artifact Store
participant DockerCtl as Docker Compose (host)
participant Container as Teleop Container
participant Node as teleop_ros2_publisher.py
participant CloudXR as CloudXR Runtime (container)
participant FS as Host Filesystem / Env
CI->>Runner: start `test-teleop-ros2` matrix (arch, python_version)
Runner->>Artifact: download `isaacteleop-install-release-${{ matrix.arch }}-py${{ matrix.python_version }}.tar.gz`
Artifact-->>Runner: deliver tarball
Runner->>FS: extract to `install/`
Runner->>DockerCtl: docker compose up -d cloudxr-runtime (mount host paths)
DockerCtl->>CloudXR: start and report health
Runner->>Container: docker run image (mode, mount runtime paths, envs)
Container->>Node: invoke `teleop_ros2_publisher.py --mode`
Node->>FS: read `ISAAC_TELEOP_PLUGIN_PATH` and call `_find_plugins_dirs(...)`
Node->>Node: build TeleopSessionConfig (include synthetic_hands PluginConfig if `use_mock_operators`)
Node->>CloudXR: connect/start TeleopSession
CloudXR-->>Node: session status / logs
Container-->>Runner: exit (timeout codes allowed/checked)
Runner->>DockerCtl: docker compose down -v
Runner-->>CI: per-mode result reported
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-ubuntu.yml:
- Around line 177-183: No code change required: the "Verify application modes"
workflow loop correctly handles timeout exit code 124 and tests the four modes
matching the application's _TELEOP_MODES; leave the docker run line and the
escaped \$? check as-is (referencing the Verify application modes block,
matrix.ros_distro and the tested modes controller_teleop, hand_teleop,
controller_raw, full_body).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: df4d90ba-198e-4995-9016-4e9da65eb253
📒 Files selected for processing (1)
.github/workflows/build-ubuntu.yml
life1ess
force-pushed
the
sgrizan/branch1
branch
from
3c9122c to
f86f763
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-ubuntu.yml:
- Around line 177-183: The new smoke test job (teleop-ros2-docker) must be added
as a release gate by making the publish jobs depend on it: update the
publish-wheel and publish-github-release-assets jobs (or their parent workflow
triggers) to include teleop-ros2-docker in their needs list (in addition to
build-ubuntu and test-cloudxr) so a failing teleop-ros2-docker run blocks
publishing; locate the job named teleop-ros2-docker and the jobs publish-wheel
and publish-github-release-assets in the workflow and add teleop-ros2-docker to
their needs array (or equivalent dependency configuration).
In `@examples/teleop_ros2/python/teleop_ros2_publisher.py`:
- Around line 84-91: The helper _find_plugins_dirs currently stops after the
first plugins/ directory due to the break, which can hide ancestor plugin
directories (e.g., one containing controller_synthetic_hands) and makes
PluginConfig.search_paths brittle; remove the break so the loop collects every
plugin_dir found for each parent (and the start) and return the full list of
candidate Paths so PluginConfig.search_paths can include all matching plugin
directories.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9171f9ad-98ee-497e-a326-bbcfe2bd092a
📒 Files selected for processing (2)
.github/workflows/build-ubuntu.ymlexamples/teleop_ros2/python/teleop_ros2_publisher.py
life1ess
force-pushed
the
sgrizan/branch1
branch
from
f86f763 to
1036f88
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/build-ubuntu.yml (1)
177-183:⚠️ Potential issue | 🟠 MajorAdd
teleop-ros2-dockerto the publish gates.This new verification job still does not block
publish-wheelorpublish-github-release-assets, so artifacts can be published after a failed mode check.🔒 Proposed gating change
publish-wheel: # to internal Artifactory runs-on: [self-hosted, linux] needs: - build-ubuntu + - teleop-ros2-docker - test-cloudxr environment: dev # Publish only after build and CloudXR tests succeed. - if: ${{ needs.build-ubuntu.result == 'success' && needs.test-cloudxr.result == 'success' }} + if: ${{ needs.build-ubuntu.result == 'success' && needs.teleop-ros2-docker.result == 'success' && needs.test-cloudxr.result == 'success' }} ... publish-github-release-assets: runs-on: ubuntu-latest needs: - build-ubuntu + - teleop-ros2-docker - test-cloudxr ... - if: ${{ github.event_name == 'push' && needs.build-ubuntu.result == 'success' && needs.test-cloudxr.result == 'success' }} + if: ${{ github.event_name == 'push' && needs.build-ubuntu.result == 'success' && needs.teleop-ros2-docker.result == 'success' && needs.test-cloudxr.result == 'success' }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-ubuntu.yml around lines 177 - 183, The new verification job ("Verify application modes") is not included as a prerequisite for the publish jobs, so publishing can proceed even if mode verification fails; update the workflow to add the verification job (the job that runs the for-loop with docker/teleop_ros2_ref) as a gate for the publish jobs by adding its job id (e.g., "Verify application modes" / the job key for the teleop-ros2-docker verification) to the needs/dependencies of "publish-wheel" and "publish-github-release-assets" (or the publish job keys) so those publish jobs only run if the teleop-ros2-docker verification succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-ubuntu.yml:
- Around line 177-183: The loop that runs docker and invokes "timeout 5 uv run
teleop_ros2_publisher.py ..." accepts any zero exit as success; change the
command so you capture the command's exit code and explicitly require it to be
124 (timeout) to pass. Concretely, in the for-loop around the docker run line
(the block invoking teleop_ros2_publisher.py via timeout inside docker), run the
docker command, save its exit code ($?), and then test "[ $exit_code -eq 124 ]"
(fail otherwise) so only a timeout exit from teleop_ros2_publisher.py is
considered success.
In `@examples/teleop_ros2/python/teleop_ros2_publisher.py`:
- Line 93: The _to_pose helper currently types position and orientation as
List[float], which causes false positives when callers pass numpy.ndarray;
change the annotations on _to_pose (function name _to_pose) to accept generic
sequences — e.g. use Sequence[float] for position and Sequence[float] | None (or
Optional[Sequence[float]]) for orientation — and add/import Sequence from typing
so static type checkers accept numpy arrays and other indexable iterables.
---
Duplicate comments:
In @.github/workflows/build-ubuntu.yml:
- Around line 177-183: The new verification job ("Verify application modes") is
not included as a prerequisite for the publish jobs, so publishing can proceed
even if mode verification fails; update the workflow to add the verification job
(the job that runs the for-loop with docker/teleop_ros2_ref) as a gate for the
publish jobs by adding its job id (e.g., "Verify application modes" / the job
key for the teleop-ros2-docker verification) to the needs/dependencies of
"publish-wheel" and "publish-github-release-assets" (or the publish job keys) so
those publish jobs only run if the teleop-ros2-docker verification succeeds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d1363755-fb63-43be-9187-0226b84b3003
📒 Files selected for processing (2)
.github/workflows/build-ubuntu.ymlexamples/teleop_ros2/python/teleop_ros2_publisher.py
life1ess
force-pushed
the
sgrizan/branch1
branch
from
1036f88 to
ac2c860
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-ubuntu.yml:
- Around line 177-184: The CI loop considers a 5s survival as success, but
teleop_ros2_publisher.py's OpenXR retry loop can spin forever and falsely pass;
update the test to assert the node actually enters a session by either (A)
adding a one-time CLI flag to teleop_ros2_publisher.py (e.g., --require-session
or --exit-if-no-session) that causes the process to exit non‑zero if
session.step() is never reached within the timeout, or (B) change the docker run
command to capture and check stdout/stderr for the session start marker (search
for the log line emitted when session.step() is called) within the timeout and
fail if not found; modify the test invocation in the workflow where
teleop_ros2_publisher.py is executed so it fails unless session.step() is
observed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f6263eb1-01c8-439d-806f-5e0669991511
📒 Files selected for processing (2)
.github/workflows/build-ubuntu.ymlexamples/teleop_ros2/python/teleop_ros2_publisher.py
sgrizan-nv
force-pushed
the
sgrizan/branch1
branch
from
ac2c860 to
3a2a1f9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/build-ubuntu.yml (1)
177-184:⚠️ Potential issue | 🟠 MajorTimeout-only success check can still false-pass broken mode startup.
If the node stays in the OpenXR retry loop, this still returns
124and passes without proving mode logic actually ran.🔧 Suggested hardening
- name: Verify application modes run: | touch /tmp/dummy.json for mode in controller_teleop hand_teleop controller_raw full_body; do echo "Testing mode: $mode" - docker run --rm -v /tmp/dummy.json:/tmp/dummy.json -e NV_CXR_RUNTIME_DIR=/tmp -e XR_RUNTIME_JSON=/tmp/dummy.json --entrypoint bash teleop_ros2_ref:${{ matrix.ros_distro }} -c \ - "source /usr/local/bin/teleop-env-setup && timeout 5 uv run teleop_ros2_publisher.py --ros-args -p mode:=$mode -p use_mock_operators:=true; rc=\$?; [ \$rc -eq 124 ]" + log_file="$(mktemp)" + docker run --rm -v /tmp/dummy.json:/tmp/dummy.json -e NV_CXR_RUNTIME_DIR=/tmp -e XR_RUNTIME_JSON=/tmp/dummy.json --entrypoint bash teleop_ros2_ref:${{ matrix.ros_distro }} -c \ + "source /usr/local/bin/teleop-env-setup && timeout 5 uv run teleop_ros2_publisher.py --ros-args -p mode:=$mode -p use_mock_operators:=true" \ + >"${log_file}" 2>&1 + rc=$? + cat "${log_file}" + [ "${rc}" -eq 124 ] + ! grep -q "No XR client connected" "${log_file}" done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-ubuntu.yml around lines 177 - 184, The workflow currently treats a timeout exit code (124) as success, which can false-pass OpenXR retry loops; modify the loop that runs teleop_ros2_publisher.py so it captures stdout/stderr to a temp file and, after the docker run, assert not only that rc is 124 (timeout) or 0, but also grep the captured output for a concrete startup confirmation indicating the requested mode (e.g., a log line containing the mode variable or a phrase like "mode" or "Started teleop" produced by teleop_ros2_publisher.py); update the shell snippet around docker run and the check that follows (the block invoking teleop_ros2_publisher.py) to fail the job if neither a successful exit nor the expected confirmation string for $mode is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/teleop_ros2/python/teleop_ros2_publisher.py`:
- Line 112: The _to_pose signature is failing Ruff formatting; change the
annotation from the PEP 604 union syntax to typing.Optional to satisfy the
linter: update def _to_pose(position: Sequence[float], orientation:
Sequence[float] | None = None) -> Pose: to use Optional[Sequence[float]] for the
orientation parameter and ensure Optional is imported from typing (and keep
Sequence and Pose references unchanged).
---
Duplicate comments:
In @.github/workflows/build-ubuntu.yml:
- Around line 177-184: The workflow currently treats a timeout exit code (124)
as success, which can false-pass OpenXR retry loops; modify the loop that runs
teleop_ros2_publisher.py so it captures stdout/stderr to a temp file and, after
the docker run, assert not only that rc is 124 (timeout) or 0, but also grep the
captured output for a concrete startup confirmation indicating the requested
mode (e.g., a log line containing the mode variable or a phrase like "mode" or
"Started teleop" produced by teleop_ros2_publisher.py); update the shell snippet
around docker run and the check that follows (the block invoking
teleop_ros2_publisher.py) to fail the job if neither a successful exit nor the
expected confirmation string for $mode is present.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: 3ab647ae-9652-48d7-b368-3ffb6e23e132
📒 Files selected for processing (2)
.github/workflows/build-ubuntu.ymlexamples/teleop_ros2/python/teleop_ros2_publisher.py
sgrizan-nv
force-pushed
the
sgrizan/branch1
branch
from
3a2a1f9 to
156c66b
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
176
to
-220
|
|
||
| - name: Smoke test (ROS 2 + rclpy) | ||
| run: | | ||
| docker run --rm --entrypoint bash teleop_ros2_ref:${{ matrix.ros_distro }} -c \ | ||
| "source /usr/local/bin/teleop-env-setup && ros2 --help > /dev/null && python3 -c \"import rclpy; rclpy.init(); rclpy.shutdown(); print('rclpy OK')\"" | ||
| # Run tests with CloudXR runtime | ||
| ./scripts/run_tests_with_cloudxr.sh --python-version ${{ matrix.python_version }} | ||
|
|
||
| test-cloudxr: | ||
| test-teleop-ros2: | ||
| runs-on: [self-hosted, linux, gpu, "${{ matrix.arch }}"] | ||
| needs: build-ubuntu | ||
|
|
||
| strategy: | ||
| matrix: | ||
| python_version: ['3.10', '3.11', '3.12'] | ||
| python_version: ['3.10', '3.12'] | ||
| arch: ['x64', 'arm64'] | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| submodules: recursive | ||
|
|
||
| - name: Install uv | ||
| uses: ./.github/actions/setup-uv | ||
|
|
||
| - name: Download install artifacts | ||
| uses: actions/download-artifact@v7 | ||
| with: | ||
| name: isaacteleop-install-release-${{ matrix.arch }}-py${{ matrix.python_version }} | ||
|
|
||
| - name: Extract tarball to preserve permissions | ||
| run: | | ||
| mkdir -p install | ||
| tar -xvf isaacteleop-install.tar -C install | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 | ||
|
|
||
| - name: Run Tests with CloudXR | ||
| - name: Set ROS Distro | ||
| id: ros-distro | ||
| run: | | ||
| if [ "${{ matrix.python_version }}" = "3.10" ]; then | ||
| echo "ROS_DISTRO=humble" >> $GITHUB_ENV | ||
| elif [ "${{ matrix.python_version }}" = "3.12" ]; then | ||
| echo "ROS_DISTRO=jazzy" >> $GITHUB_ENV | ||
| fi | ||
|
|
||
| - name: Build teleop_ros2 image | ||
| run: | | ||
| docker build -f examples/teleop_ros2/Dockerfile \ | ||
| --build-arg ROS_DISTRO=${{ env.ROS_DISTRO }} \ | ||
| --build-arg PYTHON_VERSION=${{ matrix.python_version }} \ | ||
| -t teleop_ros2_ref:${{ env.ROS_DISTRO }} . | ||
|
|
||
| - name: Verify application modes | ||
| env: | ||
| CI: true | ||
| ACCEPT_CLOUDXR_EULA: Y | ||
| run: | | ||
| # Create .env file with EULA acceptance | ||
| echo "ACCEPT_CLOUDXR_EULA=Y" > deps/cloudxr/.env | ||
|
|
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 18 days ago
In general, the fix is to define an explicit permissions: block either at the workflow root (so it applies to all jobs by default) or on the specific job(s). The minimal safe setting for workflows that only need to read repository content and artifacts is contents: read. This documents the required scope and ensures the workflow remains least-privilege even if repo/org defaults change.
The best fix here without changing functionality is to add a root-level permissions: block right after the on: section in .github/workflows/build-ubuntu.yml. This will apply to all jobs that don’t override permissions (including test-teleop-ros2, which is the one CodeQL flagged). Based on the visible steps, the jobs only require read access to the repository and artifacts; none of the shown steps push commits, publish releases, or modify issues/PRs. Therefore, permissions: contents: read is sufficient and safe as a minimal default. No imports, methods, or additional definitions are needed; this is a pure YAML configuration change.
Concretely:
- Edit
.github/workflows/build-ubuntu.yml. - Insert:
permissions:
contents: readafter the on: block (after the last pull_request: line and before concurrency:). This will satisfy CodeQL and enforce least-privilege GITHUB_TOKEN access for test-teleop-ros2 and other jobs unless they define their own permissions:.
Suggested changeset
1
.github/workflows/build-ubuntu.yml
| @@ -9,6 +9,9 @@ | ||
| tags: [ 'v*' ] | ||
| pull_request: | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| # For PRs, we want to cancel in-progress runs to save resources. | ||
| # For pushes to main or releases, we want to allow all runs to complete. | ||
| concurrency: |
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
sgrizan-nv
force-pushed
the
sgrizan/branch1
branch
3 times, most recently
from
8e13e8f to
e071f9e
Compare
Collaborator
|
Git diff is shown incorrectly, in reality no changes to test-cloudxr were made |
Collaborator
@sgrizan-nv try swapping |
Collaborator
|
Surely that would work, but I feel bad about bad Github tooling dictating how we should structure our code - in Cursor/VSCode the diff correctly shows that a block was removed before test-cloudxr and a new block was added after it. Logically it should be after test-cloudxr, and I'd like to add more tests in the same group in the future. |
sgrizan-nv
force-pushed
the
sgrizan/branch1
branch
from
d49dce1 to
baf321f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-ubuntu.yml:
- Around line 192-247: Replace the shared compose project name and static log
with job-unique values, make the health wait fail-fast if it times out, and
ensure cleanup always runs: change uses of "docker compose -p isaacteleop-test"
to include a unique suffix (e.g. append $GITHUB_RUN_ID or $RUNNER_JOB_ID)
wherever referenced, replace "/tmp/ros2_test_output.log" with a per-job path
(e.g. /tmp/ros2_test_output_$RUNNER_JOB_ID.log) used by the docker run command
and subsequent greps, update the health wait loop (the for i in {1..30} ... ps
... grep -q "healthy") to exit with a clear error if it never becomes healthy,
and wrap the runtime startup and test execution so that the docker compose down
-v command is executed in a trap/finally-like block (guaranteed cleanup) instead
of only on the success path.
In `@examples/teleop_ros2/python/teleop_ros2_publisher.py`:
- Around line 103-109: The plugin discovery function _find_plugins_dirs only
looks for "plugins" directories on ancestor paths and misses source-tree layouts
like "src/plugins"; update _find_plugins_dirs to also check for a "src/plugins"
sibling under each ancestor (e.g., parent / "src" / "plugins") and include those
directories in the returned candidates (avoiding duplicates), so manifests like
src/plugins/controller_synthetic_hands are found without requiring
ISAAC_TELEOP_PLUGIN_PATH; apply the same change to the other discovery usage
referenced in the file.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: ea063f3d-2427-43a0-8d7d-a3984dec6ddc
📒 Files selected for processing (3)
.github/workflows/build-ubuntu.yml.pre-commit-config.yamlexamples/teleop_ros2/python/teleop_ros2_publisher.py
sgrizan-nv
force-pushed
the
sgrizan/branch1
branch
from
baf321f to
0da9eb1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/build-ubuntu.yml (1)
200-201:⚠️ Potential issue | 🟠 MajorMake the compose project and log file unique per matrix leg.
GITHUB_RUN_IDandGITHUB_RUN_ATTEMPTare shared by everytest-teleop-ros2matrix job. If two same-arch legs land on the same Docker host, one leg can truncate the other leg’s/tmplog or tear down the other leg’s CloudXR stack through the EXIT trap.Suggested fix
- PROJECT_NAME="isaacteleop-test-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" - LOG_FILE="/tmp/ros2_test_output_${GITHUB_RUN_ID}_${GITHUB_RUN_ATTEMPT}.log" + py_tag="${{ matrix.python_version }}" + py_tag="${py_tag//./}" + PROJECT_NAME="isaacteleop-test-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}-${{ matrix.arch }}-py${py_tag}" + LOG_FILE="/tmp/ros2_test_output_${PROJECT_NAME}.log"Also applies to: 203-209, 239-257
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-ubuntu.yml around lines 200 - 201, PROJECT_NAME and LOG_FILE are not unique per matrix leg; update their declarations (PROJECT_NAME and LOG_FILE) to append a matrix-unique identifier such as a matrix variable (e.g., ${ { matrix.arch }} or another matrix key) or the runner-specific env (e.g., RUNNER_NAME or RUNNER_TEMP) so each matrix job gets a distinct compose project and logfile; apply the same change to the other occurrences referenced around the later blocks (the same PROJECT_NAME/LOG_FILE usages in the subsequent sections).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-ubuntu.yml:
- Around line 155-158: The checkout step currently leaves the job token in git
config; update every actions/checkout@v6 invocation (including the one shown and
the other occurrences around lines 185-190 and 198+) to disable credential
persistence by setting persist-credentials: false and keep submodules/config as
needed so that the token is not written into the repo config and cannot be read
by subsequent checked-in scripts or Docker builds.
---
Duplicate comments:
In @.github/workflows/build-ubuntu.yml:
- Around line 200-201: PROJECT_NAME and LOG_FILE are not unique per matrix leg;
update their declarations (PROJECT_NAME and LOG_FILE) to append a matrix-unique
identifier such as a matrix variable (e.g., ${ { matrix.arch }} or another
matrix key) or the runner-specific env (e.g., RUNNER_NAME or RUNNER_TEMP) so
each matrix job gets a distinct compose project and logfile; apply the same
change to the other occurrences referenced around the later blocks (the same
PROJECT_NAME/LOG_FILE usages in the subsequent sections).
🪄 Autofix (Beta)
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: Pro
Run ID: 2440dd25-7184-445e-ad2a-e9a716ec7fb8
📒 Files selected for processing (3)
.github/workflows/build-ubuntu.yml.pre-commit-config.yamlexamples/teleop_ros2/python/teleop_ros2_publisher.py
🚧 Files skipped from review as they are similar to previous changes (2)
- .pre-commit-config.yaml
- examples/teleop_ros2/python/teleop_ros2_publisher.py
sgrizan-nv
force-pushed
the
sgrizan/branch1
branch
11 times, most recently
from
9922043 to
fa94c14
Compare
…not able to properly handle diff
sgrizan-nv
force-pushed
the
sgrizan/branch1
branch
from
fa94c14 to
01a42b4
Compare
aristarkhovNV
approved these changes
Apr 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores