Repository navigation
ci: only enforce Sparkle monotonic check on release - #2651
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe changes relocate a Sparkle build number monotonicity validation step from the ci.yml workflow's guard-tests job to the release.yml workflow's build-sign-notarize job, replacing a conditional inline shell command with an unconditional dedicated script invocation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
|
This review could not be run because your cubic account has exceeded the monthly review limit. If you need help restoring access, please contact contact@cubic.dev. |
Greptile SummaryThis PR moves the Sparkle build-number monotonic check out of CI's
Confidence Score: 4/5Safe for net-new releases; re-runs on already-published tags will hard-fail at the Sparkle check before reaching the idempotency guard One P1 ordering bug: Sparkle check precedes guard_release_assets, breaking the re-run safety contract — score is 4 rather than 5 .github/workflows/release.yml — step ordering of Sparkle check relative to guard_release_assets Important Files Changed
Sequence DiagramsequenceDiagram
participant GH as GitHub Actions
participant Runner as macOS Runner
GH->>Runner: Checkout (submodules: recursive)
Runner-->>GH: source ready
Note over Runner: ⚠️ always runs — no guard condition
GH->>Runner: Validate Sparkle build number is monotonic
Runner-->>GH: local > published? FAIL if local == published
GH->>Runner: Guard immutable release assets
Runner-->>GH: sets skip_all output
alt skip_all == 'true' — already published
Note over GH,Runner: Would skip remaining steps, but Sparkle check already failed above
else skip_all != 'true'
GH->>Runner: Select Xcode + Install build deps
GH->>Runner: Build universal app (Release)
GH->>Runner: Codesign + Notarize
GH->>Runner: Generate Sparkle appcast
GH->>Runner: Upload release asset + R2 appcast
end
GH->>Runner: Cleanup keychain (always)
Reviews (1): Last reviewed commit: "ci: only enforce Sparkle monotonic check..." | Re-trigger Greptile |
| - name: Validate Sparkle build number is monotonic | ||
| run: ./tests/test_ci_sparkle_build_monotonic.sh |
There was a problem hiding this comment.
Sparkle check runs before the idempotency guard
The Sparkle check is placed before guard_release_assets, so a workflow_dispatch re-run on an already-fully-published tag will hard-fail here — the local build number equals the published appcast's build number (X <= X), triggering exit 1 — before the guard can run to set skip_all=true and exit gracefully. The guard exists specifically to make the release workflow safe to re-run; placing the Sparkle check first inverts that contract.
Move the step to after guard_release_assets and add the guard condition:
| - name: Validate Sparkle build number is monotonic | |
| run: ./tests/test_ci_sparkle_build_monotonic.sh | |
| - name: Validate Sparkle build number is monotonic | |
| if: steps.guard_release_assets.outputs.skip_all != 'true' | |
| run: ./tests/test_ci_sparkle_build_monotonic.sh |
This requires reordering so guard_release_assets runs first.
Summary
Summary by cubic
Only enforce Sparkle build-number monotonicity during releases. Run the check pre-tag via
./scripts/release-pretag-guard.shand first inrelease.ymlto fail fast before build/sign.ci.yml; use the shared./tests/test_ci_sparkle_build_monotonic.shinrelease.ymland the new pre-tag guard script.release.ymland update release docs/commands to require the pre-tag guard before tagging.Written for commit 05eff80. Summary will update on new commits.
Summary by CodeRabbit