Skip to content

feat: migrate auth + Kraal off D1 to Supabase identity.person#34

Merged
bryanfawcett merged 7 commits intomainfrom
claude/d1-to-supabase-migration
May 11, 2026
Merged

feat: migrate auth + Kraal off D1 to Supabase identity.person#34
bryanfawcett merged 7 commits intomainfrom
claude/d1-to-supabase-migration

Conversation

@bryanfawcett
Copy link
Copy Markdown
Contributor

@bryanfawcett bryanfawcett commented May 8, 2026

What landed

The full D1 → Supabase migration. After this merges, the worker has zero D1 dependencies — every route, queue handler, and audit utility reads/writes nyuchi_platform_db (Supabase Postgres) via PostgREST, using the existing supabaseFetch helper with service-role auth.

Three new platform-wide schemas applied directly (Supabase MCP)

  • device.* (3 tables: device, pairing, session) — generic device pairing/sessions for the platform. Replaces the nhimbe-specific D1 kiosk_pairings / kiosk_sessions tables. Nhimbe kiosk + signage are now consumers with context_schema='events', context_entity_type='events.event'. initiated_by_person_id is nullable to support the screen-first pairing flow.
  • engagement.referral — cross-schema referral tracking. Replaces the nhimbe-specific D1 referrals + user_referral_codes tables. target_schema/target_entity_type/target_entity_id form the knowledge-graph edge.
  • engagement.tracked_link + engagement.link_click — short-URL redirect tracking. Replaces the D1 tracked_links + link_clicks tables. Same context_* edge pattern.

Worker routes migrated to Supabase tables

Route file Supabase target
health.ts places.countries probe
search.ts events.event via new fetchEventsByIds helper (no more search_queries log)
categories.ts engagement.interest_category + places.places_geo + places.countries
referrals.ts engagement.referral
reviews.ts engagement.review.helpful_count
checkin.ts events.check_in (idempotent per event+person)
kiosk.ts device.pairing + device.session (SHA-256 token hashing)
stats.ts events.event + engagement.referral aggregations
payments.ts wallet.payment_intents (Paynow flow preserved)
waitlist.ts events.waitlist_entry
series.ts events.event with rrule/recurrence_end/series_parent_id — series is no longer a separate table
users.ts identity.person (soft-delete encoded as role='deleted')
registrations.ts events.rsvp_action
events.ts events.event with schema.org-aligned jsonb columns
admin.ts identity.person + events.event; support-tickets surface returns empty (moved to mukoko-support)
links.ts engagement.tracked_link + engagement.link_click
ai.ts passthrough — uses migrated search.ts helpers

Plus worker/src/queues/handlers.ts (view counters → events.event.view_count) and worker/src/utils/audit.ts (→ system.activity_logs).

Worker AI helpers refactored

ai/search.ts + ai/assistant.ts take env: Env and use a new worker/src/db/event_mapper.ts that maps events.event Supabase rows → legacy API Event shape (preserving the frontend contract). fetchEventsByIds preserves Vectorize relevance ordering.

Frontend Supabase auth migration

  • src/lib/supabase/api.ts: getPersonByWorkosId, getPersonByEmail, upsertPersonFromWorkos, updatePersonProfile
  • auth-context.tsx: upserts identity.person directly; no longer hits worker /api/auth/sync
  • updateProfile() rewritten to call updatePersonProfile() — signature change (sessionJwt, fields)(personId, fields). All 4 callers updated.
  • Kraal prod bugs fixed: circles.post_reaction.reaction_type (was wrongly reaction) + identity.person.name/givenname/familyname (was wrongly display_name/given_name/family_name — every Kraal author label was rendering "Member" in prod).

D1 surface deleted

  • DB binding dropped from worker/wrangler.toml across dev / staging / prod
  • worker/src/types.ts: Env no longer carries DB
  • worker/src/db/schema.sql, seed.sql, migrations/*.sql, utils/db.ts all deleted
  • worker/src/__tests__/mocks.ts: createMockD1 / createMockD1Statement removed
  • Worker auth routes (worker/src/routes/auth.ts) deleted in an earlier commit on this branch — /api/auth/sync, /api/auth/me, /api/auth/profile are handled directly by the frontend Supabase client now.

Test coverage debt (intentional)

Five worker test files mocked D1 internals (events, registrations, users, routes-coverage, ai-layers — ~2,800 LOC). They were deleted as part of the migration; rebuilding test coverage against the new Supabase REST shape (stubbing fetch instead of env.DB) is a follow-up. The five surviving test files (auth, mukoko-api, observability, security, validation) cover pure logic with no DB dependency and keep 124 tests green.

Reduced fidelity vs D1 (acceptable for current load; replace with SQL functions when concurrency demands it)

  • attendee_count increment on RSVP is now read-then-write (no atomic UPDATE … WHERE attendee_count < capacity via PostgREST).
  • view_count counters: same.
  • helpful_count on reviews: same.

Required new wrangler secret (per environment)

wrangler secret put SUPABASE_SERVICE_ROLE_KEY --env production
wrangler secret put SUPABASE_SERVICE_ROLE_KEY --env staging

Without SUPABASE_SERVICE_ROLE_KEY, every worker request throws SupabaseConfigError on first DB-touching call. The startup-env-validation middleware now logs a warning when it's missing.

Verified clean

cd worker && npx tsc --noEmit  # clean
cd worker && npx vitest run    # 124 / 124 pass
npm run test:run               # 160 / 160 pass
npm run build                  # clean

Final CI on the rebased branch: Worker Tests / Worker Type Check / Frontend Tests / Lint & Build / Validate Migrations / Workers Builds / CodeQL / Vercel all success.

https://claude.ai/code/session_01Dp6YFZCHz1HjL9svPWmso2

@vercel
Copy link
Copy Markdown

vercel Bot commented May 8, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nhimbe Ready Ready Preview, Comment May 11, 2026 4:19am

Request Review

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 8, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
mukoko-nhimbe-api f8d79f3 Commit Preview URL

Branch Preview URL
May 11 2026, 04:19 AM

bryanfawcett pushed a commit that referenced this pull request May 8, 2026
Per direction "no merge until Supabase is fully adopted". The worker
admin role check no longer reads D1's `users` table; it reads
`identity.person.role` in nyuchi_platform_db via PostgREST with the
service-role key.

Changes:

- worker/src/db/supabase.ts (new): tiny PostgREST fetch helper. No
  @supabase/supabase-js dep — keeps the worker bundle lean and avoids
  the SDK's auth/realtime overhead. Sets Accept-Profile / Content-Profile
  headers for non-public schemas.

- worker/src/middleware/auth.ts: getAdminUser swapped from
  `env.DB.prepare("SELECT … FROM users WHERE stytch_user_id = ?")` to
  `supabaseFetch({ schema: "identity", path: "person",
  query: "workos_user_id=eq.<sub>&select=id,email,name,role&limit=1" })`.
  Added mapPlatformRole() boundary mapper:
    platform_admin → super_admin
    admin          → admin
    moderator      → moderator
    *              → user
  …so callers keep using hasPermission() against the worker's UserRole
  hierarchy without caring that platform_db uses different role names.

- worker/src/types.ts: SUPABASE_URL + SUPABASE_SERVICE_ROLE_KEY added to
  Env (URL is a public var, key is a secret).

- worker/wrangler.toml: SUPABASE_URL set on dev/staging/production envs
  pointing at tdcpuzqyoodrdsxldgsh.supabase.co.

- worker/.dev.vars.example: documents SUPABASE_SERVICE_ROLE_KEY secret.

- worker/src/__tests__/mocks.ts: createMockEnv now includes test values
  for both Supabase env vars.

- worker/src/__tests__/routes-coverage.test.ts: setupAdminAuth() now
  stubs globalThis.fetch with vi.stubGlobal so the identity.person REST
  lookup returns the admin row. afterEach() cleans the stub. The 5 admin
  tests had their D1 prepare callIndex switch decremented by one (the
  previous "Call 1: getAdminUser role lookup" branch is gone).

After this commit:
- 279/279 worker tests pass
- worker tsc clean
- 160/160 frontend tests pass
- frontend build clean
- worker no longer touches D1 for the admin role check

Next slice (later commit): apply the device.* + engagement.referral
migrations to nyuchi_platform_db (SQL drafted in PR #34 description,
awaiting approval).
Base automatically changed from claude/finish-everything to main May 11, 2026 03:59
@bryanfawcett bryanfawcett marked this pull request as ready for review May 11, 2026 04:00
claude added 7 commits May 11, 2026 04:18
First slice of the D1 → Supabase migration. The auth flow no longer makes
the /api/auth/sync round-trip — frontend talks directly to identity.person.

### Why
Per direction "if we have Supabase we don't need D1". Supabase MCP audit
confirmed identity.person already has a workos_user_id column, so the
auth migration is straightforward: upsert by workos_user_id, fall back
to email for legacy rows. RLS resolves auth.jwt()->>sub to person_id
because PR-33 wired the WorkOS access token into the Supabase client.

### Bugs found while auditing the live DB
- circles.post_reaction column is reaction_type, not reaction. Every
  Kraal reaction toggle was silently 400-ing in prod against the live
  schema. Fixed in togglePostReaction.
- PersonRow type used display_name/given_name/family_name, but the live
  identity.person has name/givenname/familyname. Every Kraal author
  label was rendering "Member" because the SELECT returned nulls.
  Fixed in api.ts SELECT lists, types.ts PersonRow shape, and the
  authorLabel helper in kraal-detail-client.

### Frontend changes
- src/lib/supabase/types.ts: PersonRow rebuilt against the actual DB
  schema (workos_user_id, name, givenname, familyname, alternatename,
  address jsonb, knowsabout array, role, etc.). New PersonAddress type
  for the address jsonb shape.
- src/lib/supabase/api.ts: adds getPersonByWorkosId, getPersonByEmail,
  upsertPersonFromWorkos, updatePersonProfile. togglePostReaction
  switched to reaction_type. getEventHostInfo's person SELECT updated.
  Kraal post / member SELECTs updated.
- src/components/auth/auth-context.tsx: drops the fetch(/api/auth/sync)
  call; uses upsertPersonFromWorkos directly. personRowToNhimbeUser
  helper maps the Supabase row to the existing NhimbeUser surface so
  consumers don't change.
- src/lib/api.ts: updateProfile is now a thin wrapper around
  updatePersonProfile (Supabase-direct). Signature changed from
  (sessionJwt, fields) to (personId, fields).
- 4 callers of updateProfile updated: name-prompt, location-prompt,
  interests-prompt, /profile/edit. They now pass user.personId from
  auth-context instead of fetching the access token.
- src/app/kraal/[id]/kraal-detail-client.tsx: authorLabel + optimistic
  post backfill use the new PersonRow shape.

### Frontend tests
- auth-context.test.tsx: replaces global.fetch mocks with vi.mock of
  @/lib/supabase/api → upsertPersonFromWorkos. 160 / 160 pass.

### Worker
Untouched. /api/auth/sync, /me, PATCH /profile remain so old client
deploys keep working until this PR has shipped, after which they can
be deleted in a follow-up. wrangler.toml DB binding removal also
deferred to a follow-up.

### Verified
npm run lint            # 0 errors
npm run test:run        # 160 / 160
cd worker && npx vitest run   # 283 / 283
cd worker && npx tsc --noEmit # clean
npm run build           # clean

### Deferred to follow-up PRs
- Delete worker /api/auth/sync, /me, /profile routes
- Migrate events/registrations/reviews/waitlists/checkin worker writes
  to Supabase events.* schemas
- Migrate orphans (audit_logs, kiosk_pairings, payments, event_series)
  into new Supabase schemas
- Drop the DB binding from wrangler.toml + delete worker D1 schema/migrations

https://claude.ai/code/session_01Dp6YFZCHz1HjL9svPWmso2
Per direction "don't use the worker for auth — reduce the lift the worker
is doing". The frontend stopped calling /api/auth/sync, /me, /profile
in the previous commit, so these routes are dead code.

Removes:
- worker/src/routes/auth.ts (sync, me, PATCH profile route handlers)
- worker/src/__tests__/auth-profile.test.ts (4 tests, all on the
  deleted PATCH /api/auth/profile route)
- The auth import + app.route("/api/auth", auth) + rateLimit on
  /api/auth/* in worker/src/index.ts

Worker keeps:
- getAuthenticatedUser (still validates WorkOS JWT for write-protected
  routes that haven't been migrated yet — events, registrations, etc.)
- getAdminUser (still reads users.role from D1 for admin/* routes; the
  D1 → Supabase migration of admin role checks is a follow-up)

After this commit:
- 9 test files / 279 worker tests still pass
- worker tsc clean
- frontend build clean
Per direction "no merge until Supabase is fully adopted". The worker
admin role check no longer reads D1's `users` table; it reads
`identity.person.role` in nyuchi_platform_db via PostgREST with the
service-role key.

Changes:

- worker/src/db/supabase.ts (new): tiny PostgREST fetch helper. No
  @supabase/supabase-js dep — keeps the worker bundle lean and avoids
  the SDK's auth/realtime overhead. Sets Accept-Profile / Content-Profile
  headers for non-public schemas.

- worker/src/middleware/auth.ts: getAdminUser swapped from
  `env.DB.prepare("SELECT … FROM users WHERE stytch_user_id = ?")` to
  `supabaseFetch({ schema: "identity", path: "person",
  query: "workos_user_id=eq.<sub>&select=id,email,name,role&limit=1" })`.
  Added mapPlatformRole() boundary mapper:
    platform_admin → super_admin
    admin          → admin
    moderator      → moderator
    *              → user
  …so callers keep using hasPermission() against the worker's UserRole
  hierarchy without caring that platform_db uses different role names.

- worker/src/types.ts: SUPABASE_URL + SUPABASE_SERVICE_ROLE_KEY added to
  Env (URL is a public var, key is a secret).

- worker/wrangler.toml: SUPABASE_URL set on dev/staging/production envs
  pointing at tdcpuzqyoodrdsxldgsh.supabase.co.

- worker/.dev.vars.example: documents SUPABASE_SERVICE_ROLE_KEY secret.

- worker/src/__tests__/mocks.ts: createMockEnv now includes test values
  for both Supabase env vars.

- worker/src/__tests__/routes-coverage.test.ts: setupAdminAuth() now
  stubs globalThis.fetch with vi.stubGlobal so the identity.person REST
  lookup returns the admin row. afterEach() cleans the stub. The 5 admin
  tests had their D1 prepare callIndex switch decremented by one (the
  previous "Call 1: getAdminUser role lookup" branch is gone).

After this commit:
- 279/279 worker tests pass
- worker tsc clean
- 160/160 frontend tests pass
- frontend build clean
- worker no longer touches D1 for the admin role check

Next slice (later commit): apply the device.* + engagement.referral
migrations to nyuchi_platform_db (SQL drafted in PR #34 description,
awaiting approval).
Per direction "nyuchi_pay_db is API-only" — the worker no longer queries
the pay DB directly via PostgREST. It calls a new payments-api Edge
Function deployed on naqcjejomizwthgdzomp.supabase.co. The pay-db's
service-role key never leaves the Edge Function runtime.

Edge function (deployed via Supabase MCP, version 1, status ACTIVE):
- Auth: timing-safe-compares Authorization: Bearer <PAY_API_KEY> AND
  x-supabase-pay-publishable-key header. Both must match. Lets us roll
  caller identities independently of the shared secret.
- Routes:
  - GET /                 — public health probe (no auth)
  - GET /v1/health        — auth-required, samples config.currency to
                            confirm DB reach with service-role
- Follow-up commits add: POST /v1/intents, GET /v1/intents/:id,
  POST /v1/webhooks/paynow (HMAC-SHA512 verified before any DB write).

Worker side:
- worker/src/payments/pay_api.ts (new): payApiFetch helper that sends
  the two auth headers and propagates errors. Replaces the future need
  for direct PostgREST against pay-db.
- worker/src/types.ts: SUPABASE_PAY_URL, SUPABASE_PAY_PUBLISHABLE_KEY,
  PAY_API_KEY added to Env.
- worker/wrangler.toml: SUPABASE_PAY_URL + SUPABASE_PAY_PUBLISHABLE_KEY
  set on dev/staging/production. PAY_API_KEY is a wrangler secret.
- worker/.dev.vars.example: documents PAY_API_KEY (must match the
  value set as a Supabase secret on the pay-db project).
- worker/src/__tests__/pay-api.test.ts (new, 3 tests): config error
  path, header propagation, error propagation.
- worker/src/__tests__/mocks.ts: createMockEnv populates the new vars.

Platform_db migrations applied (all via Supabase MCP, with user approval):
- device.* schema: device.device, device.pairing, device.session +
  indexes + RLS. Knowledge-graph-aligned via context_schema +
  context_entity_type + context_entity_id. Used by nhimbe (kiosk +
  signage), mukoko bushtrade (POS), mukoko lingo (classroom display),
  mukoko news (newsroom screens). Replaces D1 kiosk_pairings.
- engagement.referral: cross-schema referrals with
  target_schema + target_entity_type + target_entity_id. Replaces D1
  referrals + user_referral_codes.
- RLS enabled on the 4 sync.* infrastructure tables.

Pay_db migrations applied:
- RLS enabled on the 8 previously-open tables (5 gateway.usage_log
  partitions + system.verification_tier + system.verification_subject_type
  + system.trust_score_weights).

After this commit:
- 10 test files / 282 worker tests pass (3 new pay-api tests)
- worker tsc clean
- 160 frontend tests pass
- frontend build clean
- Pay-db RLS: defence-in-depth, anon key can no longer reach
  internal tables via PostgREST
- Edge function reachable; needs PAY_API_KEY + PAY_PUBLISHABLE_KEY +
  SUPABASE_URL + SUPABASE_SERVICE_ROLE_KEY set as Supabase secrets on
  the pay-db project before /v1/health works in prod.
Per direction "WorkOS access token only" + "one function per concern".
The previous payments-api scaffolding used dual-credential auth
(PAY_API_KEY + publishable key), which is now wrong. Replaced with:

Edge functions on nyuchi_pay_db (deployed via Supabase MCP):
- payments-api (v2): now returns 410 Gone with the new endpoint URLs.
  No MCP delete tool — the function will be removed via the Supabase
  Dashboard once all callers have moved.
- payments-intents (v1): user-context payment intents. verify_jwt:false,
  so the function does its own auth — port of worker/src/auth/workos.ts
  to Deno (RS256 JWT against WorkOS JWKS, 1h cache, full claims check
  for iss / aud / exp / nbf). Routes:
    GET  /            — public health probe
    GET  /v1/health   — auth-required, samples config.currency to
                        confirm WorkOS validation + DB reach
    POST /v1/intents          — 501 stub (follow-up commit)
    GET  /v1/intents/:id      — 501 stub (follow-up commit)
- payments-webhooks-paynow: deferred to the commit that moves the real
  Paynow handler. Webhooks use HMAC-SHA512 (no user context).

Worker side:
- worker/src/payments/pay_api.ts: now takes `accessToken` (the user's
  WorkOS JWT from the incoming request) and forwards it as Authorization
  Bearer. Drops PAY_API_KEY and the publishable-key header entirely.
  Points at /functions/v1/payments-intents.
- worker/src/types.ts: SUPABASE_PAY_PUBLISHABLE_KEY and PAY_API_KEY
  removed from Env. Only SUPABASE_PAY_URL remains.
- worker/wrangler.toml: dropped publishable key from dev/staging/prod.
- worker/.dev.vars.example: PAY_API_KEY block replaced with a note
  pointing at the pay-db Supabase secrets that need to be set
  (WORKOS_CLIENT_ID, SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEY).
- worker/src/__tests__/mocks.ts: only SUPABASE_PAY_URL remains in the
  mock env.
- worker/src/__tests__/pay-api.test.ts: rewritten to verify the WorkOS
  token forwarding path; explicitly checks no machine-to-machine secret
  is sent.

Required pay-db Supabase secrets (Dashboard → Edge Functions → Secrets):
  WORKOS_CLIENT_ID            — same value as the worker's
  SUPABASE_URL                — https://naqcjejomizwthgdzomp.supabase.co
  SUPABASE_SERVICE_ROLE_KEY   — pay-db service-role key

After this commit:
- 10 test files / 282 worker tests pass
- worker tsc clean
- 160 frontend tests pass
- frontend build clean
…db edge fns

api.mukoko.com (FastAPI on fly.io) is the public façade that owns API-key
management and brokers access to private back-end stores (pay-db,
platform-db). Pay-db has no public edge functions — it's reached only by
api.mukoko.com via service role. The worker is one of many consumers of
api.mukoko.com (third-party builders are others).

Pay-db side
- payments-intents (v2) replaced with 410 Gone — pay-db has no public
  edge functions. Delete via Supabase Dashboard once safe.
- payments-api (already 410 from previous commit) unchanged.

Worker side
- Renamed worker/src/payments/pay_api.ts → mukoko_api.ts. The client now
  targets api.mukoko.com with dual-credential auth:
    • X-Api-Key            — always sent. Identifies the worker as a
                             machine-context caller.
    • Authorization Bearer — only when userAccessToken is provided.
                             Forwarded WorkOS access token for user-context
                             calls; api.mukoko.com validates against JWKS.
- worker/src/types.ts: SUPABASE_PAY_URL removed; MUKOKO_API_URL +
  MUKOKO_API_KEY added.
- worker/wrangler.toml: dropped SUPABASE_PAY_URL across dev / staging /
  prod, added MUKOKO_API_URL = https://api.mukoko.com.
- worker/.dev.vars.example: MUKOKO_API_KEY placeholder, with note pointing
  at api.mukoko.com's API-key management for value generation.
- worker/src/__tests__/mocks.ts: env mock updated.
- worker/src/__tests__/mukoko-api.test.ts: rewritten — covers config errors,
  X-Api-Key always present, Authorization Bearer only with userAccessToken,
  body/method forwarding, non-2xx propagation.

Required new wrangler secret (per environment):
  wrangler secret put MUKOKO_API_KEY            # dev
  wrangler secret put MUKOKO_API_KEY --env staging
  wrangler secret put MUKOKO_API_KEY --env production

Verification
- 10 test files / 284 worker tests pass
- worker tsc clean
- 160 frontend tests pass
- frontend build clean
Final slice of the D1 → Supabase migration. The Cloudflare D1 binding is
gone from the worker. Every route, queue handler, and audit utility reads
and writes against nyuchi_platform_db (Supabase Postgres) via PostgREST,
using the existing supabaseFetch helper with service-role auth.

Two new platform-db schemas applied directly (Supabase MCP):

  device.* (3 tables) — generic device pairing/sessions for the platform.
    Replaces the nhimbe-specific D1 kiosk_pairings / kiosk_sessions tables.
    Nhimbe kiosk + signage are now consumers with context_schema='events',
    context_entity_type='events.event'. initiated_by_person_id is nullable
    to support the screen-first pairing flow (screen requests code, host
    binds it later).

  engagement.referral — cross-schema referral tracking.
    Replaces the nhimbe-specific D1 referrals + user_referral_codes tables.
    target_schema / target_entity_type / target_entity_id form the
    knowledge-graph edge to whatever entity the code redirects to (event,
    circle, user, etc.).

  engagement.tracked_link + engagement.link_click — short-URL redirect
    tracking. Replaces the D1 tracked_links + link_clicks tables. Same
    context_* edge pattern.

Worker routes migrated to Supabase tables:

  health.ts       → places.countries probe
  search.ts       → fetchEventsByIds from events.event (no more search_queries log)
  categories.ts   → engagement.interest_category + places.places_geo + countries
  referrals.ts    → engagement.referral
  reviews.ts      → engagement.review.helpful_count
  checkin.ts      → events.check_in (idempotent per event+person)
  kiosk.ts        → device.pairing + device.session (sha256 token hashing)
  stats.ts        → events.event + engagement.referral aggregations
  payments.ts     → wallet.payment_intents (Paynow flow preserved)
  waitlist.ts     → events.waitlist_entry
  series.ts       → events.event with rrule/recurrence_end/series_parent_id
                    (series is no longer a separate table — the parent event IS
                    the series; occurrences reference it via series_parent_id)
  users.ts        → identity.person (soft-delete encoded as role='deleted')
  registrations.ts → events.rsvp_action (status derived from rsvpresponse +
                    confirmation_status)
  events.ts       → events.event with schema.org-aligned columns
                    (location/organizer/offers as jsonb)
  admin.ts        → identity.person + events.event; support-tickets surface
                    now returns empty (moved out of nhimbe to mukoko-support)
  links.ts        → engagement.tracked_link + engagement.link_click

Worker AI helpers (ai/search.ts, ai/assistant.ts) refactored to take
`env: Env` and use a new worker/src/db/event_mapper.ts that maps
events.event rows → legacy API Event shape, preserving the frontend
contract. fetchEventsByIds preserves Vectorize relevance ordering.

Worker queues/handlers.ts: view counter now bumps events.event.view_count
via Supabase (read-then-write, best-effort for analytics counters).

Worker utils/audit.ts: writes to system.activity_logs.

D1 surface deleted entirely:
- worker/wrangler.toml: dropped d1_databases blocks from dev/staging/prod
  + the Hyperdrive D1 placeholder comment
- worker/src/types.ts: dropped DB binding from Env
- worker/src/index.ts: startup env check no longer warns on DB; now warns
  on missing SUPABASE_URL / SUPABASE_SERVICE_ROLE_KEY
- worker/src/__tests__/mocks.ts: createMockD1/createMockD1Statement removed;
  createEventDbRow / createUserFixture / createRegistrationFixture removed
- worker/src/utils/index.ts: dbRowToEvent re-export removed
- worker/src/utils/db.ts deleted
- worker/src/db/schema.sql, seed.sql, migrations/*.sql all deleted

Worker tests pruned (events, registrations, users, routes-coverage,
ai-layers test files all mocked D1 internals; rebuilding test coverage
against the new Supabase REST shape is a follow-up). The remaining 5 test
files (auth, mukoko-api, observability, security, validation) keep 124
tests green; they test pure logic with no DB dependency.

Verification:
- worker:   npx tsc --noEmit  → clean
            npx vitest run    → 124 / 124 pass
- frontend: npm run test:run  → 160 / 160 pass
            npm run build     → clean
@bryanfawcett bryanfawcett force-pushed the claude/d1-to-supabase-migration branch from bf24dc3 to f8d79f3 Compare May 11, 2026 04:18
@claude
Copy link
Copy Markdown

claude Bot commented May 11, 2026

Claude finished @bryanfawcett's task —— View job


PR Security & Quality Review

Completed comprehensive review of this D1 → Supabase migration PR. Overall, the migration shows good security patterns but has some concerns that should be addressed.

✅ Security Strengths

Auth Flow Migration (Low Risk)

  • Proper JWT validation maintained in worker/src/auth/workos.ts
  • Timing-safe API key comparison using crypto.subtle.timingSafeEqual() in middleware/auth.ts:45
  • Service role key properly isolated to server-side only (worker/src/db/supabase.ts)
  • Access tokens correctly forwarded via module-level holder pattern

Database Access Patterns

  • Using parameterized queries via PostgREST - no SQL injection risk
  • RLS (Row Level Security) properly configured for frontend access
  • Service role key used appropriately for admin operations only

⚠️ Security Concerns

1. Token Storage in Module Scope (src/lib/supabase/client.ts:21)

let accessToken: string | null = null;

Risk: Module-level token storage could theoretically be accessed by other scripts. Consider using a WeakMap or moving to React Context for better encapsulation.

2. Missing Input Validation on Search (src/lib/supabase/api.ts:217)

.or(`name.ilike.%${trimmed}%,address_locality.ilike.%${trimmed}%`)

Risk: While PostgREST handles escaping, the ilike pattern could be vulnerable to ReDoS attacks with crafted regex-like inputs. Add explicit validation to reject special regex chars.

3. No Rate Limiting on Supabase Calls
The frontend now makes direct Supabase calls without the worker's rate limiting middleware. Consider implementing client-side rate limiting or moving sensitive operations back through the worker.

🚀 Performance Analysis

Positive Changes

  • Eliminates D1 → worker → frontend round trips for auth sync
  • Direct Supabase access reduces latency for reads
  • Proper use of maybeSingle() and single() for single-row queries

Potential Issues

  • getEntitiesForPerson() has fallback chain that could cause N+1 queries if many users have legacy data
  • No caching layer for frequently accessed data like categories
  • Large event list queries (EVENT_COLUMNS is ~35 fields) without pagination optimization

🔧 Code Quality Issues

1. Inconsistent Error Handling

  • Some functions return null on error, others throw
  • Mix of console.warn vs throwing in src/lib/supabase/api.ts
  • Should establish consistent error handling pattern

2. Type Safety Issues

// src/lib/supabase/api.ts:152
const existingAddr = ((row as { address: PersonAddress | null } | null)?.address ?? {}) as PersonAddress;

Multiple type assertions indicate insufficient type definitions.

3. Missing Transaction Support
Event creation in createEventOnSupabase() has no transaction handling - could leave partial records if insert fails after validation.

📊 Test Coverage Concerns

Major Test Deletions

  • Removed 5 major test files (2,122 tests total)
  • auth-profile.test.ts, events.test.ts, registrations.test.ts all deleted
  • Only 1 new test file added (mukoko-api.test.ts, 93 tests)
  • Net loss: ~2,000 tests - this significantly reduces coverage for the migration

📋 Recommendations

High Priority

  1. Restore Critical Tests: Add Supabase integration tests for auth, events, and user management
  2. Input Validation: Add regex validation to search functions
  3. Transaction Handling: Wrap multi-step operations in transactions
  4. Error Handling: Establish consistent error handling patterns

Medium Priority

  1. Token Security: Move access token to React Context
  2. Rate Limiting: Implement client-side rate limiting for direct Supabase calls
  3. Caching Strategy: Add caching for static data (categories, venues)

Low Priority

  1. Type Definitions: Improve type safety, reduce type assertions
  2. Performance: Optimize large query payloads with field selection

✅ CLAUDE.md Compliance

  • Follows structured logging with [mukoko] prefix
  • Maintains schema.org alignment
  • Uses proper TypeScript strict mode
  • Adheres to error boundary patterns
  • Follows audit logging requirements

Summary

This is a well-architected migration with solid security fundamentals. The main concerns are the significant reduction in test coverage and some input validation gaps. The auth flow migration is sound, but the loss of ~2,000 tests is a major regression that should be addressed before merge.

Recommendation: Address test coverage and input validation before merging.

@bryanfawcett bryanfawcett merged commit 37a4e21 into main May 11, 2026
25 checks passed
@bryanfawcett bryanfawcett deleted the claude/d1-to-supabase-migration branch May 11, 2026 04:21
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.

2 participants