fix: resolve Clerk-Supabase auth synchronization and chat preview#710
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
ngoiyaeric seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR fixes Clerk-to-Supabase user synchronization by adding ChangesClerk auth synchronization
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ClerkWebhook as Webhook route
participant AppDB as App DB (Drizzle)
participant Supabase as Supabase RPC
participant PublicUsers as public.users
ClerkWebhook->>AppDB: lookup/update/create user by clerkUserId or email
ClerkWebhook->>Supabase: call sync_clerk_user(clerkId, email, profile fields)
Supabase->>PublicUsers: link by clerk_user_id or email, or insert new user
PublicUsers-->>Supabase: return user UUID
Supabase-->>ClerkWebhook: RPC result/error (non-blocking)
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cdcfdfd to
7b4ddd8
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/auth/get-current-user.ts (1)
67-79: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winAuthorization bypass: email-matched user with a different Clerk ID returns the wrong DB identity.
When a DB user is found by email but already has a
clerkUserIdset (line 69), the update is skipped, yet line 79 still returns that user's DB id. If Clerk user B shares the same email as Clerk user A (who is already linked), Clerk user B receives Clerk user A's DB user id — granting access to A's chats and data.This is an account-takeover vector via email collision. The fix should verify the existing
clerkUserIdmatches the incoming one before returning the id; otherwise returnnullor create a new record.🔒 Proposed fix
if (existingEmailUser) { - // Link the existing user to this Clerk ID if not already linked - if (!existingEmailUser.clerkUserId) { + // Link the existing user to this Clerk ID if not already linked + if (!existingEmailUser.clerkUserId) { await db.update(users) .set({ clerkUserId, firstName, lastName, avatarUrl, }) .where(eq(users.id, existingEmailUser.id)); + } else if (existingEmailUser.clerkUserId !== clerkUserId) { + // Email matches but is already linked to a different Clerk user — do not grant access + console.warn(`[Auth] Email ${email} is already linked to a different Clerk user.`); + return null; } return existingEmailUser.id; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/auth/get-current-user.ts` around lines 67 - 79, The email-lookup branch in getCurrentUser currently returns existingEmailUser.id even when that row is already linked to a different clerkUserId, which can map a new Clerk identity onto the wrong DB account. Update the existingEmailUser handling so it only returns the id when the stored clerkUserId is either missing and can be linked, or already matches the incoming clerkUserId; otherwise do not reuse that row and return null or create a separate user record. Make the check and return behavior consistent around the db.update(users) and return existingEmailUser.id flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/clerk/webhook/route.ts`:
- Around line 75-82: The update branch in the Clerk webhook handler is
unconditionally writing `email`, which can overwrite a stored value with `null`
when `user.updated` has no primary email. Update the `db.update(users).set(...)`
logic in the webhook route so `email` is only included when a real value exists,
just like `firstName`, `lastName`, and `avatarUrl`, keeping the existing
identity needed by the email-linking flow and `users.clerkUserId` update path.
In `@CLERK_SUPABASE_FIX.md`:
- Around line 53-61: Fix the verification summary in the CLERK_SUPABASE_FIX.md
section so the stated component count matches the actual list. Update the
summary text that says “All 6 critical components” to reflect the full set of
items currently enumerated, and keep the wording consistent with the
verification bullets that mention sync_clerk_user, clerk_id, is_clerk_user,
clerk_user_id, on_auth_user_created, users_updated_at, database-access, and the
RLS policies.
In `@drizzle/migrations/0003_clerk_auth_sync.sql`:
- Around line 16-23: The SECURITY DEFINER functions sync_clerk_user and
handle_new_user need a fixed search_path to avoid resolving unqualified objects
from a caller-controlled schema. Update both function definitions to explicitly
pin the search_path to the intended schema(s), keeping the change within the
existing CREATE OR REPLACE FUNCTION blocks for those named functions.
- Around line 11-12: The users table migration uses a table-level unique
constraint and a normal index build, which can block writes during deploy.
Update the migration around the clerk_user_id change to avoid locking by
creating the unique index concurrently first, then attaching it as a constraint
with USING INDEX if needed, and keep the plain index creation non-blocking as
well. Make sure the logic in this migration is split so the CONCURRENTLY step is
not wrapped in a transaction, and reference the clerk_user_id column/index
creation in 0003_clerk_auth_sync.sql.
- Around line 72-76: The clerk_user_id lookup in the INSERT/ON CONFLICT block is
reading from auth.jwt(), which is unreliable in this trigger path; update the
trigger logic to read the Clerk ID from NEW.raw_user_meta_data instead. Make the
change in the users upsert inside the clerk auth sync migration so the AFTER
INSERT trigger uses the row’s metadata directly when populating clerk_user_id,
while keeping the existing conflict update behavior intact.
In `@lib/auth/get-current-user.ts`:
- Around line 45-48: Remove the per-request `syncUserProfile` call from
`getCurrentUserIdOnServer` so existing users return their ID without triggering
a Clerk fetch and DB write on every request. Keep the hot path in
`get-current-user.ts` focused on lookup/return, and rely on the webhook-based
profile sync already present in this PR for updates; if you still need a
fallback, gate `syncUserProfile` behind a clear staleness check using the
existing user record fields before calling it.
In `@lib/db/schema.ts`:
- Around line 29-36: Add a database migration to bring public.users in sync with
the columns introduced in lib/db/schema.ts. Update the users table to include
created_at, updated_at, avatar_url, first_name, last_name, username,
phone_number, and metadata, and make sure the migration order works with
existing scripts like 0003_clerk_auth_sync.sql that already write first_name,
last_name, and avatar_url. Use the users table definition in schema.ts and the
existing migration pattern for users-related changes as the reference point.
---
Outside diff comments:
In `@lib/auth/get-current-user.ts`:
- Around line 67-79: The email-lookup branch in getCurrentUser currently returns
existingEmailUser.id even when that row is already linked to a different
clerkUserId, which can map a new Clerk identity onto the wrong DB account.
Update the existingEmailUser handling so it only returns the id when the stored
clerkUserId is either missing and can be linked, or already matches the incoming
clerkUserId; otherwise do not reuse that row and return null or create a
separate user record. Make the check and return behavior consistent around the
db.update(users) and return existingEmailUser.id flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3ec6c5eb-8692-4b9e-9e1e-f2dd6969d268
📒 Files selected for processing (8)
CLERK_SUPABASE_FIX.mdapp/api/clerk/webhook/route.tsdrizzle/migrations/0003_clerk_auth_sync.sqldrizzle/migrations/meta/_journal.jsonedge_function/deno.jsonedge_function/index.tslib/auth/get-current-user.tslib/db/schema.ts
📜 Review details
🧰 Additional context used
🪛 LanguageTool
CLERK_SUPABASE_FIX.md
[style] ~33-~33: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...m: SELECT auth.jwt() ->> 'sub'. 4. Updated is_clerk_user(user_id) function: Pr...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~33-~33: Consider an alternative adverb to strengthen your wording.
Context: ...ed is_clerk_user(user_id) function**: Properly checks that the user's clerk_user_id ...
(PROPERLY_THOROUGHLY)
[style] ~35-~35: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...s the current session's Clerk ID. 5. Updated all RLS policies on 8 tables: users...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~76-~76: Ensure spelling is correct
Context: ...already live** in QCX-BACKEND (project: mofqapxwyphzjrqegjeq). - The edge function is deployed as vers...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.22.1)
CLERK_SUPABASE_FIX.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 SQLFluff (4.2.2)
drizzle/migrations/0003_clerk_auth_sync.sql
[error] 12-12: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 105-105: Do not use special characters in identifiers.
(RF05)
[error] 106-106: Do not use special characters in identifiers.
(RF05)
[error] 114-114: Do not use special characters in identifiers.
(RF05)
[error] 115-115: Do not use special characters in identifiers.
(RF05)
[error] 122-122: Do not use special characters in identifiers.
(RF05)
[error] 123-123: Do not use special characters in identifiers.
(RF05)
[error] 130-130: Do not use special characters in identifiers.
(RF05)
[error] 131-131: Do not use special characters in identifiers.
(RF05)
[error] 167-167: Do not use special characters in identifiers.
(RF05)
[error] 168-168: Do not use special characters in identifiers.
(RF05)
[error] 176-176: Do not use special characters in identifiers.
(RF05)
[error] 177-177: Do not use special characters in identifiers.
(RF05)
[error] 185-185: Do not use special characters in identifiers.
(RF05)
[error] 186-186: Do not use special characters in identifiers.
(RF05)
[error] 194-194: Do not use special characters in identifiers.
(RF05)
[error] 195-195: Do not use special characters in identifiers.
(RF05)
[error] 203-203: Do not use special characters in identifiers.
(RF05)
[error] 204-204: Do not use special characters in identifiers.
(RF05)
[error] 214-214: Do not use special characters in identifiers.
(RF05)
[error] 215-215: Do not use special characters in identifiers.
(RF05)
[error] 225-225: Do not use special characters in identifiers.
(RF05)
[error] 226-226: Do not use special characters in identifiers.
(RF05)
[error] 236-236: Do not use special characters in identifiers.
(RF05)
[error] 237-237: Do not use special characters in identifiers.
(RF05)
🪛 Squawk (2.59.0)
drizzle/migrations/0003_clerk_auth_sync.sql
[warning] 11-11: Adding a UNIQUE constraint requires an ACCESS EXCLUSIVE lock which blocks reads and writes to the table while the index is built. Create an index CONCURRENTLY and create the constraint using the index.
(disallowed-unique-constraint)
[warning] 12-12: During normal index creation, table updates are blocked, but reads are still allowed. Use concurrently to avoid blocking writes.
(require-concurrent-index-creation)
🔇 Additional comments (5)
drizzle/migrations/meta/_journal.json (1)
25-34: LGTM!edge_function/deno.json (1)
1-5: LGTM!app/api/clerk/webhook/route.ts (1)
108-141: LGTM!edge_function/index.ts (2)
78-83: 🎯 Functional CorrectnessReferenced file is outside this repository
edge_function/index.tsis not present here, andtsconfig.jsonexcludesedge_function, so theerror.messageconcern doesn’t apply in this codebase.> Likely an incorrect or invalid review comment.
15-49: 🔒 Security & PrivacyIncorrect target: Clerk webhook already verifies Svix signatures. The relevant handler is
app/api/clerk/webhook/route.ts; it readsCLERK_WEBHOOK_SECRET, requires thesvix-*headers, and callswh.verify(...)before syncing users.> Likely an incorrect or invalid review comment.
| await db.update(users) | ||
| .set({ | ||
| email, | ||
| ...(firstName && { firstName }), | ||
| ...(lastName && { lastName }), | ||
| ...(avatarUrl && { avatarUrl }), | ||
| }) | ||
| .where(eq(users.clerkUserId, clerkId)); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Avoid overwriting email with null on update.
email is set unconditionally in the update branch, while name/avatar are guarded. If a user.updated event arrives without a primary email, email becomes null, wiping the stored identity and breaking the email-based linking path (Line 87) and unique-identity assumptions. Guard it like the other fields.
Suggested change
await db.update(users)
.set({
- email,
+ ...(email && { email }),
...(firstName && { firstName }),
...(lastName && { lastName }),
...(avatarUrl && { avatarUrl }),
})
.where(eq(users.clerkUserId, clerkId));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await db.update(users) | |
| .set({ | |
| email, | |
| ...(firstName && { firstName }), | |
| ...(lastName && { lastName }), | |
| ...(avatarUrl && { avatarUrl }), | |
| }) | |
| .where(eq(users.clerkUserId, clerkId)); | |
| await db.update(users) | |
| .set({ | |
| ...(email && { email }), | |
| ...(firstName && { firstName }), | |
| ...(lastName && { lastName }), | |
| ...(avatarUrl && { avatarUrl }), | |
| }) | |
| .where(eq(users.clerkUserId, clerkId)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/clerk/webhook/route.ts` around lines 75 - 82, The update branch in
the Clerk webhook handler is unconditionally writing `email`, which can
overwrite a stored value with `null` when `user.updated` has no primary email.
Update the `db.update(users).set(...)` logic in the webhook route so `email` is
only included when a real value exists, just like `firstName`, `lastName`, and
`avatarUrl`, keeping the existing identity needed by the email-linking flow and
`users.clerkUserId` update path.
| All 6 critical components verified as active in QCX-BACKEND: | ||
| - `sync_clerk_user` function: active | ||
| - `clerk_id` function: active | ||
| - `is_clerk_user` function: active | ||
| - `clerk_user_id` column on `users`: present | ||
| - `on_auth_user_created` trigger: active | ||
| - `users_updated_at` trigger: active | ||
| - Edge function `database-access` v13: deployed and ACTIVE | ||
| - RLS policies: 12 policies across 8 tables, all using `is_clerk_user()` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the verification count.
The section says “All 6 critical components,” but the list contains 8 items. That makes the verification summary internally inconsistent and can mislead readers about what was actually checked.
📝 Proposed fix
-All 6 critical components verified as active in QCX-BACKEND:
+All 8 critical components verified as active in QCX-BACKEND:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| All 6 critical components verified as active in QCX-BACKEND: | |
| - `sync_clerk_user` function: active | |
| - `clerk_id` function: active | |
| - `is_clerk_user` function: active | |
| - `clerk_user_id` column on `users`: present | |
| - `on_auth_user_created` trigger: active | |
| - `users_updated_at` trigger: active | |
| - Edge function `database-access` v13: deployed and ACTIVE | |
| - RLS policies: 12 policies across 8 tables, all using `is_clerk_user()` | |
| All 8 critical components verified as active in QCX-BACKEND: | |
| - `sync_clerk_user` function: active | |
| - `clerk_id` function: active | |
| - `is_clerk_user` function: active | |
| - `clerk_user_id` column on `users`: present | |
| - `on_auth_user_created` trigger: active | |
| - `users_updated_at` trigger: active | |
| - Edge function `database-access` v13: deployed and ACTIVE | |
| - RLS policies: 12 policies across 8 tables, all using `is_clerk_user()` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLERK_SUPABASE_FIX.md` around lines 53 - 61, Fix the verification summary in
the CLERK_SUPABASE_FIX.md section so the stated component count matches the
actual list. Update the summary text that says “All 6 critical components” to
reflect the full set of items currently enumerated, and keep the wording
consistent with the verification bullets that mention sync_clerk_user, clerk_id,
is_clerk_user, clerk_user_id, on_auth_user_created, users_updated_at,
database-access, and the RLS policies.
| INSERT INTO public.users (id, email, clerk_user_id) | ||
| VALUES (NEW.id, NEW.email, (auth.jwt() ->> 'raw_user_meta_data')::jsonb ->> 'clerk_user_id'::text) | ||
| ON CONFLICT (id) DO UPDATE | ||
| SET email = EXCLUDED.email, | ||
| clerk_user_id = COALESCE(EXCLUDED.clerk_user_id, public.users.clerk_user_id); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Supabase handle_new_user trigger auth.users raw_user_meta_data vs auth.jwt user_metadata
💡 Result:
The difference between raw_user_meta_data and user_metadata in Supabase relates to the distinction between the underlying database storage and the client-facing API representation. Database Storage (raw_user_meta_data) In the Supabase database, the auth.users table stores custom user data in the raw_user_meta_data column [1][2]. This is the actual JSONB column where the data resides [3]. When creating database triggers—such as the common handle_new_user function—you must query this column directly using PostgreSQL's JSONB operators (e.g., new.raw_user_meta_data ->> 'key') [2][4]. API Representation (user_metadata) The term user_metadata refers to how this data is exposed to developers via the Supabase client SDKs and the REST API [5][3]. When you retrieve a user object using methods like supabase.auth.getUser, the SDK maps the raw_user_meta_data column from the database to the user_metadata field in the returned object [5][2]. Similarly, the user_metadata claim within a JWT is a representation of this data [6]. Key Takeaways for Triggers When writing triggers for the auth.users table: 1. Always reference raw_user_meta_data (or raw_app_meta_data for admin-only fields) [3][7]. 2. Do not attempt to use user_metadata in your SQL functions, as that column does not exist in the database schema [8]. 3. Security Warning: Because raw_user_meta_data can be updated by end users via client-side SDKs, you should not rely on it for sensitive authorization or security-critical logic in your RLS policies or database triggers [7]. Use raw_app_meta_data for fields that only administrators should modify [3].
Citations:
- 1: https://supabase.com/docs/guides/auth/managing-user-data
- 2: https://supabase.com/docs/guides/auth/managing-user-data?language=js&queryGroups=language
- 3: https://deepwiki.com/supabase/auth/8.2-user-metadata
- 4: https://dev.to/faris_perwira/extending-supabase-auth-users-table-433p
- 5: docs(auth): clarify mapping of user metadata in updateUser example supabase/supabase#35047
- 6: https://github.com/supabase/supabase/blob/master/apps/docs/content/guides/auth/jwt-fields.mdx
- 7: supabase/supabase@4574951
- 8: Auth RLS docs inconsistent with
auth.usersschema supabase/supabase#21545
🏁 Script executed:
sed -n '1,180p' drizzle/migrations/0003_clerk_auth_sync.sqlRepository: QueueLab/QCX
Length of output: 6898
🏁 Script executed:
rg -n "auth\.jwt\(\)|raw_user_meta_data|clerk_user_id|handle_new_user|trigger" drizzle/migrations -SRepository: QueueLab/QCX
Length of output: 2987
🏁 Script executed:
rg -n "raw_user_meta_data|raw_app_meta_data|user_metadata|clerk_user_id|clerk_id\(" -S .Repository: QueueLab/QCX
Length of output: 4986
🌐 Web query:
Supabase auth.jwt() inside auth.users AFTER INSERT trigger current request JWT raw_user_meta_data
💡 Result:
In a PostgreSQL trigger function acting on the auth.users table, you cannot reliably use auth.jwt to retrieve the request's raw_user_meta_data because the trigger does not execute within the context of a PostgREST request [1][2]. The auth.jwt function and the request.jwt.claims setting are only available when a request is processed by PostgREST [1]. Instead, to access the user's metadata during an AFTER INSERT trigger on auth.users, you should directly reference the NEW row object provided by the trigger [3][4]. The column raw_user_meta_data contains the user metadata as a JSONB object, which you can query using standard PostgreSQL JSONB operators [3][5]. Key Implementation Details: 1. Direct Access: Access the metadata using NEW.raw_user_meta_data within your PL/pgSQL function [3][6]. Example: insert into public.profiles (id, first_name) values (new.id, new.raw_user_meta_data ->> 'first_name'); 2. Security Definer: You must define your trigger function with SECURITY DEFINER and a restricted search_path to ensure it has sufficient privileges to perform operations on tables outside the auth schema [3][7]. Example: create function public.handle_new_user returns trigger language plpgsql security definer set search_path = '' as
Citations:
- 1: JWT claims not being passed to PostgreSQL - auth.uid() and current_setting('request.jwt.claims') return NULL supabase/supabase#43066
- 2: If using SQL editor with authenticated role to test policies with custom claims, the auth.jwt() doesn't include custom claims that makes the policies not work supabase/supabase#22487
- 3: https://supabase.com/docs/guides/auth/managing-user-data?language=js&queryGroups=language
- 4: https://dev.to/sruhleder/creating-user-profiles-on-sign-up-in-supabase-5037
- 5: https://github.com/supabase/supabase/blob/master/apps/docs/content/guides/auth/managing-user-data.mdx
- 6: https://dev.to/faris_perwira/extending-supabase-auth-users-table-433p
- 7: https://stackoverflow.com/questions/78996250/supabase-cannot-add-users-use-a-signup-form-when-a-trigger-function-is-lis
- 8: Security definer's 'new' object only has app_metadata after update not after insert supabase/auth#975
Read the Clerk ID from NEW.raw_user_meta_data here. In this AFTER INSERT trigger, auth.jwt() won’t reliably carry the signup metadata, so clerk_user_id can stay NULL.
Suggested change
INSERT INTO public.users (id, email, clerk_user_id)
- VALUES (NEW.id, NEW.email, (auth.jwt() ->> 'raw_user_meta_data')::jsonb ->> 'clerk_user_id'::text)
+ VALUES (NEW.id, NEW.email, NEW.raw_user_meta_data ->> 'clerk_user_id')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| INSERT INTO public.users (id, email, clerk_user_id) | |
| VALUES (NEW.id, NEW.email, (auth.jwt() ->> 'raw_user_meta_data')::jsonb ->> 'clerk_user_id'::text) | |
| ON CONFLICT (id) DO UPDATE | |
| SET email = EXCLUDED.email, | |
| clerk_user_id = COALESCE(EXCLUDED.clerk_user_id, public.users.clerk_user_id); | |
| INSERT INTO public.users (id, email, clerk_user_id) | |
| VALUES (NEW.id, NEW.email, NEW.raw_user_meta_data ->> 'clerk_user_id') | |
| ON CONFLICT (id) DO UPDATE | |
| SET email = EXCLUDED.email, | |
| clerk_user_id = COALESCE(EXCLUDED.clerk_user_id, public.users.clerk_user_id); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drizzle/migrations/0003_clerk_auth_sync.sql` around lines 72 - 76, The
clerk_user_id lookup in the INSERT/ON CONFLICT block is reading from auth.jwt(),
which is unreliable in this trigger path; update the trigger logic to read the
Clerk ID from NEW.raw_user_meta_data instead. Make the change in the users
upsert inside the clerk auth sync migration so the AFTER INSERT trigger uses the
row’s metadata directly when populating clerk_user_id, while keeping the
existing conflict update behavior intact.
| createdAt: timestamp('created_at', { withTimezone: true }).defaultNow(), | ||
| avatarUrl: text('avatar_url'), | ||
| firstName: text('first_name'), | ||
| lastName: text('last_name'), | ||
| updatedAt: timestamp('updated_at', { withTimezone: true }).defaultNow(), | ||
| username: text('username'), | ||
| phoneNumber: text('phone_number'), | ||
| metadata: jsonb('metadata'), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find any migration adding the new profile/audit columns to public.users
rg -nP 'ADD COLUMN.*\b(first_name|last_name|avatar_url|username|phone_number|metadata|created_at|updated_at)\b' drizzle/migrations
# List all migration files to confirm coverage
fd . drizzle/migrations -e sqlRepository: QueueLab/QCX
Length of output: 545
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the schema and the referenced migration to see whether the users table
# columns in lib/db/schema.ts are backed by migrations.
sed -n '1,120p' lib/db/schema.ts
printf '\n--- migration 0003 ---\n'
sed -n '1,220p' drizzle/migrations/0003_clerk_auth_sync.sql
printf '\n--- users-related migrations ---\n'
rg -n '"users"|public\.users|CREATE TABLE "users"|ALTER TABLE "users"|ADD COLUMN' drizzle/migrations/*.sqlRepository: QueueLab/QCX
Length of output: 18772
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the original users table definition and the tail of the auth sync migration.
sed -n '1,120p' drizzle/migrations/0000_sweet_metal_master.sql
printf '\n--- migration 0003 tail ---\n'
sed -n '220,320p' drizzle/migrations/0003_clerk_auth_sync.sqlRepository: QueueLab/QCX
Length of output: 2945
Add a migration for the new users columns
lib/db/schema.ts now declares created_at, updated_at, avatar_url, first_name, last_name, username, phone_number, and metadata, but the existing migrations only create users(id) and later add email, role, selected_model, system_prompt, and clerk_user_id. 0003_clerk_auth_sync.sql also writes first_name, last_name, and avatar_url into public.users, so those paths will break until the table is migrated to match the schema.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/db/schema.ts` around lines 29 - 36, Add a database migration to bring
public.users in sync with the columns introduced in lib/db/schema.ts. Update the
users table to include created_at, updated_at, avatar_url, first_name,
last_name, username, phone_number, and metadata, and make sure the migration
order works with existing scripts like 0003_clerk_auth_sync.sql that already
write first_name, last_name, and avatar_url. Use the users table definition in
schema.ts and the existing migration pattern for users-related changes as the
reference point.
There was a problem hiding this comment.
Blocking feedback
- This migration uses new
usersprofile/timestamp fields but never adds those columns, so environments built from the repo migrations can fail on the new auth-sync writes — drizzle/migrations/0003_clerk_auth_sync.sql handle_new_userreads Clerk metadata fromauth.jwt()inside anauth.userstrigger, where request JWT claims are not reliable, soclerk_user_idcan remain unset — drizzle/migrations/0003_clerk_auth_sync.sql- The webhook still returns
200when backendsync_clerk_userRPC fails, which suppresses Clerk retries and can leave QCX-BACKEND user linkage permanently unsynced for affected users — app/api/clerk/webhook/route.ts
If you want me to push fixes, reply with item numbers (for example: please fix 1,3).
| -- ============================================= | ||
|
|
||
| -- 1. Ensure clerk_user_id column exists (may already exist from live migration) | ||
| ALTER TABLE public.users ADD COLUMN IF NOT EXISTS clerk_user_id TEXT UNIQUE; |
There was a problem hiding this comment.
This migration does not add the new users columns introduced in lib/db/schema.ts (created_at, updated_at, avatar_url, first_name, last_name, username, phone_number, metadata), but this same migration and app code already write several of them.
On DBs bootstrapped from repo migrations, those columns are missing, so auth sync writes will fail at runtime. Suggested fix: add ALTER TABLE public.users ADD COLUMN IF NOT EXISTS ... for the new columns before creating/using sync_clerk_user.
| RETURNS TRIGGER AS $$ | ||
| BEGIN | ||
| INSERT INTO public.users (id, email, clerk_user_id) | ||
| VALUES (NEW.id, NEW.email, (auth.jwt() ->> 'raw_user_meta_data')::jsonb ->> 'clerk_user_id'::text) |
There was a problem hiding this comment.
auth.jwt() is request-scoped and unreliable in an AFTER INSERT ON auth.users trigger context (runs under internal auth role, not an end-user request). This can leave clerk_user_id null and keep the Clerk↔UUID link broken for trigger-created rows.
Suggested fix: read from row metadata (NEW.raw_user_meta_data ->> 'clerk_user_id', or raw_app_meta_data if that is the trusted source).
| avatar_url: avatarUrl, | ||
| }) | ||
|
|
||
| if (rpcError) { |
There was a problem hiding this comment.
When this RPC call fails, the handler logs and still returns HTTP 200. Clerk treats 2xx as success and won’t retry delivery, so transient Supabase failures can permanently leave clerk_user_id unsynced in QCX-BACKEND for that user.
Suggested fix: return non-2xx on backend sync failure (or enqueue a durable retry) so this critical sync path is eventually consistent.
Root cause: clerk_user_id was never populated on public.users, causing is_clerk_user() to always return false and all Clerk-aware RLS policies to deny access to authenticated users. Changes: - Updated webhook route to sync users to both QCX app DB and QCX-BACKEND via sync_clerk_user RPC (using new prefixed parameter names) - Updated migration 0003_clerk_auth_sync.sql with all SQL changes: - sync_clerk_user() with aliased parameters (p_clerk_id, p_email, etc.) - handle_new_user() trigger with explicit table references - clerk_id() and is_clerk_user() with search_path restrictions - save_chat_with_messages() with input validation - hybrid_search() with input validation - make_creator_owner() with security fixes - All RLS policies across 7 tables recreated - users.id default set to gen_random_uuid() - users_updated_at trigger for timestamp consistency - Edge function v14 deployed to Supabase (not tracked in git) Security improvements: - Parameter name prefixing (p_) to avoid PL/pgSQL ambiguity - Explicit table-qualified column references - SEARCH_PATH restrictions on all SECURITY DEFINER functions - Input validation on save_chat_with_messages and hybrid_search - Edge function uses Clerk verifyWebhook for signature verification
7b4ddd8 to
9c73575
Compare
…onnection errors The original code created a new Pool() at module load time, which tried to connect to the database during Next.js static optimization on Vercel. When DATABASE_URL wasn't available yet, it fell back to a localhost URL and the pg client crashed with 'Cannot read properties of undefined'. Fix: Use lazy initialization so the Pool is only created when db is first used in a server-side request handler, where DATABASE_URL is guaranteed to be available as a Vercel env var.
The native pg Pool driver fails on Vercel Lambda with: TypeError: Cannot read properties of undefined (reading 'searchParams') This happens because pg is a TCP-based driver that doesn't work reliably in serverless environments. The postgres package (postgres-js) is a pure JavaScript PostgreSQL driver that works in all serverless contexts. Changes: - package.json: replaced 'pg' with 'postgres' - bun.lock: updated to include postgres@3.4.9 - lib/db/index.ts: switched from drizzle-orm/node-postgres to drizzle-orm/postgres-js - lib/db/migrate.ts: updated to use postgres-js for migrations No schema or query changes needed — same Drizzle ORM API.
6ece8c4 to
85fcd36
Compare
During Next.js static optimization (build phase), DATABASE_URL is not available, causing the postgres-js driver to crash when the Proxy tried to initialize. This fix detects the build environment and returns a stub object instead of crashing. The stub only throws when an actual database operation is attempted at runtime. Also added explicit type annotation for transaction parameter (tx: typeof db) to resolve TypeScript implicit any error.
On Vercel, the Supabase database connection URL is set as POSTGRES_URL, not DATABASE_URL. The previous code only checked DATABASE_URL, which caused the build to fail with 'Invalid URL' because the .env placeholder value 'postgresql://user:password@host:port/db' was being used. Changes: - lib/db/index.ts: checks POSTGRES_URL || DATABASE_URL, also rejects placeholder URLs that contain 'user:password@host:port' - lib/db/migrate.ts: same env var fallback - drizzle.config.ts: same env var fallback
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/clerk/webhook/route.ts`:
- Around line 145-148: The user.deleted handler in the webhook route is logging
the full evt.data payload, which includes sensitive PII. Update the eventType
=== 'user.deleted' branch in the route.ts handler to log only a non-sensitive
identifier such as the Clerk user id from evt.data, and avoid printing any
profile or email fields in the console.log.
In `@drizzle/migrations/0003_clerk_auth_sync.sql`:
- Around line 18-19: The migration adds two indexes for clerk_user_id in
public.users, which is redundant because the UNIQUE column constraint already
creates a unique index. Update the migration by keeping only one indexing
strategy in the clerk auth sync change: either retain ALTER TABLE ... ADD COLUMN
... TEXT UNIQUE and remove the separate CREATE INDEX, or remove UNIQUE and leave
a single unique index definition. Use the ADD COLUMN and CREATE INDEX statements
in this migration as the targets for the change.
- Around line 43-46: The header comment for sync_clerk_user is misleading
because it mentions an email-format check that the body does not implement.
Update the migration so the comment matches the actual validation in
sync_clerk_user, either by adding the missing email validation alongside the
existing p_clerk_id NULL check or by revising the header text to describe only
the clerk_id requirement.
- Around line 223-257: The `chat_participants` ownership policies are too
permissive because `OR public.is_clerk_user(user_id)` allows a Clerk user to
self-insert or self-escalate. Update the `CREATE POLICY` definitions for
`chat_participants` so `FOR INSERT`, `FOR UPDATE`, and `FOR DELETE` rely only on
the existing owner-existence check, and remove the self-bypass from each policy.
Keep `make_creator_owner()` as the path that creates the initial owner row, and
make sure the `chat_participants` policy names remain aligned with the
migration.
In `@lib/actions/chat-db.ts`:
- Line 90: The transaction callback in chat-db’s db.transaction call is
incorrectly annotated with tx: typeof db, which treats the transaction handle as
the full database object. Remove the explicit tx type annotation and let Drizzle
infer the proper PgTransaction type for the callback parameter, keeping the
callback aligned with the actual transaction handle used in chat-db and
db.transaction.
In `@lib/auth/get-current-user.ts`:
- Around line 71-76: In get-current-user.ts, the email-linking update block is
overwriting existing profile data by setting firstName, lastName, and avatarUrl
unconditionally. Update the .set call in the linking path to match
syncUserProfile’s null-guarded behavior so only non-null Clerk fields are
written. Keep clerkUserId unchanged, and ensure existing stored profile values
are preserved when Clerk omits any of those fields.
In `@lib/db/index.ts`:
- Around line 63-94: The exported db value is widening to any because the stub
branch in the db initializer returns a Proxy typed as any, which removes Drizzle
query typing downstream. Update the db export in the db initializer to use an
explicit Drizzle database type so both the stub path and the real drizzle(sql, {
schema, logger: ... }) path conform to the same type. Use the existing db IIFE,
hasValidConnectionUrl(), getSql(), and drizzle() symbols to locate the fix, and
ensure the stub is cast/typed to the real database interface instead of any.
In `@lib/db/migrate.ts`:
- Around line 32-36: runMigrations() is being called without handling its
returned promise, so failures like the missing database URL error can surface as
unhandled rejections. Update the EXECUTE_MIGRATIONS branch in migrate.ts to
attach proper async error handling to runMigrations, using a .catch on the call
(or equivalent top-level await handling) that logs the error and exits with a
non-zero status. Keep the fix localized to runMigrations and the
EXECUTE_MIGRATIONS conditional so all migration failures are caught
consistently.
In `@package.json`:
- Line 85: The dependency list is out of sync with the database imports:
`lib/db/index.ts` and `lib/db/migrate.ts` use `postgres`, but `package.json`
still depends on `pg`. Update the `dependencies` section to remove `pg` and add
`postgres` so the package matches the actual database client used by
`createDb`/migration code. Keep the existing version constraints consistent with
the rest of the project.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e88c513c-1f3f-4e22-81c3-d0605bea241c
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
.gitignoreCLERK_SUPABASE_FIX.mdapp/api/clerk/webhook/route.tsdrizzle.config.tsdrizzle/migrations/0003_clerk_auth_sync.sqldrizzle/migrations/meta/_journal.jsonlib/actions/chat-db.tslib/auth/get-current-user.tslib/db/index.tslib/db/migrate.tslib/db/schema.tspackage.jsontsconfig.json
📜 Review details
🧰 Additional context used
🪛 LanguageTool
CLERK_SUPABASE_FIX.md
[style] ~33-~33: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...m: SELECT auth.jwt() ->> 'sub'. 4. Updated is_clerk_user(user_id) function: Pr...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~33-~33: Consider an alternative adverb to strengthen your wording.
Context: ...ed is_clerk_user(user_id) function**: Properly checks that the user's clerk_user_id ...
(PROPERLY_THOROUGHLY)
[style] ~35-~35: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...s the current session's Clerk ID. 5. Updated all RLS policies on 8 tables: users...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~76-~76: Ensure spelling is correct
Context: ...already live** in QCX-BACKEND (project: mofqapxwyphzjrqegjeq). - The edge function is deployed as vers...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.22.1)
CLERK_SUPABASE_FIX.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 SQLFluff (4.2.2)
drizzle/migrations/0003_clerk_auth_sync.sql
[error] 19-19: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 152-152: Do not use special characters in identifiers.
(RF05)
[error] 153-153: Do not use special characters in identifiers.
(RF05)
[error] 160-160: Do not use special characters in identifiers.
(RF05)
[error] 161-161: Do not use special characters in identifiers.
(RF05)
[error] 172-172: Do not use special characters in identifiers.
(RF05)
[error] 173-173: Do not use special characters in identifiers.
(RF05)
[error] 178-178: Do not use special characters in identifiers.
(RF05)
[error] 179-179: Do not use special characters in identifiers.
(RF05)
[error] 203-203: Do not use special characters in identifiers.
(RF05)
[error] 204-204: Do not use special characters in identifiers.
(RF05)
[error] 208-208: Do not use special characters in identifiers.
(RF05)
[error] 209-209: Do not use special characters in identifiers.
(RF05)
[error] 213-213: Do not use special characters in identifiers.
(RF05)
[error] 214-214: Do not use special characters in identifiers.
(RF05)
[error] 218-218: Do not use special characters in identifiers.
(RF05)
[error] 219-219: Do not use special characters in identifiers.
(RF05)
[error] 223-223: Do not use special characters in identifiers.
(RF05)
[error] 224-224: Do not use special characters in identifiers.
(RF05)
[error] 235-235: Do not use special characters in identifiers.
(RF05)
[error] 236-236: Do not use special characters in identifiers.
(RF05)
[error] 247-247: Do not use special characters in identifiers.
(RF05)
[error] 248-248: Do not use special characters in identifiers.
(RF05)
[error] 259-259: Do not use special characters in identifiers.
(RF05)
[error] 260-260: Do not use special characters in identifiers.
(RF05)
🪛 Squawk (2.59.0)
drizzle/migrations/0003_clerk_auth_sync.sql
[warning] 18-18: Adding a UNIQUE constraint requires an ACCESS EXCLUSIVE lock which blocks reads and writes to the table while the index is built. Create an index CONCURRENTLY and create the constraint using the index.
(disallowed-unique-constraint)
[warning] 19-19: During normal index creation, table updates are blocked, but reads are still allowed. Use concurrently to avoid blocking writes.
(require-concurrent-index-creation)
🔇 Additional comments (15)
lib/db/schema.ts (1)
29-36: Schema columns lack a backing migration.
created_at,updated_at,username,phone_number, andmetadataare declared here but are not added topublic.usersby any migration (0003_clerk_auth_sync.sqlonly addsclerk_user_id).sync_clerk_userand the app-side sync paths write several of these, so they will fail at runtime on DBs bootstrapped from repo migrations.drizzle/migrations/0003_clerk_auth_sync.sql (3)
18-19: Index/unique build takes table-level locks.
ADD ... UNIQUE(Line 18) andCREATE INDEX(Line 19) takeACCESS EXCLUSIVE/write locks on a populateduserstable. PreferCREATE UNIQUE INDEX CONCURRENTLY+ADD CONSTRAINT ... USING INDEXoutside the wrapping transaction.Source: Linters/SAST tools
54-59:sync_clerk_userwrites columns the migration never creates.Lines 54-59 (and 71-76, plus the
updated_attrigger) referencefirst_name,last_name,avatar_url,updated_atonpublic.users, but this migration only addsclerk_user_id. On a DB bootstrapped from repo migrations these columns don't exist and the function will error. AddALTER TABLE public.users ADD COLUMN IF NOT EXISTS ...for the profile/audit columns before defining/usingsync_clerk_user.
102-106:auth.jwt()is unreliable inside thisAFTER INSERTtrigger.
handle_new_userruns outside a PostgREST request (internal auth role), soauth.jwt()won't reliably carry signup metadata andclerk_user_idcan stay NULL, keeping the Clerk↔UUID link broken. Read from the row instead, e.g.NEW.raw_user_meta_data ->> 'sub'(or'clerk_user_id', matching the trusted source).drizzle/migrations/meta/_journal.json (1)
25-34: LGTM!app/api/clerk/webhook/route.ts (2)
75-82: Avoid overwritingnullon update.
user.updatedevent without a primary email will null out the stored identity, breaking the email-based linking path and unique-identity assumptions. Guard it like the other fields.
116-141: Backend RPC failure is swallowed while returning HTTP 200.On
rpcError(Line 130) or exception, the handler logs and still returns 200, so Clerk won't retry. Transient Supabase failures can permanently leaveclerk_user_idunsynced in QCX-BACKEND. Return non-2xx on backend sync failure or enqueue a durable retry.lib/auth/get-current-user.ts (2)
45-48: Per-request profile sync degrades the hot path.
syncUserProfile(clerkUserId)runs on every authenticated request for existing users, adding a ClerkcurrentUser()call plus a DB UPDATE to a path invoked on every API request. Prefer event-driven sync (the webhook) or gate behind a staleness check.
100-126: LGTM!drizzle.config.ts (1)
6-16: LGTM!lib/db/index.ts (1)
47-50: 🩺 Stability & AvailabilityNeed to confirm the connection target
POSTGRES_URL/DATABASE_URLmay be a direct Postgres URL here.prepare: falseis only required for a transaction-mode pooler, so don’t apply this change unless this environment is definitely pointing at Supavisor.CLERK_SUPABASE_FIX.md (2)
53-61: 📐 Maintainability & Code Quality | ⚡ Quick winVerification count still incorrect.
Says "All 6 critical components" but the list enumerates 8 items (sync_clerk_user, clerk_id, is_clerk_user, clerk_user_id column, on_auth_user_created trigger, users_updated_at trigger, edge function, RLS policies). This was already flagged in a prior review and remains unfixed.
📝 Proposed fix
-All 6 critical components verified as active in QCX-BACKEND: +All 8 critical components verified as active in QCX-BACKEND:
11-20: 📐 Maintainability & Code Quality | ⚡ Quick winMarkdown heading spacing.
Headings at lines 11, 14, 17, 20 lack a blank line before them per markdownlint (MD022).
Source: Linters/SAST tools
.gitignore (1)
60-61: LGTM!tsconfig.json (1)
37-40: LGTM!
| if (eventType === 'user.deleted') { | ||
| // Optionally handle user deletion | ||
| // For safety, we might not want to delete user data entirely | ||
| console.log('[Webhook] User deleted event received:', evt.data) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Avoid logging the full user payload (PII).
evt.data for user.deleted contains PII (email, name, profile fields). Logging the entire payload persists personal data in application logs, a GDPR/CCPA retention concern. Log only the non-sensitive identifier.
🔒️ Proposed fix
if (eventType === 'user.deleted') {
// Optionally handle user deletion
// For safety, we might not want to delete user data entirely
- console.log('[Webhook] User deleted event received:', evt.data)
+ console.log('[Webhook] User deleted event received:', evt.data?.id)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (eventType === 'user.deleted') { | |
| // Optionally handle user deletion | |
| // For safety, we might not want to delete user data entirely | |
| console.log('[Webhook] User deleted event received:', evt.data) | |
| if (eventType === 'user.deleted') { | |
| // Optionally handle user deletion | |
| // For safety, we might not want to delete user data entirely | |
| console.log('[Webhook] User deleted event received:', evt.data?.id) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/clerk/webhook/route.ts` around lines 145 - 148, The user.deleted
handler in the webhook route is logging the full evt.data payload, which
includes sensitive PII. Update the eventType === 'user.deleted' branch in the
route.ts handler to log only a non-sensitive identifier such as the Clerk user
id from evt.data, and avoid printing any profile or email fields in the
console.log.
| ALTER TABLE public.users ADD COLUMN IF NOT EXISTS clerk_user_id TEXT UNIQUE; | ||
| CREATE INDEX IF NOT EXISTS idx_users_clerk_user_id ON public.users(clerk_user_id); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
Redundant index on clerk_user_id.
ADD COLUMN clerk_user_id TEXT UNIQUE already creates a unique btree index on the column. The explicit CREATE INDEX idx_users_clerk_user_id (Line 19) is a second, non-unique index on the same column — pure write/storage overhead with no read benefit. Drop it, or drop UNIQUE from the column and keep only a single (unique) index.
🧰 Tools
🪛 SQLFluff (4.2.2)
[error] 19-19: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
🪛 Squawk (2.59.0)
[warning] 18-18: Adding a UNIQUE constraint requires an ACCESS EXCLUSIVE lock which blocks reads and writes to the table while the index is built. Create an index CONCURRENTLY and create the constraint using the index.
(disallowed-unique-constraint)
[warning] 19-19: During normal index creation, table updates are blocked, but reads are still allowed. Use concurrently to avoid blocking writes.
(require-concurrent-index-creation)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drizzle/migrations/0003_clerk_auth_sync.sql` around lines 18 - 19, The
migration adds two indexes for clerk_user_id in public.users, which is redundant
because the UNIQUE column constraint already creates a unique index. Update the
migration by keeping only one indexing strategy in the clerk auth sync change:
either retain ALTER TABLE ... ADD COLUMN ... TEXT UNIQUE and remove the separate
CREATE INDEX, or remove UNIQUE and leave a single unique index definition. Use
the ADD COLUMN and CREATE INDEX statements in this migration as the targets for
the change.
| -- Input validation | ||
| IF p_clerk_id IS NULL THEN | ||
| RAISE EXCEPTION 'clerk_id is required'; | ||
| END IF; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Header comment claims an email-format check that isn't implemented.
The file header (Line 14) states "Input validation on sync_clerk_user (email format check)", but the body only validates p_clerk_id IS NOT NULL. There is no email validation. Either add the check or correct the misleading comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drizzle/migrations/0003_clerk_auth_sync.sql` around lines 43 - 46, The header
comment for sync_clerk_user is misleading because it mentions an email-format
check that the body does not implement. Update the migration so the comment
matches the actual validation in sync_clerk_user, either by adding the missing
email validation alongside the existing p_clerk_id NULL check or by revising the
header text to describe only the clerk_id requirement.
| DROP POLICY IF EXISTS "Only owners can insert participants" ON public.chat_participants; | ||
| CREATE POLICY "Only owners can insert participants" ON public.chat_participants | ||
| FOR INSERT WITH CHECK ( | ||
| EXISTS ( | ||
| SELECT 1 FROM public.chat_participants cp | ||
| WHERE cp.chat_id = chat_participants.chat_id | ||
| AND (cp.user_id = auth.uid() OR public.is_clerk_user(cp.user_id)) | ||
| AND cp.role = 'owner' | ||
| ) | ||
| OR public.is_clerk_user(user_id) | ||
| ); | ||
|
|
||
| DROP POLICY IF EXISTS "Only owners can update participants" ON public.chat_participants; | ||
| CREATE POLICY "Only owners can update participants" ON public.chat_participants | ||
| FOR UPDATE USING ( | ||
| EXISTS ( | ||
| SELECT 1 FROM public.chat_participants cp | ||
| WHERE cp.chat_id = chat_participants.chat_id | ||
| AND (cp.user_id = auth.uid() OR public.is_clerk_user(cp.user_id)) | ||
| AND cp.role = 'owner' | ||
| ) | ||
| OR public.is_clerk_user(user_id) | ||
| ); | ||
|
|
||
| DROP POLICY IF EXISTS "Only owners can delete participants" ON public.chat_participants; | ||
| CREATE POLICY "Only owners can delete participants" ON public.chat_participants | ||
| FOR DELETE USING ( | ||
| EXISTS ( | ||
| SELECT 1 FROM public.chat_participants cp | ||
| WHERE cp.chat_id = chat_participants.chat_id | ||
| AND (cp.user_id = auth.uid() OR public.is_clerk_user(cp.user_id)) | ||
| AND cp.role = 'owner' | ||
| ) | ||
| OR public.is_clerk_user(user_id) | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target migration around the reported lines.
file="drizzle/migrations/0003_clerk_auth_sync.sql"
wc -l "$file"
sed -n '1,120p' "$file"
printf '\n--- MIDDLE ---\n'
sed -n '120,320p' "$file"
# Find related definitions/usages.
printf '\n--- SEARCH ---\n'
rg -n "is_clerk_user|make_creator_owner|chat_participants|Only owners can (insert|update|delete) participants|SECURITY DEFINER" -S drizzle . || trueRepository: QueueLab/QCX
Length of output: 29868
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the table schema and any role constraints.
file="lib/db/schema.ts"
wc -l "$file"
sed -n '1,170p' "$file"
# Narrow search for participant-related schema or docs.
printf '\n--- PARTICIPANT SEARCH ---\n'
rg -n "chatParticipants|chat_participants|role|owner" lib drizzle CLERK_SUPABASE_FIX.md -S || trueRepository: QueueLab/QCX
Length of output: 19322
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="lib/db/schema.ts"
wc -l "$file"
sed -n '1,170p' "$file"
printf '\n--- PARTICIPANT SEARCH ---\n'
rg -n "chatParticipants|chat_participants|role|owner" lib drizzle CLERK_SUPABASE_FIX.md -S || trueRepository: QueueLab/QCX
Length of output: 19322
🌐 Web query:
PostgreSQL row level security UPDATE USING WITH CHECK omitted default semantics
💡 Result:
In PostgreSQL Row Level Security (RLS) policies for UPDATE operations, the USING and WITH CHECK clauses serve distinct purposes [1][2]. The USING expression determines which existing rows the user has permission to select or target for an update [1][3]. The WITH CHECK expression defines the constraints that the new, modified row must satisfy before it can be stored in the table [1][3]. If the WITH CHECK clause is omitted in an UPDATE policy, PostgreSQL automatically defaults to using the USING expression as the WITH CHECK expression [1][3]. This ensures that the same constraints applied to selecting rows are also applied to the final state of the updated rows [4][1]. While this fallback behavior often functions as intended, it can be a security risk if the USING expression is insufficient to validate the new state of the row [2]. For example, if a USING clause only checks if a user owns a row, but does not prevent them from modifying a sensitive column (e.g., changing an is_admin flag), the omitted WITH CHECK will implicitly allow that modification because it uses the same USING logic [2]. For this reason, it is considered best practice to explicitly define a WITH CHECK expression for UPDATE policies to ensure that the resulting data satisfies all necessary integrity and security requirements [2].
Citations:
- 1: https://www.postgresql.org/docs/19/sql-createpolicy.html
- 2: https://zenn.dev/cosoado/articles/supabase-rls-with-check-vs-using-pitfall?locale=en
- 3: https://www.postgresql.org/docs/17/sql-createpolicy.html
- 4: https://www.postgresql.org/docs/current/ddl-rowsecurity.html
Remove the self-bypass from chat_participants ownership policies.
OR public.is_clerk_user(user_id) lets a Clerk user insert themselves into any chat, which then grants chat/message access through the participant-based RLS rules. On UPDATE, PostgreSQL reuses USING as WITH CHECK when omitted, so the same self row can be changed to role = 'owner'. make_creator_owner() already covers the initial owner row, so this clause is broader than intended.
🧰 Tools
🪛 SQLFluff (4.2.2)
[error] 223-223: Do not use special characters in identifiers.
(RF05)
[error] 224-224: Do not use special characters in identifiers.
(RF05)
[error] 235-235: Do not use special characters in identifiers.
(RF05)
[error] 236-236: Do not use special characters in identifiers.
(RF05)
[error] 247-247: Do not use special characters in identifiers.
(RF05)
[error] 248-248: Do not use special characters in identifiers.
(RF05)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@drizzle/migrations/0003_clerk_auth_sync.sql` around lines 223 - 257, The
`chat_participants` ownership policies are too permissive because `OR
public.is_clerk_user(user_id)` allows a Clerk user to self-insert or
self-escalate. Update the `CREATE POLICY` definitions for `chat_participants` so
`FOR INSERT`, `FOR UPDATE`, and `FOR DELETE` rely only on the existing
owner-existence check, and remove the self-bypass from each policy. Keep
`make_creator_owner()` as the path that creates the initial owner row, and make
sure the `chat_participants` policy names remain aligned with the migration.
|
|
||
| // Transaction to ensure atomicity | ||
| return db.transaction(async (tx) => { | ||
| return db.transaction(async (tx: typeof db) => { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
tx: typeof db is the wrong type for a transaction callback.
The transaction handle is a PgTransaction, not the full database, so annotating it as typeof db is inaccurate; it currently only compiles because db is inferred as any (see lib/db/index.ts Line 63-94). Once db is properly typed, drizzle already infers the correct tx type — drop the explicit annotation.
♻️ Suggested change
- return db.transaction(async (tx: typeof db) => {
+ return db.transaction(async (tx) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return db.transaction(async (tx: typeof db) => { | |
| return db.transaction(async (tx) => { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/actions/chat-db.ts` at line 90, The transaction callback in chat-db’s
db.transaction call is incorrectly annotated with tx: typeof db, which treats
the transaction handle as the full database object. Remove the explicit tx type
annotation and let Drizzle infer the proper PgTransaction type for the callback
parameter, keeping the callback aligned with the actual transaction handle used
in chat-db and db.transaction.
| .set({ | ||
| clerkUserId, | ||
| firstName, | ||
| lastName, | ||
| avatarUrl, | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Guard profile fields when linking to avoid wiping existing data.
When linking an email-matched user (possibly created by Supabase/edge function with populated profile fields), firstName, lastName, and avatarUrl are written unconditionally. If the Clerk profile lacks any of these, the existing values are overwritten with null. This is inconsistent with the null-guarded update in syncUserProfile (Lines 116-119). Apply the same guards.
🛡️ Proposed fix
await db.update(users)
.set({
clerkUserId,
- firstName,
- lastName,
- avatarUrl,
+ ...(firstName !== null && { firstName }),
+ ...(lastName !== null && { lastName }),
+ ...(avatarUrl !== null && { avatarUrl }),
})
.where(eq(users.id, existingEmailUser.id));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .set({ | |
| clerkUserId, | |
| firstName, | |
| lastName, | |
| avatarUrl, | |
| }) | |
| .set({ | |
| clerkUserId, | |
| ...(firstName !== null && { firstName }), | |
| ...(lastName !== null && { lastName }), | |
| ...(avatarUrl !== null && { avatarUrl }), | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/auth/get-current-user.ts` around lines 71 - 76, In get-current-user.ts,
the email-linking update block is overwriting existing profile data by setting
firstName, lastName, and avatarUrl unconditionally. Update the .set call in the
linking path to match syncUserProfile’s null-guarded behavior so only non-null
Clerk fields are written. Keep clerkUserId unchanged, and ensure existing stored
profile values are preserved when Clerk omits any of those fields.
| export const db = (function () { | ||
| // If no valid connection URL (build time or missing env var), return a stub | ||
| if (!hasValidConnectionUrl()) { | ||
| const stub: any = new Proxy({}, { | ||
| get(_, prop) { | ||
| if (prop === Symbol.toPrimitive || prop === 'then') { | ||
| return undefined; | ||
| } | ||
| const source = process.env.POSTGRES_URL | ||
| ? 'POSTGRES_URL' | ||
| : process.env.DATABASE_URL | ||
| ? 'DATABASE_URL' | ||
| : 'no database URL'; | ||
| return function (...args: any[]) { | ||
| throw new Error( | ||
| `Cannot execute ${String(prop)}() — ${source} is not a valid connection string. ` + | ||
| 'Ensure the database URL environment variable is set correctly.' | ||
| ); | ||
| }; | ||
| }, | ||
| }); | ||
| return stub; | ||
| } | ||
|
|
||
| // Runtime: create the real database instance | ||
| const sql = getSql(); | ||
| if (!sql) { | ||
| throw new Error('No valid database connection string at runtime'); | ||
| } | ||
|
|
||
| export const db = drizzle(pool, { schema, logger: process.env.NODE_ENV === 'development' }); | ||
| return drizzle(sql, { schema, logger: process.env.NODE_ENV === 'development' }); | ||
| })(); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Stub branch erases the Drizzle type of db.
Because the if (!hasValidConnectionUrl()) branch returns stub: any, the inferred type of the exported db becomes any, so all downstream query typing is lost (e.g. tx: typeof db in lib/actions/chat-db.ts resolves to any). Give the export the real Drizzle type explicitly so the stub path doesn’t widen it.
♻️ Suggested typing
-export const db = (function () {
+export const db: PostgresJsDatabase<typeof schema> = (function () {Add the import:
-import { drizzle } from 'drizzle-orm/postgres-js';
+import { drizzle, type PostgresJsDatabase } from 'drizzle-orm/postgres-js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const db = (function () { | |
| // If no valid connection URL (build time or missing env var), return a stub | |
| if (!hasValidConnectionUrl()) { | |
| const stub: any = new Proxy({}, { | |
| get(_, prop) { | |
| if (prop === Symbol.toPrimitive || prop === 'then') { | |
| return undefined; | |
| } | |
| const source = process.env.POSTGRES_URL | |
| ? 'POSTGRES_URL' | |
| : process.env.DATABASE_URL | |
| ? 'DATABASE_URL' | |
| : 'no database URL'; | |
| return function (...args: any[]) { | |
| throw new Error( | |
| `Cannot execute ${String(prop)}() — ${source} is not a valid connection string. ` + | |
| 'Ensure the database URL environment variable is set correctly.' | |
| ); | |
| }; | |
| }, | |
| }); | |
| return stub; | |
| } | |
| // Runtime: create the real database instance | |
| const sql = getSql(); | |
| if (!sql) { | |
| throw new Error('No valid database connection string at runtime'); | |
| } | |
| export const db = drizzle(pool, { schema, logger: process.env.NODE_ENV === 'development' }); | |
| return drizzle(sql, { schema, logger: process.env.NODE_ENV === 'development' }); | |
| })(); | |
| import { drizzle, type PostgresJsDatabase } from 'drizzle-orm/postgres-js'; | |
| export const db: PostgresJsDatabase<typeof schema> = (function () { | |
| // If no valid connection URL (build time or missing env var), return a stub | |
| if (!hasValidConnectionUrl()) { | |
| const stub: any = new Proxy({}, { | |
| get(_, prop) { | |
| if (prop === Symbol.toPrimitive || prop === 'then') { | |
| return undefined; | |
| } | |
| const source = process.env.POSTGRES_URL | |
| ? 'POSTGRES_URL' | |
| : process.env.DATABASE_URL | |
| ? 'DATABASE_URL' | |
| : 'no database URL'; | |
| return function (...args: any[]) { | |
| throw new Error( | |
| `Cannot execute ${String(prop)}() — ${source} is not a valid connection string. ` + | |
| 'Ensure the database URL environment variable is set correctly.' | |
| ); | |
| }; | |
| }, | |
| }); | |
| return stub; | |
| } | |
| // Runtime: create the real database instance | |
| const sql = getSql(); | |
| if (!sql) { | |
| throw new Error('No valid database connection string at runtime'); | |
| } | |
| return drizzle(sql, { schema, logger: process.env.NODE_ENV === 'development' }); | |
| })(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/db/index.ts` around lines 63 - 94, The exported db value is widening to
any because the stub branch in the db initializer returns a Proxy typed as any,
which removes Drizzle query typing downstream. Update the db export in the db
initializer to use an explicit Drizzle database type so both the stub path and
the real drizzle(sql, { schema, logger: ... }) path conform to the same type.
Use the existing db IIFE, hasValidConnectionUrl(), getSql(), and drizzle()
symbols to locate the fix, and ensure the stub is cast/typed to the real
database interface instead of any.
| if (process.env.EXECUTE_MIGRATIONS === 'true') { | ||
| runMigrations(); | ||
| } else { | ||
| console.log('Skipping migrations. Set EXECUTE_MIGRATIONS=true to run them.'); | ||
| console.log('To run migrations, use the "npm run db:migrate" or "bun run db:migrate" script, which sets this variable.'); | ||
| console.log('To run migrations, use the "bun run db:migrate" script, which sets this variable.'); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
runMigrations() is invoked without awaiting or catching rejections.
The early throw new Error('Neither POSTGRES_URL nor DATABASE_URL is set for migrations') (and any rejection from migrate) becomes an unhandled promise rejection rather than a clean non-zero exit. Attach a .catch that logs and exits.
🔧 Suggested fix
if (process.env.EXECUTE_MIGRATIONS === 'true') {
- runMigrations();
+ runMigrations().catch((error) => {
+ console.error('Migration run failed:', error);
+ process.exit(1);
+ });
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (process.env.EXECUTE_MIGRATIONS === 'true') { | |
| runMigrations(); | |
| } else { | |
| console.log('Skipping migrations. Set EXECUTE_MIGRATIONS=true to run them.'); | |
| console.log('To run migrations, use the "npm run db:migrate" or "bun run db:migrate" script, which sets this variable.'); | |
| console.log('To run migrations, use the "bun run db:migrate" script, which sets this variable.'); | |
| if (process.env.EXECUTE_MIGRATIONS === 'true') { | |
| runMigrations().catch((error) => { | |
| console.error('Migration run failed:', error); | |
| process.exit(1); | |
| }); | |
| } else { | |
| console.log('Skipping migrations. Set EXECUTE_MIGRATIONS=true to run them.'); | |
| console.log('To run migrations, use the "bun run db:migrate" script, which sets this variable.'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/db/migrate.ts` around lines 32 - 36, runMigrations() is being called
without handling its returned promise, so failures like the missing database URL
error can surface as unhandled rejections. Update the EXECUTE_MIGRATIONS branch
in migrate.ts to attach proper async error handling to runMigrations, using a
.catch on the call (or equivalent top-level await handling) that logs the error
and exits with a non-zero status. Keep the fix localized to runMigrations and
the EXECUTE_MIGRATIONS conditional so all migration failures are caught
consistently.
| "next-themes": "^0.3.0", | ||
| "open-codex": "^0.1.30", | ||
| "pg": "^8.16.2", | ||
| "pg": "^8.22.0", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Any remaining pg / node-postgres usage?
rg -nP "from ['\"]pg['\"]|require\(['\"]pg['\"]\)|drizzle-orm/node-postgres" -g '!**/node_modules/**'
# Is postgres declared as a dependency?
rg -nP '"postgres"\s*:' package.jsonRepository: QueueLab/QCX
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- package.json dependency block ---'
sed -n '1,220p' package.json | nl -ba | sed -n '70,120p'
printf '\n%s\n' '--- pg / postgres references ---'
rg -n --hidden -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' \
-e "from ['\"]pg['\"]" \
-e "require\\(['\"]pg['\"]\\)" \
-e "drizzle-orm/node-postgres" \
-e "\"postgres\"\\s*:" \
-e "from ['\"]postgres['\"]" \
-e "require\\(['\"]postgres['\"]\\)" \
.Repository: QueueLab/QCX
Length of output: 228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- package.json dependency block ---'
sed -n '70,120p' package.json
printf '\n%s\n' '--- pg / postgres references ---'
rg -n --hidden -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' \
-e "from ['\"]pg['\"]" \
-e "require\\(['\"]pg['\"]\\)" \
-e "drizzle-orm/node-postgres" \
-e "\"postgres\"\\s*:" \
-e "from ['\"]postgres['\"]" \
-e "require\\(['\"]postgres['\"]\\)" \
.Repository: QueueLab/QCX
Length of output: 3418
Replace pg with postgres in dependencies
lib/db/index.ts and lib/db/migrate.ts import postgres, while package.json still lists pg. Remove pg and add postgres to dependencies.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 85, The dependency list is out of sync with the
database imports: `lib/db/index.ts` and `lib/db/migrate.ts` use `postgres`, but
`package.json` still depends on `pg`. Update the `dependencies` section to
remove `pg` and add `postgres` so the package matches the actual database client
used by `createDb`/migration code. Keep the existing version constraints
consistent with the rest of the project.
The .env file had a placeholder DATABASE_URL that was being loaded by Next.js env system and interfering with Vercel builds. The stub error message referenced DATABASE_URL even though the fix checked POSTGRES_URL, because Next.js loads .env files by default and DATABASE_URL was set to the placeholder value. Changes: - lib/db/index.ts: only checks POSTGRES_URL, no DATABASE_URL fallback - lib/db/migrate.ts: only checks POSTGRES_URL - drizzle.config.ts: only checks POSTGRES_URL - .env: replaced placeholder DATABASE_URL with POSTGRES_URL comment
Problem
Authenticated users were unable to preview their chats in QCX because the Clerk authentication system and the QCX-BACKEND Supabase backend were not properly synchronized. The root cause was that the
clerk_user_idcolumn on thepublic.userstable was never being populated, which caused all Clerk-aware Row Level Security (RLS) policies to fail — specifically, theis_clerk_user()function always returnedfalse, blocking all authenticated users from accessing any data.Root Cause
Three conflicting user creation paths each created user records differently without establishing the Clerk-to-Supabase ID bridge:
handle_new_usertrigger — createdpublic.userswith Supabase UUID but never setclerk_user_idclerk_user_idSince all RLS policies depend on
is_clerk_user(user_id)which checksclerk_user_id = clerk_id(), andclerk_user_idwas alwaysNULL, every RLS policy denied access.Changes
QCX-BACKEND (Supabase) — Direct SQL/Migration Fixes
sync_clerk_user()RPC function for canonical user creation/linkinghandle_new_usertrigger to extract Clerk ID from JWT metadataclerk_id()to extract fromauth.jwt() ->> 'sub'is_clerk_user()to properly checkclerk_user_idis_clerk_user()users_updated_attrigger for timestamp consistencysync_clerk_userRPCQCX (Next.js app) — Codebase Fixes
first_name,last_name,avatar_url, etc.resolveClerkUserToDbUserto sync full profile data0003_clerk_auth_sync.sqlVerification
All 6 critical database components verified as active:
sync_clerk_userfunctionclerk_idfunctionis_clerk_userfunctionclerk_user_idcolumn onuserson_auth_user_createdtriggerusers_updated_attriggerdatabase-accessv13: deployed and ACTIVEis_clerk_user()How It Works After the Fix
subclaimresolveClerkUserToDbUser()resolves the Clerk ID to the Supabase UUIDsync_clerk_user()is_clerk_user(user_id)which now correctly returnstrueSummary by CodeRabbit
New Features
Bug Fixes
Chores