ci(pr): run E2E when a PR touches tests/e2e paths (advisory)

.github/workflows/e2e.yml

@@ -2,6 +2,10 @@ name: E2E
 
 run-name: E2E ${{ inputs.ref || github.ref }}
 
+# Why: checkout + artifact upload only; callers can only further restrict.
+permissions:
+  contents: read
+
 on:
   workflow_call:
     inputs:

.github/workflows/pr.yml

@@ -265,6 +265,55 @@ jobs:
       - name: Smoke packaged CLI
         run: node config/scripts/smoke-packaged-cli.mjs --app-dir=dist/linux-unpacked
 
+  # Why: regression specs under tests/e2e/** used to merge green without ever
+  # running — e2e.yml only fired on schedule/release (#10518). Path-filter so
+  # ordinary PRs stay light; any E2E suite change still gets a full shard run.
+  e2e-paths:
+    name: detect e2e path changes
+    runs-on: ubuntu-latest
+    if: github.event.pull_request.draft != true
+    # Why: detector only needs to read the checkout; do not inherit repo defaults.
+    permissions:
+      contents: read
+    outputs:
+      should_run: ${{ steps.filter.outputs.should_run }}
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v6
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+
+      - name: Filter E2E-relevant paths
+        id: filter
+        run: |
+          set -euo pipefail
+          BASE="${{ github.event.pull_request.base.sha }}"
+          HEAD="${{ github.event.pull_request.head.sha }}"
+          # Why: capture first so a failed git diff does not look like "no matches"
+          # (pipeline status in `if` is not aborted by set -e). Merge-base limits the
+          # list to files this PR introduced, not base-branch drift.
+          CHANGED="$(git diff --name-only --merge-base "$BASE" "$HEAD")"
+          # Why: tests/playwright.config.ts sits beside tests/e2e/, not inside it, so
+          # it needs its own pattern — a bare `playwright.` prefix matches no tracked
+          # file and would silently skip E2E when the runner config changes.
+          if printf '%s\n' "$CHANGED" | grep -E '^(tests/e2e/|tests/playwright\.|\.github/workflows/e2e\.yml$)' >/dev/null; then
+            echo "should_run=true" >> "$GITHUB_OUTPUT"
+            echo "E2E path changes detected"
+          else
+            echo "should_run=false" >> "$GITHUB_OUTPUT"
+            echo "No E2E path changes"
+          fi
+
+  e2e:
+    name: e2e
+    needs: e2e-paths
+    if: needs.e2e-paths.outputs.should_run == 'true'
+    # Why: reusable e2e.yml only checkouts, builds, and uploads artifacts.
+    permissions:
+      contents: read
+    uses: ./.github/workflows/e2e.yml
+
   verify:
     if: always()
     needs:
@@ -277,6 +326,14 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
+      # Why: e2e is deliberately absent from needs. The suite is currently red on
+      # main (every scheduled run), so gating merges on it would block any PR that
+      # touches tests/e2e/** — including the ones fixing the suite. Until it is
+      # green the job runs and reports for E2E-path PRs without blocking. To flip
+      # it on: add `e2e` to needs, add E2E to the env below, and require
+      # `"$E2E" = success || skipped` after the loop — skipped is the normal
+      # result for a path-filtered job and must keep passing, so it has to be
+      # checked outside the loop or it would excuse the jobs above.
       - name: Require successful checks
         env:
           STATIC_ANALYSIS: ${{ needs.static_analysis.result }}

config/scripts/pr-e2e-gate-contract.test.mjs

@@ -0,0 +1,64 @@
+import { readFileSync } from 'node:fs'
+import { join, resolve } from 'node:path'
+import { describe, expect, it } from 'vitest'
+import { parse } from 'yaml'
+
+const projectDir = resolve(import.meta.dirname, '../..')
+const prWorkflow = parse(readFileSync(join(projectDir, '.github/workflows/pr.yml'), 'utf8'))
+
+const filterStep = prWorkflow.jobs['e2e-paths'].steps.find(
+  (step) => step.name === 'Filter E2E-relevant paths'
+)
+const verifyStep = prWorkflow.jobs.verify.steps.find(
+  (step) => step.name === 'Require successful checks'
+)
+
+describe('PR E2E gate contract', () => {
+  it('keeps E2E advisory while the suite is red on main', () => {
+    // Why: pin the deliberate choice so it reads as intentional rather than as
+    // the "forgot to wire the gate" bug this file originally caught. Gating on a
+    // suite that fails every scheduled run would block the PRs that fix it.
+    // Flipping to blocking means updating this expectation too — see the comment
+    // on verify's Require-successful-checks step for the exact wiring.
+    expect(prWorkflow.jobs.verify.needs).not.toContain('e2e')
+    expect(verifyStep.env.E2E).toBeUndefined()
+    expect(verifyStep.run).not.toContain('$E2E')
+  })
+
+  it('runs E2E only when the detector says the PR touches E2E paths', () => {
+    // Why: without this the job could lose its filter and run on every PR — the
+    // cost the path filter exists to avoid — while the gate assertions above
+    // stay green.
+    expect(prWorkflow.jobs.e2e.needs).toBe('e2e-paths')
+    expect(prWorkflow.jobs.e2e.if).toBe("needs.e2e-paths.outputs.should_run == 'true'")
+    expect(prWorkflow.jobs['e2e-paths'].outputs.should_run).toBe(
+      '${{ steps.filter.outputs.should_run }}'
+    )
+  })
+
+  it('enforces every job verify depends on', () => {
+    // Why: derive from verify.needs rather than hardcoding, so adding a required
+    // job without adding it to the strict loop fails here instead of silently
+    // leaving that job unenforced. This is what caught GIT_COMPATIBILITY and
+    // SHELL_CONTRACTS being absent from an earlier hardcoded list.
+    const strictLoop = verifyStep.run.slice(0, verifyStep.run.indexOf('done'))
+    for (const job of prWorkflow.jobs.verify.needs) {
+      const envVar = job.toUpperCase()
+      expect(verifyStep.env[envVar]).toBe(`\${{ needs.${job}.result }}`)
+      expect(strictLoop).toContain(`"$${envVar}"`)
+    }
+  })
+
+  it('matches the Playwright config where it actually lives', () => {
+    // Why: the config is tests/playwright.config.ts, beside tests/e2e/ rather
+    // than inside it. A bare `playwright.` prefix matches no tracked file, so
+    // editing the runner config would silently skip E2E.
+    expect(filterStep.run).toContain('tests/playwright\\.')
+    expect(filterStep.run).not.toMatch(/\(\^?\|\|]tests\/e2e\/\|playwright\\\./)
+  })
+
+  it('scopes detection to the PR range so base drift cannot false-trigger', () => {
+    expect(filterStep.run).toContain('git diff --name-only --merge-base "$BASE" "$HEAD"')
+    expect(filterStep.run).toContain('set -euo pipefail')
+  })
+})