[zizmor] ignore superfluous-actions#7548
Conversation
📝 WalkthroughWalkthroughReplaced several uses of a pinned or manual Rust toolchain installation with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
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)
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 |
Sorry, something went wrong.
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 (2)
.github/workflows/ci.yaml (2)
414-427: ⚠️ Potential issue | 🔴 CriticalUpdate the miri command to use the correct toolchain variable.
Line 414 defines the job environment variable
RUSTUP_TOOLCHAIN: nightly, but Line 427 references the non-existentenv.NIGHTLY_CHANNEL. SinceRUSTUP_TOOLCHAINis already set at the job level, the+${{ env.NIGHTLY_CHANNEL }}prefix should be removed entirely—cargo will automatically use the nightly toolchain without it.🛠️ Proposed fix
- name: Run tests under miri - run: cargo +${{ env.NIGHTLY_CHANNEL }} miri test -p rustpython-vm -- miri_test + run: cargo miri test -p rustpython-vm -- miri_test🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 414 - 427, The workflow uses RUSTUP_TOOLCHAIN: nightly but the "Run tests under miri" step calls cargo with a non-existent toolchain variable (+${{ env.NIGHTLY_CHANNEL }}); remove the +${{ env.NIGHTLY_CHANNEL }} prefix from the "cargo +... miri test -p rustpython-vm -- miri_test" command so it simply runs "cargo miri test -p rustpython-vm -- miri_test" and relies on the job-level RUSTUP_TOOLCHAIN setting.
360-364: ⚠️ Potential issue | 🔴 CriticalSplit the
rustfmtinstall into its own step.This step has both
usesandrun, which violates GitHub Actions workflow syntax. A single step cannot define both keys. Moverustup component add rustfmt --toolchain=stableinto a separate step.🛠️ Proposed fix
- uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 with: python-version: ${{ env.PYTHON_VERSION }} - run: rustup component add rustfmt --toolchain=stable + - name: Install rustfmt + run: rustup component add rustfmt --toolchain=stable🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 360 - 364, The workflow step currently mixes an action usage and a run command (the step with uses: actions/setup-python@a309ff8b426b58ec0e45f0f869d46889d02405 and the run: rustup component add rustfmt --toolchain=stable), which is invalid; fix it by removing the run from that step and creating a new standalone step that only runs the command "rustup component add rustfmt --toolchain=stable" (ensure the new step appears after the setup-python step and uses only the run key), referencing the existing actions/setup-python step to keep ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 414-427: The workflow uses RUSTUP_TOOLCHAIN: nightly but the "Run
tests under miri" step calls cargo with a non-existent toolchain variable (+${{
env.NIGHTLY_CHANNEL }}); remove the +${{ env.NIGHTLY_CHANNEL }} prefix from the
"cargo +... miri test -p rustpython-vm -- miri_test" command so it simply runs
"cargo miri test -p rustpython-vm -- miri_test" and relies on the job-level
RUSTUP_TOOLCHAIN setting.
- Around line 360-364: The workflow step currently mixes an action usage and a
run command (the step with uses:
actions/setup-python@a309ff8b426b58ec0e45f0f869d46889d02405 and the run: rustup
component add rustfmt --toolchain=stable), which is invalid; fix it by removing
the run from that step and creating a new standalone step that only runs the
command "rustup component add rustfmt --toolchain=stable" (ensure the new step
appears after the setup-python step and uses only the run key), referencing the
existing actions/setup-python step to keep ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: b9692e1e-c0cf-422a-9d2f-06acfe2af859
📒 Files selected for processing (1)
.github/workflows/ci.yaml
Sorry, something went wrong.
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 (1)
.github/workflows/ci.yaml (1)
414-427: ⚠️ Potential issue | 🔴 CriticalUpdate the Miri command to reference the correct environment variable.
Line 427 references
env.NIGHTLY_CHANNEL, which is undefined. The job definesRUSTUP_TOOLCHAIN: nightlyat line 414. The cargo command will fail when expanded tocargo +(with empty toolchain). Update to use the defined variable:Suggested fix
- run: cargo +${{ env.NIGHTLY_CHANNEL }} miri test -p rustpython-vm -- miri_test + run: cargo +${{ env.RUSTUP_TOOLCHAIN }} miri test -p rustpython-vm -- miri_test🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 414 - 427, The Miri test step uses an undefined env var NIGHTLY_CHANNEL; update the "Run tests under miri" step to use the defined RUSTUP_TOOLCHAIN variable (or the literal "nightly") so the cargo +<toolchain> invocation expands correctly — change the cargo command that references env.NIGHTLY_CHANNEL to reference env.RUSTUP_TOOLCHAIN (or hardcode "nightly") in that step.
🤖 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/ci.yaml:
- Around line 50-53: The Miri job is using an undefined env var `${{
env.NIGHTLY_CHANNEL }}` and several "stable" jobs install stable but don't force
it for subsequent cargo commands; update the Miri step that runs `cargo +${{
env.NIGHTLY_CHANNEL }} miri test` to use a concrete toolchain (either `cargo
+nightly miri test` or `cargo +${{ env.RUSTUP_TOOLCHAIN }} miri test`) and
ensure the MUST-FIX env is `RUSTUP_TOOLCHAIN: nightly` where that job sets it;
for each job that installs `stable` (where `rustup toolchain install stable` is
run) either add `RUSTUP_TOOLCHAIN: stable` at the job-level env or change
subsequent invocations like `cargo clippy`, `cargo shear`, and other `cargo ...`
steps to explicitly run `cargo +stable ...` so they deterministically use the
stable toolchain.
---
Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 414-427: The Miri test step uses an undefined env var
NIGHTLY_CHANNEL; update the "Run tests under miri" step to use the defined
RUSTUP_TOOLCHAIN variable (or the literal "nightly") so the cargo +<toolchain>
invocation expands correctly — change the cargo command that references
env.NIGHTLY_CHANNEL to reference env.RUSTUP_TOOLCHAIN (or hardcode "nightly") in
that step.
🪄 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.yml
Review profile: CHILL
Plan: Pro
Run ID: 5bda2a95-4ca3-4b4f-9a59-1eb46e269f5e
📒 Files selected for processing (1)
.github/workflows/ci.yaml
Sorry, something went wrong.
|
any reason to change? |
Sorry, something went wrong.
The initial motivation of this PR was to resolve "note" level warnings from zizmor, as the functionality of |
Sorry, something went wrong.
|
After reading zizmorcore/zizmor#1817 I've changed my mind. I'll make zizmor to ignore this, and shift this PR to ignore this rule instead |
Sorry, something went wrong.
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/ci.yaml:
- Line 53: Replace all uses of the reusable action reference
dtolnay/rust-toolchain that use mutable tags (e.g., `@stable` and `@master`) with
their corresponding full 40-character commit SHAs to avoid mutable refs; update
each occurrence (the instances currently written as
dtolnay/rust-toolchain@stable and dtolnay/rust-toolchain@master) to the exact
commit SHA and add a short comment on the same line preserving the original
tag/intent (for example: # originally `@stable`) so CI remains reproducible and
reviewers know the intended channel.
In @.github/workflows/release.yml:
- Line 55: Replace both mutable references of the dtolnay/rust-toolchain action
("uses: dtolnay/rust-toolchain@stable") with the repository's immutable
40-character commit SHA for the current stable tag and add an inline comment
noting which stable tag the SHA corresponds to; locate and update the two
occurrences in the release workflow where "uses: dtolnay/rust-toolchain@stable"
appears, fetch the current commit SHA from the dtolnay/rust-toolchain GitHub
repo for the stable tag, and use that SHA in place of "@stable" so the workflow
is pinned to a specific commit.
🪄 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.yml
Review profile: CHILL
Plan: Pro
Run ID: a74f15d1-21fe-4bae-b98f-96e3a0c6ab80
📒 Files selected for processing (3)
.github/workflows/ci.yaml.github/workflows/release.yml.github/zizmor.yml
✅ Files skipped from review due to trivial changes (1)
- .github/zizmor.yml
Sorry, something went wrong.
344b7a5
into
RustPython:main
Apr 2, 2026
Summary by CodeRabbit