ci: promote BaseballHelm authed smoke to a required PR gate (#372)

.github/branch-protection.md

@@ -20,6 +20,9 @@ so a job split/rename does not silently break protection.
   - `Next build`
   - `Route Hygiene P0/P1`
   - `Supabase lint + RLS tests`
+  - `BaseballHelm authenticated smoke` (coach + player, #372) — skips (does
+    not fail) on fork/Dependabot PRs, which receive no repo secrets from
+    GitHub by design
 - `Review Gate / all` — hard aggregate for ast-grep, semgrep, gitleaks,
   actionlint, yamllint, shellcheck, markdownlint, ruff+pylint, sqlfluff, and
   hadolint.
@@ -48,6 +51,20 @@ Advisory checks:
 `CI / all` aggregate once the baseball pgTAP RLS suite went green on `main`
 (#517, supersedes #423). RLS regressions now block merge.
 
+`BaseballHelm authenticated smoke` (the `baseball-auth-smoke` job in
+`ci.yml`) was promoted the same way (#372): the coach/player smoke suite
+(`e2e/baseball-smoke.spec.ts` + `e2e/baseball-onboarding-smoke.spec.ts`) and
+its fail-loud auth setup already existed and ran on every `main` push via
+`playwright.yml`'s `e2e` job, but only post-merge — a real authenticated
+regression could land on `main` before this ever ran. `ci.yml` now runs the
+same specs (steps copied, not moved) as a required PR gate. It skips rather
+than fails on fork/Dependabot PRs (no repo secrets available to them);
+same-repo, non-Dependabot pushes and PRs must have the required secrets
+configured or the job fails loudly. Note the added cost: a second full
+`npm run build` + Playwright-chromium install on every same-repo,
+non-Dependabot PR, on top of the existing `Next build` / `Smoke checks`
+builds.
+
 `Greptile Review` is intentionally advisory, not a required check: Greptile's
 `.greptile/config.json` skips `dependabot`-titled PRs, so it never posts a
 passing `Greptile Review` on them — making it a required context would leave

.github/workflows/ci.yml

@@ -312,6 +312,126 @@ jobs:
       - name: Check for new runtime import cycles
         run: npm run check:cycles
 
+  baseball-auth-smoke:
+    name: BaseballHelm authenticated smoke
+    # #372: promotes the already-built, already-mandatory baseball-smoke +
+    # onboarding-smoke specs (e2e/baseball-smoke.spec.ts,
+    # e2e/baseball-onboarding-smoke.spec.ts) from push/workflow_dispatch-only
+    # (.github/workflows/playwright.yml's `e2e` job) into a required PR gate —
+    # the same mechanism that promoted Supabase RLS tests into `all` (#517)
+    # and the import-cycle ratchet (#808). Steps below are copied verbatim
+    # from playwright.yml's readiness-check + fail-loud pattern, not moved:
+    # the push/manual job keeps running the full mandatory suite (plus the
+    # mobile-viewport regression) unchanged.
+    #
+    # Skips (does not fail) on fork/Dependabot pull_request runs, which
+    # receive no repo secrets from GitHub by design — `all`'s aggregate step
+    # only treats 'failure'/'cancelled' as blocking, so a skipped job here
+    # never blocks an external contribution that structurally cannot supply
+    # these secrets. Same-repo, non-Dependabot pushes and PRs still hard-fail
+    # on missing secrets, same as the push-triggered version always has.
+    # Dependabot PRs are opened against this repo (not a fork), so the
+    # same-repo check alone does not exclude them — `github.actor` is
+    # checked explicitly for that reason.
+    #
+    # COST NOTE: this doubles Playwright-browser-install + a full `npm run
+    # build` on every PR from this repo, on top of the existing next-build /
+    # Smoke-checks builds — a real, ongoing CI-minutes tradeoff the repo had
+    # previously avoided by scoping full E2E to push-only. Revisit if PR
+    # wall-clock time becomes a problem.
+    if: >-
+      github.event_name == 'push' ||
+      (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository && github.actor != 'dependabot[bot]')
+    runs-on: ubuntu-latest
+    timeout-minutes: 30
+    env:
+      NEXT_PUBLIC_SUPABASE_URL: ${{ secrets.NEXT_PUBLIC_SUPABASE_URL || 'https://dummy-ci-build.supabase.co' }}
+      NEXT_PUBLIC_SUPABASE_ANON_KEY: ${{ secrets.NEXT_PUBLIC_SUPABASE_ANON_KEY || 'dummy-anon-key-ci-build-only' }}
+      SUPABASE_SERVICE_ROLE_KEY: ${{ secrets.SUPABASE_SERVICE_ROLE_KEY }}
+      E2E_BASEBALL_COACH_EMAIL: ${{ secrets.E2E_BASEBALL_COACH_EMAIL }}
+      E2E_BASEBALL_COACH_PASSWORD: ${{ secrets.E2E_BASEBALL_COACH_PASSWORD }}
+      E2E_BASEBALL_PLAYER_EMAIL: ${{ secrets.E2E_BASEBALL_PLAYER_EMAIL }}
+      E2E_BASEBALL_PLAYER_PASSWORD: ${{ secrets.E2E_BASEBALL_PLAYER_PASSWORD }}
+      NODE_OPTIONS: --max-old-space-size=8192
+    steps:
+      - name: Checkout
+        uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0
+        with:
+          persist-credentials: false
+
+      - name: Setup Node
+        uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e
+        with:
+          node-version: 22
+          cache: npm
+
+      - name: Install dependencies
+        run: npm ci
+
+      - name: Compute BaseballHelm auth readiness
+        id: baseball-auth-ready
+        run: |
+          missing=()
+          [ -z "$SUPABASE_SERVICE_ROLE_KEY" ] && missing+=("SUPABASE_SERVICE_ROLE_KEY")
+          [ -z "$E2E_BASEBALL_COACH_EMAIL" ] && missing+=("E2E_BASEBALL_COACH_EMAIL")
+          [ -z "$E2E_BASEBALL_COACH_PASSWORD" ] && missing+=("E2E_BASEBALL_COACH_PASSWORD")
+          [ -z "$E2E_BASEBALL_PLAYER_EMAIL" ] && missing+=("E2E_BASEBALL_PLAYER_EMAIL")
+          [ -z "$E2E_BASEBALL_PLAYER_PASSWORD" ] && missing+=("E2E_BASEBALL_PLAYER_PASSWORD")
+
+          if [ ${#missing[@]} -gt 0 ]; then
+            echo "ready=false" >> "$GITHUB_OUTPUT"
+            printf 'missing=%s\n' "${missing[*]}" >> "$GITHUB_OUTPUT"
+            echo "BaseballHelm auth secrets unavailable: ${missing[*]}"
+          else
+            echo "ready=true" >> "$GITHUB_OUTPUT"
+            echo "missing=" >> "$GITHUB_OUTPUT"
+            echo "BaseballHelm auth secrets available."
+          fi
+
+      - name: Fail missing BaseballHelm secrets
+        if: steps.baseball-auth-ready.outputs.ready != 'true'
+        env:
+          MISSING_BASEBALL_SECRETS: ${{ steps.baseball-auth-ready.outputs.missing }}
+        run: |
+          echo "::error::Missing required secret(s): ${MISSING_BASEBALL_SECRETS}"
+          echo "BaseballHelm authenticated smoke (#372) is a required PR gate on this repo and needs these secrets configured."
+          exit 1
+
+      - name: Cache Playwright browsers
+        uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9
+        with:
+          path: ~/.cache/ms-playwright
+          key: playwright-${{ runner.os }}-${{ hashFiles('package-lock.json') }}
+          restore-keys: |
+            playwright-${{ runner.os }}-
+
+      - name: Install Playwright browsers
+        run: npx playwright install --with-deps chromium
+
+      - name: Build (so dev server boots fast)
+        run: npm run build
+
+      - name: Seed BaseballHelm CI accounts
+        run: npm run seed:baseball:ci
+
+      - name: Run BaseballHelm mandatory smoke (blocking)
+        run: |
+          npx playwright test \
+            --project=chromium \
+            --project=baseball-coach \
+            --project=baseball-player \
+            e2e/baseball-onboarding-smoke.spec.ts \
+            e2e/baseball-smoke.spec.ts
+
+      - name: Upload Playwright report
+        if: always()
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a
+        with:
+          name: baseball-auth-smoke-playwright-report
+          path: playwright-report/
+          retention-days: 3
+          if-no-files-found: ignore
+
   all:
     name: all
     if: always()
@@ -321,6 +441,11 @@ jobs:
     # baseball pgTAP RLS suite was greened on main by #517 (supersedes #423), so
     # RLS regressions now block merge through the required `all` check instead of
     # being advisory-only — the gap that let the anon-access hole reach main.
+    #
+    # `baseball-auth-smoke` (BaseballHelm authenticated coach/player smoke,
+    # #372) is part of the hard gate for the same reason: an authenticated
+    # smoke suite that only ran post-merge could not have caught a real
+    # regression before it landed on `main`.
     needs:
       - database-types
       - schema-invariants
@@ -334,6 +459,7 @@ jobs:
       - route-hygiene
       - supabase
       - import-cycles
+      - baseball-auth-smoke
     steps:
       - name: Fail if any required CI check failed
         if: contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')

.github/workflows/playwright.yml

@@ -173,10 +173,19 @@ jobs:
             e2e/baseball-smoke.spec.ts \
             e2e/mobile-viewports.spec.ts
 
+      # NOTE: previously `|| echo "Playwright suite has failures..."`, which
+      # masked the exit code for every spec in this job — a real failure
+      # here looked identical to a clean pass in the GitHub Actions UI
+      # (flagged as blocking trust in the e2e gate on #812 review). This job
+      # ("Playwright (chromium)") is advisory, not a required check (see
+      # docs/CI_RUNBOOK.md / .github/branch-protection.md), so unmasking it
+      # cannot newly block a merge — it only makes this job's own status
+      # honestly reflect whether the suite passed. The report/results-json
+      # upload steps below still run regardless (`if: always()`).
       - name: Run Playwright tests
         env:
           PLAYWRIGHT_BASEBALL_SEEDED: ${{ env.SUPABASE_SERVICE_ROLE_KEY != '' && '1' || '' }}
-        run: timeout 45m npx playwright test --project=chromium || echo "Playwright suite has failures or timed out — see report artifact"
+        run: timeout 45m npx playwright test --project=chromium
 
       - name: Upload Playwright report
         if: always()

docs/CI_RUNBOOK.md

@@ -18,14 +18,14 @@ rolls up into one of those. (The context name `all` is shared by both the
 
 | Check | Source | What it validates | Gate type |
 |---|---|---|---|
-| `all` (CI) | `ci.yml` | aggregate: DB-types drift, schema invariants, feature knowledge, typecheck, ESLint, lint-ratchet, unit tests, business contracts, `next build`, route hygiene, **Supabase lint + RLS tests** | **Hard gate** (required context `all`) |
+| `all` (CI) | `ci.yml` | aggregate: DB-types drift, schema invariants, feature knowledge, typecheck, ESLint, lint-ratchet, unit tests, business contracts, `next build`, route hygiene, **Supabase lint + RLS tests**, **BaseballHelm authenticated coach/player smoke (#372)** | **Hard gate** (required context `all`) |
 | `all` (Review Gate) | `review-gate.yml` | aggregate: ast-grep, semgrep, gitleaks, actionlint, yamllint, shellcheck, markdownlint, ruff+pylint, sqlfluff, hadolint | **Hard gate** (required context `all`) |
 | `Smoke checks` | `playwright.yml` (PRs + main push) | build-only smoke: `npm ci` + `next build` (no full E2E) | **Hard gate** |
 | `Playwright PR smoke (a11y)` | `pr-smoke.yml` | public-route accessibility Playwright only when frontend/e2e paths change | Advisory |
 | `CodeRabbit` | CodeRabbit GitHub App | assertive line-level review + blocking custom checks (`.coderabbit.yaml`) | **Hard gate** |
 | `CodeQL` | `codeql.yml` | code-scanning security analysis | **Hard gate** |
 | `Greptile Review` | Greptile GitHub App | whole-codebase review + hard rules (`.greptile/rules.md`) | Advisory — *not* a required context; Greptile skips `dependabot` PRs, so requiring it would block the bot flow. CodeRabbit is the blocking AI reviewer. |
-| `Playwright (chromium)` / `Course picker screenshots` / `BaseballHelm seeded smoke` | `playwright.yml` | full E2E — **main push + manual `workflow_dispatch` only** (not PRs) | Advisory on main; manual for feature branches |
+| `Playwright (chromium)` / `Course picker screenshots` / `BaseballHelm seeded smoke` | `playwright.yml` | full E2E (mandatory Baseball smoke + mobile-viewport regression + broader chromium suite) — **main push + manual `workflow_dispatch` only** (not PRs) | Advisory on main; manual for feature branches. **Note:** `Playwright (chromium)`'s broader-suite step no longer masks its exit code (`|| echo ...` removed) — a red run here now means a real failure, not just "see artifact." |
 | `ci/circleci: lighthouse-preview` | CircleCI | Lighthouse against the Vercel preview URL; usually skips when no preview exists (non-main Vercel builds disabled) | Advisory |
 | `ci/circleci: ios-compile` | CircleCI | iOS Capacitor compile, only relevant when `ios/**` / `capacitor.config.ts` changed | Advisory unless the PR touches iOS |
 | `migration-lockdown / block-historical-edits` | `migration-lockdown.yml` | blocks edits to already-applied migrations | Advisory |
@@ -45,6 +45,13 @@ Don't treat a check as "stuck" before its normal window has passed:
 - **Full Playwright** (`playwright.yml`, main + manual only) — `e2e` job
   75-minute budget; `picker-screenshots` and `baseball-smoke` 20 minutes each;
   main-push `Smoke checks` 15 minutes.
+- **`CI / all`'s `baseball-auth-smoke` job (#372)** — 30-minute budget; on
+  every same-repo, non-Dependabot PR/push it installs Playwright chromium,
+  runs a full `npm run build`, seeds BaseballHelm CI accounts, then runs the
+  mandatory coach/player smoke. This is separate from — and in addition to —
+  the broader `Smoke checks` build. On fork/Dependabot PR runs it **skips**
+  (no repo secrets available) rather than running or failing; a skip here
+  is expected, not stuck.
 - **Web server / auth waits** (why Playwright can be slow to even start) —
   120s dev-server startup, 45s auth navigation per spec.
 
@@ -96,7 +103,9 @@ your diff — `main` itself was already red when you branched.
 - **Don't misread intentional skips as failures**: Lighthouse skips
   `docs/*` and `*-noop` branches by design; Playwright specs self-skip when
   their env vars aren't set (`PLAYWRIGHT_BASEBALL_SEEDED`, `E2E_GOLF_*`,
-  `GOLFHELM_*`). A skip is not a failure.
+  `GOLFHELM_*`). `CI / all`'s `baseball-auth-smoke` job (#372) similarly
+  skips (not fails) on fork/Dependabot `pull_request` runs, which never
+  receive repo secrets. A skip is not a failure.
 
 ---