Skip to content

Comments

fix: prevent non-E2E labels from triggering workflow runs#23459

Merged
keithwillcode merged 3 commits intomainfrom
devin/1756510618-fix-workflow-label-triggers
Sep 2, 2025
Merged

fix: prevent non-E2E labels from triggering workflow runs#23459
keithwillcode merged 3 commits intomainfrom
devin/1756510618-fix-workflow-label-triggers

Conversation

@keithwillcode
Copy link
Contributor

What does this PR do?

Fixes an annoyance with the ready-for-e2e label where adding any label to a PR (like "Improvements") triggers unnecessary workflow runs that consume CI resources.

Problem: Currently, the PR workflow triggers on all labeled events, but only controls E2E test execution - not whether the entire workflow should run. This means labels like "Improvements" trigger full workflow runs including type checking, linting, unit tests, and builds, even though only the ready-for-e2e label should trigger these comprehensive checks.

Solution: Added conditional logic github.event.action != 'labeled' || github.event.label.name == 'ready-for-e2e' to all jobs in the PR workflow. This ensures:

  • All other trigger types (opened, synchronize, reopened) continue to work normally
  • Only the ready-for-e2e label triggers workflow runs when labels are added
  • All other labels are ignored by the workflow

How should this be tested?

Critical testing steps:

  1. Test non-E2E label behavior:

    • Add a non-E2E label (like "Improvements", "bug", etc.) to this PR
    • Verify that NO workflow runs are triggered
    • Check that the PR checks remain in their previous state
  2. Test ready-for-e2e label behavior:

    • Add the ready-for-e2e label to this PR
    • Verify that the full workflow runs including all E2E tests
    • Confirm all expected jobs execute
  3. Test other trigger types:

    • Push additional commits to verify synchronize triggers still work
    • Test that new PRs (opened) trigger workflows normally
    • Verify reopened PRs trigger workflows
  4. Edge case testing:

    • Test adding multiple labels simultaneously
    • Test removing and re-adding the ready-for-e2e label

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code
  • N/A - No documentation changes required for internal CI optimization
  • I confirm automated tests are in place that prove my fix is effective or that my feature works ⚠️ Cannot test GitHub Actions logic without live PR testing

⚠️ Review Focus Areas

High Priority - Manual Testing Required:

  • Live workflow behavior: This change cannot be validated without actually testing label additions on real PRs
  • Conditional logic correctness: Verify the boolean logic github.event.action != 'labeled' || github.event.label.name == 'ready-for-e2e' works as intended across all 22+ modified jobs
  • Consistency check: Ensure all jobs have identical conditional logic applied correctly

Medium Priority:

  • Backward compatibility: Confirm other workflow triggers (push, synchronize, reopened) remain unaffected
  • YAML syntax: Validate workflow file syntax (yaml validation failed locally due to missing dependencies)

Potential Risks:

  • Complex nested conditions may be hard to maintain long-term
  • Untested changes to critical CI infrastructure
  • Risk of accidentally blocking legitimate workflow runs

Session Details:

- Add conditional logic to all jobs to skip when non-ready-for-e2e labels are added
- Only allow ready-for-e2e label additions to trigger workflows
- Preserve all other trigger types (opened, synchronize, reopened)
- Fixes issue where labels like 'Improvements' cause unnecessary CI runs

Resolves the annoyance where adding labels like 'Improvements' to PRs
triggers full workflow runs including all checks, when only the
ready-for-e2e label should trigger E2E tests and associated builds.

Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 29, 2025

Walkthrough

The PR updates .github/workflows/pr.yml to introduce an env variable SKIP_WORKFLOW, evaluated true when the event is a label action and the label is not ready-for-e2e. This variable is used to gate most jobs via if conditions, including changes, check-label, deps, type-check, lint, unit-test, build variants, integration and e2e jobs, analyze, report merge/publish/cleanup, and the required fail step. The check-label job’s run-e2e decision is reworked to depend on SKIP_WORKFLOW. When SKIP_WORKFLOW is true, these jobs are skipped.

Possibly related PRs

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/1756510618-fix-workflow-label-triggers

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

…iable

- Replace repeated conditional logic with single SKIP_WORKFLOW env variable
- Simplify all job conditions from complex expression to env.SKIP_WORKFLOW != 'true'
- Maintain exact same functional behavior while improving maintainability
- Reduces code duplication across 20+ jobs in the workflow

This addresses feedback to consolidate the repeated conditional logic:
github.event.action != 'labeled' || github.event.label.name == 'ready-for-e2e'

The refactored approach is cleaner and easier to maintain.

Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
@vercel
Copy link

vercel bot commented Aug 30, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Aug 30, 2025 1:06am
cal-eu Ignored Ignored Aug 30, 2025 1:06am

- Remove 'skipped' from failure conditions in required job
- Add SKIP_WORKFLOW condition to required job logic
- Prevents false failures when E2E jobs legitimately skip
- Maintains proper failure detection for actual job failures

This fixes the CI failure where the required job was treating
legitimately skipped E2E jobs as failures when no ready-for-e2e
label is present.

Co-Authored-By: keith@cal.com <keithwillcode@gmail.com>
@github-actions
Copy link
Contributor

github-actions bot commented Aug 30, 2025

E2E results are ready!

@keithwillcode keithwillcode marked this pull request as ready for review September 2, 2025 11:11
@graphite-app graphite-app bot requested a review from a team September 2, 2025 11:11
@dosubot dosubot bot added the ci area: CI, DX, pipeline, github actions label Sep 2, 2025
Copy link
Contributor

@anikdhabal anikdhabal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@keithwillcode keithwillcode enabled auto-merge (squash) September 2, 2025 11:13
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
.github/workflows/pr.yml (1)

95-97: Minor hardening: guard against odd label payloads

Unlikely, but to be defensive you can normalize labels before mapping.

-            const labelFound = labels.map(l => l.name).includes('ready-for-e2e');
+            const names = Array.isArray(labels) ? labels.map(l => l?.name).filter(Boolean) : [];
+            const labelFound = names.includes('ready-for-e2e');
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between bfa23dd and 36457c6.

📒 Files selected for processing (1)
  • .github/workflows/pr.yml (5 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/pr.yml

25-25: label "buildjet-2vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


26-26: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


50-50: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


102-102: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


108-108: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


115-115: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


122-122: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


129-129: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


136-136: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


143-143: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


150-150: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


157-157: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


164-164: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


171-171: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


178-178: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


185-185: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


192-192: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


199-199: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


206-206: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


213-213: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


219-219: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


226-226: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)


237-237: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)

🔇 Additional comments (1)
.github/workflows/pr.yml (1)

25-26: Verify runner labels exist in the org

buildjet-2vcpu-ubuntu-2204 is unknown to actionlint. If these are self-hosted, you’re fine; otherwise provide a fallback or update actionlint config.

You can validate runner labels and add an actionlint config entry if needed.

Also applies to: 48-49

Comment on lines +19 to +21
env:
SKIP_WORKFLOW: ${{ github.event.action == 'labeled' && github.event.label.name != 'ready-for-e2e' }}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Job-level if cannot use env context; gating will not work as intended

actionlint is correct: env is unavailable in job.if. This will either error or be ignored, defeating the skip intent across all jobs. Inline the label gate expression (or derive it via a needs output) and drop the workflow env.

Apply this diff to remove the global env and rely on direct expressions:

- env:
-   SKIP_WORKFLOW: ${{ github.event.action == 'labeled' && github.event.label.name != 'ready-for-e2e' }}

Then update job conditions as shown in the next comment. If you prefer a single source of truth, add a tiny “gate” job that outputs skip=true/false and reference it via needs.gate.outputs.skip in job.if (needs context is allowed at job level).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
env:
SKIP_WORKFLOW: ${{ github.event.action == 'labeled' && github.event.label.name != 'ready-for-e2e' }}
🤖 Prompt for AI Agents
.github/workflows/pr.yml around lines 19-21: the workflow sets a global env
SKIP_WORKFLOW using an expression that references github.event and expects it to
be usable inside job.if, but job-level if cannot access env values so the gate
will be ignored or error; remove the global env entry and instead put the
label-check expression directly into each job's if (or create a tiny “gate” job
that computes skip=true/false and exposes it as an output which other jobs
reference via needs.gate.outputs.skip), updating all job.if conditions to use
the inline expression or needs.gate.outputs.skip as appropriate.

Comment on lines +54 to 55
run-e2e: ${{ steps.check-if-pr-has-label.outputs.run-e2e == 'true' && env.SKIP_WORKFLOW != 'true' }}
steps:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Do not couple run-e2e output to SKIP gating

Since the job itself is gated, keep run-e2e purely about label presence.

-      run-e2e: ${{ steps.check-if-pr-has-label.outputs.run-e2e == 'true' && env.SKIP_WORKFLOW != 'true' }}
+      run-e2e: ${{ steps.check-if-pr-has-label.outputs.run-e2e == 'true' }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
run-e2e: ${{ steps.check-if-pr-has-label.outputs.run-e2e == 'true' && env.SKIP_WORKFLOW != 'true' }}
steps:
run-e2e: ${{ steps.check-if-pr-has-label.outputs.run-e2e == 'true' }}
steps:
🤖 Prompt for AI Agents
.github/workflows/pr.yml around lines 54-55: the run-e2e job condition currently
combines the label check with an environment SKIP_WORKFLOW gate; remove the
SKIP_WORKFLOW portion so run-e2e only depends on the label output. Update the if
expression to check only that steps.check-if-pr-has-label.outputs.run-e2e ==
'true' and leave any SKIP_WORKFLOW gating to the job-level if that already
exists.

@keithwillcode keithwillcode merged commit e45f9c5 into main Sep 2, 2025
109 of 112 checks passed
@keithwillcode keithwillcode deleted the devin/1756510618-fix-workflow-label-triggers branch September 2, 2025 11:44
@keithwillcode
Copy link
Contributor Author

This was reverted out of main due to an issue not triggering all jobs properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci area: CI, DX, pipeline, github actions core area: core, team members only foundation ready-for-e2e

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants