fix(ci): resolve CI/CD release failures - #25
Conversation
test(git): skip pre-commit hook test on Windows CI fix(observer): add serial_test to prevent race conditions in env var tests
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a multi-platform Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
|
|
||
| - name: Build and release | ||
| uses: tauri-apps/tauri-action@v0 | ||
| uses: tauri-apps/tauri-action@dev |
There was a problem hiding this comment.
🔥 The Roast: Using @dev branch for a CI/CD action is like choosing "trust me, bro" as your deployment strategy. This is the digital equivalent of Russian roulette with your release pipeline — except the gun always has at least one bullet.
🩹 The Fix: Pin to a specific version tag like tauri-apps/tauri-action@v0.3.0 or wait for the next stable release. Your future self (and your users) will thank you when the build doesn't mysteriously break because someone pushed a commit to dev.
📏 Severity: critical
Code Review Roast 🔥Verdict: Issues Addressed | Recommendation: Merge Overview
Issue Details (click to expand)
🏆 Best part: Where do I start? The pinned SHA in release.yml ( 💀 Worst part: The 📊 Overall: This PR is a net positive — fixes the critical security concern, adds robust test infrastructure, and improves test reliability with serial execution. The only remaining "issue" is a cosmetic dependency inconsistency that has zero runtime impact. Merge with confidence. Files Reviewed (6 files)
Note: The existing inline comments about |
Greptile SummaryThis PR addresses three CI/CD failures in
Confidence Score: 2/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[git push tag v*] --> B[release.yml triggered]
B --> C{Matrix platform}
C --> D[ubuntu-22.04]
C --> E[macos-latest]
C --> F[windows-latest]
D & E & F --> G[Checkout + Setup Node + Rust]
G --> H[Install deps]
H --> I[npm ci + npm test]
I --> J["cargo test\n(all tests)"]
J -->|Windows| J1["test_commit_changes_pre_commit_hook_failure\n→ IGNORED via #[cfg_attr(windows,ignore)]"]
J -->|Linux| J2["observer backend tests\n→ run SERIALLY via #[serial]"]
J -->|macOS| J3[All tests run normally]
J1 & J2 & J3 --> K["tauri-apps/tauri-action@dev ⚠️\n(mutable branch — security risk)"]
K --> L[Build & upload release artifacts]
L --> M[GitHub Release Draft]
subgraph ci.yml [ci.yml — new rust-test job]
N[push to main/develop or PR] --> O{Matrix platform}
O --> P[ubuntu-latest]
O --> Q[macos-latest]
O --> R[windows-latest]
P & Q & R --> S["cargo test --lib --bin ronin-observer"]
end
|
|
|
||
| - name: Build and release | ||
| uses: tauri-apps/tauri-action@v0 | ||
| uses: tauri-apps/tauri-action@dev |
There was a problem hiding this comment.
Unpinned mutable branch reference is a supply chain risk
Using tauri-apps/tauri-action@dev pins to the tip of a mutable branch rather than an immutable tag or commit SHA. This means any commit pushed to the dev branch — including a potentially malicious one — will silently execute in your release pipeline on the next run with full access to GITHUB_TOKEN and other secrets.
The original @v0 issue was a schema validation error, which is likely already resolved in a newer stable release. The safer alternatives are:
- Pin to a specific commit SHA (most secure):
| uses: tauri-apps/tauri-action@dev | |
| uses: tauri-apps/tauri-action@5def7825be7cd2d62e7b0c3f6cf4ab3d93db68e |
- Pin to a newer release tag (if available):
| uses: tauri-apps/tauri-action@dev | |
| uses: tauri-apps/tauri-action@v0.5 |
Please check the tauri-action releases for the latest stable tag that includes the schema fix, and pin to that instead.
| - name: Install system dependencies (Linux only) | ||
| if: matrix.platform == 'ubuntu-latest' | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y \ | ||
| libwebkit2gtk-4.1-dev \ | ||
| build-essential \ | ||
| libssl-dev \ | ||
| libayatana-appindicator3-dev \ | ||
| librsvg2-dev |
There was a problem hiding this comment.
Missing libxdo-dev compared to other Linux jobs
The release.yml Linux step and the existing build job in ci.yml both install libxdo-dev, but the new rust-test job omits it. If any crate compiled during cargo test --lib --bin ronin-observer links against libxdo, the Linux runner will fail to compile.
Currently the cargo-check job also omits libxdo-dev and presumably works, so this may not be an immediate issue — but for consistency and to avoid future surprises consider adding it:
| - name: Install system dependencies (Linux only) | |
| if: matrix.platform == 'ubuntu-latest' | |
| run: | | |
| sudo apt-get update | |
| sudo apt-get install -y \ | |
| libwebkit2gtk-4.1-dev \ | |
| build-essential \ | |
| libssl-dev \ | |
| libayatana-appindicator3-dev \ | |
| librsvg2-dev | |
| - name: Install system dependencies (Linux only) | |
| if: matrix.platform == 'ubuntu-latest' | |
| run: | | |
| sudo apt-get update | |
| sudo apt-get install -y \ | |
| libwebkit2gtk-4.1-dev \ | |
| build-essential \ | |
| libxdo-dev \ | |
| libssl-dev \ | |
| libayatana-appindicator3-dev \ | |
| librsvg2-dev |
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/release.yml:
- Around line 77-78: Replace the unstable branch pin in the "Build and release"
step: instead of uses: tauri-apps/tauri-action@dev, pin the action to a specific
commit SHA that contains the schema fix (replace the `@dev` suffix with
@<commit-sha>) so the workflow uses a reproducible, immutable version; update
the "Build and release" step (the one with uses: tauri-apps/tauri-action@dev) to
reference that commit SHA and commit the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f371a8b6-4630-4e6e-9675-e3abe0caac32
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/ci.yml.github/workflows/release.ymlsrc-tauri/Cargo.tomlsrc-tauri/src/bin/observer.rssrc-tauri/src/commands/git.rs
Replace @dev with specific commit SHA for reproducible builds. Commit: 850c373b94f35e16e4de61cbb9f6e4876af8c0b4
Summary
Fix 3 CI/CD failures in
release.yml:1. Ubuntu - Observer Test Race Condition
Problem: Environment variable tests (
test_detect_backend_*) run in parallel, causing race conditions.Fix: Add
serial_testcrate and#[serial]attribute to all 5 backend detection tests.2. macOS - tauri-action Schema Error
Problem:
tauri-action@v0fails with"overlay" is not validschema error.Fix: Upgrade to
tauri-action@devwhich has proper schema support.3. Windows - Pre-commit Hook Test Failure
Problem:
test_commit_changes_pre_commit_hook_failurefails on Windows CI runners.Fix: Skip this test on Windows with
#[cfg_attr(windows, ignore)].Bonus
rust-testjob toci.ymlto catch test failures before releaseChanges
src-tauri/Cargo.tomlserial_test = "3.0"to dev-dependenciessrc-tauri/src/bin/observer.rs#[serial]to 5 test functionssrc-tauri/src/commands/git.rs.github/workflows/release.ymltauri-action@dev.github/workflows/ci.ymlrust-testjobTesting
Summary by CodeRabbit
Tests
Chores