fix: throw on missing env vars in backend createServerSupabase()

[bmersereau]

Summary

• backend/src/lib/supabase.ts now throws Error("SUPABASE_URL and SUPABASE_SECRET_KEY must be set") when either env var is absent, matching the frontend fix from PR fix: remove auth bypass dev fallback in getUserIdFromRequest #73
• Removes dead getUserIdFromRequest export (no callers) which still used the old || "" pattern

Closes #97
Closes #114
Closes #117
Closes #124

Changes

• backend/src/lib/supabase.ts — replace || "" fallbacks with explicit guard; remove dead getUserIdFromRequest export
• backend/src/lib/__tests__/supabaseServer.test.ts — new test: throws on missing URL, missing key, both missing; succeeds when both set
• backend/vitest.config.ts — vitest configuration with include filter and isolate: true; "test" script added to package.json

Test plan

• Unit tests added and passing (4/4)
• Backend build passes

[bmersereau]

PR Review

Summary

Fixes a silent-fail bug in createServerSupabase() where missing env vars produced a broken client with empty-string credentials instead of throwing. The fix is correct, minimal, and well-tested. One pre-existing function in the same file (getUserIdFromRequest) still uses the old || "" pattern, but that function is currently dead code (no callers) and is out of scope for this PR.

Findings

• [severity:low] getUserIdFromRequest in supabase.ts (lines 29-30) still uses the old || "" fallback pattern and creates a new createClient on every invocation — both issues this PR fixed for createServerSupabase. The function has no callers today, but should be cleaned up before it's wired to a route.
• [severity:nit] Tests use dynamic import() inside each it block with isolate: true in vitest config. This works, but vi.resetModules() in a beforeEach would make the isolation intent explicit and guard against accidental module-cache sharing if isolate is ever removed.

Verdict

Approve with nits

What I verified

• Tests: pass (4/4)
• vitest.config.ts has include filter: pass ("src/**/__tests__/**/*.test.ts")
• package.json has "test": "vitest run": pass
• createServerSupabase throws on missing URL, missing key, both missing, and returns client when both set: pass

[bmersereau]

PR Review

Summary

Replaces the silent || "" fallbacks in createServerSupabase() with an explicit guard that throws when either env var is absent. Four unit tests cover all combinations of missing/present env vars. Simple, correct, well-tested change.

Findings

• [severity:praise] Error message "SUPABASE_URL and SUPABASE_SECRET_KEY must be set" is clear and actionable
• [severity:praise] persistSession: false already present — service-role client correctly stateless
• [severity:nit] Tests import via dynamic await import("../supabase.js") without module reset between tests — env var deletions in beforeEach may not fully isolate if the module is cached. Tests still pass because vitest's isolate: true in vitest.config.ts handles this

Specific checks

• "test": "vitest run" in package.json ✓
• vitest.config.ts include filter present ✓
• Tests pass: 4/4 ✓
• documents.ts unchanged from origin/main ✓

Verdict

Approve — clean.

[bmersereau]

Follow-up Review (pass 2)

Change

Removed dead getUserIdFromRequest export — function had no callers and still used the old || "" pattern that this PR was eliminating.

Findings

• [severity:praise] File is now clean — only createServerSupabase remains, with the correct guard
• No regressions. Tests still pass 4/4.

Verdict

Approve — all pass-1 findings resolved.