diff --git a/.claude/skills/api-contract/SKILL.md b/.claude/skills/api-contract/SKILL.md index 08846da8..1c92a979 100644 --- a/.claude/skills/api-contract/SKILL.md +++ b/.claude/skills/api-contract/SKILL.md @@ -262,9 +262,9 @@ Other implementation details worth knowing: - **`STORAGE_UNCONFIGURED` 503**: if `env.RATINGS_DB` is unbound, `handleReviews` returns `503 UNAVAILABLE`/`STORAGE_UNCONFIGURED` before touching D1 (`reviews.ts:158-166`). This is also the state of the *real* - production database today: `wrangler.toml:26` has a placeholder - `database_id = "00000000-0000-0000-0000-000000000000"` — D1 has not been - provisioned. Tests and `wrangler dev` use a simulated local D1 via + production worker today: D1 has not been provisioned, so the + `[[d1_databases]]` block in `wrangler.toml` is commented out and the binding + is genuinely absent. Tests and `wrangler dev` use a simulated local D1 via `@cloudflare/vitest-pool-workers`, so the whole test suite runs green without a real database. Provisioning is out of scope for this skill — see `run-and-operate` for the `wrangler d1 create` runbook, or diff --git a/.claude/skills/architecture-contract/SKILL.md b/.claude/skills/architecture-contract/SKILL.md index 8fe1ba6f..070e6508 100644 --- a/.claude/skills/architecture-contract/SKILL.md +++ b/.claude/skills/architecture-contract/SKILL.md @@ -328,8 +328,8 @@ found" — they're tracked. the class without checking every import, and never rename the route. - **The `/v1alpha` catalogue API is contract-only** (issue #432/PR #433): proto + generated OpenAPI + a read-only worker endpoint exist, but there - is no production server backing MyFestival sync yet — D1 has a - placeholder `database_id` (`cloudflare-worker/wrangler.toml:26`). See + is no production server backing MyFestival sync yet — D1 is unprovisioned + and its binding is commented out in `cloudflare-worker/wrangler.toml`. See `run-and-operate` for the provisioning gap and `api-contract` for the proto surface itself. diff --git a/.claude/skills/build-and-env/SKILL.md b/.claude/skills/build-and-env/SKILL.md index b215a004..c30bbda4 100644 --- a/.claude/skills/build-and-env/SKILL.md +++ b/.claude/skills/build-and-env/SKILL.md @@ -241,7 +241,7 @@ train. | `TEST_LOG` / `ANALYZE_LOG` | `mktemp /tmp/test-XXXXXX.log` / `/tmp/analyze-XXXXXX.log` per run (`mise-tasks/test.sh:6`, `mise-tasks/analyze.sh:6`) | the `test`/`analyze` file-tasks themselves (tee output, preserve exit code via `${PIPESTATUS[0]}`) | Same on both; override to a stable path when you need to grep the same log across multiple invocations without re-running | | `ENVIRONMENT` (wrangler `[vars]`) | `"production"` (`cloudflare-worker/wrangler.toml:6-7`) | Worker code (`worker.js`/`shared.ts`) for any environment-conditional behavior | Only one value committed — there's no separate staging `[vars]` block in `wrangler.toml`; staging behavior is driven by **origin-based** CORS/bucket logic instead (below), not this var | | `RATINGS_BUCKET` (env override) | unset — falls back to `resolveBucket(origin, env)` (`cloudflare-worker/shared.ts:23-27`): `origin === "https://cambeerfestival.app"` → `"prod"`, else `"test"` | `shared.ts` bucket resolution for the D1 `reviews` table's composite key | Not set anywhere in committed config today; it's an escape hatch for forcing a bucket regardless of request origin. Do not set it in production without understanding it silently overrides the origin check | -| `RATINGS_DB` (D1 binding, not an env var) | `[[d1_databases]] binding = "RATINGS_DB"`, `database_id = "00000000-0000-0000-0000-000000000000"` **placeholder** (`wrangler.toml:20-25`) | `reviews.ts` via `env.RATINGS_DB`; missing/misconfigured → worker returns 503 `STORAGE_UNCONFIGURED` | Tests/local dev use wrangler's simulated local D1 (id ignored); a real deploy needs `wrangler d1 create cbf-myfestival` + paste the real id + `wrangler d1 migrations apply` — see `run-and-operate` for the provisioning runbook | +| `RATINGS_DB` (D1 binding, not an env var) | `[[d1_databases]]` block **commented out** in `wrangler.toml` (a placeholder `database_id` broke every deploy — error 10181); the test-only binding lives in `vitest.config.js` (`miniflare.d1Databases`) | `reviews.ts` via `env.RATINGS_DB`; missing/misconfigured → worker returns 503 `STORAGE_UNCONFIGURED` | Tests/local dev use a simulated local D1 (id ignored; `--dry-run` doesn't check resource existence either, so `test/wrangler-config.test.js` guards the placeholder case); enabling it needs `wrangler d1 create cbf-myfestival` + uncomment the block with the real id + `wrangler d1 migrations apply --remote` — see `run-and-operate` for the provisioning runbook | | `BASE_URL` (Playwright) | `"http://127.0.0.1:8080"` (`playwright.config.ts:34`) | `test-e2e/*.spec.ts` via `page.goto`/`baseURL` | Local/CI default targets a locally-served build; CI's `smoke-test-preview` job sets `BASE_URL=` to run `csp-smoke.spec.ts` against a real deployed CSP policy — the only place that check is meaningful (`web/_headers` CSP isn't exercised any other way) | ## 7. Adding a tool or task correctly diff --git a/.claude/skills/debugging-playbook/SKILL.md b/.claude/skills/debugging-playbook/SKILL.md index f2ab9d32..4d4df35a 100644 --- a/.claude/skills/debugging-playbook/SKILL.md +++ b/.claude/skills/debugging-playbook/SKILL.md @@ -92,7 +92,8 @@ most battles in this repo have already been fought once. | Symptom | Likely cause | Discriminating experiment | Fix pattern | History | |---|---|---|---|---| -| `/v1alpha/...` returns 503 with reason `STORAGE_UNCONFIGURED` | `env.RATINGS_DB` D1 binding missing — `cloudflare-worker/wrangler.toml` ships a placeholder `database_id = "00000000-..."` | `curl -s https://data.cambeerfestival.app/v1alpha/festivals/x/drinks/y/review -H 'X-Device-Id: t'` and check the error body (`reviews.ts:158-166`) | Provision D1: `wrangler d1 create cbf-myfestival` → paste real id into wrangler.toml → `wrangler d1 migrations apply cbf-myfestival --remote` (`--remote` migrates the real DB, not the local sim); token needs D1:Edit. Full runbook: skill `run-and-operate` | v1alpha arc (PR #426); deploy still pending | +| `/v1alpha/...` returns 503 with reason `STORAGE_UNCONFIGURED` | Expected, not a bug: `env.RATINGS_DB` is absent because the `[[d1_databases]]` block in `cloudflare-worker/wrangler.toml` is **deliberately commented out** until a database is provisioned | `curl -s https://data.cambeerfestival.app/v1alpha/festivals/x/drinks/y/review -H 'X-Device-Id: t'` and check the error body (`reviews.ts:158-166`) | Provision D1: `wrangler d1 create cbf-myfestival` → uncomment the block and paste the real id → `wrangler d1 migrations apply cbf-myfestival --remote` (`--remote` migrates the real DB, not the local sim); token needs D1:Edit. Full runbook: skill `run-and-operate` | v1alpha arc (PR #426); binding disabled to unblock deploys | +| `wrangler deploy` fails with `D1 binding 'X' references database '…' which was not found [code: 10181]`, while `npm test` is green | A binding names a resource that doesn't exist in the account. Cloudflare validates bindings at upload; the vitest pool and `wrangler dev` use simulated local resources and never check the id, so tests cannot catch it | **Not** `--dry-run` — it exits 0 on a dangling binding (verified, wrangler 4.98.0). Read the deploy log for the resource id, then confirm the resource exists in the account (dashboard, or `wrangler d1 list`). `cloudflare-worker/test/wrangler-config.test.js` catches the placeholder case offline | Either provision the resource, or remove/comment out the binding so the rest of the worker can deploy (the code already degrades to 503 `STORAGE_UNCONFIGURED`). Never leave a placeholder id in place: it blocks *every* deploy, proxy included | Broke all worker deploys 2026-06-13 → 2026-08-10; the live worker silently stayed on pre-#426 code for ~2 months | | Browser: CORS error calling the worker; curl works fine | Origin not in the allow-list — the worker returns NO CORS headers for unknown origins (silent reject) | Compare your page's `Origin` against `ALLOWED_ORIGINS` (`cloudflare-worker/worker.js:29-37`) and the wildcard suffixes in `getCorsHeaders` (worker.js:279-329): `*.cambeerfestival.pages.dev`, `*.staging-cambeerfestival.pages.dev`, `*.trycloudflare.com` | Add the origin to the allow-list — but `cloudflare-worker/` is on the Do-Not-Modify list; needs explicit maintainer request + PR | Allow-list design; ops trap (trycloudflare wildcard live in prod) | | Worker returns 502 | Upstream `data.cambridgebeerfestival.com` fetch failed — the worker proxies everything not matched by its own routes | `curl -si https://data.cambridgebeerfestival.com/` — is upstream itself down/erroring? | Nothing to fix app-side; upstream CAMRA feeds are untouchable (unwritten rule). The app's SWR cache is the mitigation — verify cached data still renders with the refresh notice | worker.js:102-137 | diff --git a/.claude/skills/my-festival-campaign/SKILL.md b/.claude/skills/my-festival-campaign/SKILL.md index b23fd06a..0e4b1fb6 100644 --- a/.claude/skills/my-festival-campaign/SKILL.md +++ b/.claude/skills/my-festival-campaign/SKILL.md @@ -330,10 +330,11 @@ account. `cloudflare-worker/` is on the **Do-Not-Modify list** — it needs an **explicit maintainer request**, and the **festival-freeze window** must be clear (`change-control`). Do not provision on your own initiative. -**Provision** (one-time — `wrangler.toml:26` `database_id` is the placeholder -`00000000-...`; binding `RATINGS_DB`, db name `cbf-myfestival`). Follow the exact -command sequence in skill `run-and-operate` §4 — `wrangler d1 create -cbf-myfestival`, paste the returned id into `wrangler.toml`, then +**Provision** (one-time — the `[[d1_databases]]` block in `wrangler.toml` is +commented out, so there is no binding at all today; binding name `RATINGS_DB`, +db name `cbf-myfestival`). Follow the exact command sequence in skill +`run-and-operate` §4 — `wrangler d1 create cbf-myfestival`, uncomment the block +and paste the returned id into `wrangler.toml`, then `wrangler d1 migrations apply cbf-myfestival --remote`. The **`--remote` flag is mandatory**: without it `wrangler` migrates only the local simulated D1 and the real production database is left unmigrated while the command reports success. @@ -359,8 +360,11 @@ curl -sS -X PATCH \ curl -sS https://data.cambeerfestival.app/v1alpha/festivals/cbf2025/reviewSummaries/beer-1 # expect: {"name":"...","ratingCount":>=1,"averageRating":...,"recommendRate":...} ``` -- **See 503 after provisioning?** The binding didn't resolve — the `database_id` - paste is wrong or the deploy predates the migration. Re-apply, redeploy. +- **See 503 after provisioning?** The binding didn't resolve — the block is + still commented out, the `database_id` paste is wrong, or the deploy predates + the migration. Note `--dry-run` lists the binding + even when the database does not exist, so it proves nothing here — check the + real deploy log. Re-apply the migration and redeploy. - **See `test` data leaking to `prod`?** Bucket resolution keys on `Origin`; only `https://cambeerfestival.app` → `prod`, everything else → `test` (or the `RATINGS_BUCKET` var). That's correct isolation, not a bug. @@ -573,7 +577,7 @@ Written 2026-07-02. Verified against the working tree at that date: - Worker: `Review` API (`/review`, `/reviews`, `/reviewSummaries`) implemented in `cloudflare-worker/reviews.ts`; 503 `STORAGE_UNCONFIGURED` guard on `env.RATINGS_DB`; `X-Device-Id` identity; bucket resolution in `shared.ts`; - placeholder `database_id` in `wrangler.toml:26`; migration + D1 binding commented out in `wrangler.toml` (unprovisioned); migration `0001_create_reviews_table.sql`; curl set + provisioning in `cloudflare-worker/README.md`. The `DrinkEntry`/`drinkEntries` proto endpoints are **not** implemented (grep over `cloudflare-worker/*.ts`). @@ -598,8 +602,8 @@ rg -n 'favorites' lib/router.dart # Current goldens on disk find test -path '*goldens*' -name '*.png' -# D1 still a placeholder? -rg -n 'database_id' cloudflare-worker/wrangler.toml +# D1 still unprovisioned? (expect: [[d1_databases]] commented out) +rg -n 'd1_databases' -A5 cloudflare-worker/wrangler.toml # Baseline ./bin/mise run check # add MISE_ENV=claude-code-web on a 403-sandboxed box diff --git a/.claude/skills/reference/SKILL.md b/.claude/skills/reference/SKILL.md index e9e079db..831a6f97 100644 --- a/.claude/skills/reference/SKILL.md +++ b/.claude/skills/reference/SKILL.md @@ -273,7 +273,7 @@ are at different maturity levels and it's easy to conflate them. | Surface | Status | Where implemented | What it does | |---|---|---|---| | **Static beverage feeds** | Live in production, untouchable upstream data | `data.cambridgebeerfestival.com` (CAMRA's own static files) proxied through `cloudflare-worker/worker.js` at `data.cambeerfestival.app/{festivalId}/{category}.json`; also `/festivals.json` (embedded registry, `no-cache, must-revalidate`) and `/{festivalId}/available_beverage_types.json` (scrapes an upstream Apache directory listing, 1h cache) | The entire catalogue the app renders — Producers→Products, per festival per category (§3). This is what `BeerApiService`/`FestivalService` fetch. | -| **v1alpha Review API** | Live code, deployed worker, **D1 database not yet provisioned** (`wrangler.toml` ships a placeholder `database_id`) | `cloudflare-worker/reviews.ts` + `shared.ts`, routed at `/v1alpha/...` inside the same worker; D1 table `reviews` (`cloudflare-worker/migrations/0001_create_reviews_table.sql`) | Anonymous star-rating + "would recommend" reviews, keyed by `X-Device-Id` header (not signed-in identity yet — `user_id` column reserved for a future sign-in upgrade). GET/PATCH/DELETE one review, list caller's reviews, get/list aggregate summaries per drink. Any unmatched `/v1alpha/*` path 404s — it is never proxied upstream. | +| **v1alpha Review API** | Live code, deployed worker, **D1 database not yet provisioned**, so `/v1alpha` answers 503 `STORAGE_UNCONFIGURED` in production (the `[[d1_databases]]` block in `wrangler.toml` is commented out) | `cloudflare-worker/reviews.ts` + `shared.ts`, routed at `/v1alpha/...` inside the same worker; D1 table `reviews` (`cloudflare-worker/migrations/0001_create_reviews_table.sql`) | Anonymous star-rating + "would recommend" reviews, keyed by `X-Device-Id` header (not signed-in identity yet — `user_id` column reserved for a future sign-in upgrade). GET/PATCH/DELETE one review, list caller's reviews, get/list aggregate summaries per drink. Any unmatched `/v1alpha/*` path 404s — it is never proxied upstream. | | **proto CatalogService / MyFestivalService** | **Paper contract only** — defines the intended future v1alpha REST surface via `google.api.http` annotations, generates OpenAPI, but has **no server implementation** in the worker (`/v1alpha` routing only wires up `handleReviews`, i.e. the Review API above; `CatalogService`'s `ListFestivals`/`GetFestival`/`ListDrinks` RPCs have no handler) | `proto/cambeerfestival/festival/v1alpha/{catalog_service,my_festival_service,drink_entry,drink_summary,festival,drink,producer}.proto` → `buf generate` → `docs/code/api/openapi/openapi.yaml` (Redoc-published by `api-docs.yml`) | Design-time contract for where the API is headed: a typed, resource-oriented catalogue API (AIP-compliant) and a richer `DrinkEntry`/`MyFestivalService` (favourite/rating/note/pour-count sync with soft-delete tombstones and etag concurrency) intended to eventually replace/extend the anonymous Review API. Full workflow and AIP facts: skill `api-contract`. | The **static feeds** and the **v1alpha Review API** are the two surfaces that @@ -445,8 +445,8 @@ cat docs/code/api/festival-registry-schema.json # API surface: is CatalogService actually routed in the worker? (expect: no handler, only handleReviews) grep -n "v1alpha\|handleReviews\|CatalogService" cloudflare-worker/worker.js -# D1 provisioning state (expect: placeholder database_id until provisioned) -grep -n "database_id" cloudflare-worker/wrangler.toml +# D1 provisioning state (expect: [[d1_databases]] commented out until provisioned) +grep -n "d1_databases" -A5 cloudflare-worker/wrangler.toml # Proto promotion path / AIP framing sed -n '1,40p' proto/buf.yaml diff --git a/.claude/skills/run-and-operate/SKILL.md b/.claude/skills/run-and-operate/SKILL.md index 65cb0d0e..280d19fa 100644 --- a/.claude/skills/run-and-operate/SKILL.md +++ b/.claude/skills/run-and-operate/SKILL.md @@ -196,36 +196,56 @@ enrollment; every release after that is fully automated. > changes near/during the live festival; see skill `change-control`). Do not > run the commands below on your own judgement. -`cloudflare-worker/wrangler.toml` has a **placeholder** database id: - -```toml -[[d1_databases]] -binding = "RATINGS_DB" -database_name = "cbf-myfestival" -database_id = "00000000-0000-0000-0000-000000000000" -migrations_dir = "migrations" -``` +`cloudflare-worker/wrangler.toml` has **no D1 binding** — the +`[[d1_databases]]` block is commented out, so the deployed worker runs without +`env.RATINGS_DB` and every `/v1alpha` route answers 503 `STORAGE_UNCONFIGURED` +(`reviews.ts:158`). + +> **Why commented out, not a placeholder id.** It used to carry +> `database_id = "00000000-0000-0000-0000-000000000000"`. Cloudflare validates +> bindings when the script is uploaded, so that placeholder made **every** +> `wrangler deploy` fail with error 10181 — taking the CORS proxy down with it, +> even though the proxy has nothing to do with D1. The worker was undeployable +> from 2026-06-13 (PR #426, which added the binding) until 2026-08-10, and the +> live worker silently served pre-#426 code that whole time. Do not restore a +> placeholder id "so the config documents itself" — a binding is either real or +> absent. Tests and `wrangler dev` use a **simulated local D1** (via -`@cloudflare/vitest-pool-workers`) and ignore this id entirely — the whole -worker test suite (`npm test` in `cloudflare-worker/`) runs green with the -placeholder in place, so a passing `test:worker` run tells you nothing about -whether the real database exists. - -To provision the real thing (do this before any manual `wrangler deploy` that -needs to serve real `/v1alpha` review traffic): +`@cloudflare/vitest-pool-workers`) and never check the id — a green +`test:worker` run tells you nothing about whether the real database exists, or +even whether the worker can deploy at all. The binding for tests is declared in +`cloudflare-worker/vitest.config.js` (`miniflare.d1Databases`), deliberately +decoupled from `wrangler.toml` so the test suite is unaffected by the +production binding's presence. + +**`wrangler deploy --dry-run` does NOT catch a dangling binding** — verified on +wrangler 4.98.0: with the all-zeroes `database_id` restored it printed +`env.RATINGS_DB (cbf-myfestival)` in the binding table and exited 0. Resource +existence is checked server-side at upload, so CI's `validate-worker` job was +green on the PR that broke deploys, and only the post-merge `deploy-worker` job +on `main` failed. The offline guard is +`cloudflare-worker/test/wrangler-config.test.js` (runs in `test-worker`, needs +no credentials); the only positive proof a binding resolves is a real deploy. + +To provision the real thing (needed before `/v1alpha` can serve real review +traffic): ```bash cd cloudflare-worker # 1. Create the D1 database in the Cloudflare account wrangler d1 create cbf-myfestival -# → paste the returned database_id into wrangler.toml's database_id field +# → uncomment the [[d1_databases]] block in wrangler.toml and paste the +# returned id into database_id # 2. Apply migrations to the REAL (remote) database wrangler d1 migrations apply cbf-myfestival --remote # (only one migration exists today: migrations/0001_create_reviews_table.sql — # single `reviews` table, PK (bucket, festival_id, drink_id, device_id)) + +# 3. Confirm the binding resolves before pushing +npx wrangler deploy --dry-run # must list env.RATINGS_DB (cbf-myfestival) ``` The `CLOUDFLARE_API_TOKEN` used for this needs **D1:Edit** permission in @@ -399,8 +419,11 @@ Re-verification commands (run when a fact here feels stale): sed -n '1,50p' .github/workflows/release-pr.yml sed -n '1,80p' .github/workflows/release.yml -# D1 still unprovisioned (placeholder id)? -grep -A2 database_id cloudflare-worker/wrangler.toml +# D1 still unprovisioned (expect: the [[d1_databases]] block is commented out)? +grep -n "d1_databases" -A5 cloudflare-worker/wrangler.toml + +# Placeholder bindings still absent? (--dry-run does NOT check this; this does) +(cd cloudflare-worker && npx vitest run test/wrangler-config.test.js) # Deployment topology still matches reality (not the stale doc)? grep -n "project-name" .github/workflows/ci.yml .github/workflows/release-web.yml diff --git a/.claude/skills/validation-and-qa/SKILL.md b/.claude/skills/validation-and-qa/SKILL.md index cbbe2e95..435fa140 100644 --- a/.claude/skills/validation-and-qa/SKILL.md +++ b/.claude/skills/validation-and-qa/SKILL.md @@ -588,9 +588,14 @@ Per AGENTS.md's Engineering Standards: - CSP header behavior anywhere except a deployed Pages URL (`csp-smoke.spec.ts` only runs in CI's `smoke-test-preview` job). - Whether a real D1 database in production behaves like the simulated D1 in - `cloudflare-worker/test/` — migrations are exercised, but `wrangler.toml`'s - `database_id` is still the placeholder `00000000-...` until a real - `wrangler d1 create` + `wrangler d1 migrations apply` has run. + `cloudflare-worker/test/` — migrations are exercised, but no real database + exists until a `wrangler d1 create` + `wrangler d1 migrations apply --remote` + has run. Note the test D1 binding comes from `vitest.config.js`, not + `wrangler.toml` — a passing suite says nothing about whether the + deployed bindings resolve, and neither does `wrangler deploy --dry-run` + (it exits 0 on a binding naming a non-existent database). Only a real deploy + proves it; `cloudflare-worker/test/wrangler-config.test.js` catches the + placeholder-id case offline. --- diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 1ab74d3d..5ce2d4dd 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -55,6 +55,16 @@ updates: open-pull-requests-limit: 5 commit-message: prefix: chore + # TypeScript is held at 5.x here. openapi-typescript@7.13.0 — still the + # latest — peer-requires `typescript: ^5.x`, so a major bump produces a + # tree npm cannot resolve and every `npm ci` in CI fails. Without this, + # the grouped update pulled TypeScript 7.x and wedged the worker + # dependabot PR from 2026-06-15 onward (#440), failing weekly. + # Remove this ignore once openapi-typescript supports TypeScript 7. + ignore: + - dependency-name: typescript + update-types: + - version-update:semver-major groups: worker-npm-dependencies: patterns: diff --git a/cloudflare-worker/README.md b/cloudflare-worker/README.md index 92dbe6c9..10adbf10 100644 --- a/cloudflare-worker/README.md +++ b/cloudflare-worker/README.md @@ -141,19 +141,38 @@ curl https://data.cambeerfestival.app/v1alpha/festivals/cbf2025/reviewSummaries/ # -> {"name":"...","ratingCount":3,"averageRating":4.0,"responseCount":2,"recommendCount":1,"recommendRate":0.5} ``` -#### D1 provisioning (one-time, before first deploy) +#### D1 provisioning (one-time, to enable the review API) -The `database_id` in `wrangler.toml` is a placeholder. Local `wrangler dev` and -the vitest test pool use a simulated local D1 and ignore it, so the full test -suite runs with no real database. Before deploying: +**The review API is currently disabled in production.** No D1 database has been +provisioned, so the `[[d1_databases]]` block in `wrangler.toml` is commented +out and every `/v1alpha` route answers 503 `STORAGE_UNCONFIGURED`. The proxy, +`/health` and `/festivals.json` are unaffected. + +The binding is commented out rather than left pointing at a placeholder id +because Cloudflare validates bindings at upload time: a binding naming a +database that does not exist fails `wrangler deploy` outright (error 10181), +which blocked *all* worker deploys — proxy included — from 2026-06-13 until +this was fixed. + +Nothing caught it automatically: `wrangler dev` and the vitest pool use a +simulated local D1 that ignores the id, and `wrangler deploy --dry-run` (CI's +`validate-worker` job) **exits 0 with a dangling binding** — verified against +wrangler 4.98.0, it prints the binding table without checking that the resource +exists. The only signal was the real deploy on `main`, after merge. +`test/wrangler-config.test.js` is the offline guard added to close that gap. + +To enable it: ```bash cd cloudflare-worker -wrangler d1 create cbf-myfestival # prints the database_id -# paste the id into wrangler.toml ([[d1_databases]].database_id) -wrangler d1 migrations apply cbf-myfestival # applies migrations/*.sql +wrangler d1 create cbf-myfestival # prints the database_id +# uncomment [[d1_databases]] in wrangler.toml and paste the id into database_id +wrangler d1 migrations apply cbf-myfestival --remote # applies migrations/*.sql ``` +`--remote` targets the real database; without it you migrate only the local +simulation. + The deploy `CLOUDFLARE_API_TOKEN` must include **D1: Edit** in addition to Workers Scripts: Edit. To wipe test data: `DELETE FROM reviews WHERE bucket='test'`. diff --git a/cloudflare-worker/test/reviews.test.js b/cloudflare-worker/test/reviews.test.js index 1817502c..3420bb4d 100644 --- a/cloudflare-worker/test/reviews.test.js +++ b/cloudflare-worker/test/reviews.test.js @@ -37,6 +37,21 @@ async function send( return response; } +// Sends a request through an environment with no D1 binding at all, which is +// how the worker is currently deployed (see wrangler.toml — the binding is +// commented out until the database is provisioned). +async function sendWithoutStorage(method, path) { + const { RATINGS_DB: _unused, ...envWithoutDb } = env; + const request = new Request(`https://worker.example.com${path}`, { + method, + headers: { Origin: TEST_ORIGIN, "X-Device-Id": DEVICE }, + }); + const ctx = createExecutionContext(); + const response = await worker.fetch(request, envWithoutDb, ctx); + await waitOnExecutionContext(ctx); + return response; +} + const reviewPath = (f, d) => `/v1alpha/festivals/${f}/drinks/${d}/review`; const patch = (f, d, body, opts) => send("PATCH", reviewPath(f, d), { body, ...opts }); @@ -456,3 +471,40 @@ describe("reviews — bucket isolation", () => { expect(test.averageRating).toBe(1); }); }); + +// --------------------------------------------------------------------------- +// Degraded mode: deployed without a D1 binding +// --------------------------------------------------------------------------- + +describe("no D1 binding — the current production configuration", () => { + it("answers every /v1alpha route with 503 STORAGE_UNCONFIGURED", async () => { + const routes = [ + ["GET", reviewPath("cbf2025", "beer-1")], + ["PATCH", reviewPath("cbf2025", "beer-1")], + ["DELETE", reviewPath("cbf2025", "beer-1")], + ["GET", "/v1alpha/festivals/cbf2025/reviews"], + ["GET", "/v1alpha/festivals/cbf2025/reviewSummaries"], + ["GET", "/v1alpha/festivals/cbf2025/reviewSummaries/beer-1"], + ]; + + for (const [method, path] of routes) { + const response = await sendWithoutStorage(method, path); + expect(response.status, `${method} ${path}`).toBe(503); + const { error } = await response.json(); + expect(error.status, `${method} ${path}`).toBe("UNAVAILABLE"); + expect(error.details[0].reason, `${method} ${path}`).toBe( + "STORAGE_UNCONFIGURED", + ); + } + }); + + it("keeps the proxy endpoints working without storage", async () => { + const health = await sendWithoutStorage("GET", "/health"); + expect(health.status).toBe(200); + expect(await health.json()).toEqual({ status: "ok" }); + + const festivals = await sendWithoutStorage("GET", "/festivals.json"); + expect(festivals.status).toBe(200); + expect((await festivals.json()).default_festival_id).toBeTruthy(); + }); +}); diff --git a/cloudflare-worker/test/wrangler-config.test.js b/cloudflare-worker/test/wrangler-config.test.js new file mode 100644 index 00000000..8ad80454 --- /dev/null +++ b/cloudflare-worker/test/wrangler-config.test.js @@ -0,0 +1,98 @@ +import { describe, it, expect } from "vitest"; +// Inlined at build time by vite — workerd itself has no filesystem access. +import wranglerToml from "../wrangler.toml?raw"; + +/** + * Regression guard for the 2026-06-13 → 2026-08-10 deploy outage. + * + * `wrangler.toml` carried a placeholder `database_id` of all zeroes for a D1 + * database that was never provisioned. Cloudflare validates bindings when the + * script is uploaded, so every `wrangler deploy` failed with error 10181 and + * took the whole worker — CORS proxy included — with it. + * + * Nothing caught it: the vitest pool and `wrangler dev` use simulated local + * resources that ignore the id, and `wrangler deploy --dry-run` (CI's + * `validate-worker` job) exits 0 with a dangling binding — verified against + * wrangler 4.98.0. The only signal was the real deploy on `main`, after merge. + * + * So this test asserts the one thing that is checkable offline: a binding + * declared in `wrangler.toml` must not reference a placeholder resource. A + * binding is either real or absent — comment it out until it is provisioned. + */ + +/** Config lines that wrangler actually reads (comments stripped). */ +const activeLines = wranglerToml + .split("\n") + .map((line) => line.trim()) + .filter((line) => line.length > 0 && !line.startsWith("#")); + +/** + * A resource id that names nothing: empty, the nil UUID (any zeros-and-dashes + * spelling), or unfilled `<...>` template text. + * + * Deliberately narrow. Matching anything merely zero-ish would reject real ids + * — roughly 1 in 256 random UUIDs both start and end with `0` — and the failure + * would land precisely on whoever is provisioning D1 for the first time, which + * is the worst possible moment for a spurious error. + */ +export function isPlaceholderId(value) { + return /^(?:[0-]*|<.*>)$/.test(value); +} + +describe("wrangler.toml — deployability", () => { + it("recognises placeholder ids without rejecting real ones", () => { + // The nil UUID is the exact value that broke production. + expect(isPlaceholderId("00000000-0000-0000-0000-000000000000")).toBe(true); + expect(isPlaceholderId("")).toBe(true); + expect(isPlaceholderId("0")).toBe(true); + expect(isPlaceholderId("")).toBe( + true, + ); + + // Real ids, including the awkward ones that start and/or end with 0. + expect(isPlaceholderId("0f9e8d7c-6b5a-4321-8f0e-1d2c3b4a5960")).toBe(false); + expect(isPlaceholderId("0a000000-0000-0000-0000-000000000000")).toBe(false); + expect(isPlaceholderId("00000000-0000-0000-0000-00000000000a")).toBe(false); + expect(isPlaceholderId("xxxxxxxx-1234-5678-9abc-def012345678")).toBe(false); + }); + + it("declares no binding pointing at a placeholder resource id", () => { + const ids = activeLines + .filter((line) => /^(database_id|id|bucket_name)\s*=/.test(line)) + .map((line) => ({ + line, + value: + line + .split("=")[1] + ?.trim() + .replace(/^["']|["']$/g, "") ?? "", + })); + + const placeholders = ids.filter(({ value }) => isPlaceholderId(value)); + + expect( + placeholders.map(({ line }) => line), + "A binding references a placeholder/unprovisioned resource. This fails " + + "`wrangler deploy` with error 10181 and takes the entire worker down, " + + "including the proxy. Provision the resource and paste the real id, or " + + "comment the binding out until it exists.", + ).toEqual([]); + }); + + it("keeps the D1 binding and the review API's degraded mode in sync", () => { + // If someone provisions D1 and uncomments the binding, this test fails and + // points them at the STORAGE_UNCONFIGURED tests in reviews.test.js, which + // assert the no-storage behaviour that is no longer the deployed reality. + const hasD1Binding = activeLines.some((line) => + line.startsWith("[[d1_databases]]"), + ); + + expect( + hasD1Binding, + "wrangler.toml now declares a D1 binding, so /v1alpha no longer returns " + + "503 STORAGE_UNCONFIGURED in production. Update the 'no D1 binding' " + + "tests in test/reviews.test.js and the README's D1 section, then flip " + + "this expectation to true.", + ).toBe(false); + }); +}); diff --git a/cloudflare-worker/vitest.config.js b/cloudflare-worker/vitest.config.js index 6374e7e6..fc8611f1 100644 --- a/cloudflare-worker/vitest.config.js +++ b/cloudflare-worker/vitest.config.js @@ -15,6 +15,13 @@ export default defineConfig(async () => { cloudflareTest({ wrangler: { configPath: "./wrangler.toml" }, miniflare: { + // The RATINGS_DB binding is declared here rather than in + // wrangler.toml: production has no D1 database provisioned yet, and a + // binding pointing at a non-existent database fails `wrangler deploy` + // outright. Declaring the simulated D1 here keeps the review-API + // tests running against a real (local) database while the deployed + // worker runs without the binding. + d1Databases: ["RATINGS_DB"], bindings: { TEST_MIGRATIONS: migrations }, }, }), diff --git a/cloudflare-worker/wrangler.toml b/cloudflare-worker/wrangler.toml index cd9e7811..e85672d5 100644 --- a/cloudflare-worker/wrangler.toml +++ b/cloudflare-worker/wrangler.toml @@ -10,18 +10,36 @@ ENVIRONMENT = "production" # "My festival" aggregate storage (D1 — reviews, tastings, bookmarks). # -# `database_id` is a placeholder until the database is provisioned in the -# Cloudflare account. Local dev (`wrangler dev`) and the vitest test pool use a -# simulated local D1 and ignore this id, so the whole test suite runs without a -# real database. Before the first `wrangler deploy`, run: +# DISABLED until the database is provisioned in the Cloudflare account. # -# wrangler d1 create cbf-myfestival +# This block previously carried a placeholder `database_id` of all zeroes. That +# is not deploy-neutral: Cloudflare validates every binding when the script is +# uploaded, so `wrangler deploy` failed with # -# then paste the returned id below and apply migrations with: +# D1 binding 'RATINGS_DB' references database +# '00000000-0000-0000-0000-000000000000' which was not found. [code: 10181] # -# wrangler d1 migrations apply cbf-myfestival -[[d1_databases]] -binding = "RATINGS_DB" -database_name = "cbf-myfestival" -database_id = "00000000-0000-0000-0000-000000000000" -migrations_dir = "migrations" +# and took the whole worker down with it — including the CORS data proxy, which +# has nothing to do with D1. Tests never caught this because the vitest pool and +# `wrangler dev` use a simulated local D1 that ignores the id entirely. +# +# With the binding absent, `env.RATINGS_DB` is undefined and the /v1alpha review +# routes return 503 UNAVAILABLE / STORAGE_UNCONFIGURED (see `reviews.ts`), which +# is the documented degraded mode. The proxy, /health and /festivals.json are +# unaffected. +# +# To enable the review API, provision the database and restore the binding: +# +# wrangler d1 create cbf-myfestival # → returns the real database_id +# # uncomment the block below and paste the id into database_id +# wrangler d1 migrations apply cbf-myfestival --remote +# +# The API token needs D1:Edit in addition to its Workers/Pages scopes. +# Tests keep their simulated D1 either way — it is declared in +# `vitest.config.js`, not here. +# +# [[d1_databases]] +# binding = "RATINGS_DB" +# database_name = "cbf-myfestival" +# database_id = "" +# migrations_dir = "migrations"