chore(ci): make Verify E2E Passed a required check#264
Conversation
The verify-e2e job now runs on all PRs to main (not just release-please PRs). On non-release PRs it passes instantly; on release PRs it performs the full E2E verification. This allows it to be added as a required status check without blocking feature PRs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: a6408bad87ba
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a release-PR detection step that sets Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
🚥 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #264 +/- ##
==========================================
+ Coverage 46.31% 47.92% +1.61%
==========================================
Files 106 109 +3
Lines 8266 8321 +55
Branches 591 639 +48
==========================================
+ Hits 3828 3988 +160
+ Misses 4271 4163 -108
- Partials 167 170 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Updates the “Release Gate” GitHub Actions workflow so the Verify E2E Passed job is intended to run on all PRs to main, becoming a viable required status check while still only performing full E2E gating for release-please PRs.
Changes:
- Make
verify-e2erun for all PRs (with a fast “not applicable” path for non-release PRs). - Gate Web/Desktop E2E verification steps so they only execute on
release-please--*PRs. - Gate the desktop verification/skip/summary steps behind the release-please PR condition.
You can also share your feedback on Copilot code review. Take the survey.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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-gate.yml:
- Around line 75-76: Remove the job-level "if: '!cancelled()'" from the
verify-e2e job (currently declared with needs: detect-changes) and add an
explicit step (e.g., named "ensure-detect-changes-success") at the start of the
verify-e2e job that checks needs.detect-changes.result and fails the job for
release PRs when that result is not "success"; implement the check using
needs.detect-changes.result != 'success' combined with your release-PR detection
logic (for example, checking github.event.pull_request.head.ref startsWith
'release/' or presence of a "release" label) and exit non‑zero so verify-e2e
cannot be skipped silently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 723aa7dd-f720-4705-8222-d4064db56602
📒 Files selected for processing (1)
.github/workflows/release-gate.yml
Make detect-changes run on all PRs (short-circuits for non-release PRs) so verify-e2e's needs dependency is always satisfied. This prevents the required check from being skipped and silently passing branch protection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: c08fdccd40bd
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/release-gate.yml (1)
85-93:⚠️ Potential issue | 🔴 Critical
Verify E2E Passedcan still be skipped ifdetect-changesis skipped/failed.Because this job has
needs: detect-changesand noalways()override, upstream non-success can skip this required check. A skipped required check can satisfy protection, which weakens the gate.🧩 Suggested fix
verify-e2e: name: Verify E2E Passed needs: detect-changes + if: ${{ always() && !cancelled() }} runs-on: ubuntu-latest timeout-minutes: 40 steps: + - name: Ensure detect-changes succeeded (release PR) + if: needs.detect-changes.outputs.is_release == 'true' && needs.detect-changes.result != 'success' + run: | + echo "::error::detect-changes must succeed before E2E verification." + exit 1 + - name: Skip (not a release PR) if: needs.detect-changes.outputs.is_release != 'true' run: echo "Not a release PR — E2E gate not applicable"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-gate.yml around lines 85 - 93, The verify-e2e job can be skipped when its dependency detect-changes is skipped/failed; change the job-level condition so it always runs regardless of detect-changes result and let the existing step-level check handle skipping based on outputs. Specifically, update the verify-e2e job (named "verify-e2e") to include an always-run condition (e.g., add if: ${{ always() }}) at the job level while keeping the step that checks needs.detect-changes.outputs.is_release to decide whether to actually run E2E, ensuring the job cannot be bypassed by a skipped/failed detect-changes.
🤖 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/release-gate.yml:
- Around line 85-93: The verify-e2e job can be skipped when its dependency
detect-changes is skipped/failed; change the job-level condition so it always
runs regardless of detect-changes result and let the existing step-level check
handle skipping based on outputs. Specifically, update the verify-e2e job (named
"verify-e2e") to include an always-run condition (e.g., add if: ${{ always() }})
at the job level while keeping the step that checks
needs.detect-changes.outputs.is_release to decide whether to actually run E2E,
ensuring the job cannot be bypassed by a skipped/failed detect-changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b7ddf01f-3c03-44f3-924c-ae3bd3ff985c
📒 Files selected for processing (1)
.github/workflows/release-gate.yml
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
You can also share your feedback on Copilot code review. Take the survey.
- Set explicit default outputs (desktop=false, prev_tag=) for non-release PRs so downstream outputs are always defined - Add if: !cancelled() to verify-e2e so it runs even if detect-changes fails unexpectedly - Add explicit failure step for release PRs when detect-changes didn't succeed Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: b63abff3adfd
|
Re: CodeRabbit duplicate comment about Fixed in 3c9ecc6:
This ensures the required check cannot be bypassed by an upstream failure. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
You can also share your feedback on Copilot code review. Take the survey.
Explicit always() ensures the job runs even when detect-changes is skipped/failed, removing ambiguity about !cancelled() alone. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 61cdbe0daf6d
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
You can also share your feedback on Copilot code review. Take the survey.
Detect release PRs via startsWith(github.head_ref) instead of needs.detect-changes.outputs.is_release. This fails closed if detect-changes crashes before producing outputs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 85ab87bf85f5
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-gate.yml:
- Around line 19-21: Do not interpolate `${{ github.head_ref }}` directly in the
run script; instead add an env entry (e.g., HEAD_REF: ${{ github.head_ref }}) to
the step and reference that env var in the bash if-test (use `"$HEAD_REF"` and
`[[ "$HEAD_REF" == release-please--* ]]` to avoid word-splitting and globbing),
then write to the GITHUB_OUTPUT as before; update the step that contains the run
block to read the branch from the env variable rather than direct interpolation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9e12d50e-d981-42c9-9d54-1bad9c2a7a8b
📒 Files selected for processing (1)
.github/workflows/release-gate.yml
github.head_ref is user-controlled (PR branch name). Pass it through an environment variable instead of interpolating directly in the run script. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: bf29201a8eff
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
You can also share your feedback on Copilot code review. Take the survey.
Summary
verify-e2ejob inrelease-gate.ymlnow runs on all PRs to main, not just release-please PRsVerify E2E Passedhas been added as a required status check onmainbranch protectionThis ensures release-please PRs cannot be merged without passing E2E tests, while feature PRs are unaffected.
Test plan
Verify E2E Passedcheck passes (non-release PR path)🤖 Generated with Claude Code
Summary by CodeRabbit