feat: Add ROS 2 Lyrical installers#70
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for ROS 2 Lyrical (LTS) on Ubuntu 26.04 (resolute) by adding two new installation scripts, updating the README documentation, and adding a test case to verify OS compatibility checks. The review feedback highlights two main areas of improvement across both new scripts: adding robust error handling to the curl commands fetching the latest ros-apt-source version, and replacing the non-standard which command with command -v for checking command existence.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ROS_APT_SOURCE_VERSION=$(curl -s https://api.github.com/repos/ros-infrastructure/ros-apt-source/releases/latest | grep -F "tag_name" | awk -F'"' '{print $4}') | ||
| curl -L -o /tmp/ros2-apt-source.deb "https://github.com/ros-infrastructure/ros-apt-source/releases/download/${ROS_APT_SOURCE_VERSION}/ros2-apt-source_${ROS_APT_SOURCE_VERSION}.$(. /etc/os-release && echo ${UBUNTU_CODENAME:-${VERSION_CODENAME}})_all.deb" |
There was a problem hiding this comment.
The curl command lacks error handling. If the GitHub API rate limit is exceeded or a network error occurs, ROS_APT_SOURCE_VERSION will be empty, leading to a subsequent silent failure or a corrupted package download because curl without -f does not return a non-zero exit code on HTTP errors (like 404). Adding -f / --fail and checking if the version string is empty improves robustness.
| ROS_APT_SOURCE_VERSION=$(curl -s https://api.github.com/repos/ros-infrastructure/ros-apt-source/releases/latest | grep -F "tag_name" | awk -F'"' '{print $4}') | |
| curl -L -o /tmp/ros2-apt-source.deb "https://github.com/ros-infrastructure/ros-apt-source/releases/download/${ROS_APT_SOURCE_VERSION}/ros2-apt-source_${ROS_APT_SOURCE_VERSION}.$(. /etc/os-release && echo ${UBUNTU_CODENAME:-${VERSION_CODENAME}})_all.deb" | |
| ROS_APT_SOURCE_VERSION=$(curl -sSf https://api.github.com/repos/ros-infrastructure/ros-apt-source/releases/latest | grep -F "tag_name" | awk -F'"' '{print $4}') | |
| if [[ -z "$ROS_APT_SOURCE_VERSION" ]]; then | |
| echo "ERROR: Failed to fetch the latest ros-apt-source version." >&2 | |
| exit 1 | |
| fi | |
| curl -sSLf -o /tmp/ros2-apt-source.deb "https://github.com/ros-infrastructure/ros-apt-source/releases/download/${ROS_APT_SOURCE_VERSION}/ros2-apt-source_${ROS_APT_SOURCE_VERSION}.$(. /etc/os-release && echo ${UBUNTU_CODENAME:-${VERSION_CODENAME}})_all.deb" |
| ROS_APT_SOURCE_VERSION=$(curl -s https://api.github.com/repos/ros-infrastructure/ros-apt-source/releases/latest | grep -F "tag_name" | awk -F'"' '{print $4}') | ||
| curl -L -o /tmp/ros2-apt-source.deb "https://github.com/ros-infrastructure/ros-apt-source/releases/download/${ROS_APT_SOURCE_VERSION}/ros2-apt-source_${ROS_APT_SOURCE_VERSION}.$(. /etc/os-release && echo ${UBUNTU_CODENAME:-${VERSION_CODENAME}})_all.deb" |
There was a problem hiding this comment.
The curl command lacks error handling. If the GitHub API rate limit is exceeded or a network error occurs, ROS_APT_SOURCE_VERSION will be empty, leading to a subsequent silent failure or a corrupted package download because curl without -f does not return a non-zero exit code on HTTP errors (like 404). Adding -f / --fail and checking if the version string is empty improves robustness.
| ROS_APT_SOURCE_VERSION=$(curl -s https://api.github.com/repos/ros-infrastructure/ros-apt-source/releases/latest | grep -F "tag_name" | awk -F'"' '{print $4}') | |
| curl -L -o /tmp/ros2-apt-source.deb "https://github.com/ros-infrastructure/ros-apt-source/releases/download/${ROS_APT_SOURCE_VERSION}/ros2-apt-source_${ROS_APT_SOURCE_VERSION}.$(. /etc/os-release && echo ${UBUNTU_CODENAME:-${VERSION_CODENAME}})_all.deb" | |
| ROS_APT_SOURCE_VERSION=$(curl -sSf https://api.github.com/repos/ros-infrastructure/ros-apt-source/releases/latest | grep -F "tag_name" | awk -F'"' '{print $4}') | |
| if [[ -z "$ROS_APT_SOURCE_VERSION" ]]; then | |
| echo "ERROR: Failed to fetch the latest ros-apt-source version." >&2 | |
| exit 1 | |
| fi | |
| curl -sSLf -o /tmp/ros2-apt-source.deb "https://github.com/ros-infrastructure/ros-apt-source/releases/download/${ROS_APT_SOURCE_VERSION}/ros2-apt-source_${ROS_APT_SOURCE_VERSION}.$(. /etc/os-release && echo ${UBUNTU_CODENAME:-${VERSION_CODENAME}})_all.deb" |
| TARGET_OS=resolute | ||
|
|
||
| # Check OS version | ||
| if ! which lsb_release > /dev/null ; then |
There was a problem hiding this comment.
Using which to check for command existence is non-standard and can behave inconsistently across different POSIX shells or minimal environments. It is recommended to use command -v instead.
| if ! which lsb_release > /dev/null ; then | |
| if ! command -v lsb_release > /dev/null ; then |
| TARGET_OS=resolute | ||
|
|
||
| # Check OS version | ||
| if ! which lsb_release > /dev/null ; then |
There was a problem hiding this comment.
Using which to check for command existence is non-standard and can behave inconsistently across different POSIX shells or minimal environments. It is recommended to use command -v instead.
| if ! which lsb_release > /dev/null ; then | |
| if ! command -v lsb_release > /dev/null ; then |
📝 WalkthroughWalkthroughAdds ROS 2 Lyrical (LTS) support: two new installer scripts (desktop and ros-base), CI workflow jobs to run and smoke-test them on Ubuntu 26.04, a Bats OS-validation test, and README updates listing Lyrical installers and supported versions. ChangesROS 2 Lyrical Installation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/installers.yml (1)
6-20:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude the Bats OS-check files in these path filters.
The new Noble test only runs in this PR because the workflow file itself also changed. A future PR that edits
.test/test-os-check-noble.batsor its shared helpers alone will be filtered out here, so the OS-gate CI can drift silently.Suggested fix
paths: - "ros2-humble-**.sh" - "ros2-jazzy-**.sh" - "ros2-kilted-**.sh" - "ros2-lyrical-**.sh" + - ".test/test-os-check-*.bats" + - ".test/test_helper/**" - ".github/workflows/installers.yml" @@ paths: - "ros2-humble-**.sh" - "ros2-jazzy-**.sh" - "ros2-kilted-**.sh" - "ros2-lyrical-**.sh" + - ".test/test-os-check-*.bats" + - ".test/test_helper/**" - ".github/workflows/installers.yml"🤖 Prompt for 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. In @.github/workflows/installers.yml around lines 6 - 20, Update the installers workflow path filters to include the Bats OS-check test files so changes to those tests trigger the workflow; specifically, add the relevant test paths (e.g. ".test/test-os-check-*.bats" or the exact file names like ".test/test-os-check-noble.bats" and any shared helpers) to both the push and pull_request paths lists in .github/workflows/installers.yml next to the existing "ros2-*-**.sh" entries so edits to the OS-check tests no longer get filtered out.
🧹 Nitpick comments (1)
.test/test-os-check-noble.bats (1)
21-25: ⚡ Quick winCover the
ros-baseOS gate here too.With no Ubuntu 26.04 runner yet, this Bats file is the only automated execution path for Lyrical. It exercises
./ros2-lyrical-desktop-main.sh, but./ros2-lyrical-ros-base-main.shis still untested, so an OS-check typo there would merge without any signal.Suggested addition
`@test` "Lyrical fail on Ubuntu 24.04" { run ./ros2-lyrical-desktop-main.sh [ "$status" -eq 1 ] assert_output --partial "ERROR: This OS (version: noble) is not supported" } + +@test "Lyrical ros-base fail on Ubuntu 24.04" { + run ./ros2-lyrical-ros-base-main.sh + [ "$status" -eq 1 ] + assert_output --partial "ERROR: This OS (version: noble) is not supported" +}🤖 Prompt for 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. In @.test/test-os-check-noble.bats around lines 21 - 25, Add a parallel Bats test that covers the ros-base script: duplicate the existing test "Lyrical fail on Ubuntu 24.04" in .test/test-os-check-noble.bats but run ./ros2-lyrical-ros-base-main.sh instead of ./ros2-lyrical-desktop-main.sh and assert the same exit status and partial output ("ERROR: This OS (version: noble) is not supported"); this ensures the OS-check in the ros-base entrypoint (./ros2-lyrical-ros-base-main.sh) is exercised and will catch any typographical or logic errors.
🤖 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 `@ros2-lyrical-desktop-main.sh`:
- Around line 48-50: Add back the pre-install system upgrade step before
installing the ROS apt package: after installing /tmp/ros2-apt-source.deb (the
block that uses sudo dpkg -i) and before running sudo apt-get install -y
ros-$CHOOSE_ROS_DISTRO-$INSTALL_PACKAGE, run an apt update/upgrade sequence
(e.g., sudo apt-get update && sudo apt-get -y upgrade) to ensure the system
packages are up-to-date per the Lyrical install docs; update the script around
the sudo dpkg -i and sudo apt-get install lines in ros2-lyrical-desktop-main.sh
so the upgrade runs first and then proceeds to install the ROS package.
In `@ros2-lyrical-ros-base-main.sh`:
- Around line 48-50: Restore the pre-install upgrade step before installing ROS
packages: after running "sudo dpkg -i /tmp/ros2-apt-source.deb" and immediately
following "sudo apt-get update", run an upgrade (e.g., "sudo apt-get upgrade -y"
or the project's preferred upgrade command) to ensure packages are up-to-date
before the "sudo apt-get install -y ros-$CHOOSE_ROS_DISTRO-$INSTALL_PACKAGE"
step; update the script around the dpkg/update/install block to include this
upgrade command.
---
Outside diff comments:
In @.github/workflows/installers.yml:
- Around line 6-20: Update the installers workflow path filters to include the
Bats OS-check test files so changes to those tests trigger the workflow;
specifically, add the relevant test paths (e.g. ".test/test-os-check-*.bats" or
the exact file names like ".test/test-os-check-noble.bats" and any shared
helpers) to both the push and pull_request paths lists in
.github/workflows/installers.yml next to the existing "ros2-*-**.sh" entries so
edits to the OS-check tests no longer get filtered out.
---
Nitpick comments:
In @.test/test-os-check-noble.bats:
- Around line 21-25: Add a parallel Bats test that covers the ros-base script:
duplicate the existing test "Lyrical fail on Ubuntu 24.04" in
.test/test-os-check-noble.bats but run ./ros2-lyrical-ros-base-main.sh instead
of ./ros2-lyrical-desktop-main.sh and assert the same exit status and partial
output ("ERROR: This OS (version: noble) is not supported"); this ensures the
OS-check in the ros-base entrypoint (./ros2-lyrical-ros-base-main.sh) is
exercised and will catch any typographical or logic errors.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c31da2c-494e-4adf-965f-e00a50ab3d7c
📒 Files selected for processing (5)
.github/workflows/installers.yml.test/test-os-check-noble.batsREADME.mdros2-lyrical-desktop-main.shros2-lyrical-ros-base-main.sh
| sudo dpkg -i /tmp/ros2-apt-source.deb | ||
| sudo apt-get update | ||
| sudo apt-get install -y ros-$CHOOSE_ROS_DISTRO-$INSTALL_PACKAGE |
There was a problem hiding this comment.
Restore the pre-install apt upgrade step.
These installers skip the upgrade that the existing distro scripts already perform before installing ROS packages. The current Lyrical Ubuntu-deb instructions still call out upgrading first because ROS packages are built against frequently updated Ubuntu systems; omitting it here makes fresh Resolute installs more fragile. (docs.ros.org)
Suggested fix
sudo dpkg -i /tmp/ros2-apt-source.deb
sudo apt-get update
+sudo apt-get upgrade -y
sudo apt-get install -y ros-$CHOOSE_ROS_DISTRO-$INSTALL_PACKAGE📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sudo dpkg -i /tmp/ros2-apt-source.deb | |
| sudo apt-get update | |
| sudo apt-get install -y ros-$CHOOSE_ROS_DISTRO-$INSTALL_PACKAGE | |
| sudo dpkg -i /tmp/ros2-apt-source.deb | |
| sudo apt-get update | |
| sudo apt-get upgrade -y | |
| sudo apt-get install -y ros-$CHOOSE_ROS_DISTRO-$INSTALL_PACKAGE |
🤖 Prompt for 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.
In `@ros2-lyrical-desktop-main.sh` around lines 48 - 50, Add back the pre-install
system upgrade step before installing the ROS apt package: after installing
/tmp/ros2-apt-source.deb (the block that uses sudo dpkg -i) and before running
sudo apt-get install -y ros-$CHOOSE_ROS_DISTRO-$INSTALL_PACKAGE, run an apt
update/upgrade sequence (e.g., sudo apt-get update && sudo apt-get -y upgrade)
to ensure the system packages are up-to-date per the Lyrical install docs;
update the script around the sudo dpkg -i and sudo apt-get install lines in
ros2-lyrical-desktop-main.sh so the upgrade runs first and then proceeds to
install the ROS package.
| sudo dpkg -i /tmp/ros2-apt-source.deb | ||
| sudo apt-get update | ||
| sudo apt-get install -y ros-$CHOOSE_ROS_DISTRO-$INSTALL_PACKAGE |
There was a problem hiding this comment.
Restore the pre-install apt upgrade step.
This variant has the same regression as the desktop installer: it goes straight from apt-get update to apt install ros-lyrical-ros-base. The Lyrical deb-install guide still recommends upgrading first, so skipping it here makes the new installer less reliable on fresh Resolute machines. (docs.ros.org)
Suggested fix
sudo dpkg -i /tmp/ros2-apt-source.deb
sudo apt-get update
+sudo apt-get upgrade -y
sudo apt-get install -y ros-$CHOOSE_ROS_DISTRO-$INSTALL_PACKAGE📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sudo dpkg -i /tmp/ros2-apt-source.deb | |
| sudo apt-get update | |
| sudo apt-get install -y ros-$CHOOSE_ROS_DISTRO-$INSTALL_PACKAGE | |
| sudo dpkg -i /tmp/ros2-apt-source.deb | |
| sudo apt-get update | |
| sudo apt-get upgrade -y | |
| sudo apt-get install -y ros-$CHOOSE_ROS_DISTRO-$INSTALL_PACKAGE |
🤖 Prompt for 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.
In `@ros2-lyrical-ros-base-main.sh` around lines 48 - 50, Restore the pre-install
upgrade step before installing ROS packages: after running "sudo dpkg -i
/tmp/ros2-apt-source.deb" and immediately following "sudo apt-get update", run
an upgrade (e.g., "sudo apt-get upgrade -y" or the project's preferred upgrade
command) to ensure packages are up-to-date before the "sudo apt-get install -y
ros-$CHOOSE_ROS_DISTRO-$INSTALL_PACKAGE" step; update the script around the
dpkg/update/install block to include this upgrade command.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/installers.yml (1)
123-170:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHarden CI security for the new Lyrical jobs (token permissions + checkout credentials + pin action)
In
.github/workflows/installers.yml, jobslyrical-ros-base-mainandlyrical-desktop-mainuse unpinnedactions/checkout@v6, there is no explicitpermissions:block, and the checkout step does not setpersist-credentials: false.
- Pin checkout:
actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10- Add least-privilege
permissions(e.g.,contents: read)- Disable persisted credentials on checkout
Suggested hardening patch
lyrical-ros-base-main: + permissions: + contents: read runs-on: ubuntu-24.04 container: ubuntu:26.04 @@ - - uses: actions/checkout@v6 + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 + with: + persist-credentials: false - name: Run the install script run: | ./ros2-lyrical-ros-base-main.sh @@ lyrical-desktop-main: + permissions: + contents: read runs-on: ubuntu-24.04 container: ubuntu:26.04 @@ - - uses: actions/checkout@v6 + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 + with: + persist-credentials: false - name: Run the install script run: | ./ros2-lyrical-desktop-main.sh🤖 Prompt for 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. In @.github/workflows/installers.yml around lines 123 - 170, The CI jobs lyrical-ros-base-main and lyrical-desktop-main use unpinned checkout, lack a permissions block, and leave persisted credentials enabled; update each job to use the pinned actions/checkout commit id (actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10), add a least-privilege permissions entry (e.g., permissions: contents: read) at the job level, and set persist-credentials: false on the checkout step to avoid exposing tokens (locate the uses: actions/checkout@... and the job blocks for lyrical-ros-base-main and lyrical-desktop-main to apply these changes).
🤖 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.
Outside diff comments:
In @.github/workflows/installers.yml:
- Around line 123-170: The CI jobs lyrical-ros-base-main and
lyrical-desktop-main use unpinned checkout, lack a permissions block, and leave
persisted credentials enabled; update each job to use the pinned
actions/checkout commit id
(actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10), add a
least-privilege permissions entry (e.g., permissions: contents: read) at the job
level, and set persist-credentials: false on the checkout step to avoid exposing
tokens (locate the uses: actions/checkout@... and the job blocks for
lyrical-ros-base-main and lyrical-desktop-main to apply these changes).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1d96b496-b089-4199-8b48-bfbd7c4218a6
📒 Files selected for processing (1)
.github/workflows/installers.yml
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/installers.yml (2)
27-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSet explicit least-privilege workflow permissions.
No
permissionsblock is defined, soGITHUB_TOKENpermissions depend on repository defaults. These jobs only need checkout + local script execution; setcontents: readexplicitly.Suggested patch
name: ROS2 installers test on: @@ schedule: - cron: "0 1 * * 1" +permissions: + contents: read + env: PATH: /usr/bin:/usr/sbin:/bin:/sbin:/usr/local/bin:/usr/local/sbin🤖 Prompt for 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. In @.github/workflows/installers.yml at line 27, The workflow currently lacks an explicit permissions block so GITHUB_TOKEN falls back to repo defaults; add a top-level permissions declaration in the workflow (near the existing jobs: block) and set permissions.contents to read to enforce least-privilege for checkout and local script execution. Update the workflow file to include a permissions: section with contents: read (and no broader rights) so jobs that use actions/checkout and run scripts only get the minimal required permission.
6-20:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd test-related paths to workflow triggers.
At Line 46 this workflow runs
.test/test-os-check-noble.bats, but Line 6-Line 20 path filters don’t include.test/**/*.bats(ortutorial.sh, which is executed in Line 59/71/84/96/109/121/145/168). Changes to those files can bypass CI validation.Suggested patch
on: push: branches: - main paths: - "ros2-humble-**.sh" - "ros2-jazzy-**.sh" - "ros2-kilted-**.sh" - "ros2-lyrical-**.sh" + - ".test/**/*.bats" + - "tutorial.sh" - ".github/workflows/installers.yml" pull_request: branches: - main paths: - "ros2-humble-**.sh" - "ros2-jazzy-**.sh" - "ros2-kilted-**.sh" - "ros2-lyrical-**.sh" + - ".test/**/*.bats" + - "tutorial.sh" - ".github/workflows/installers.yml"🤖 Prompt for 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. In @.github/workflows/installers.yml around lines 6 - 20, The workflow path filters in .github/workflows/installers.yml currently list only installer scripts and the workflow file itself; add test file patterns so changes to tests trigger CI — update both the push and pull_request "paths" arrays to include patterns for tests, e.g. ".test/**/*.bats" and any tutorial scripts like ".test/**/tutorial.sh" (or "tutorial.sh") so files referenced by the workflow (e.g., the test runner invoked at Line 46 and tutorial.sh executed at Lines 59/71/84/96/109/121/145/168) will cause the workflow to run when changed.
🤖 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 @.github/workflows/installers.yml:
- Around line 141-145: Replace the heavy tutorial build flow invoked in the "Run
the test script" step (the sed lines that edit tutorial.sh and the bash <(head
-n -2 ./tutorial.sh) invocation) with explicit lightweight smoke checks for
Lyrical jobs: remove the sed edits and the tutorial.sh execution and instead run
targeted, fast validations (for example: a minimal colcon build of only lyrical
packages or a quick smoke test command such as running a small unit-test subset
or a CLI sanity check) so the job becomes a fast smoke check rather than
cloning/building the full tutorial; update the step named "Run the test script"
to run these explicit smoke commands.
---
Outside diff comments:
In @.github/workflows/installers.yml:
- Line 27: The workflow currently lacks an explicit permissions block so
GITHUB_TOKEN falls back to repo defaults; add a top-level permissions
declaration in the workflow (near the existing jobs: block) and set
permissions.contents to read to enforce least-privilege for checkout and local
script execution. Update the workflow file to include a permissions: section
with contents: read (and no broader rights) so jobs that use actions/checkout
and run scripts only get the minimal required permission.
- Around line 6-20: The workflow path filters in
.github/workflows/installers.yml currently list only installer scripts and the
workflow file itself; add test file patterns so changes to tests trigger CI —
update both the push and pull_request "paths" arrays to include patterns for
tests, e.g. ".test/**/*.bats" and any tutorial scripts like
".test/**/tutorial.sh" (or "tutorial.sh") so files referenced by the workflow
(e.g., the test runner invoked at Line 46 and tutorial.sh executed at Lines
59/71/84/96/109/121/145/168) will cause the workflow to run when changed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: adf8da94-d6ec-4974-b6cc-99cd761c84fe
📒 Files selected for processing (1)
.github/workflows/installers.yml
| - name: Run the test script | ||
| run: | | ||
| sed -e 's/^\(ROS_DISTRO=.*\)/#\1\nROS_DISTRO=lyrical/g' -i tutorial.sh | ||
| sed -e 's/^\(source \/opt.*bash\)$/\1\nrosdep install -r -y -i --from-paths ./g' -i tutorial.sh | ||
| bash <(head -n -2 ./tutorial.sh) |
There was a problem hiding this comment.
Use explicit smoke checks for Lyrical jobs instead of tutorial build flow.
Line 145 and Line 168 still execute tutorial.sh (which performs clone/build/test), so these jobs are not lightweight smoke checks and are likely to be slower/flakier in ubuntu:26.04 containers.
Suggested patch
lyrical-ros-base-main:
@@
- - name: Run the test script
+ - name: Run smoke tests
run: |
- sed -e 's/^\(ROS_DISTRO=.*\)/#\1\nROS_DISTRO=lyrical/g' -i tutorial.sh
- sed -e 's/^\(source \/opt.*bash\)$/\1\nrosdep install -r -y -i --from-paths ./g' -i tutorial.sh
- bash <(head -n -2 ./tutorial.sh)
+ source /opt/ros/lyrical/setup.bash
+ ros2 pkg prefix rclcpp
+ ros2 interface show std_msgs/msg/String
lyrical-desktop-main:
@@
- - name: Run the test script
+ - name: Run smoke tests
run: |
- sed -e 's/^\(ROS_DISTRO=.*\)/#\1\nROS_DISTRO=lyrical/g' -i tutorial.sh
- bash <(head -n -2 ./tutorial.sh)
+ source /opt/ros/lyrical/setup.bash
+ ros2 pkg prefix rclcpp
+ ros2 interface show std_msgs/msg/String
+ ros2 pkg prefix rviz2Also applies to: 165-168
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 123-145: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 Prompt for 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.
In @.github/workflows/installers.yml around lines 141 - 145, Replace the heavy
tutorial build flow invoked in the "Run the test script" step (the sed lines
that edit tutorial.sh and the bash <(head -n -2 ./tutorial.sh) invocation) with
explicit lightweight smoke checks for Lyrical jobs: remove the sed edits and the
tutorial.sh execution and instead run targeted, fast validations (for example: a
minimal colcon build of only lyrical packages or a quick smoke test command such
as running a small unit-test subset or a CLI sanity check) so the job becomes a
fast smoke check rather than cloning/building the full tutorial; update the step
named "Run the test script" to run these explicit smoke commands.
1 participant
Summary
Testing
Note: GitHub Actions does not have a GitHub-hosted Ubuntu 26.04 runner yet, so the Lyrical checks run on ubuntu-24.04 with an ubuntu:26.04 container.
Summary by CodeRabbit
New Features
Tests
Documentation
Chores