feat: add Clerk auth to marketing site and improve web app UI#340
feat: add Clerk auth to marketing site and improve web app UI#340saddlepaddle merged 9 commits intomainfrom
Conversation
|
Warning Rate limit exceeded@saddlepaddle has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (17)
WalkthroughMigrates authentication to Clerk across apps (admin, web, marketing, api), adds Clerk middleware and webhooks, updates TRPC context/procedures to use Clerk sessions and clerkId, introduces user management UI and DB migrations for clerk_id/deleted_at, replaces lucide-react with react-icons, and updates environment/CI configs accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Middleware as Clerk Middleware
participant ClerkAPI as Clerk Backend
participant NextApp as Next.js App
participant API as tRPC/API
participant DB as Database
Browser->>Middleware: Request (any protected route)
Middleware->>ClerkAPI: validate session
ClerkAPI-->>Middleware: session (userId) / no session
alt authenticated
Middleware->>NextApp: forward with session
NextApp->>API: trpc.user.me() (uses session.userId)
API->>DB: SELECT user WHERE clerk_id = session.userId
DB-->>API: user record
API-->>NextApp: user data
NextApp-->>Browser: render authenticated UI
else unauthenticated
Middleware-->>Browser: redirect to sign-in (or allow public route)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
saddlepaddle
force-pushed
the
xeric-cat-124424
branch
from
6508596 to
3f7bd13
Compare
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 Preview resources have been processed for cleanup |
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
force-pushed
the
xeric-cat-124424
branch
from
3f7bd13 to
b781f05
Compare
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
force-pushed
the
xeric-cat-124424
branch
from
b781f05 to
02b31e0
Compare
saddlepaddle
force-pushed
the
xeric-cat-124424
branch
from
02b31e0 to
cefe25c
Compare
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/trpc/src/trpc.ts (1)
74-98: Preserve existing ctx when addinguserinadminProcedure—prevent losingsessionand follow middleware composition patterns.The current implementation overwrites
ctxentirely with onlyuser, discarding thesessionproperty passed down fromprotectedProcedure. While current admin handlers don't accessctx, this breaks the middleware composition contract and creates a latent bug: future admin handlers or procedures that depend onctx.sessionwill fail silently.export const adminProcedure = protectedProcedure.use(async ({ ctx, next }) => { const user = await db.query.users.findFirst({ where: eq(users.clerkId, ctx.session.userId), }); ... return next({ ctx: { + ...ctx, user, }, }); });apps/marketing/package.json (1)
13-30: @clerk/nextjs@6.36.2 is incompatible with Next.js 16.0.10. The package requires Next.js^13.5.7 || ^14.2.25 || ^15.2.3(React 18+ is satisfied by 19.2.3). Upgrade to @clerk/nextjs v7 or higher, or downgrade Next.js to v15.apps/web/package.json (1)
13-36: @clerk/nextjs v6.36.2 is not compatible with Next.js 16.0.10 + React 19.2.3.
Clerk's peerDependencies officially support React 18+ and Next.js up through v15. Using v6 with Next.js 16 will trigger peer-dependency warnings and risks runtime breakage in authentication flows (middleware, RSC). Either downgrade to Next.js 15, wait for an official Clerk v7+ release with Next.js 16 support, or force-install with--legacy-peer-depsand test thoroughly in staging.apps/admin/src/app/(dashboard)/layout.tsx (1)
17-33: ImportReactNodetype to fix undefined reference in component props.The component uses
React.ReactNodeat line 25 without importing React—while the project ignores build errors, this should be fixed for code quality. Additionally, consider wrappingtrpc.user.me.query()in error handling to defensively catch any exceptions before the null check.+import type { ReactNode } from "react"; import { redirect } from "next/navigation"; import { env } from "@/env"; import { api } from "@/trpc/server"; @@ export default async function DashboardLayout({ children, }: { - children: React.ReactNode; + children: ReactNode; }) { const trpc = await api(); - const user = await trpc.user.me.query(); + const user = await trpc.user.me.query().catch(() => null); // Redirect unauthorized users to web app if (!user) { redirect(env.NEXT_PUBLIC_WEB_URL); }packages/trpc/src/router/organization.ts (1)
60-76: Inconsistent error handling compared totask.ts.In
task.ts, when the user lookup fails, an error is thrown (throw new Error("User not found")). Here, if the user is not found, the organization is still created but the creator is silently not added as a member. This could lead to orphaned organizations without any members.Consider aligning the error handling:
.mutation(async ({ ctx, input }) => { const user = await db.query.users.findFirst({ where: eq(users.clerkId, ctx.session.userId), }); + if (!user) throw new Error("User not found"); const [organization] = await db .insert(organizations) .values(input) .returning(); - if (user && organization) { + if (organization) { await db.insert(organizationMembers).values({ organizationId: organization.id, userId: user.id, }); } return organization; }),
🧹 Nitpick comments (19)
apps/web/src/app/providers.tsx (1)
11-11: Remove redundantdefaultThemeprop.When
forcedThemeis set, thedefaultThemeprop has no effect since the forced theme always takes precedence. You can simplify the configuration.Apply this diff:
- <ThemeProvider attribute="class" defaultTheme="dark" forcedTheme="dark"> + <ThemeProvider attribute="class" forcedTheme="dark">apps/web/src/app/page.tsx (1)
31-39: Consider using Next.jsLinkfor internal navigation.Using a plain
<a>tag for internal routes causes a full page reload. The Next.jsLinkcomponent provides client-side navigation for better performance.+import Link from "next/link";- <a href="/"> + <Link href="/"> <Image src="/title.svg" alt="Superset" width={140} height={24} priority /> - </a> + </Link>apps/admin/src/app/(dashboard)/users/deleted/components/DeletedUsersTable/DeletedUsersTable.tsx (2)
51-66: Remove unused callback parameters.The
_and_variablesparameters inonSuccessare not used.const restoreMutation = useMutation( trpc.admin.restoreUser.mutationOptions({ - onSuccess: (_, _variables) => { + onSuccess: () => { queryClient.invalidateQueries({ queryKey: trpc.admin.listActiveUsers.queryKey(), });
92-116: Consider using lucide-react's AlertTriangle icon for consistency.The component already imports icons from
lucide-react. UsingAlertTriangleinstead of the inline SVG would improve consistency.-import { Loader2, RotateCcw, Trash2, UserX } from "lucide-react"; +import { AlertTriangle, Loader2, RotateCcw, Trash2, UserX } from "lucide-react";<div className="text-destructive mb-4"> - <svg - aria-hidden="true" - className="h-12 w-12" - fill="none" - viewBox="0 0 24 24" - stroke="currentColor" - > - <path - strokeLinecap="round" - strokeLinejoin="round" - strokeWidth={2} - d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z" - /> - </svg> + <AlertTriangle aria-hidden="true" className="h-12 w-12" /> </div>apps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsx (2)
91-105: Consider using lucide-react's AlertTriangle icon for consistency.Same as in DeletedUsersTable - the inline SVG could be replaced with
AlertTrianglefromlucide-reactfor consistency across both components.-import { Loader2, MoreHorizontal, Trash2, User } from "lucide-react"; +import { AlertTriangle, Loader2, MoreHorizontal, Trash2, User } from "lucide-react";Then replace the inline SVG with:
<AlertTriangle aria-hidden="true" className="h-12 w-12" />
44-245: Consider extracting shared logic with DeletedUsersTable.Both
UsersTableandDeletedUsersTableshare significant code:
- Loading/error/empty state Card patterns (Lines 77-128 here, similar in DeletedUsersTable)
- Avatar fallback logic (Lines 157-162)
- AlertDialog confirmation pattern
Extracting these to shared components would reduce duplication and ease maintenance:
- A
UserAvatarcomponent with built-in initials fallback- A
TableStateCardfor loading/error/empty states- A
ConfirmDeleteDialogcomponentapps/web/src/app/(auth)/sso-callback/layout.tsx (1)
1-7: Prefer importingReactNodeinstead of relying on globalReacttypes.This is fine as-is, but to avoid TS config surprises, consider:
+import type { ReactNode } from "react"; + export default function SSOCallbackLayout({ children, }: { - children: React.ReactNode; + children: ReactNode; }) { return <>{children}</>; }apps/web/src/app/download/page.tsx (2)
1-23: Avoid rendering “Sign Out” when signed out (or ensure/downloadis auth-protected).
If this page is reachable while signed out, the header action will be confusing.import { SignOutButton } from "@clerk/nextjs"; +import { SignedIn } from "@clerk/nextjs"; ... <div className="absolute top-4 right-4 z-10"> - <SignOutButton> - <Button variant="outline" size="sm"> - Sign Out - </Button> - </SignOutButton> + <SignedIn> + <SignOutButton> + <Button variant="outline" size="sm"> + Sign Out + </Button> + </SignOutButton> + </SignedIn> </div>
42-72: Consider making “Coming Soon” CTAs more explicit for accessibility.
Disabled buttons are OK, but atitle(or helper text) improves clarity.- <Button className="w-full" disabled> + <Button className="w-full" disabled title="Download is not available yet"> <Download className="mr-2 h-4 w-4" /> Coming Soon </Button>packages/trpc/src/trpc.ts (1)
57-72: Use a stricter type check foruserIdinstead of truthiness comparison.The current check
!ctx.session.userIdwill work because it catches bothnullandundefined, but Clerk's documentation indicates thatuserIdcan be either depending on session state (signed-out vs. pending). A stricter check makes the intent explicit and improves type safety:export const protectedProcedure = t.procedure.use(async ({ ctx, next }) => { - if (!ctx.session.userId) { + if (typeof ctx.session.userId !== "string") { throw new TRPCError({ code: "UNAUTHORIZED", message: "Not authenticated. Please sign in.", }); }This approach clarifies that
userIdmust be astringfor the procedure to continue, aligns with theSignedInAuthObjecttype extraction pattern already in use (line 12), and follows Clerk's own guidance on handling signed-out/pending sessions.packages/db/drizzle/0002_add_deleted_at_users.sql (1)
1-2: Prefertimestamptz(and consider a partial index) for soft-deletes.
If this is Postgres,timestampcan introduce timezone ambiguity;timestamptzis usually the safer default for audit fields. Also, if the main pattern isWHERE deleted_at IS NULL, a partial index ondeleted_at IS NOT NULLcan sometimes be a better fit (depends on your query patterns).-ALTER TABLE "users" ADD COLUMN "deleted_at" timestamp;--> statement-breakpoint +ALTER TABLE "users" ADD COLUMN "deleted_at" timestamptz;--> statement-breakpoint CREATE INDEX "users_deleted_at_idx" ON "users" USING btree ("deleted_at");apps/marketing/src/app/components/AuthButtons/AuthButtons.tsx (1)
22-24: Make theDownloadicon decorative for screen readers.- <Download className="size-4" /> + <Download className="size-4" aria-hidden="true" focusable="false" /> ... - <Download className="size-4" /> + <Download className="size-4" aria-hidden="true" focusable="false" />Also applies to: 41-43
turbo.jsonc (1)
4-18: Consider scopingCLERK_WEBHOOK_SECRETandBLOB_READ_WRITE_TOKENto theapiapp instead ofglobalEnv. These secrets are only used byapps/api(for webhook handling), while other apps don't need them. Keeping them inglobalEnvunnecessarily invalidates the cache for unrelated tasks. The other variables (CLERK_SECRET_KEY,NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY, and the public URLs) are legitimately shared across multiple apps (admin, web, marketing, api) and belong inglobalEnv.apps/marketing/src/app/terms-of-service/page.tsx (1)
11-356: Hardcoded “Last updated” date can drift from actual edits. Consider sourcing it from a single constant (or metadata) so future updates don’t miss it.apps/web/src/app/layout.tsx (1)
30-35: Viewport theme color doesn’t match the app’s dark background. If you’re standardizing dark mode tooklch(0.178 0 0), consider using a matching hex forthemeColor(so browser UI matches on mobile).apps/web/src/proxy.ts (1)
14-20: Consider usingisPublicRoutematcher for consistency.The manual
pathname.startsWithchecks for/sign-inand/sign-upwork correctly but are inconsistent with theisPublicRoutematcher defined above. Note that/sso-callbackis intentionally excluded from this redirect (allowing authenticated users to complete OAuth flows), which is correct.For better maintainability, consider refactoring to:
- if ( - userId && - (req.nextUrl.pathname.startsWith("/sign-in") || - req.nextUrl.pathname.startsWith("/sign-up")) - ) { + const isAuthPage = createRouteMatcher(["/sign-in(.*)", "/sign-up(.*)"]); + if (userId && isAuthPage(req)) { return NextResponse.redirect(new URL("/", req.url)); }apps/admin/src/env.ts (1)
22-23: Consider validating NEXT_PUBLIC_COOKIE_DOMAIN format.The cookie domain should follow specific format constraints (e.g., valid domain characters, optional leading dot). Consider adding validation.
Apply this diff to add domain validation:
- NEXT_PUBLIC_COOKIE_DOMAIN: z.string(), + NEXT_PUBLIC_COOKIE_DOMAIN: z.string().regex(/^\.?[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*$/i, { + message: "Invalid cookie domain format" + }),apps/marketing/src/env.ts (1)
17-19: Consider validating NEXT_PUBLIC_COOKIE_DOMAIN format.Similar to the admin app, the cookie domain should follow specific format constraints.
Apply this diff to add domain validation:
- NEXT_PUBLIC_COOKIE_DOMAIN: z.string(), + NEXT_PUBLIC_COOKIE_DOMAIN: z.string().regex(/^\.?[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*$/i, { + message: "Invalid cookie domain format" + }),apps/admin/src/app/(dashboard)/components/AppSidebar/components/NavUser/NavUser.tsx (1)
39-41: Consider error handling for sign-out.The
handleSignOutfunction doesn't handle potential errors from the ClerksignOutoperation.Apply this diff to add error handling:
- const handleSignOut = () => signOut({ redirectUrl: env.NEXT_PUBLIC_WEB_URL }); + const handleSignOut = async () => { + try { + await signOut({ redirectUrl: env.NEXT_PUBLIC_WEB_URL }); + } catch (error) { + console.error("Sign out failed:", error); + // Optionally show a toast notification + } + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
apps/web/public/assets/auth-bg-dark.jpgis excluded by!**/*.jpgapps/web/public/assets/auth-bg-light.jpgis excluded by!**/*.jpgapps/web/public/assets/icon-dark.pngis excluded by!**/*.pngapps/web/public/assets/icon-light.pngis excluded by!**/*.pngapps/web/public/assets/social/google.svgis excluded by!**/*.svgapps/web/public/hero/change-themes.gifis excluded by!**/*.gifapps/web/public/hero/manage-terminals.gifis excluded by!**/*.gifapps/web/public/hero/open-worktrees.gifis excluded by!**/*.gifapps/web/public/hero/use-agents.gifis excluded by!**/*.gifapps/web/public/title.svgis excluded by!**/*.svgbun.lockis excluded by!**/*.lock
📒 Files selected for processing (71)
apps/admin/package.json(2 hunks)apps/admin/src/app/(dashboard)/components/AppSidebar/AppSidebar.tsx(2 hunks)apps/admin/src/app/(dashboard)/components/AppSidebar/components/NavUser/NavUser.tsx(5 hunks)apps/admin/src/app/(dashboard)/layout.tsx(2 hunks)apps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsx(1 hunks)apps/admin/src/app/(dashboard)/users/components/UsersTable/index.ts(1 hunks)apps/admin/src/app/(dashboard)/users/deleted/components/DeletedUsersTable/DeletedUsersTable.tsx(1 hunks)apps/admin/src/app/(dashboard)/users/deleted/components/DeletedUsersTable/index.ts(1 hunks)apps/admin/src/app/(dashboard)/users/deleted/page.tsx(1 hunks)apps/admin/src/app/(dashboard)/users/page.tsx(1 hunks)apps/admin/src/app/layout.tsx(2 hunks)apps/admin/src/app/providers.tsx(1 hunks)apps/admin/src/env.ts(1 hunks)apps/admin/src/lib/auth/client.tsx(0 hunks)apps/admin/src/lib/auth/index.ts(0 hunks)apps/admin/src/lib/auth/server.ts(0 hunks)apps/admin/src/lib/auth/types.ts(0 hunks)apps/admin/src/proxy.ts(2 hunks)apps/admin/src/trpc/react.tsx(1 hunks)apps/api/next.config.ts(1 hunks)apps/api/package.json(1 hunks)apps/api/src/app/api/trpc/[trpc]/route.ts(1 hunks)apps/api/src/app/api/webhooks/clerk/route.ts(1 hunks)apps/api/src/env.ts(1 hunks)apps/api/src/proxy.ts(1 hunks)apps/api/src/trpc/context.ts(1 hunks)apps/marketing/package.json(1 hunks)apps/marketing/src/app/components/AuthButtons/AuthButtons.tsx(1 hunks)apps/marketing/src/app/components/AuthButtons/index.ts(1 hunks)apps/marketing/src/app/components/Header/Header.tsx(1 hunks)apps/marketing/src/app/globals.css(0 hunks)apps/marketing/src/app/layout.tsx(2 hunks)apps/marketing/src/app/page.tsx(1 hunks)apps/marketing/src/app/privacy-policy/page.tsx(1 hunks)apps/marketing/src/app/scripts/page.tsx(1 hunks)apps/marketing/src/app/terms-of-service/page.tsx(1 hunks)apps/marketing/src/env.ts(1 hunks)apps/marketing/src/proxy.ts(1 hunks)apps/web/package.json(2 hunks)apps/web/src/app/(auth)/layout.tsx(1 hunks)apps/web/src/app/(auth)/sign-in/[[...sign-in]]/page.tsx(1 hunks)apps/web/src/app/(auth)/sign-up/[[...sign-up]]/page.tsx(1 hunks)apps/web/src/app/(auth)/sso-callback/layout.tsx(1 hunks)apps/web/src/app/(auth)/sso-callback/page.tsx(1 hunks)apps/web/src/app/download/page.tsx(1 hunks)apps/web/src/app/globals.css(1 hunks)apps/web/src/app/layout.tsx(2 hunks)apps/web/src/app/page.tsx(1 hunks)apps/web/src/app/providers.tsx(1 hunks)apps/web/src/env.ts(1 hunks)apps/web/src/proxy.ts(1 hunks)apps/web/src/trpc/react.tsx(1 hunks)biome.jsonc(1 hunks)packages/db/drizzle/0001_add_clerk_auth.sql(1 hunks)packages/db/drizzle/0002_add_deleted_at_users.sql(1 hunks)packages/db/drizzle/meta/0001_snapshot.json(1 hunks)packages/db/drizzle/meta/0002_snapshot.json(1 hunks)packages/db/drizzle/meta/_journal.json(1 hunks)packages/db/src/index.ts(1 hunks)packages/db/src/schema/schema.ts(1 hunks)packages/trpc/package.json(1 hunks)packages/trpc/src/env.ts(1 hunks)packages/trpc/src/index.ts(1 hunks)packages/trpc/src/root.ts(1 hunks)packages/trpc/src/router/admin.ts(1 hunks)packages/trpc/src/router/organization.ts(2 hunks)packages/trpc/src/router/task.ts(2 hunks)packages/trpc/src/router/user.ts(1 hunks)packages/trpc/src/trpc.ts(3 hunks)packages/ui/src/globals.css(1 hunks)turbo.jsonc(1 hunks)
💤 Files with no reviewable changes (5)
- apps/admin/src/lib/auth/server.ts
- apps/marketing/src/app/globals.css
- apps/admin/src/lib/auth/types.ts
- apps/admin/src/lib/auth/index.ts
- apps/admin/src/lib/auth/client.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting, running at root level for speed
Files:
packages/trpc/package.jsonapps/api/src/trpc/context.tsapps/admin/src/app/(dashboard)/users/components/UsersTable/index.tsapps/admin/src/app/(dashboard)/users/deleted/components/DeletedUsersTable/index.tsapps/marketing/package.jsonpackages/trpc/src/index.tsapps/marketing/src/app/components/AuthButtons/AuthButtons.tsxapps/marketing/src/proxy.tsapps/marketing/src/app/components/AuthButtons/index.tsapps/api/src/app/api/webhooks/clerk/route.tsapps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsxpackages/db/src/index.tsapps/admin/package.jsonapps/web/src/app/(auth)/sign-up/[[...sign-up]]/page.tsxapps/admin/src/app/(dashboard)/layout.tsxapps/admin/src/app/(dashboard)/users/deleted/components/DeletedUsersTable/DeletedUsersTable.tsxpackages/db/src/schema/schema.tsapps/admin/src/app/(dashboard)/users/deleted/page.tsxapps/web/package.jsonapps/marketing/src/app/components/Header/Header.tsxapps/api/package.jsonapps/admin/src/app/providers.tsxpackages/db/drizzle/meta/0001_snapshot.jsonapps/web/src/app/(auth)/layout.tsxpackages/trpc/src/router/task.tsapps/web/src/trpc/react.tsxapps/marketing/src/app/page.tsxpackages/db/drizzle/meta/_journal.jsonapps/web/src/app/providers.tsxapps/web/src/proxy.tsapps/admin/src/app/(dashboard)/users/page.tsxapps/web/src/app/download/page.tsxapps/admin/src/app/layout.tsxapps/admin/src/trpc/react.tsxapps/web/src/app/(auth)/sign-in/[[...sign-in]]/page.tsxapps/marketing/src/app/privacy-policy/page.tsxapps/marketing/src/app/layout.tsxpackages/trpc/src/router/admin.tspackages/trpc/src/root.tspackages/db/drizzle/meta/0002_snapshot.jsonapps/marketing/src/app/scripts/page.tsxapps/api/src/proxy.tsapps/web/src/app/(auth)/sso-callback/layout.tsxapps/api/src/env.tsapps/api/next.config.tsapps/web/src/app/layout.tsxapps/web/src/app/(auth)/sso-callback/page.tsxapps/marketing/src/app/terms-of-service/page.tsxapps/admin/src/env.tspackages/trpc/src/router/organization.tsapps/admin/src/proxy.tsapps/admin/src/app/(dashboard)/components/AppSidebar/AppSidebar.tsxapps/api/src/app/api/trpc/[trpc]/route.tspackages/trpc/src/router/user.tsapps/web/src/app/page.tsxpackages/trpc/src/env.tsapps/marketing/src/env.tsapps/admin/src/app/(dashboard)/components/AppSidebar/components/NavUser/NavUser.tsxpackages/trpc/src/trpc.tsapps/web/src/env.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid
anytype and prioritize type safety in TypeScript code
Files:
apps/api/src/trpc/context.tsapps/admin/src/app/(dashboard)/users/components/UsersTable/index.tsapps/admin/src/app/(dashboard)/users/deleted/components/DeletedUsersTable/index.tspackages/trpc/src/index.tsapps/marketing/src/app/components/AuthButtons/AuthButtons.tsxapps/marketing/src/proxy.tsapps/marketing/src/app/components/AuthButtons/index.tsapps/api/src/app/api/webhooks/clerk/route.tsapps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsxpackages/db/src/index.tsapps/web/src/app/(auth)/sign-up/[[...sign-up]]/page.tsxapps/admin/src/app/(dashboard)/layout.tsxapps/admin/src/app/(dashboard)/users/deleted/components/DeletedUsersTable/DeletedUsersTable.tsxpackages/db/src/schema/schema.tsapps/admin/src/app/(dashboard)/users/deleted/page.tsxapps/marketing/src/app/components/Header/Header.tsxapps/admin/src/app/providers.tsxapps/web/src/app/(auth)/layout.tsxpackages/trpc/src/router/task.tsapps/web/src/trpc/react.tsxapps/marketing/src/app/page.tsxapps/web/src/app/providers.tsxapps/web/src/proxy.tsapps/admin/src/app/(dashboard)/users/page.tsxapps/web/src/app/download/page.tsxapps/admin/src/app/layout.tsxapps/admin/src/trpc/react.tsxapps/web/src/app/(auth)/sign-in/[[...sign-in]]/page.tsxapps/marketing/src/app/privacy-policy/page.tsxapps/marketing/src/app/layout.tsxpackages/trpc/src/router/admin.tspackages/trpc/src/root.tsapps/marketing/src/app/scripts/page.tsxapps/api/src/proxy.tsapps/web/src/app/(auth)/sso-callback/layout.tsxapps/api/src/env.tsapps/api/next.config.tsapps/web/src/app/layout.tsxapps/web/src/app/(auth)/sso-callback/page.tsxapps/marketing/src/app/terms-of-service/page.tsxapps/admin/src/env.tspackages/trpc/src/router/organization.tsapps/admin/src/proxy.tsapps/admin/src/app/(dashboard)/components/AppSidebar/AppSidebar.tsxapps/api/src/app/api/trpc/[trpc]/route.tspackages/trpc/src/router/user.tsapps/web/src/app/page.tsxpackages/trpc/src/env.tsapps/marketing/src/env.tsapps/admin/src/app/(dashboard)/components/AppSidebar/components/NavUser/NavUser.tsxpackages/trpc/src/trpc.tsapps/web/src/env.ts
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.{ts,tsx}: Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Use one component per file (no multi-component files)
Files:
apps/admin/src/app/(dashboard)/users/components/UsersTable/index.tsapps/admin/src/app/(dashboard)/users/deleted/components/DeletedUsersTable/index.tsapps/marketing/src/app/components/AuthButtons/AuthButtons.tsxapps/marketing/src/app/components/AuthButtons/index.tsapps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsxapps/admin/src/app/(dashboard)/users/deleted/components/DeletedUsersTable/DeletedUsersTable.tsxapps/marketing/src/app/components/Header/Header.tsxapps/admin/src/app/(dashboard)/components/AppSidebar/AppSidebar.tsxapps/admin/src/app/(dashboard)/components/AppSidebar/components/NavUser/NavUser.tsx
packages/db/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations in the
packages/dbpackage
Files:
packages/db/src/index.tspackages/db/src/schema/schema.ts
🧠 Learnings (9)
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx,js,jsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/api/src/trpc/context.tspackages/trpc/src/index.tsapps/admin/src/app/(dashboard)/layout.tsxapps/web/src/trpc/react.tsxpackages/trpc/src/root.tsapps/api/src/app/api/trpc/[trpc]/route.tspackages/trpc/src/trpc.ts
📚 Learning: 2025-12-12T05:45:09.672Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.672Z
Learning: Applies to **/components/**/*.{ts,tsx} : Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Applied to files:
apps/marketing/package.jsonapps/admin/package.jsonapps/web/package.json
📚 Learning: 2025-12-12T05:45:09.672Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.672Z
Learning: Applies to packages/ui/**/*.{ts,tsx} : Use React + TailwindCSS v4 + shadcn/ui for UI components in `packages/ui`
Applied to files:
apps/marketing/package.jsonapps/admin/package.jsonapps/web/package.jsonapps/web/src/app/globals.css
📚 Learning: 2025-12-12T05:45:09.672Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.672Z
Learning: Applies to **/*.{ts,tsx,js,jsx,json} : Use Biome for code formatting and linting, running at root level for speed
Applied to files:
biome.jsonc
📚 Learning: 2025-12-12T05:45:09.672Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.672Z
Learning: Applies to packages/db/src/**/*.{ts,tsx} : Use Drizzle ORM for all database operations in the `packages/db` package
Applied to files:
packages/db/src/index.tspackages/trpc/src/router/task.tspackages/trpc/src/router/organization.tspackages/trpc/src/router/user.ts
📚 Learning: 2025-12-12T05:45:09.672Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.672Z
Learning: Applies to **/components/**/*.{ts,tsx} : Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Applied to files:
apps/marketing/src/app/scripts/page.tsx
📚 Learning: 2025-12-12T05:45:09.673Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.673Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in apps/desktop/src/main/index.ts with override: true before any imports
Applied to files:
apps/api/src/env.tsapps/marketing/src/env.tsapps/web/src/env.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Please use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/admin/src/app/(dashboard)/components/AppSidebar/AppSidebar.tsx
📚 Learning: 2025-12-12T05:45:09.673Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.673Z
Learning: Applies to apps/desktop/src/main/lib/*.ts : Implement IPC handlers accepting object parameters (not positional parameters) in apps/desktop/src/main/lib/*.ts files
Applied to files:
packages/trpc/src/trpc.ts
🧬 Code graph analysis (17)
apps/api/src/app/api/webhooks/clerk/route.ts (3)
apps/web/src/env.ts (1)
env(5-43)packages/db/src/index.ts (1)
db(2-2)packages/db/src/schema/schema.ts (1)
users(15-35)
apps/admin/src/app/(dashboard)/users/deleted/page.tsx (1)
apps/admin/src/app/(dashboard)/users/deleted/components/DeletedUsersTable/DeletedUsersTable.tsx (1)
DeletedUsersTable(38-296)
apps/web/src/app/(auth)/layout.tsx (1)
apps/web/src/env.ts (1)
env(5-43)
apps/web/src/proxy.ts (2)
apps/api/src/proxy.ts (1)
config(36-43)apps/admin/src/proxy.ts (1)
config(41-46)
apps/admin/src/app/(dashboard)/users/page.tsx (2)
apps/admin/src/app/(dashboard)/users/components/UsersTable/UsersTable.tsx (1)
UsersTable(44-245)apps/admin/src/app/(dashboard)/users/components/UsersTable/index.ts (1)
UsersTable(1-1)
apps/web/src/app/download/page.tsx (2)
packages/ui/src/components/ui/button.tsx (1)
Button(60-60)packages/ui/src/components/ui/card.tsx (5)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardDescription(90-90)CardContent(91-91)
apps/admin/src/app/layout.tsx (1)
apps/admin/src/env.ts (1)
env(5-37)
apps/web/src/app/(auth)/sign-in/[[...sign-in]]/page.tsx (1)
apps/web/src/env.ts (1)
env(5-43)
packages/trpc/src/router/admin.ts (1)
packages/db/src/schema/schema.ts (1)
users(15-35)
packages/trpc/src/root.ts (6)
packages/trpc/src/index.ts (5)
appRouter(3-3)createTRPCRouter(10-10)AppRouter(2-2)RouterInputs(2-2)RouterOutputs(2-2)packages/trpc/src/trpc.ts (1)
createTRPCRouter(51-51)packages/trpc/src/router/admin.ts (1)
adminRouter(9-62)packages/trpc/src/router/organization.ts (1)
organizationRouter(8-138)packages/trpc/src/router/repository.ts (1)
repositoryRouter(8-100)packages/trpc/src/router/task.ts (1)
taskRouter(9-130)
apps/api/src/proxy.ts (4)
apps/admin/src/env.ts (1)
env(5-37)apps/web/src/env.ts (1)
env(5-43)apps/web/src/proxy.ts (1)
config(30-35)apps/admin/src/proxy.ts (1)
config(41-46)
apps/web/src/app/layout.tsx (2)
apps/web/src/env.ts (1)
env(5-43)packages/ui/src/lib/utils.ts (1)
cn(4-6)
packages/trpc/src/router/organization.ts (1)
packages/db/src/schema/schema.ts (3)
users(15-35)organizations(40-55)organizationMembers(60-80)
apps/admin/src/proxy.ts (4)
apps/admin/src/env.ts (1)
env(5-37)packages/db/src/index.ts (1)
db(2-2)packages/db/src/schema/schema.ts (1)
users(15-35)packages/shared/src/constants.ts (1)
COMPANY(2-6)
apps/admin/src/app/(dashboard)/components/AppSidebar/AppSidebar.tsx (2)
packages/trpc/src/root.ts (1)
RouterOutputs(20-20)packages/trpc/src/index.ts (1)
RouterOutputs(2-2)
apps/api/src/app/api/trpc/[trpc]/route.ts (1)
packages/trpc/src/root.ts (1)
appRouter(10-16)
apps/admin/src/app/(dashboard)/components/AppSidebar/components/NavUser/NavUser.tsx (4)
packages/trpc/src/root.ts (1)
RouterOutputs(20-20)packages/trpc/src/index.ts (1)
RouterOutputs(2-2)apps/admin/src/env.ts (1)
env(5-37)packages/ui/src/components/ui/avatar.tsx (1)
AvatarImage(53-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (61)
packages/ui/src/globals.css (1)
79-81: Dark theme background color standardized tooklch(0.178 0 0).The dark background is slightly lighter than before (lightness increased from 0.145 to 0.178). This aligns with the PR objective to standardize dark mode colors across the marketing and web apps.
Please verify that this color value is consistently applied across marketing and web app components and matches your design specifications.
biome.jsonc (1)
8-10: Good addition of file scoping to Biome config.The new
filesconfiguration appropriately includes all files while excluding thedrizzledirectory. This aligns with best practices for excluding auto-generated or database schema files from linting/formatting, reducing unnecessary processing in CI pipelines.Verify that the
**/drizzleexclusion is intentional (e.g., Drizzle manages auto-generated schema files). If other directories with auto-generated or vendored code exist (e.g.,.next,dist), consider whether they should also be excluded.apps/web/src/app/providers.tsx (1)
11-11: Verify accessibility implications of forcing dark theme.Forcing dark mode removes user choice and could create accessibility barriers for users with photosensitivity, visual impairments, or those who work in bright environments. While the PR objectives indicate this is intentional for design consistency, please confirm:
- Have you considered the accessibility impact on users who may need light mode?
- Is there a plan to support theme preferences in the future?
- Have you tested this with users who have visual accessibility needs?
apps/web/src/trpc/react.tsx (1)
50-55: Don’t blindly overrideoptions.credentials, and double-check cross-origin CORS behavior.If
env.NEXT_PUBLIC_API_URLcan be cross-origin,credentials: "include"requires explicit server CORS support (no*origin +Access-Control-Allow-Credentials: true) or requests will fail. Also, consider preserving an explicitly-setoptions.credentialsto avoid surprising callers.- fetch(url, options) { - return fetch(url, { - ...options, - credentials: "include", - }); - }, + fetch(url, options) { + return fetch(url, { + ...options, + credentials: options?.credentials ?? "include", + }); + },In @trpc/client v11.7.1, does httpBatchStreamLink’s fetch override receive standard RequestInfo + RequestInit, and will setting RequestInit.credentials to "include" reliably propagate to all batched/streamed requests?apps/web/src/app/page.tsx (1)
107-124: LGTM!Using plain
<a>tags for cross-app navigation to the marketing site is appropriate. The environment variable usage for constructing URLs is correct.apps/admin/src/app/(dashboard)/users/deleted/page.tsx (1)
1-16: LGTM!Clean page structure with proper barrel import and consistent layout with the Users page.
apps/admin/src/app/(dashboard)/users/components/UsersTable/index.ts (1)
1-1: LGTM!Proper barrel export following the project's component folder conventions.
apps/admin/src/app/(dashboard)/users/page.tsx (1)
1-15: LGTM!Clean page structure consistent with the DeletedUsersPage pattern.
apps/admin/src/app/(dashboard)/users/deleted/components/DeletedUsersTable/index.ts (1)
1-1: LGTM!Proper barrel export following the project's component folder conventions.
apps/admin/src/app/providers.tsx (1)
8-16: Forced dark theme is consistent—double-check UX intent + next-themes behavior.If admin should never respect system/user theme, this is good. Please just confirm there’s no expectation of persisted preference anywhere else (since forcedTheme overrides it).
apps/api/src/env.ts (1)
4-22: Confirm these env vars are truly required in every runtime (or make optional + gate).
CLERK_SECRET_KEYis likely required, butCLERK_WEBHOOK_SECRETandBLOB_READ_WRITE_TOKENmay be route-specific. If some environments don’t use those routes, consider making them optional and validating only where needed to avoid blocking boot.apps/api/next.config.ts (1)
3-7: CORS moved to runtime: verify OPTIONS + credentialed flows are fully covered.Please ensure route-level CORS handles:
OPTIONSpreflight for/api/trpc(and any other cross-origin endpoints),Access-Control-Allow-Credentialswith a non-*origin,- consistent behavior across Node/Edge runtimes if applicable.
packages/db/src/index.ts (1)
1-4: Re-exporting Drizzle operators improves DX; no conflicts or version concerns identified.These operator re-exports provide convenient single-import access to common query builders. Verification confirms no naming collisions across the package exports (schema, utils, and drizzle-orm all export distinct symbols), drizzle-orm is pinned consistently at
0.45.1across all direct consumers, and the pattern is already in use (e.g.,apps/admin/src/proxy.tsimportseqfrom@superset/db). This approach aligns with the package's role as the centralized database layer.apps/admin/src/trpc/react.tsx (1)
44-56:credentials: "include"is correct for cookie auth—CORS is properly hardened.The API correctly implements credential-safe CORS via middleware:
Access-Control-Allow-Credentials: trueis set (paired with specific origins, not wildcard).Access-Control-Allow-Originis conditional: only reflects the origin if it matches the allowlist (NEXT_PUBLIC_WEB_URLorNEXT_PUBLIC_ADMIN_URL), otherwise returns empty string.- Preflight OPTIONS requests are handled.
NEXT_PUBLIC_API_URLis constrained to allowlisted admin/web origins, preventing cookie leakage to untrusted domains.No action needed; the setup is secure.
packages/trpc/package.json (1)
16-25: Version alignment is correct; server-only usage is properly implemented.
@clerk/backend^2.27.0 is consistently used across packages/trpc and apps/api, and@clerk/nextjsalso depends on the same version, ensuring no duplicate instances. Usage is properly isolated to server contexts: tRPC middleware inpackages/trpc/src/trpc.tsand webhook verification inapps/api/src/app/api/webhooks/clerk/route.ts. The dependency addition is appropriate and well-motivated (documented in trpc.ts comments explaining support for Bearer tokens from mobile/desktop clients).apps/web/src/app/globals.css (1)
4-9: @source path is correct; font variables are properly wired.The
@sourcedirective correctly points topackages/ui/src/**/*.{ts,tsx}from the monorepo structure. The font variables--font-family-sansand--font-family-monoare properly defined in@themeand correctly reference--font-interand--font-ibm-plex-mono, which are provided bylayout.tsxvia Next.js font imports. No changes needed.packages/trpc/src/env.ts (1)
3-9: Server schema cleanup is safe;MOCK_USER_IDis fully removed.No dangling references remain in the codebase. The empty
server: {}is appropriate here.apps/admin/package.json (1)
13-36: No action needed.date-fns@^4.1.0is stable and production-ready. The only function used in this codebase (formatDistanceToNow) has no breaking changes in v4; the minimal breaking changes (type-related and ESM-first orientation) do not affect this usage.apps/marketing/src/app/components/AuthButtons/index.ts (1)
1-1: Barrel export matches the components folder convention.packages/trpc/src/index.ts (1)
1-3: Nice public API improvement for type-safe tRPC consumption.apps/api/src/trpc/context.ts (1)
1-7: Fix type mismatch:auth()from@clerk/nextjs/serverreturns incompatible type.The
createTRPCContextexpectsSessionAuthObjectfrom@clerk/backend, butauth()from@clerk/nextjs/serverreturns a differentAuthobject. Per the documented tRPC setup, use@clerk/backendauth utilities instead to ensure type compatibility across web, Expo, and desktop clients. The context correctly passes{ session }, but the session type must matchSessionAuthObject.import { auth as clerkAuth } from "@clerk/backend"; import { createTRPCContext } from "@superset/trpc"; export const createContext = async () => { const session = await clerkAuth(); return createTRPCContext({ session }); };Alternatively, verify that
@clerk/nextjs/server#auth()is assignable toSessionAuthObjectif that's the intended design.⛔ Skipped due to learnings
Learnt from: CR Repo: superset-sh/superset PR: 0 File: apps/desktop/AGENTS.md:0-0 Timestamp: 2025-11-24T21:33:13.267Z Learning: Applies to apps/desktop/**/*.{ts,tsx,js,jsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`apps/api/package.json (1)
14-15: LGTM!The Clerk dependencies are appropriate for the auth integration.
@clerk/backendprovides server-side utilities and@clerk/nextjsenables Next.js-specific integration.packages/trpc/src/router/task.ts (1)
75-78: Good error handling pattern.The explicit check and error throw ensures tasks aren't created without a valid creator. Same note as
organization.ts: consider filtering out soft-deleted users (deletedAt IS NULL) if Clerk sessions can outlive user soft-deletion.apps/marketing/src/app/layout.tsx (1)
38-62: LGTM!The ClerkProvider is correctly positioned at the root level. The
domainprop for cookie sharing andisSatellite={false}configuration are appropriate for the primary marketing app. The layout structure with Header, children, and Footer is clean and maintainable.packages/db/drizzle/meta/_journal.json (1)
1-27: LGTM!The migration journal correctly tracks the new migrations for Clerk authentication (
0001_add_clerk_auth) and soft-delete support (0002_add_deleted_at_users). The sequential ordering and consistent version/breakpoint settings are appropriate.apps/marketing/src/app/components/AuthButtons/AuthButtons.tsx (1)
1-8: Avoid hardcoding/sign-inand/sign-upif Clerk URLs are env-configurable.
Clerk supportsNEXT_PUBLIC_CLERK_SIGN_IN_URLandNEXT_PUBLIC_CLERK_SIGN_UP_URLenvironment variables to configure custom sign-in/sign-up routes. Using these prevents drift and aligns with Clerk's recommended setup, especially for custom OAuth flows or satellite apps.apps/admin/src/app/layout.tsx (1)
1-9: The review comment is based on an incorrect assumption about how Clerk'sdomainprop works. According to Clerk's official documentation, thedomainprop is only used and required whenisSatelliteistrue(for multi-domain/satellite setups). WhenisSatelliteisfalse, thedomainprop is ignored by Clerk and has no effect.The code explicitly sets
isSatellite={false}on line 32, meaning thedomainprop is ignored at runtime regardless of the format or value ofNEXT_PUBLIC_COOKIE_DOMAIN. The format concern about leading dots (e.g.,.example.com) is irrelevant in this context since Clerk does not process this prop when not in satellite mode.No action is required.
Likely an incorrect or invalid review comment.
apps/web/src/app/(auth)/layout.tsx (1)
11-25: Consider usingnext/linkonly if you want client-side navigation/prefetch; otherwise this is fine for an external URL.
If this link ever becomestarget="_blank", please addrel="noopener noreferrer"(currently not needed since notarget).apps/marketing/src/app/page.tsx (1)
1-17: Re-check whether\"use client\"is actually required for this page. If all client-only usage is inside the section components already, removing\"use client\"here can reduce JS shipped.apps/api/src/app/api/trpc/[trpc]/route.ts (1)
1-14: ConfirmcreateContextsignature matches@trpc/server/adapters/fetchexpectations. IfcreateContextwas previously(opts: { headers }) => ..., you may need a wrapper(opts) => createContext(opts)or an adapter-specific shim. Also consider reducing prod log verbosity inonErrorto avoid leaking sensitive internals.apps/web/src/app/layout.tsx (1)
43-50: [rewritten comment]
[classification tag]apps/web/src/env.ts (3)
16-16: LGTM!The Clerk secret key configuration is correct. The validator relies on
emptyStringAsUndefinedto handle empty strings, which is the proper pattern per the PR's environment cleanup approach.
23-26: LGTM!The client-side Clerk configuration looks correct:
- All variables are properly prefixed with
NEXT_PUBLIC_- Default values for sign-in/sign-up URLs are sensible
- Cookie domain configuration is appropriately included
34-38: LGTM!The runtime environment shim correctly propagates all Clerk-related client variables, ensuring they're accessible at runtime in the Next.js app.
packages/trpc/src/root.ts (3)
1-3: LGTM!The type imports and admin router integration follow the established pattern used by other routers in the codebase.
11-11: LGTM!The admin router is correctly integrated into the main app router, following the same pattern as organization, repository, task, and user routers.
19-20: LGTM!Exporting
RouterInputsandRouterOutputstypes provides type-safe access to all router input and output schemas for consuming code, which is a best practice for TRPC applications.apps/marketing/src/app/components/Header/Header.tsx (2)
7-11: LGTM!The refactor to accept
authButtonsas a prop improves composability and separates concerns. The server component can now control authentication UI while the client component handles interactivity.
48-48: LGTM!The
authButtonsprop is correctly rendered alongside the social links, maintaining the right-side action cluster pattern.apps/web/src/app/(auth)/sign-up/[[...sign-up]]/page.tsx (3)
15-24: LGTM!The Google OAuth sign-up flow is correctly implemented:
- Checks
isLoadedbefore proceeding- Sets loading state to prevent double-submission
- Uses appropriate redirect URLs for the OAuth callback flow
37-51: LGTM!The sign-up button correctly:
- Disables during loading and before Clerk is ready
- Shows loading feedback to the user
- Uses the UI package's Button component consistently
52-72: LGTM!The Terms of Service and Privacy Policy links are correctly implemented with:
- Proper
target="_blank"andrel="noopener noreferrer"for security- Dynamic URLs from environment configuration
- Clear user communication about agreement
apps/marketing/src/app/scripts/page.tsx (1)
22-163: LGTM!The removal of Header and Footer from this page is correct. These components are now rendered by the centralized layout in
apps/marketing/src/app/layout.tsx, which reduces duplication and provides consistent chrome across all marketing pages.apps/web/src/proxy.ts (3)
4-8: LGTM!The public route matcher correctly includes the sign-in, sign-up, and SSO callback routes that should be accessible without authentication.
22-25: LGTM!The unauthenticated user redirect logic is correct, using the
isPublicRoutematcher to allow access to auth pages and callbacks while redirecting all other routes to sign-in.
30-35: LGTM!The middleware matcher configuration correctly:
- Excludes Next.js internals and static files
- Includes all dynamic routes
- Explicitly targets API and TRPC routes
This matches the pattern used in other apps in the monorepo.
packages/db/src/schema/schema.ts (3)
19-19: LGTM!The
clerkIdfield is correctly configured as:
- Non-nullable (required for user identification)
- Unique (prevents duplicate Clerk users)
- Indexed via
users_clerk_id_idxfor efficient auth lookups
23-23: LGTM!The
deletedAttimestamp field correctly implements the soft delete pattern. Being nullable allowsNULLto represent active users and a timestamp to represent soft-deleted users.
30-34: LGTM!The indexes are appropriately defined:
users_email_idxon email for existing lookupsusers_clerk_id_idxon clerkId for authentication queriesusers_deleted_at_idxon deletedAt for filtering active/deleted usersThese indexes will improve query performance for common access patterns in the admin router and authentication flows.
packages/db/drizzle/meta/0001_snapshot.json (1)
1-714: Snapshot version correctly matches migration state.The snapshot file reflects the database state after the
0001_add_clerk_auth.sqlmigration. It correctly includesclerk_idin the users table with the unique constraint and index, and does not contain thedeleted_atfield (which is added in the subsequent0002_add_deleted_at_users.sqlmigration). The migration sequence is properly ordered and aligned.packages/db/drizzle/meta/0002_snapshot.json (1)
1-735: LGTM! Database snapshot aligns with Clerk integration.The snapshot correctly defines the schema with Clerk-related fields (
clerk_id,deleted_at) and appropriate indexes to support authentication and soft-delete queries.apps/admin/src/env.ts (1)
16-16: LGTM! Clerk secret key configuration is correct.The required
CLERK_SECRET_KEYproperly replaces the mock authentication setup. The removal of.min(1)is appropriate givenemptyStringAsUndefined: truehandles empty strings.packages/trpc/src/router/admin.ts (3)
10-15: LGTM! Active users query is correct.The query properly filters users by
deletedAt IS NULLand orders by creation date.
17-22: LGTM! Deleted users query is correct.The query properly filters users by
deletedAt IS NOT NULLand orders by deletion date.
24-41: LGTM! Restore user mutation handles edge cases.The mutation properly validates input, updates the user, and throws NOT_FOUND when appropriate.
apps/admin/src/app/(dashboard)/components/AppSidebar/AppSidebar.tsx (1)
3-3: LGTM! Type alias from TRPC outputs improves type safety.Using
RouterOutputs["user"]["me"]withNonNullableensures strong typing aligned with your TRPC API contract and eliminates dependence on manually maintained type definitions.Also applies to: 38-38
apps/admin/src/proxy.ts (1)
17-23: LGTM! Middleware structure and public route handling is correct.The Clerk middleware wrapper and public route bypass logic are properly implemented.
apps/marketing/src/app/privacy-policy/page.tsx (1)
11-20: LGTM! Page structure and styling are consistent.The self-contained layout with dark mode background matches the PR objectives.
apps/marketing/src/env.ts (1)
12-14: LGTM! Clerk server configuration is correct.The required
CLERK_SECRET_KEYproperly enables server-side authentication for the marketing site.apps/admin/src/app/(dashboard)/components/AppSidebar/components/NavUser/NavUser.tsx (2)
3-4: LGTM! Type alias and Clerk integration are correct.Using
RouterOutputs["user"]["me"]withNonNullableensures type safety, and the Clerk hook provides proper sign-out functionality.Also applies to: 29-31
58-61: LGTM! Avatar URL updates align with schema changes.The change from
imageUrltoavatarUrlmatches the database schema, and the null handling with?? undefinedis appropriate for theAvatarImagecomponent.Also applies to: 82-85
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={() => | ||
| restoreMutation.mutate({ userId: user.id }) | ||
| } | ||
| disabled={restoreMutation.isPending} | ||
| > | ||
| {restoreMutation.isPending ? ( | ||
| <Loader2 className="h-4 w-4 animate-spin" /> | ||
| ) : ( | ||
| <RotateCcw className="h-4 w-4" /> | ||
| )} | ||
| <span className="ml-2">Restore</span> | ||
| </Button> |
There was a problem hiding this comment.
Restore spinner/disabled state affects all rows simultaneously.
restoreMutation.isPending is true for the entire mutation, not per-user. When restoring one user, all restore buttons show the spinner and become disabled. Use the mutation's variables to conditionally render per-row:
<Button
variant="outline"
size="sm"
onClick={() =>
restoreMutation.mutate({ userId: user.id })
}
- disabled={restoreMutation.isPending}
+ disabled={restoreMutation.isPending && restoreMutation.variables?.userId === user.id}
>
- {restoreMutation.isPending ? (
+ {restoreMutation.isPending && restoreMutation.variables?.userId === user.id ? (
<Loader2 className="h-4 w-4 animate-spin" />
) : (
<RotateCcw className="h-4 w-4" />
)}
<span className="ml-2">Restore</span>
</Button>📝 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.
| <Button | |
| variant="outline" | |
| size="sm" | |
| onClick={() => | |
| restoreMutation.mutate({ userId: user.id }) | |
| } | |
| disabled={restoreMutation.isPending} | |
| > | |
| {restoreMutation.isPending ? ( | |
| <Loader2 className="h-4 w-4 animate-spin" /> | |
| ) : ( | |
| <RotateCcw className="h-4 w-4" /> | |
| )} | |
| <span className="ml-2">Restore</span> | |
| </Button> | |
| <Button | |
| variant="outline" | |
| size="sm" | |
| onClick={() => | |
| restoreMutation.mutate({ userId: user.id }) | |
| } | |
| disabled={restoreMutation.isPending && restoreMutation.variables?.userId === user.id} | |
| > | |
| {restoreMutation.isPending && restoreMutation.variables?.userId === user.id ? ( | |
| <Loader2 className="h-4 w-4 animate-spin" /> | |
| ) : ( | |
| <RotateCcw className="h-4 w-4" /> | |
| )} | |
| <span className="ml-2">Restore</span> | |
| </Button> |
🤖 Prompt for AI Agents
In
apps/admin/src/app/(dashboard)/users/deleted/components/DeletedUsersTable/DeletedUsersTable.tsx
around lines 214 to 228, the restore button uses restoreMutation.isPending
globally so when one row is restoring every row shows a spinner and is disabled;
change the condition to be per-row by checking the mutation's variables (or
store the pending user id) and only show spinner/disable when
restoreMutation.isPending && (restoreMutation.variables?.userId === user.id) —
if your mutation API doesn't expose variables, set a local pendingUserId state
when triggering restore (set pendingUserId=user.id before mutate and clear on
success/error) and use pendingUserId === user.id to control the button UI.
| <ClerkProvider domain={env.NEXT_PUBLIC_COOKIE_DOMAIN} isSatellite={false}> | ||
| <html lang="en" suppressHydrationWarning> | ||
| <body | ||
| className={cn( | ||
| "bg-background text-foreground min-h-screen font-sans antialiased", | ||
| GeistSans.variable, | ||
| GeistMono.variable, | ||
| )} | ||
| > | ||
| <Providers> | ||
| {children} | ||
| <Toaster /> | ||
| </Providers> | ||
| </body> | ||
| </html> | ||
| </ClerkProvider> |
There was a problem hiding this comment.
RootLayout provider wrapping <html> may break App Router invariants—move ClerkProvider inside <body>.
Next.js root layouts typically must return <html> as the top-level element; placing ClerkProvider above it can trigger runtime/layout issues.
Proposed change:
export default function RootLayout({
children,
}: {
children: React.ReactNode;
}) {
return (
- <ClerkProvider domain={env.NEXT_PUBLIC_COOKIE_DOMAIN} isSatellite={false}>
- <html lang="en" suppressHydrationWarning>
- <body
- className={cn(
- "bg-background text-foreground min-h-screen font-sans antialiased",
- GeistSans.variable,
- GeistMono.variable,
- )}
- >
- <Providers>
- {children}
- <Toaster />
- </Providers>
- </body>
- </html>
- </ClerkProvider>
+ <html lang="en" suppressHydrationWarning>
+ <body
+ className={cn(
+ "bg-background text-foreground min-h-screen font-sans antialiased",
+ GeistSans.variable,
+ GeistMono.variable,
+ )}
+ >
+ <ClerkProvider domain={env.NEXT_PUBLIC_COOKIE_DOMAIN} isSatellite={false}>
+ <Providers>
+ {children}
+ <Toaster />
+ </Providers>
+ </ClerkProvider>
+ </body>
+ </html>
);
}📝 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.
| <ClerkProvider domain={env.NEXT_PUBLIC_COOKIE_DOMAIN} isSatellite={false}> | |
| <html lang="en" suppressHydrationWarning> | |
| <body | |
| className={cn( | |
| "bg-background text-foreground min-h-screen font-sans antialiased", | |
| GeistSans.variable, | |
| GeistMono.variable, | |
| )} | |
| > | |
| <Providers> | |
| {children} | |
| <Toaster /> | |
| </Providers> | |
| </body> | |
| </html> | |
| </ClerkProvider> | |
| <html lang="en" suppressHydrationWarning> | |
| <body | |
| className={cn( | |
| "bg-background text-foreground min-h-screen font-sans antialiased", | |
| GeistSans.variable, | |
| GeistMono.variable, | |
| )} | |
| > | |
| <ClerkProvider domain={env.NEXT_PUBLIC_COOKIE_DOMAIN} isSatellite={false}> | |
| <Providers> | |
| {children} | |
| <Toaster /> | |
| </Providers> | |
| </ClerkProvider> | |
| </body> | |
| </html> |
🤖 Prompt for AI Agents
In apps/admin/src/app/layout.tsx around lines 32 to 47, the ClerkProvider is
wrapped outside the top-level <html> which violates Next.js App Router root
layout invariants; move ClerkProvider inside the <body> so that <html> remains
the top-level element. Update the layout to render <html lang="en"
suppressHydrationWarning> at the root, then place <body> which contains
ClerkProvider wrapping <Providers> (and the {children} and <Toaster />) so
authentication context still covers the app while preserving the required HTML
root structure.
| const { userId: clerkId } = await auth(); | ||
|
|
||
| // Get current user (throws if not authenticated) | ||
| const user = await caller.user.me(); | ||
| if (!clerkId) { | ||
| return NextResponse.redirect(new URL(env.NEXT_PUBLIC_WEB_URL)); | ||
| } | ||
|
|
||
| // Validate domain access | ||
| if (!user?.email.endsWith(COMPANY.emailDomain)) { | ||
| return NextResponse.redirect(new URL(env.NEXT_PUBLIC_WEB_URL)); | ||
| } | ||
| const user = await db.query.users.findFirst({ | ||
| where: eq(users.clerkId, clerkId), | ||
| }); |
There was a problem hiding this comment.
Critical: Soft-deleted users can access the admin panel.
The user lookup doesn't filter by deletedAt, allowing soft-deleted users to authenticate and access admin functionality.
Apply this diff to block soft-deleted users:
const user = await db.query.users.findFirst({
- where: eq(users.clerkId, clerkId),
+ where: (users, { eq, and, isNull }) =>
+ and(eq(users.clerkId, clerkId), isNull(users.deletedAt)),
});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/admin/src/proxy.ts around lines 24 to 32, the user lookup does not
exclude soft-deleted users, allowing users with deletedAt set to access admin
pages; update the DB query to include a condition that deletedAt is null (or is
not set) so only non-deleted users are returned, and handle the case where no
user is found by redirecting or denying access as before.
| if (!user?.email.endsWith(COMPANY.emailDomain)) { | ||
| return NextResponse.redirect(new URL(env.NEXT_PUBLIC_WEB_URL)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add null check before accessing user.email.
The optional chaining prevents a runtime error, but the logic could be clearer by handling the null case explicitly.
Apply this diff for explicit null handling:
- if (!user?.email.endsWith(COMPANY.emailDomain)) {
+ if (!user || !user.email.endsWith(COMPANY.emailDomain)) {
return NextResponse.redirect(new URL(env.NEXT_PUBLIC_WEB_URL));
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/admin/src/proxy.ts around lines 34 to 36, the code uses optional
chaining on user.email but doesn't explicitly handle a null/undefined user;
update the logic to first check if user is falsy or user.email is missing and,
in that case, return NextResponse.redirect(new URL(env.NEXT_PUBLIC_WEB_URL));
otherwise check that user.email.endsWith(COMPANY.emailDomain) and redirect if it
doesn't; make the null/empty email check explicit so you never call .endsWith on
an undefined value.
| async function uploadAvatar( | ||
| imageUrl: string | undefined, | ||
| userId: string, | ||
| ): Promise<string | null> { | ||
| if (!imageUrl) return null; | ||
|
|
||
| try { | ||
| const response = await fetch(imageUrl); | ||
| if (!response.ok) return null; | ||
|
|
||
| const blob = await response.blob(); | ||
| const { url } = await put(`users/${userId}/avatar.png`, blob, { | ||
| access: "public", | ||
| token: env.BLOB_READ_WRITE_TOKEN, | ||
| }); | ||
| return url; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| export async function POST(req: Request) { | ||
| try { | ||
| const evt = await verifyWebhook(req, { | ||
| signingSecret: env.CLERK_WEBHOOK_SECRET, | ||
| }); | ||
|
|
||
| if (evt.type === "user.created" || evt.type === "user.updated") { | ||
| const clerkUser = evt.data; | ||
| const primaryEmail = clerkUser.email_addresses.find( | ||
| (email) => email.id === clerkUser.primary_email_address_id, | ||
| )?.email_address; | ||
|
|
||
| if (!primaryEmail) { | ||
| return new Response("No primary email", { status: 200 }); | ||
| } | ||
|
|
||
| const name = | ||
| [clerkUser.first_name, clerkUser.last_name].filter(Boolean).join(" ") || | ||
| primaryEmail.split("@")[0] || | ||
| "User"; | ||
|
|
||
| // Insert/update user first to get the internal UUID | ||
| const [user] = await db | ||
| .insert(users) | ||
| .values({ | ||
| clerkId: clerkUser.id, | ||
| email: primaryEmail, | ||
| name, | ||
| }) | ||
| .onConflictDoUpdate({ | ||
| target: users.clerkId, | ||
| set: { | ||
| email: primaryEmail, | ||
| name, | ||
| }, | ||
| }) | ||
| .returning({ id: users.id }); | ||
|
|
||
| // Upload avatar using internal UUID, then update user | ||
| if (user) { | ||
| const avatarUrl = await uploadAvatar(clerkUser.image_url, user.id); | ||
| if (avatarUrl) { | ||
| await db | ||
| .update(users) | ||
| .set({ avatarUrl }) | ||
| .where(eq(users.id, user.id)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (evt.type === "user.deleted" && evt.data.id) { | ||
| await db.delete(users).where(eq(users.clerkId, evt.data.id)); | ||
| } | ||
|
|
||
| return new Response("Success", { status: 200 }); | ||
| } catch (err) { | ||
| console.error("Webhook verification failed:", err); | ||
| return new Response("Webhook verification failed", { status: 400 }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the users schema to verify deletedAt and email unique constraint
find . -type f \( -name "*.ts" -o -name "*.sql" \) | xargs grep -l "users\|schema" | grep -E "(schema|db|prisma|drizzle)" | head -20Repository: superset-sh/superset
Length of output: 493
🏁 Script executed:
# Look for drizzle schema definition
fd -t f "schema\|migrate" --extension ts --extension sql | head -20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for users table definition
rg -t ts "export const users" -A 20Repository: superset-sh/superset
Length of output: 2812
🏁 Script executed:
# Check if verifyWebhook is imported and what it expects
rg -t ts "verifyWebhook" -B 3 -A 3Repository: superset-sh/superset
Length of output: 884
🏁 Script executed:
# Check uploadAvatar for timeout handling
cat -n apps/api/src/app/api/webhooks/clerk/route.ts | head -30Repository: superset-sh/superset
Length of output: 1003
🏁 Script executed:
# Look for fetch timeout patterns in the codebase
rg "AbortSignal|timeout" -t ts apps/api/src --max-count 20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check for any type usage in the webhook file
rg "any" -t ts apps/api/src/app/api/webhooks/clerk/route.ts -B 2 -A 2Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for Clerk webhook handler examples or documentation in the codebase
fd -t ts "webhook" | xargs grep -l "user.created\|user.updated" 2>/dev/nullRepository: superset-sh/superset
Length of output: 235
🏁 Script executed:
# Check what imports are available from env or config
rg "BLOB_READ_WRITE_TOKEN|CLERK_WEBHOOK_SECRET" -t tsRepository: superset-sh/superset
Length of output: 347
🏁 Script executed:
# Search for Clerk webhook documentation or comments
rg "Clerk|webhook" -t ts -i --max-count 30 | grep -i "400\|500\|retry\|status"Repository: superset-sh/superset
Length of output: 179
🏁 Script executed:
# Check if there are other webhook handlers or docs
find . -type f \( -name "*.ts" -o -name "*.md" \) -path "*/webhook*" | head -20Repository: superset-sh/superset
Length of output: 110
🏁 Script executed:
# Look for any email conflict handling patterns in the codebase
rg "email.*conflict|uniqueness|duplicate" -t ts -iRepository: superset-sh/superset
Length of output: 1031
🏁 Script executed:
# Search package.json for @clerk/backend version to understand expected behavior
rg "@clerk/backend" --type jsonRepository: superset-sh/superset
Length of output: 174
🌐 Web query:
Clerk webhook handler HTTP status code retry behavior 2.27.0
💡 Result:
Short answer: Clerk (which delivers webhooks via Svix) treats any 2xx response from your handler as success and stops retries; non-2xx (4xx/5xx) or no response will be retried according to Svix’s retry schedule. See Clerk docs (Webhooks / Sync guide) and the Webhooks reference about Svix retries for details and the retry schedule link. [1][2][3]
Sources:
- Clerk docs — Sync Clerk data to your app with webhooks (explains 2xx stops retries; 4xx/5xx or no code => retried). [1]
- Clerk reference — Webhooks (notes Svix is used and links to Svix retry schedule). [2]
- Clerk debugging webhooks (common HTTP response codes / diagnostic details). [3]
Webhook handler has correctness and reliability gaps: hard delete, timeout exposure, email conflict, and error status codes.
user.deletedhard-deletes despitedeletedAtfield existing in schema; should use soft delete for data integrity and cascading relationships.fetch(imageUrl)inuploadAvatarhas no timeout, risking indefinite hangs on slow or unresponsive external sources.- Upsert only targets
clerkIdconflicts; insert fails silently onusers.emailunique constraint violation when a new Clerk user shares an email with an existing row under a differentclerkId. - Catch-all returns 400 for both verification and processing errors; should return 400 only for failed webhook signature verification and 500 for transient server errors (database, network) to enable Clerk's retry logic.
🤖 Prompt for AI Agents
In apps/api/src/app/api/webhooks/clerk/route.ts around lines 9 to 89: fix four
issues — replace hard delete on user.deleted with a soft delete (update
users.deletedAt = new Date()) so related data remains intact; make uploadAvatar
use a fetch timeout via AbortController (pass signal and abort after e.g. 5–10s)
and handle aborts/errors returning null; handle email-unique conflicts during
upsert by performing a transaction: attempt the insert/upsert by clerkId, and if
it fails with a unique constraint on users.email, lookup the existing user by
email and update its clerkId/name (or merge as appropriate) to return that user
id instead of silently failing; and adjust error handling so that signature
verification failures return 400 while all other processing errors (db, network,
upload) return 500 to allow Clerk retries, logging details for diagnostics.
| ALTER TABLE "users" ADD COLUMN "clerk_id" text NOT NULL;--> statement-breakpoint | ||
| CREATE INDEX "users_clerk_id_idx" ON "users" USING btree ("clerk_id");--> statement-breakpoint | ||
| ALTER TABLE "users" ADD CONSTRAINT "users_clerk_id_unique" UNIQUE("clerk_id"); No newline at end of file |
There was a problem hiding this comment.
Migration can fail on existing data: ADD COLUMN ... NOT NULL needs a backfill plan.
If users already has rows, Postgres will reject adding a NOT NULL column without a default. Consider a safer sequence: add nullable clerk_id, backfill, then ALTER COLUMN ... SET NOT NULL, then add the unique constraint (and only after ensuring uniqueness).
🤖 Prompt for AI Agents
In packages/db/drizzle/0001_add_clerk_auth.sql around lines 1 to 3, the
migration adds clerk_id as NOT NULL which will fail if users already exist;
instead: add clerk_id as nullable, run a backfill step to populate clerk_id for
existing rows (or set a temporary default for new rows), then run ALTER TABLE
... ALTER COLUMN clerk_id SET NOT NULL once backfill is complete, and finally
create the unique constraint (and index if desired) after verifying no
duplicates; ensure the migration is split into safe transactional steps or
separate migrations to avoid failures on live data.
| permanentlyDeleteUser: adminProcedure | ||
| .input(z.object({ userId: z.string().uuid() })) | ||
| .mutation(async ({ input }) => { | ||
| const [user] = await db | ||
| .delete(users) | ||
| .where(eq(users.id, input.userId)) | ||
| .returning(); | ||
|
|
||
| if (!user) { | ||
| throw new TRPCError({ | ||
| code: "NOT_FOUND", | ||
| message: "User not found", | ||
| }); | ||
| } | ||
|
|
||
| // TODO: Delete from Clerk, delete avatar from blob storage, etc. | ||
|
|
||
| return { success: true }; | ||
| }), |
There was a problem hiding this comment.
Critical: Complete the cleanup TODO before merging.
The permanentlyDeleteUser mutation hard-deletes the user without cleaning up the Clerk account or blob storage assets. This creates orphaned resources and potential security/billing issues.
Additionally, consider:
- Verifying the user is already soft-deleted before permanent deletion
- Wrapping cleanup operations in a transaction
- The CASCADE delete on
tasks.creator_idwill remove all tasks created by this user - ensure this is intended behavior
Do you want me to generate an implementation that includes Clerk cleanup and blob storage deletion?
🤖 Prompt for AI Agents
In packages/trpc/src/router/admin.ts around lines 43 to 61, the
permanentlyDeleteUser mutation currently hard-deletes the DB user but does not
perform required external cleanup; update it to (1) verify the user is already
soft-deleted (e.g., a deleted_at or status flag) before proceeding, (2) perform
all cleanup inside a DB transaction that first deletes/returns the user row and
any related rows if needed (or confirm CASCADE behavior is intended), (3) call
Clerk API to permanently delete the Clerk account for that user, (4) delete any
avatar or other blobs from your blob storage provider, and (5) handle and log
errors/partial failures (rollback the DB transaction on failure of external
calls or implement a compensating strategy), returning a clear success/failure
response; ensure sensitive operations are permission-checked and add tests for
the full path.
| const user = await db.query.users.findFirst({ | ||
| where: eq(users.clerkId, ctx.session.userId), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check schema for deletedAt column in users table
fd -t ts -t sql . | xargs grep -l "users\|schema" | head -20Repository: superset-sh/superset
Length of output: 233
🏁 Script executed:
#!/bin/bash
# Search for deletedAt usage in the codebase
rg "deletedAt" --type ts -B 2 -A 2Repository: superset-sh/superset
Length of output: 4472
🏁 Script executed:
#!/bin/bash
# Check the users table schema definition
fd schema | xargs cat 2>/dev/null || fd "users" --type ts | grep -i schemaRepository: superset-sh/superset
Length of output: 7852
🏁 Script executed:
#!/bin/bash
# Search for other user queries to see if they filter deletedAt
rg "db\.query\.users\.findFirst|db\.query\.users\.findMany" --type ts -B 2 -A 5Repository: superset-sh/superset
Length of output: 3599
Add soft-deleted user filter to the user lookup.
The user query doesn't exclude soft-deleted users. Since admin.ts explicitly filters where: isNull(users.deletedAt) for active users, the same pattern should apply here. If a user is soft-deleted but their Clerk session remains active, they could still create organizations.
const user = await db.query.users.findFirst({
- where: eq(users.clerkId, ctx.session.userId),
+ where: and(
+ eq(users.clerkId, ctx.session.userId),
+ isNull(users.deletedAt),
+ ),
});Note: This same pattern is missing in other user lookups (task.ts, user.ts, trpc.ts adminProcedure, proxy.ts).
🤖 Prompt for AI Agents
In packages/trpc/src/router/organization.ts around lines 60 to 62, the user
lookup does not exclude soft-deleted users; update the query to add a filter
like where: and(eq(users.clerkId, ctx.session.userId), isNull(users.deletedAt))
so soft-deleted accounts cannot create organizations, and apply the same
isNull(users.deletedAt) filter pattern to other user lookups (task.ts, user.ts,
trpc.ts adminProcedure, proxy.ts).
| me: protectedProcedure.query(async ({ ctx }) => { | ||
| return db.query.users.findFirst({ | ||
| where: eq(users.id, ctx.session.userId), | ||
| }); | ||
| }), | ||
|
|
||
| all: publicProcedure.query(() => { | ||
| return db.query.users.findMany({ | ||
| orderBy: desc(users.createdAt), | ||
| where: eq(users.clerkId, ctx.session.userId), | ||
| }); | ||
| }), | ||
|
|
||
| byId: publicProcedure.input(z.string().uuid()).query(({ input }) => { | ||
| byId: protectedProcedure.input(z.string().uuid()).query(({ input }) => { | ||
| return db.query.users.findFirst({ | ||
| where: eq(users.id, input), | ||
| }); | ||
| }), |
There was a problem hiding this comment.
user.byId may leak user data to any authenticated user unless additional authorization is enforced. If this is intended to be “self only”, gate it by ctx.session.userId/users.clerkId (or move to an admin-only procedure). Also consider filtering out soft-deleted users (deletedAt IS NULL) in both me/byId.
🤖 Prompt for AI Agents
In packages/trpc/src/router/user.ts around lines 10 to 20, user.byId currently
returns any user record to any authenticated caller and neither endpoint filters
out soft-deleted rows; change byId to enforce authorization (either require
ctx.session.userId === input for self-only access or restrict the procedure to
admin roles) and add the soft-delete filter to both me and byId (e.g., include
condition deletedAt IS NULL alongside existing where clauses) so only
non-deleted users are returned and users can only fetch their own record unless
explicitly admin-authorized.
2 participants
Summary
Changes
AuthButtonsserver component, ClerkProvider, and Clerk middlewareoklch(0.178 0 0)).min(1)validators (handled byemptyStringAsUndefined)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Style
Chores
✏️ Tip: You can customize this high-level summary in your review settings.