Skip to content

fix(worker): unblock deploys by removing the unprovisioned D1 binding - #542

Open
richardthe3rd wants to merge 3 commits into
mainfrom
claude/cloudflare-worker-deploy-failure-actcgd
Open

fix(worker): unblock deploys by removing the unprovisioned D1 binding#542
richardthe3rd wants to merge 3 commits into
mainfrom
claude/cloudflare-worker-deploy-failure-actcgd

Conversation

@richardthe3rd

Copy link
Copy Markdown
Owner

Fixes two deterministic (not flaky) worker CI failures.

1. deploy-worker has been broken since 2026-06-13

cloudflare-worker/wrangler.toml declared a D1 binding whose database_id was a placeholder of all zeroes, for a database that was never created. Cloudflare validates bindings when the script is uploaded, so every wrangler deploy failed:

D1 binding 'RATINGS_DB' references database
'00000000-0000-0000-0000-000000000000' which was not found. [code: 10181]

That took the whole worker down with it, including the CORS data proxy, which has nothing to do with D1. #426 introduced the binding; its own deploy failed, and so has every deploy since. Runs that look green are ones where deploy-worker was skipped because no worker/festivals path was touched.

Production impact, confirmed:

  • https://data.cambeerfestival.app/health200 {"status":"ok"} — the worker is up
  • /v1alpha/festivals/cbf2026/reviewSummaries404 from Apache at data.cambridgebeerfestival.com — the request fell through to the upstream origin, so the deployed worker has no /v1alpha route at all

The running worker predates #426 by ~2 months. More urgent than the missing route: data/festivals.json changes also trigger this deploy, so festival data edits could not reach production.

The fix

Comment the binding out rather than leave a placeholder id. reviews.ts:158 already returns 503 UNAVAILABLE / STORAGE_UNCONFIGURED when env.RATINGS_DB is absent, so the review API keeps exactly the behaviour it has had all along — it has never served traffic — while the proxy, /health and /festivals.json deploy again.

npx wrangler deploy --dry-run now succeeds with only the ENVIRONMENT binding.

Provisioning D1 stays a one-time maintainer step against the Cloudflare account, now documented as uncomment-and-paste. This PR does not enable the review API; it restores deployability.

Why nothing caught it

  • the vitest pool and wrangler dev use a simulated local D1 that ignores the id, so the suite stayed green
  • wrangler deploy --dry-run — CI's validate-worker job — exits 0 on a dangling binding. Verified by restoring the placeholder: it printed env.RATINGS_DB (cbf-myfestival) and exited 0 (wrangler 4.98.0). Resource existence is checked server-side at upload, so the gate you'd expect to catch this structurally cannot.

Since neither gate can catch this class of bug, added cloudflare-worker/test/wrangler-config.test.js:

  • fails on a binding pointing at a placeholder/unprovisioned resource id
  • fails if a D1 binding reappears without the degraded-mode tests being updated

Verified red against the exact config that broke production, green after. Also added tests asserting all six /v1alpha routes return 503 STORAGE_UNCONFIGURED and that /health and /festivals.json still work with no storage — i.e. the configuration actually being deployed. The test D1 binding moved to vitest.config.js, decoupling the suite from the deployed binding set.

No festival data drift: the live /festivals.json and data/festivals.json are semantically identical (minified vs pretty-printed).

2. Worker dependabot PRs fail at npm ci (#440)

The grouped update bumps TypeScript to 7.x, but openapi-typescript@7.13.0 — still the latest release — peer-requires typescript: ^5.x. main is fine (cloudflare-worker/package.json pins typescript: ^5.9.3); only dependabot's own tree cannot resolve. Failing weekly since 2026-06-15.

Proved both directions rather than assuming:

  • typescript@^7.0.2ERESOLVE, peer typescript@"^5.x" from openapi-typescript@7.13.0
  • typescript@^5.9.3 plus the group's other four bumps (@cloudflare/vitest-pool-workers 0.20.2, @cloudflare/workers-types 5.20260804.1, vitest 4.1.10, wrangler 4.119.0) → resolves cleanly

So ignoring major TypeScript bumps unwedges the group rather than merely shrinking it. Added the ignore to the /cloudflare-worker entry with a documented removal condition.

Testing

  • cloudflare-worker: 90 tests pass (88 existing + 2 new files' worth)
  • ./bin/mise run check: passes (analyze + full Flutter suite; no Dart changed)
  • npx wrangler deploy --dry-run: succeeds
  • guard test verified red/green against the breaking config

Pre-existing and unrelated: npm run typecheck in cloudflare-worker/ fails on a missing generated ./src/api-types. Not run by CI's worker job (npm test only), untouched here.

Follow-ups for the maintainer


Generated by Claude Code

claude added 2 commits August 10, 2026 20:29
`wrangler.toml` declared a D1 binding whose `database_id` was a placeholder
of all zeroes for a database that was never created. Cloudflare validates
bindings when the script is uploaded, so every `wrangler deploy` failed:

  D1 binding 'RATINGS_DB' references database
  '00000000-0000-0000-0000-000000000000' which was not found. [code: 10181]

That took the whole worker down with it, including the CORS data proxy,
which has nothing to do with D1. The worker has been undeployable since
#426 landed on 2026-06-13; production still serves pre-#426 code, and
`/v1alpha` falls through to the upstream origin as a 404. Critically,
`data/festivals.json` edits also trigger this deploy, so festival data
changes could not reach production either.

Comment the binding out rather than leave a placeholder id. `reviews.ts`
already degrades to 503 UNAVAILABLE / STORAGE_UNCONFIGURED when
`env.RATINGS_DB` is absent, so the review API keeps the behaviour it has
had all along (it has never served traffic) while the proxy, /health and
/festivals.json deploy again. Provisioning D1 stays a one-time maintainer
step, now documented as uncomment-and-paste.

Nothing caught this: the vitest pool and `wrangler dev` use a simulated
local D1 that ignores the id, and `wrangler deploy --dry-run` — CI's
`validate-worker` job — exits 0 on a dangling binding (verified against
wrangler 4.98.0). The PR that broke deploys had a green CI run. So:

- move the test D1 binding to `vitest.config.js`, decoupling the suite
  from the deployed binding set
- add `test/wrangler-config.test.js`, which fails on a placeholder id and
  on a D1 binding reappearing without the degraded-mode tests being
  updated (verified red against the exact config that broke production)
- add tests asserting every /v1alpha route returns 503
  STORAGE_UNCONFIGURED and that /health and /festivals.json still work
  with no storage — the configuration actually being deployed

Docs corrected across the worker README and seven skills, which described
the placeholder as deploy-neutral and named `--dry-run` as the check that
would catch this.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E7Rma9m9unUYXYgpon6Rph
The grouped worker update bumps TypeScript to 7.x, but
openapi-typescript@7.13.0 — still the latest release — peer-requires
`typescript: ^5.x`. The resulting tree cannot resolve, so `npm ci` fails
and the PR has been red every week since 2026-06-15 (#440). `main` itself
is fine: cloudflare-worker/package.json pins typescript ^5.9.3. Only
dependabot's own tree is broken.

Verified by resolving both trees directly:

- with typescript ^7.0.2: ERESOLVE, "peer typescript@^5.x from
  openapi-typescript@7.13.0"
- with typescript held at ^5.9.3 and the group's other four bumps applied
  (@cloudflare/vitest-pool-workers 0.20.2, @cloudflare/workers-types
  5.20260804.1, vitest 4.1.10, wrangler 4.119.0): resolves cleanly

So ignoring major TypeScript bumps unwedges the group rather than merely
shrinking it. Remove the ignore once openapi-typescript supports
TypeScript 7.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E7Rma9m9unUYXYgpon6Rph

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR restores Cloudflare Worker deployability by removing an invalid/unprovisioned D1 binding from the deployed wrangler.toml, while keeping the review API in an explicit degraded mode (503 STORAGE_UNCONFIGURED). It also adds test/config guards and Dependabot configuration to prevent recurrence of the two CI/deploy failures described in the PR.

Changes:

  • Disable the production D1 binding in wrangler.toml and move the test-only D1 binding into vitest.config.js.
  • Add regression tests to prevent placeholder resource IDs from reappearing and to assert /v1alpha degraded-mode behavior when storage is absent.
  • Hold Dependabot’s worker TypeScript updates at 5.x to avoid peer-dependency resolution failures.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
cloudflare-worker/wrangler.toml Comments out the D1 binding and documents why placeholder IDs break deploys.
cloudflare-worker/vitest.config.js Declares a simulated D1 binding for tests (decoupled from production deploy config).
cloudflare-worker/test/wrangler-config.test.js Adds an offline guard to catch placeholder/unprovisioned resource IDs and D1-binding mode drift.
cloudflare-worker/test/reviews.test.js Adds degraded-mode tests asserting 503 for /v1alpha when RATINGS_DB is absent, while proxy endpoints still work.
cloudflare-worker/README.md Updates operational docs to reflect “D1 unprovisioned / binding commented out” reality and the new guard test.
.github/dependabot.yml Adds an ignore rule to prevent TypeScript major bumps in the worker group.
.claude/skills/validation-and-qa/SKILL.md Updates guidance to reflect test-vs-deploy binding decoupling and limitations of --dry-run.
.claude/skills/run-and-operate/SKILL.md Updates the provisioning/deploy runbook to reflect the binding being commented out and adds verification notes.
.claude/skills/reference/SKILL.md Updates repo reference docs to reflect current production behavior (503 on /v1alpha until D1 is provisioned).
.claude/skills/my-festival-campaign/SKILL.md Updates campaign notes for the new “binding commented out” state and provisioning steps.
.claude/skills/debugging-playbook/SKILL.md Updates symptom→cause mappings for 503 and deploy error 10181 with the new guard.
.claude/skills/build-and-env/SKILL.md Updates environment/binding documentation for RATINGS_DB and test-only binding behavior.
.claude/skills/architecture-contract/SKILL.md Updates architecture notes to reflect D1 unprovisioned state (binding commented out).
.claude/skills/api-contract/SKILL.md Updates API-contract notes describing the production 503 degraded mode due to missing binding.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/dependabot.yml
Comment on lines +64 to +67
ignore:
- dependency-name: typescript
update-types:
- version-update:semver-major

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipping this one — the premise is incorrect, so the suggested change isn't needed.

YAML only forms a mapping when the colon is followed by a space (or ends the line). version-update:semver-major has no space after the colon, so it is a plain scalar string, not a {version-update: null} map.

Verified by parsing the file as committed:

$ python3 -c "import yaml; d=yaml.safe_load(open('.github/dependabot.yml')); ..."
parsed value : ['version-update:semver-major']
element type : str

A list of one str, which is exactly what Dependabot expects. This is also the unquoted form GitHub's own dependabot.yml documentation uses for update-types.

Quoting it would be harmless, but it isn't a correctness fix, and the rule will apply as written.


Generated by Claude Code

.map((line) => line.trim())
.filter((line) => line.length > 0 && !line.startsWith("#"));

const PLACEHOLDER_ID = /^(0[0-9a-f-]*0|<.*>|)$/i;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — this was a real bug, fixed in f7463c3.

0[0-9a-f-]*0 rejects any id that starts and ends with 0, which is about 1 in 256 random UUIDs. The failure would have landed on whoever provisions D1 for the first time — the worst possible moment for a spurious error, and in the exact workflow the guard exists to support.

Narrowed to ids that genuinely name nothing:

/^(?:[0-]*|<.*>)$/

which covers empty, the nil UUID in any zeros-and-dashes spelling, and unfilled <...> template text. A real UUID cannot be all zeros and dashes — that spelling is the placeholder.

Also extracted it as isPlaceholderId and table-tested it, so the predicate's behaviour is covered directly rather than only via its effect on the current wrangler.toml.

Verified end to end against the real file:

  • 00000000-0000-0000-0000-000000000000 → still fails the check (the config that broke production)
  • 0f9e8d7c-6b5a-4321-8f0e-1d2c3b4a5960 (starts and ends with 0) → now passes, and trips only the degraded-mode-drift test, which is the intended signal that D1 has been provisioned

Generated by Claude Code

The guard matched `/^(0[0-9a-f-]*0|<.*>|)$/i`, which rejects any id that
starts and ends with `0` — roughly 1 in 256 random UUIDs. A real
`database_id` of that shape would have failed the check, and the error
would have landed on whoever was provisioning D1 for the first time: the
worst possible moment for a spurious failure, in the exact workflow this
guard exists to support.

Narrow it to ids that genuinely name nothing: empty, the nil UUID (any
zeros-and-dashes spelling), or unfilled `<...>` template text. Extract the
predicate as `isPlaceholderId` and table-test it, so the guard's own
behaviour is covered rather than only its effect on the current file.

Verified end to end against wrangler.toml: the nil UUID still fails the
check, while `0f9e8d7c-6b5a-4321-8f0e-1d2c3b4a5960` (starts and ends with
`0`) now passes it and trips only the degraded-mode-drift test, which is
the intended signal that D1 has been provisioned.

Raised by Copilot review on #542.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E7Rma9m9unUYXYgpon6Rph
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants