diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index d85ee659..2e62daac 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -9,9 +9,13 @@ # # The suite is green with NO secret: the 4 LLM-dependent specs (chat + critical # path) skip themselves when ANTHROPIC_API_KEY is absent (see e2e/llm.ts), so a -# keyless run passes the other ~23 specs. Set the ANTHROPIC_API_KEY secret under -# Settings > Secrets and variables > Actions to also run and enforce those 4. -# See docs/e2e-ci.md for the full picture. +# keyless run passes the other 23 specs. The gate exists because the app has no +# keyless model — with no provider key configured, ChatInput blocks every send — +# not because of the (best-effort) title-generation call. To run and enforce +# those 4, set the ANTHROPIC_API_KEY repository secret; exact steps, the +# fork-PR caveat (fork `pull_request` runs never receive secrets, so they +# always take the skip path), cost, and how to verify the specs ran are in +# docs/e2e-ci.md ("Enable the LLM specs"). # # The e2e user (e2e@mike.local) is bootstrapped in-job by e2e/auth.setup.ts against # the local Supabase admin API, so no E2E_PASSWORD secret is needed — the defaults @@ -40,8 +44,9 @@ jobs: 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. + # secret is set they run and are enforced; when absent they skip, so a keyless + # run (e.g. a fork PR, which never receives this secret) is still green on + # the other 23 specs. Setup: docs/e2e-ci.md, "Enable the LLM specs". ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} steps: - uses: actions/checkout@v4 diff --git a/docs/e2e-ci.md b/docs/e2e-ci.md index bb2ab934..2b7b9fb2 100644 --- a/docs/e2e-ci.md +++ b/docs/e2e-ci.md @@ -35,20 +35,94 @@ and on manual `workflow_dispatch`, the `e2e / playwright` job: the local Supabase admin API, so no login secret is needed — the credentials baked into that file are the single source of truth. -Typical run: **~7 minutes**, **23 passed / 4 skipped / 0 failed** with no secret. +A keyless run is expected to end **23 passed / 4 skipped / 0 failed** — the +suite has 27 specs, 4 of them LLM-gated (see "Confirm the specs ran" below). ## Optional secret (fuller coverage) | Secret | What it unlocks | Without it | |---|---|---| -| `ANTHROPIC_API_KEY` | The 4 LLM-dependent specs (chat rename/delete/submit, critical-path "ask a question") send a message and assert a **streamed** answer. With the key set they run and must pass. | Those 4 specs **skip** (see `e2e/llm.ts`) instead of hanging, so the run is still green on the other ~23 specs. | +| `ANTHROPIC_API_KEY` | The 4 LLM-dependent specs (chat rename/delete/submit, critical-path "ask a question") send a message and assert a **streamed** answer. With the key set they run and are enforced. | Those 4 specs **skip** (see `e2e/llm.ts`) instead of hanging, so the run is still green on the other 23 specs. | The suite is green **without** any secret — the LLM specs skip themselves via `test.skip(!process.env.ANTHROPIC_API_KEY, …)`, which keeps keyless runs (local, -and fork PRs with no secret access) green and fast. Add the key under -**Settings → Secrets and variables → Actions → New repository secret** to also -run and enforce the LLM specs. For fork PRs, GitHub withholds secrets until a -maintainer approves the run. +and fork PRs with no secret access) green and fast. The gate is a blanket one on +purpose: the app ships **no keyless model** — every model in the picker +(`frontend/src/app/components/assistant/ModelToggle.tsx`) needs an +Anthropic/Google/OpenAI key before `ChatInput` will send at all — so without a +key the 4 specs cannot even submit a message. (The auto title-generation call is +*not* the reason for the gate; keyless it returns a 500 the specs already +tolerate.) Setup steps below. + +## Enable the LLM specs + +### 1. Add the repository secret + +UI path: + +1. Open the repository on GitHub → **Settings**. +2. In the left sidebar: **Secrets and variables → Actions**. +3. On the **Secrets** tab, click **New repository secret**. +4. **Name:** `ANTHROPIC_API_KEY` — exactly this name; both the workflow env and + `e2e/llm.ts` read it. **Secret:** an Anthropic API key (`sk-ant-…`) from + . +5. Click **Add secret**. + +CLI equivalent (repo admin): + +```bash +gh secret set ANTHROPIC_API_KEY --repo Open-Legal-Products/mike +# paste the key at the prompt (or pipe it: --body "$ANTHROPIC_API_KEY") +``` + +### 2. The fork-PR caveat + +On `pull_request` events from **forks**, GitHub withholds repository secrets, so +fork PRs — most external contributions — still run keyless and skip the 4 specs. +That is by design and keeps those runs green. Runs that actually receive the +secret and exercise the specs are: + +- PRs from branches pushed to this repository (maintainer branches), and +- manual runs: **Actions → e2e → Run workflow** (`workflow_dispatch`) on any + branch. + +So after adding the secret, the quickest way to see the specs run is a +`workflow_dispatch` run from the Actions tab. + +### 3. Expected cost per run + +A handful of short completions: one streamed chat answer per LLM spec plus a few +small title generations (`claude-haiku-4-5`, 64-token cap). On the order of a +few cents per run — negligible next to the CI minutes. + +### 4. Confirm the specs ran (not skipped) + +Open the **Run Playwright** step in the Actions log: + +- **Keyless run:** the summary ends with `4 skipped` / `23 passed`, and each + skipped spec carries the reason + `requires a model key — set the ANTHROPIC_API_KEY secret to run LLM-dependent specs`. +- **With the secret:** the summary shows `27 passed` and **no `skipped` line**; + searching the log for `requires a model key` finds nothing. + +The uploaded `playwright-report` artifact shows the same per-spec statuses. + +### Model selection (gap fixed in this branch) + +Earlier revisions of the four specs drove the model picker to a keyless +**"Demo (no key needed)"** entry that exists in the amal66 fork but **not in +this repository** (no `mike-demo` model id, no demo provider), so setting the +secret unskipped them and they then failed at model selection. This branch +carries the fix (commit `test(e2e): select a real Claude model in LLM-gated +specs`): the specs' `selectClaudeModel` helper picks **Claude Sonnet 4.6** in +the ModelToggle whenever the key is set, and the critical-path response +assertion checks for a nonempty streamed assistant answer instead of the +fork's canned demo reply. The **23 passed / 4 skipped** keyless and +**27 passed / 0 skipped** with-key figures come from that commit's own +verification against a full local stack (backend, prod-build frontend, local +Supabase + MinIO — see its commit message); they have not been re-measured +since this branch was rebased onto current `main`. The secret setup above is +all that is needed. ## Make it merge-blocking diff --git a/e2e/llm.ts b/e2e/llm.ts index 3d9c52f8..d5f2a097 100644 --- a/e2e/llm.ts +++ b/e2e/llm.ts @@ -1,16 +1,34 @@ /** * 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. + * Four specs (chat rename, chat delete, project-assistant create+submit, and + * the critical-path "ask a question" flow) create/populate a chat by sending a + * message. This codebase ships no keyless model: every entry in the model + * picker (frontend/src/app/components/assistant/ModelToggle.tsx MODELS) is an + * Anthropic/Google/OpenAI model whose availability requires a configured + * provider key — backend env var or user-stored key + * (backend/src/lib/userApiKeys.ts) — and ChatInput.handleSubmit refuses to + * send when the selected model's key is missing (ApiKeyMissingPopup). The + * backend enforces the same model set server-side + * (backend/src/lib/llm/models.ts ALL_MODELS/providerForModel). So with no key + * there is no way for these specs to send a message at all; unguarded they + * 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. + * The auto title-generation call (POST /chat/:id/generate-title) is NOT why + * the gate exists: keyless it just returns 500, and the specs already treat it + * as best-effort (`.catch(() => null)`). + * + * In CI the key is the `ANTHROPIC_API_KEY` repository secret, which + * `.github/workflows/e2e.yml` exposes both to the backend (backend/.env) and + * to the Playwright process. Guarding with + * `test.skip(!hasLlmKey, LLM_SKIP_REASON)` keeps a keyless run (a plain local + * run, or a fork PR with no secret access) green and fast on the other 23 + * specs, while still running — and enforcing — the LLM specs whenever the key + * is present. Setup steps: docs/e2e-ci.md, "Enable the LLM specs". + * + * When the key IS set, the specs' selectClaudeModel helper picks "Claude + * Sonnet 4.6" in the ModelToggle (see docs/e2e-ci.md, "Model selection"), so + * the unskipped specs submit against a model this repository actually ships. */ export const hasLlmKey = Boolean(process.env.ANTHROPIC_API_KEY);