chore: strip redundant CI steps for self-hosted runner#31
chore: strip redundant CI steps for self-hosted runner#31
Conversation
📝 WalkthroughWalkthroughWorkflows were simplified across CI, CodeQL, release, and security configs by removing or guarding explicit Rust toolchain installs and cargo registry caching; CI gains conditionals for the ChangesWorkflow Simplification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
Review rate limit: 3/5 reviews remaining, refill in 20 minutes and 52 seconds. Comment |
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.yml:
- Around line 43-47: The step that uses dtolnay/rust-toolchain is skipped on the
cachekit runner except for rust '1.85' because the if condition is "if:
matrix.runner != 'cachekit' || matrix.rust == '1.85'", which causes the beta
matrix entry to run with the runner default toolchain; update the condition so
the toolchain setup runs on cachekit when matrix.rust == 'beta' (for example add
"|| matrix.rust == 'beta'" or remove the cachekit exclusion), ensuring the uses:
dtolnay/rust-toolchain step actually installs the beta toolchain for the
cachekit runner.
In @.github/workflows/security.yml:
- Around line 70-71: Replace the step that runs the shell command "rustup
default nightly" (the job step named "Use nightly toolchain" that currently
mutates the host-global toolchain) with invocations that prefix cargo commands
with +nightly (for example use "cargo +nightly fuzz build ..." and "cargo
+nightly fuzz run ..." where relevant) so the workflow uses the nightly
toolchain only for those cargo actions without changing the runner's default
toolchain; update both occurrences that currently call "rustup default nightly"
to the targeted "cargo +nightly ..." variants and remove the global rustup
default change.
🪄 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: 399213ee-bedc-49e8-9299-6e8010904446
📒 Files selected for processing (4)
.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/release.yml.github/workflows/security.yml
💤 Files with no reviewable changes (2)
- .github/workflows/release.yml
- .github/workflows/codeql.yml
| if: matrix.runner != 'cachekit' || matrix.rust == '1.85' | ||
| uses: dtolnay/rust-toolchain@3c5f7ea28cd621ae0bf5283f0e981fb97b8a7af9 # master | ||
| with: | ||
| toolchain: ${{ matrix.rust }} | ||
| components: ${{ matrix.rust != '1.85' && 'rustfmt, clippy' || '' }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/ci.yml | head -60Repository: cachekit-io/cachekit-core
Length of output: 2329
Install the beta toolchain on cachekit to preserve real beta coverage.
At Line 43, the condition if: matrix.runner != 'cachekit' || matrix.rust == '1.85' skips setup for beta on cachekit. This causes the beta matrix entry to run with the runner's default toolchain instead of beta, making the beta coverage misleading.
Suggested change
- if: matrix.runner != 'cachekit' || matrix.rust == '1.85'
+ if: matrix.runner != 'cachekit' || matrix.rust != 'stable'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 43 - 47, The step that uses
dtolnay/rust-toolchain is skipped on the cachekit runner except for rust '1.85'
because the if condition is "if: matrix.runner != 'cachekit' || matrix.rust ==
'1.85'", which causes the beta matrix entry to run with the runner default
toolchain; update the condition so the toolchain setup runs on cachekit when
matrix.rust == 'beta' (for example add "|| matrix.rust == 'beta'" or remove the
cachekit exclusion), ensuring the uses: dtolnay/rust-toolchain step actually
installs the beta toolchain for the cachekit runner.
| - name: Use nightly toolchain | ||
| run: rustup default nightly |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/security.yml | head -120Repository: cachekit-io/cachekit-core
Length of output: 4566
🏁 Script executed:
wc -l .github/workflows/security.ymlRepository: cachekit-io/cachekit-core
Length of output: 104
🏁 Script executed:
cat -n .github/workflows/security.yml | tail -64Repository: cachekit-io/cachekit-core
Length of output: 2590
🏁 Script executed:
cat -n .github/workflows/ci.yml | head -80Repository: cachekit-io/cachekit-core
Length of output: 3018
🏁 Script executed:
fd -type f -name "*.yml" -o -name "*.yaml" | grep -E "(runner|setup)" | head -20Repository: cachekit-io/cachekit-core
Length of output: 240
🏁 Script executed:
fd -type f "runner|setup" .github/ 2>/dev/null || find .github -type f \( -name "*runner*" -o -name "*setup*" \)Repository: cachekit-io/cachekit-core
Length of output: 51
🏁 Script executed:
ls -la .github/Repository: cachekit-io/cachekit-core
Length of output: 239
🏁 Script executed:
rg "cargo fuzz" .github/workflows/ -A 3 -B 3Repository: cachekit-io/cachekit-core
Length of output: 1382
🏁 Script executed:
rg "rustup" .github/workflows/ -B 2 -A 2Repository: cachekit-io/cachekit-core
Length of output: 625
Use cargo +nightly instead of rustup default nightly to avoid host-global state mutation on persistent self-hosted runners.
At Lines 70–71 and 109–110, rustup default nightly changes the runner's global toolchain default. On the cachekit self-hosted runner, this persists across job boundaries and can cause subsequent jobs (e.g., kani, cargo-vet) to unexpectedly use nightly instead of the stable toolchain they assume.
Replace with: cargo +nightly fuzz build ... and cargo +nightly fuzz run ...
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/security.yml around lines 70 - 71, Replace the step that
runs the shell command "rustup default nightly" (the job step named "Use nightly
toolchain" that currently mutates the host-global toolchain) with invocations
that prefix cargo commands with +nightly (for example use "cargo +nightly fuzz
build ..." and "cargo +nightly fuzz run ..." where relevant) so the workflow
uses the nightly toolchain only for those cargo actions without changing the
runner's default toolchain; update both occurrences that currently call "rustup
default nightly" to the targeted "cargo +nightly ..." variants and remove the
global rustup default change.
…d in The cachekit ARC runner image (ghcr.io/cachekit-io/runner) includes Rust stable, rustfmt, clippy, cargo-audit, cargo-deny, and cargo-fuzz. - Remove dtolnay/rust-toolchain steps from cachekit jobs (already installed) - Remove actions/cache steps from cachekit jobs (host cache mounted at /cache) - Remove cargo install steps for tools baked into the image - Skip toolchain install on self-hosted unless MSRV (1.85) is needed - Keep toolchain/cache for GitHub-hosted runners (macos, windows) - Add cargo generate-lockfile before cargo vet (library has no Cargo.lock) - Use rustup default nightly for fuzz jobs (simpler than dtolnay action)
c1e77f8 to
2a458d9
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/security.yml (1)
70-71:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse per-command nightly instead of
rustup default nightlyin fuzz jobs.
rustup default nightlymutates host-global toolchain state on self-hosted runners, which can leak into later jobs. Prefercargo +nightly ...for fuzz commands in both jobs.Suggested patch
- - name: Use nightly toolchain - run: rustup default nightly - - name: Run quick fuzz (corpus only) run: | cd fuzz # Build first - fail fast on compile errors - cargo fuzz build ${{ matrix.target }} + cargo +nightly fuzz build ${{ matrix.target }} # Run corpus - timeout exit code 124 is acceptable (means it ran) - timeout 120 cargo fuzz run ${{ matrix.target }} -- -runs=0 -max_total_time=120 || [ $? -eq 124 ] + timeout 120 cargo +nightly fuzz run ${{ matrix.target }} -- -runs=0 -max_total_time=120 || [ $? -eq 124 ] @@ - - name: Use nightly toolchain - run: rustup default nightly - - name: Run deep fuzz (30 minutes per target) run: | cd fuzz # Build first - fail fast on compile errors - cargo fuzz build ${{ matrix.target }} + cargo +nightly fuzz build ${{ matrix.target }} # Run fuzz - timeout exit code 124 is acceptable (means it ran the full duration) - timeout 1800 cargo fuzz run ${{ matrix.target }} -- -max_total_time=1800 || [ $? -eq 124 ] + timeout 1800 cargo +nightly fuzz run ${{ matrix.target }} -- -max_total_time=1800 || [ $? -eq 124 ]#!/bin/bash # Verify there are no runner-global nightly mutations and fuzz commands are toolchain-scoped. rg -n -C2 'rustup default nightly|cargo(\s+\+nightly)?\s+fuzz\s+(build|run)' .github/workflows/security.ymlAlso applies to: 77-79, 109-110, 116-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/security.yml around lines 70 - 71, Replace any global toolchain mutation using the command "rustup default nightly" in the fuzz job steps with per-command scoped invocations; remove the "rustup default nightly" step and change fuzz invocations to use the nightly toolchain prefix (e.g., invoke "cargo +nightly fuzz build" and "cargo +nightly fuzz run" wherever fuzz commands appear) so runner-global state is not mutated and all fuzz commands are toolchain-scoped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/security.yml:
- Around line 70-71: Replace any global toolchain mutation using the command
"rustup default nightly" in the fuzz job steps with per-command scoped
invocations; remove the "rustup default nightly" step and change fuzz
invocations to use the nightly toolchain prefix (e.g., invoke "cargo +nightly
fuzz build" and "cargo +nightly fuzz run" wherever fuzz commands appear) so
runner-global state is not mutated and all fuzz commands are toolchain-scoped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bfdde1b0-e1ab-400b-9122-2c77507b597f
📒 Files selected for processing (4)
.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/release.yml.github/workflows/security.yml
💤 Files with no reviewable changes (2)
- .github/workflows/release.yml
- .github/workflows/codeql.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
Summary
cachekitself-hosted runner (image has Rust stable, cargo-audit, cargo-deny, cargo-fuzz baked in)cargo generate-lockfilebeforecargo vet(fixes fix: generate lockfile before cargo vet #30 — library has no committed Cargo.lock)rustup default nightlyfor fuzz jobs instead of dtolnay actionNet: -134 lines, faster CI (no redundant installs on every run).
Summary by CodeRabbit