From 7039461cd3e40dee23b7eebb5665047a08cd7dbc Mon Sep 17 00:00:00 2001 From: QA Runner Date: Fri, 17 Jul 2026 16:08:09 -0700 Subject: [PATCH] ci(e2e): apply migrations, serve prod build, skip LLM specs without a key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes that take the suite from ~everything-failing to green (verified end to end against a local stack): 1. schema.sql lags the migrations (missing e.g. workflow_open_source_submissions, which GET /workflows/:id queries → 500). Apply every dated migration on top of schema.sql (idempotent; 0 errors on a fresh DB), grant service_role AFTER so new tables are covered, then reload PostgREST. Fixes the workflow specs. 2. Serve a production build (next build + next start) instead of next dev. The dev server's on-demand compilation (slow first hit → waitForResponse timeouts) and hydration-error overlay (injects nextjs__container_errors DOM that pollutes text locators, e.g. getByText('All') matching 'Call Stack') made the suite flaky. Fixes tabular-reviews list render + the timing flakes. 3. LLM-dependent specs (chat rename/delete/submit, critical-path) require a model key to send a message. Guard them with test.skip(!hasLlmKey) (e2e/llm.ts) and expose ANTHROPIC_API_KEY to the Playwright process, so a keyless run is green on the ~23 other specs and the LLM specs run + enforce only when the secret is set. Local result (no key, with MinIO+LibreOffice as on ubuntu-latest): 23 passed, 4 skipped, 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/e2e.yml | 59 ++++++++++++++++++++++++++++--------- e2e/chat-management.spec.ts | 4 +++ e2e/critical-path.spec.ts | 2 ++ e2e/llm.ts | 18 +++++++++++ 4 files changed, 69 insertions(+), 14 deletions(-) create mode 100644 e2e/llm.ts diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index a09a0621..7b5ee2e8 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -38,6 +38,11 @@ jobs: # Defaults match e2e/auth.setup.ts; overriding the password here would break # the valid-login specs, so only the base URL is pinned. PLAYWRIGHT_BASE_URL: http://localhost:3000 + # Exposed to the Playwright process (not just backend/.env) so the LLM-gated + # specs can skip themselves when no key is present — see e2e/llm.ts. When the + # secret is set they run and must pass; when absent they skip, so a keyless + # run (e.g. a fork PR) is still green on the other ~20 specs. + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} steps: - uses: actions/checkout@v4 @@ -104,28 +109,42 @@ jobs: uses: supabase/setup-cli@v1 with: version: latest - - name: Init + start Supabase, load schema + - name: Init + start Supabase, load schema + migrations run: | supabase init --force --with-vscode-settings=false || supabase init --force supabase start DB_URL=$(supabase status -o json | jq -r '.DB_URL') - # README: "For a new Supabase database ... run backend/schema.sql". The - # schema file already includes the latest shape, so no migrations are - # replayed on a fresh CI database. + # 1) Base schema. README: "For a new Supabase database ... run + # backend/schema.sql". It is meant to be the latest shape but in practice + # lags the migrations (e.g. it is missing workflow_open_source_submissions, + # which GET /workflows/:id queries — a 500 that breaks the workflow specs). psql "$DB_URL" -v ON_ERROR_STOP=1 -f backend/schema.sql - # schema.sql revokes client grants (anon/authenticated) on purpose, but - # it assumes a HOSTED Supabase project where service_role already holds - # full table access. On a fresh CLI stack loaded via psql, service_role - # gets no grants on the newly-created public tables, so the backend — - # which queries exclusively as service_role (lib/supabase.ts) — 500s with - # "permission denied for table user_profiles" on the first write. Restore - # the production grant posture so the API behaves as it does in prod. + # 2) Apply every dated migration on top. They are written to be safe to + # re-run (IF NOT EXISTS / ADD COLUMN IF NOT EXISTS), so the ones already + # in schema.sql are no-ops and the newer ones fill the gaps. Verified on a + # fresh CLI DB: all migrations apply with zero errors. Kept fail-tolerant + # so one non-idempotent migration can't wedge the whole gate — mismatches + # surface as a spec failure downstream, not an opaque CI abort. + for m in $(ls backend/migrations/*.sql | sort); do + psql "$DB_URL" -v ON_ERROR_STOP=1 -f "$m" >/dev/null 2>&1 \ + || echo "::warning::migration returned a non-zero status (already applied?): $m" + done + # 3) schema.sql revokes client grants (anon/authenticated) on purpose, but + # assumes a HOSTED Supabase where service_role already holds full table + # access. On a fresh CLI stack loaded via psql, service_role gets no grants + # on the new public tables, so the backend — which queries exclusively as + # service_role (lib/supabase.ts) — 500s with "permission denied for table + # user_profiles" on the first write. Grant AFTER migrations so the tables + # they add are covered too. psql "$DB_URL" -v ON_ERROR_STOP=1 <<'SQL' GRANT USAGE ON SCHEMA public TO service_role; GRANT ALL ON ALL TABLES IN SCHEMA public TO service_role; GRANT ALL ON ALL SEQUENCES IN SCHEMA public TO service_role; GRANT ALL ON ALL FUNCTIONS IN SCHEMA public TO service_role; SQL + # 4) PostgREST cached its schema when `supabase start` booted it (before + # the DDL above). Tell it to reload so the new tables/grants are visible. + psql "$DB_URL" -v ON_ERROR_STOP=1 -c "NOTIFY pgrst, 'reload schema';" - name: Wire env from Supabase run: | @@ -161,17 +180,29 @@ jobs: echo "R2_BUCKET_NAME=mike" echo "ANTHROPIC_API_KEY=${{ secrets.ANTHROPIC_API_KEY }}" } >> backend/.env - # Frontend reads NEXT_PUBLIC_* at dev-server boot. + # next build inlines NEXT_PUBLIC_* at build time, so these must be set + # before the "Build web" step below. { echo "NEXT_PUBLIC_SUPABASE_URL=$API_URL" echo "NEXT_PUBLIC_SUPABASE_PUBLISHABLE_DEFAULT_KEY=$ANON_KEY" echo "NEXT_PUBLIC_API_BASE_URL=http://localhost:3001" } > frontend/.env.local + # Serve a PRODUCTION build, not `next dev`. Two reasons the dev server makes + # this suite flaky: (1) its on-demand route compilation makes the first hit + # of each page arbitrarily slow (20s waitForResponse assertions time out), + # and (2) its hydration-error overlay injects DOM (nextjs__container_errors…) + # that pollutes text locators — e.g. getByText('All') matched the overlay's + # "Call Stack"/"…fallback" text and failed strict mode. A production build has + # neither: no overlay, no JIT compile. Verified locally — flipping dev→start + # took the suite from intermittent overlay/timeout failures to green. + - name: Build web (production) + run: npm run build --prefix frontend + - name: Start backend API run: npm run dev --prefix backend & - - name: Start web - run: npm run dev --prefix frontend & + - name: Start web (production server) + run: npm run start --prefix frontend & - name: Wait for servers run: npx wait-on http://localhost:3001/health http://localhost:3000 --timeout 120000 diff --git a/e2e/chat-management.spec.ts b/e2e/chat-management.spec.ts index 6ef0a071..db94a0bc 100644 --- a/e2e/chat-management.spec.ts +++ b/e2e/chat-management.spec.ts @@ -9,6 +9,7 @@ * Test user: e2e@mike.local / E2eTestPass1! */ import { test, expect, type Page } from "@playwright/test"; +import { hasLlmKey, LLM_SKIP_REASON } from "./llm"; /* ─── Helpers ────────────────────────────────────────────────────────────────── */ @@ -112,6 +113,7 @@ test("cold-load: direct URL to a chat triggers the getChat history load", async /* ─── Test 2: rename a chat from sidebar ────────────────────────────────────── */ test("rename chat: sidebar rename interaction updates the title", async ({ page }) => { + test.skip(!hasLlmKey, LLM_SKIP_REASON); // REGRESSION: fails if the renameChat API call or the optimistic title update in // ChatHistoryContext.renameChatFn / SidebarChatItem.handleRenameSave is removed. @@ -203,6 +205,7 @@ test("rename chat: sidebar rename interaction updates the title", async ({ page /* ─── Test 3: delete a chat from sidebar ────────────────────────────────────── */ test("delete chat: sidebar delete action removes the chat from history", async ({ page }) => { + test.skip(!hasLlmKey, LLM_SKIP_REASON); // REGRESSION: fails if the deleteChat API call or the optimistic list removal in // ChatHistoryContext.deleteChatFn (filter by chatId) is removed. @@ -315,6 +318,7 @@ test("delete chat: sidebar delete action removes the chat from history", async ( /* ─── Test 4: project assistant — create new chat ───────────────────────────── */ test("project assistant: create a new chat and submit a question", async ({ page }) => { + test.skip(!hasLlmKey, LLM_SKIP_REASON); // REGRESSION: fails if the project chat creation route is broken — specifically if // handleNewChat() in ProjectPage.tsx (lines 515-519) fails to call saveChat() or // router.push to /projects/[id]/assistant/chat/[chatId]. (Verified by temporarily diff --git a/e2e/critical-path.spec.ts b/e2e/critical-path.spec.ts index 12e0f29a..c72abfb9 100644 --- a/e2e/critical-path.spec.ts +++ b/e2e/critical-path.spec.ts @@ -7,6 +7,7 @@ * Prerequisite: auth.setup.ts has already saved the session to e2e/.auth/user.json */ import { test, expect, type Page } from "@playwright/test"; +import { hasLlmKey, LLM_SKIP_REASON } from "./llm"; import path from "path"; const PDF_FIXTURE = path.join(__dirname, "fixtures/test.pdf"); @@ -53,6 +54,7 @@ test("authenticated user lands on the assistant page", async ({ page }) => { test("create project, upload PDF, ask a question and receive a response", async ({ page, }) => { + test.skip(!hasLlmKey, LLM_SKIP_REASON); /* This end-to-end flow (create + upload + navigate + chat) is throttled by the local Supabase stack and needs far more than the 30s default. The per-test `{ timeout }` option that test() accepts is silently ignored by diff --git a/e2e/llm.ts b/e2e/llm.ts new file mode 100644 index 00000000..3d9c52f8 --- /dev/null +++ b/e2e/llm.ts @@ -0,0 +1,18 @@ +/** + * Helpers for specs that require a live LLM turn. + * + * A few specs create/populate a chat by sending a message and awaiting a + * streamed answer. That only works when a model key is configured — in CI the + * `ANTHROPIC_API_KEY` secret, which `.github/workflows/e2e.yml` exposes to the + * Playwright process. When it is absent (a plain local run, or a fork PR with no + * secret access) the app blocks message send behind the ApiKeyMissingModal, so + * these specs would hang to their timeout. + * + * Guarding them with `test.skip(!hasLlmKey, LLM_SKIP_REASON)` keeps a keyless + * run green and fast on the ~20 specs that don't need a model, while still + * running — and enforcing — the LLM specs whenever the key is present. + */ +export const hasLlmKey = Boolean(process.env.ANTHROPIC_API_KEY); + +export const LLM_SKIP_REASON = + "requires a model key — set the ANTHROPIC_API_KEY secret to run LLM-dependent specs";