From e45f9c5a13aead66ae5c12ea1740bd67768cc3e4 Mon Sep 17 00:00:00 2001 From: Keith Williams Date: Tue, 2 Sep 2025 08:44:11 -0300 Subject: [PATCH] fix: prevent non-E2E labels from triggering workflow runs (#23459) * fix: prevent non-E2E labels from triggering workflow runs - 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 * refactor: consolidate workflow conditional logic into environment variable - 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 * fix: update required job logic to handle skipped jobs correctly - 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 --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .github/workflows/pr.yml | 48 ++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index e9f983df1a..fe8eeafd0d 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -16,10 +16,14 @@ concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: true +env: + SKIP_WORKFLOW: ${{ github.event.action == 'labeled' && github.event.label.name != 'ready-for-e2e' }} + jobs: changes: name: Detect changes runs-on: buildjet-2vcpu-ubuntu-2204 + if: ${{ env.SKIP_WORKFLOW != 'true' }} permissions: pull-requests: read outputs: @@ -43,10 +47,11 @@ jobs: needs: [changes] runs-on: buildjet-2vcpu-ubuntu-2204 name: Check for E2E label + if: ${{ env.SKIP_WORKFLOW != 'true' }} permissions: pull-requests: read outputs: - run-e2e: ${{ steps.check-if-pr-has-label.outputs.run-e2e == 'true' && (github.event.action != 'labeled' || github.event.label.name == 'ready-for-e2e') }} + run-e2e: ${{ steps.check-if-pr-has-label.outputs.run-e2e == 'true' && env.SKIP_WORKFLOW != 'true' }} steps: - name: Check if PR exists with ready-for-e2e label for this SHA id: check-if-pr-has-label @@ -94,130 +99,131 @@ jobs: deps: name: Install dependencies needs: [changes, check-label] - if: ${{ needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/yarn-install.yml type-check: name: Type check needs: [changes, check-label, deps] - if: ${{ needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/check-types.yml secrets: inherit lint: name: Linters needs: [changes, check-label, deps] - if: ${{ needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/lint.yml secrets: inherit unit-test: name: Tests needs: [changes, check-label, deps] - if: ${{ needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/unit-tests.yml secrets: inherit build-api-v1: name: Production builds needs: [changes, check-label, deps] - if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/api-v1-production-build.yml secrets: inherit build-api-v2: name: Production builds needs: [changes, check-label, deps] - if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/api-v2-production-build.yml secrets: inherit build-atoms: name: Production builds needs: [changes, check-label, deps] - if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/atoms-production-build.yml secrets: inherit build-docs: name: Production builds needs: [changes, check-label, deps] - if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/docs-build.yml secrets: inherit build: name: Production builds needs: [changes, check-label, deps] - if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/production-build-without-database.yml secrets: inherit integration-test: name: Tests needs: [changes, check-label, build, build-api-v1, build-api-v2] - if: ${{ needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/integration-tests.yml secrets: inherit e2e: name: Tests needs: [changes, check-label, build, build-api-v1, build-api-v2] - if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/e2e.yml secrets: inherit e2e-api-v2: name: Tests needs: [changes, check-label, build, build-api-v1, build-api-v2] - if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/e2e-api-v2.yml secrets: inherit e2e-app-store: name: Tests needs: [changes, check-label, build, build-api-v1, build-api-v2] - if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/e2e-app-store.yml secrets: inherit e2e-embed: name: Tests needs: [changes, check-label, build, build-api-v1, build-api-v2] - if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/e2e-embed.yml secrets: inherit e2e-embed-react: name: Tests needs: [changes, check-label, build, build-api-v1, build-api-v2] - if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/e2e-embed-react.yml secrets: inherit e2e-atoms: name: Tests needs: [changes, check-label, build, build-atoms, build-api-v2] - if: ${{ needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} uses: ./.github/workflows/e2e-atoms.yml secrets: inherit analyze: name: Analyze Build needs: [build] + if: ${{ env.SKIP_WORKFLOW != 'true' }} uses: ./.github/workflows/nextjs-bundle-analysis.yml secrets: inherit merge-reports: name: Merge reports - if: ${{ !cancelled() && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ !cancelled() && env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} needs: [changes, check-label, e2e, e2e-embed, e2e-embed-react, e2e-app-store, e2e-atoms] uses: ./.github/workflows/merge-reports.yml secrets: inherit publish-report: name: Publish HTML report - if: ${{ !cancelled() && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} + if: ${{ !cancelled() && env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' }} permissions: contents: write issues: write @@ -228,7 +234,7 @@ jobs: cleanup-report: name: Cleanup HTML report - if: ${{ !cancelled() && needs.check-label.outputs.run-e2e == 'true' && (github.event.pull_request.merged == true || github.event.pull_request.state == 'closed') }} + if: ${{ !cancelled() && env.SKIP_WORKFLOW != 'true' && needs.check-label.outputs.run-e2e == 'true' && (github.event.pull_request.merged == true || github.event.pull_request.state == 'closed') }} permissions: contents: write issues: write @@ -262,5 +268,5 @@ jobs: runs-on: buildjet-2vcpu-ubuntu-2204 steps: - name: fail if conditional jobs failed - if: needs.changes.outputs.has-files-requiring-all-checks == 'true' && (contains(needs.*.result, 'failure') || contains(needs.*.result, 'skipped') || contains(needs.*.result, 'cancelled')) + if: env.SKIP_WORKFLOW != 'true' && needs.changes.outputs.has-files-requiring-all-checks == 'true' && (contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')) run: exit 1