Add user accounts, domain ownership verification, expiry notifications #137
Conversation
…tion tables in the database schema
…der with UserMenu for user authentication
…rification methods
…iration notifications
… for certificate and nameserver changes, and implement change detection logic
…ance user experience with loading states; update domains router to include domain expiry data and verification details.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request implements a comprehensive authentication and notification system. It adds Better Auth integration with magic link and OAuth providers, email infrastructure using Resend with multiple templates, database schema for users/sessions/notifications, UI components for auth flows and domain management, Inngest functions for expiration checks and change detection, and tRPC routers for domains and notifications. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Browser as Browser/Client
participant NextJS as Next.js App
participant BetterAuth as Better Auth API
participant DB as Database
participant Email as Email Service
User->>Browser: Navigate to /login
Browser->>NextJS: GET /login
NextJS-->>Browser: Login page with magic link input
User->>Browser: Enter email & submit
Browser->>NextJS: POST signIn.magicLink
NextJS->>BetterAuth: Validate & create session
BetterAuth->>DB: Store session & verification
BetterAuth->>Email: Send magic link email
Email-->>Browser: Email sent confirmation
User->>Email: Click magic link
Email->>Browser: Navigate to callback URL
Browser->>NextJS: GET /auth/callback?token=...
NextJS->>BetterAuth: Verify token
BetterAuth->>DB: Create authenticated session
BetterAuth-->>Browser: Set session cookie
Browser->>NextJS: GET /dashboard
NextJS-->>Browser: Dashboard with user context
sequenceDiagram
participant Admin as Administrator
participant Dashboard as Dashboard Page
participant TRPC as tRPC Router
participant Verification as Verification Service
participant DNS as DNS Provider
Admin->>Dashboard: Click "Add Domain"
Dashboard->>TRPC: domains.add(domain)
TRPC->>TRPC: Normalize & validate domain
TRPC->>TRPC: Generate verification token
TRPC-->>Dashboard: Return domain ID & token
Dashboard->>Dashboard: Navigate to verify page
Dashboard-->>Admin: Display DNS/Meta/File options
Admin->>Dashboard: Select DNS verification
Dashboard->>TRPC: domains.verify(method="dns")
TRPC->>Verification: verifyDnsTxt(domain, token)
Verification->>DNS: Resolve TXT records
DNS-->>Verification: Return records
Verification->>Verification: Check for domainstack-verify
Verification-->>TRPC: Verified ✓
TRPC->>TRPC: Update user_domains.verified_at
TRPC-->>Dashboard: Success
Dashboard->>Dashboard: Redirect to dashboard
sequenceDiagram
participant Inngest as Inngest Scheduler
participant Function as Check Expirations
participant DB as Database
participant Prefs as Notification System
participant Email as Email Service
Inngest->>Function: Daily trigger (09:00 UTC)
Function->>DB: Fetch verified user_domains
loop For each domain
Function->>DB: Get registration & certificate data
Function->>Function: Compute days until expiry
Function->>Prefs: Get user preferences
alt Should notify
Prefs->>Prefs: Check expiry threshold match
Prefs->>Prefs: Verify idempotency (canSendNotification)
Prefs-->>Function: Approved to send
Function->>Email: Send expiration email
Email-->>Function: Email ID
Function->>DB: Log notification
else Skip notification
Prefs-->>Function: Already notified or disabled
end
end
Function-->>Inngest: Return summary
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #137 +/- ##
==========================================
+ Coverage 68.13% 68.85% +0.71%
==========================================
Files 101 103 +2
Lines 2727 2854 +127
Branches 892 908 +16
==========================================
+ Hits 1858 1965 +107
- Misses 526 540 +14
- Partials 343 349 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 70
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (50)
.env.example(1 hunks)app/api/auth/[...all]/route.ts(1 hunks)app/api/inngest/route.ts(1 hunks)app/auth-demo/page.tsx(1 hunks)app/dashboard/page.tsx(1 hunks)app/domains/verify/[id]/page.tsx(1 hunks)app/login/magic-link-example.tsx(1 hunks)app/login/page.tsx(1 hunks)app/trpc-test/page.tsx(1 hunks)components/app-header.tsx(2 hunks)components/auth/user-menu.tsx(1 hunks)components/dashboard/add-domain-button.tsx(1 hunks)components/dashboard/domain-card-skeleton.tsx(1 hunks)components/dashboard/domain-card.tsx(1 hunks)components/dashboard/domain-list.tsx(1 hunks)components/dashboard/verification-instructions-skeleton.tsx(1 hunks)components/dashboard/verification-instructions.tsx(1 hunks)drizzle/0002_regular_purple_man.sql(1 hunks)drizzle/0003_dazzling_prima.sql(1 hunks)drizzle/0004_cuddly_thanos.sql(1 hunks)drizzle/meta/0002_snapshot.json(1 hunks)drizzle/meta/0003_snapshot.json(1 hunks)drizzle/meta/0004_snapshot.json(1 hunks)drizzle/meta/_journal.json(1 hunks)emails/certificate-changed.tsx(1 hunks)emails/certificate-expiring.tsx(1 hunks)emails/magic-link.tsx(1 hunks)emails/nameserver-changed.tsx(1 hunks)emails/registration-expiring.tsx(1 hunks)emails/reset-password.tsx(1 hunks)emails/verify-email.tsx(1 hunks)lib/auth/client.ts(1 hunks)lib/auth/config.ts(1 hunks)lib/auth/server.ts(1 hunks)lib/db/schema.ts(2 hunks)lib/db/zod.ts(2 hunks)lib/email/client.ts(1 hunks)lib/inngest/functions/check-expirations.ts(1 hunks)lib/inngest/functions/section-revalidate.ts(4 hunks)lib/notifications/detect-changes.ts(1 hunks)lib/notifications/helpers.ts(1 hunks)middleware.ts(1 hunks)package.json(4 hunks)server/routers/_app.ts(1 hunks)server/routers/domains.ts(1 hunks)server/routers/examples/email-usage.ts(1 hunks)server/routers/notifications.ts(1 hunks)server/routers/test.ts(1 hunks)server/services/domain-verification.ts(1 hunks)trpc/init.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-20T02:11:42.179Z
Learnt from: CR
PR: jakejarvis/domainstack.io#0
File: AGENTS.md:0-0
Timestamp: 2025-10-20T02:11:42.179Z
Learning: Applies to server/trpc.ts : Review server/trpc.ts when extending procedures to ensure auth and context remain intact
Applied to files:
trpc/init.tsapp/trpc-test/page.tsx
📚 Learning: 2025-10-20T02:11:42.179Z
Learnt from: CR
PR: jakejarvis/domainstack.io#0
File: AGENTS.md:0-0
Timestamp: 2025-10-20T02:11:42.179Z
Learning: Applies to **/*.test.{ts,tsx} : Mock TRPC/React Query for components like Favicon and Screenshot in tests
Applied to files:
app/trpc-test/page.tsx
🔇 Additional comments (24)
components/dashboard/domain-card-skeleton.tsx (1)
1-38: LGTM!The skeleton component provides a clean loading state with appropriate placeholder elements that match the expected domain card structure.
components/app-header.tsx (1)
2-2: LGTM!The UserMenu integration is clean and maintains consistent spacing with the existing header elements.
Also applies to: 30-32
package.json (1)
41-41: Dependencies verified as secure and stable.All five packages are on their latest stable versions with no known security vulnerabilities:
- @react-email/components (0.5.7): No vulnerabilities
- better-auth (1.3.32): All historical CVEs patched; latest version is safe
- react-email (4.3.2): No vulnerabilities
- resend (6.2.2): No vulnerabilities
- @better-auth/cli (1.3.32): No vulnerabilities
emails/verify-email.tsx (1)
81-96: Validate verificationUrl origin (security).Ensure verificationUrl is absolute HTTPS and same-origin to prevent phishing/open redirects.
I recommend asserting before send/render:
- new URL(verificationUrl)
- url.protocol === "https:"
- url.origin === process.env.APP_ORIGIN (or configured origin)
Want a PR-ready helper for this?
Also applies to: 180-182
emails/reset-password.tsx (1)
81-96: Validate resetUrl origin (security).Ensure resetUrl is absolute HTTPS and same-origin; otherwise, risk of phishing/open redirect.
Add the same assertion as the verify email template. I can provide a small utility and unit tests if helpful.
Also applies to: 160-162
app/api/inngest/route.ts (1)
3-4: checkExpirations registration verified and approved.The function is properly exported with a unique ID (
"check-expirations") and correctly registered in the Inngest serve configuration.drizzle/meta/_journal.json (1)
19-39: Snapshots verified—all entries and files are present and ordered correctly.The verification confirms that snapshot files exist for all entries (0002, 0003, 0004) and the journal sequence is consistent. No missing files detected.
server/services/domain-verification.ts (1)
38-46: The review comment does not apply to this codebase.The current implementation is already robust. DnsRecord.value is strictly defined as
z.string(), and there are no alternative field shapes (record.valuesorrecord.data). The normalizeAnswer() function for TXT records returns{ type, name, value: trimQuotes(a.data), ttl }, which always produces a string. The trimQuotes() function already removes leading/trailing quotes, handling the quoted fragments concern.The code at lines 38-46 correctly assumes
record.valueis a string and safely calls.includes()on it. No defensive normalization helper is needed.Likely an incorrect or invalid review comment.
components/dashboard/verification-instructions-skeleton.tsx (1)
4-31: LGTM!The skeleton component is well-structured and provides appropriate loading states for the verification instructions UI.
app/dashboard/page.tsx (1)
31-48: LGTM!The dashboard layout and structure are well-organized with clear heading hierarchy and proper spacing.
components/dashboard/add-domain-button.tsx (1)
58-118: LGTM!The form handling, validation, and user feedback flow are well-implemented with proper loading states and error handling.
server/routers/_app.ts (1)
2-11: LGTM!The router composition follows standard TRPC patterns and cleanly integrates the new domain, notifications, and test routers.
app/api/auth/[...all]/route.ts (1)
1-4: LGTM!The authentication route handler correctly integrates Better Auth with Next.js using the recommended pattern.
middleware.ts (1)
16-35: LGTM!The protected path authentication check is well-structured with appropriate redirect logic and clear flow.
lib/auth/client.ts (1)
6-11: LGTM!The auth client configuration is clean and properly uses client-side environment variables with a sensible fallback for local development.
emails/magic-link.tsx (2)
17-90: LGTM!The email template is well-structured with clear branding, appropriate styling, and good user guidance including security tips and expiration information.
156-158: Plain text magic link URL in email fallback is standard practice but warrants security review.The plain text URL at line 157 is confirmed as an accessibility fallback. While your implementation correctly limits exposure through single-use tokens and 5-minute expiry, plain text URLs in email can be logged in headers and forwarded. This is a common pattern for magic link emails, but verify it aligns with your security requirements—particularly if users forward emails or if your threat model includes email interception.
server/routers/test.ts (1)
7-9: Temporary test router acknowledged.Similar to the test page, this router should be removed after testing is complete. Consider tracking both test artifacts in a single cleanup issue.
drizzle/meta/0002_snapshot.json (1)
1-1539: Migration snapshot file looks comprehensive.This is a generated Drizzle ORM snapshot file that captures the database schema state. The schema includes proper foreign keys, indexes, and constraints (e.g.,
ck_cert_valid_windowensures certificate validity windows are logical).lib/db/zod.ts (1)
79-98: Zod schemas for notifications and snapshots look consistent with schema.No issues spotted.
drizzle/0003_dazzling_prima.sql (1)
1-18: Migration aligns with router flows and constraints.Enum values match code; uniques and FKs are appropriate.
drizzle/0004_cuddly_thanos.sql (1)
19-33: Verified: updated_at properly managed in all code paths.Application correctly handles
updated_attimestamps:
- Schema defines
$onUpdate(() => new Date())for automatic ORM-level management- Direct update (line 34 in server/routers/notifications.ts) explicitly sets
updatedAt: new Date()- Upsert fallback (line 51) also explicitly sets
updatedAt: new Date()drizzle/0002_regular_purple_man.sql (1)
1-15: Disregard this review comment—it is based on an architectural misunderstanding.The codebase uses Better Auth v1.3.32, a production-grade authentication library that manages the
accountandverificationtables through its Drizzle adapter. Better Auth internally handles password hashing and token security; it stores hashed/encrypted values in these columns, not plaintext.Renaming columns as suggested would break Better Auth's adapter, which expects the standard column names. The application never directly reads or writes the
passwordorverification.valuefields—Better Auth abstracts all authentication logic through its API.No security improvements are needed here because Better Auth already implements industry-standard password hashing and token management.
Likely an incorrect or invalid review comment.
lib/db/schema.ts (1)
339-363: The account.password field is dead code and poses no security risk.The review comment correctly identifies that plaintext passwords should never be stored. However, verification reveals this concern is inapplicable here:
- The
accounttable is never inserted to or updated anywhere in the codebase—zero references in any repository or service file- The application uses OAuth + magic link authentication (Better Auth library), not email/password auth
- The
usertable has no password field, confirming no native password authentication exists- The
account.passwordfield is unused schema scaffolding from the auth library and is never populated or readSince the password field is not part of the active authentication flow and is never written to, there is no actual vulnerability. No code changes are required for this schema field.
Likely an incorrect or invalid review comment.
| updatePreferences: protectedProcedure | ||
| .input(NotificationPreferencesUpdate.partial()) | ||
| .mutation(async ({ ctx, input }) => { | ||
| // Update the preferences | ||
| const updated = await db | ||
| .update(notificationPreferences) | ||
| .set({ | ||
| ...input, | ||
| updatedAt: new Date(), | ||
| }) | ||
| .where(eq(notificationPreferences.userId, ctx.user.id)) | ||
| .returning(); | ||
|
|
||
| if (!updated[0]) { | ||
| // If update failed (preferences don't exist), create them | ||
| const created = await db | ||
| .insert(notificationPreferences) | ||
| .values({ | ||
| userId: ctx.user.id, | ||
| ...input, | ||
| }) | ||
| .onConflictDoUpdate({ | ||
| target: [notificationPreferences.userId], | ||
| set: { | ||
| ...input, | ||
| updatedAt: new Date(), | ||
| }, | ||
| }) | ||
| .returning(); | ||
|
|
||
| return created[0]; | ||
| } | ||
|
|
||
| return updated[0]; | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Simplify preferences update logic.
The current logic attempts an update first, then falls back to an insert with onConflictDoUpdate. This pattern is unnecessarily complex since onConflictDoUpdate alone handles both insert and update cases.
Apply this diff to simplify:
updatePreferences: protectedProcedure
.input(NotificationPreferencesUpdate.partial())
.mutation(async ({ ctx, input }) => {
- // Update the preferences
- const updated = await db
- .update(notificationPreferences)
- .set({
- ...input,
- updatedAt: new Date(),
- })
- .where(eq(notificationPreferences.userId, ctx.user.id))
- .returning();
-
- if (!updated[0]) {
- // If update failed (preferences don't exist), create them
- const created = await db
- .insert(notificationPreferences)
- .values({
- userId: ctx.user.id,
- ...input,
- })
- .onConflictDoUpdate({
- target: [notificationPreferences.userId],
- set: {
- ...input,
- updatedAt: new Date(),
- },
- })
- .returning();
-
- return created[0];
- }
-
- return updated[0];
+ // Upsert the preferences
+ const [result] = await db
+ .insert(notificationPreferences)
+ .values({
+ userId: ctx.user.id,
+ ...input,
+ })
+ .onConflictDoUpdate({
+ target: [notificationPreferences.userId],
+ set: {
+ ...input,
+ updatedAt: new Date(),
+ },
+ })
+ .returning();
+
+ return result;
}),🤖 Prompt for AI Agents
In server/routers/notifications.ts around lines 26 to 60, the current code does
an update then falls back to an insert-with-onConflict, which is redundant;
replace the whole update-then-insert block with a single upsert: perform
db.insert(notificationPreferences).values({ userId: ctx.user.id, ...input,
updatedAt: new Date() }).onConflictDoUpdate({ target:
[notificationPreferences.userId], set: { ...input, updatedAt: new Date() }
}).returning() and return the first row from the result, removing the initial
update branch entirely so the single operation handles both create and update
cases.
| // Check for <meta name="domainstack-verify" content="<token>"> | ||
| const verificationMeta = $('meta[name="domainstack-verify"]').attr( | ||
| "content", | ||
| ); | ||
|
|
||
| const isVerified = | ||
| typeof verificationMeta === "string" && | ||
| verificationMeta.trim() === expectedToken; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Scan all matching meta tags; don’t rely on first only.
Choose any tag whose content matches the token.
Apply this diff:
- // Check for <meta name="domainstack-verify" content="<token>">
- const verificationMeta = $('meta[name="domainstack-verify"]').attr(
- "content",
- );
-
- const isVerified =
- typeof verificationMeta === "string" &&
- verificationMeta.trim() === expectedToken;
+ // Check all <meta name="domainstack-verify" content="<token>">
+ const metaContents = $('meta[name="domainstack-verify"]')
+ .map((_, el) => $(el).attr("content"))
+ .get()
+ .filter((v): v is string => typeof v === "string");
+ const isVerified = metaContents.some((v) => v.trim() === expectedToken);🤖 Prompt for AI Agents
In server/services/domain-verification.ts around lines 110 to 118, the code only
reads the first meta[name="domainstack-verify"] tag; change it to scan all
matching meta tags and treat the verification as successful if any tag's content
(when present and trimmed) equals expectedToken. Iterate over
$('meta[name="domainstack-verify"]'), read each tag's content safely, trim and
compare to expectedToken, and set isVerified to true if any match is found
(otherwise false).
| } else { | ||
| log.debug("Meta tag verification failed", { | ||
| domain, | ||
| found: verificationMeta, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Avoid logging verification tokens.
Logging the found meta content can leak tokens.
Apply this diff:
- log.debug("Meta tag verification failed", {
- domain,
- found: verificationMeta,
- });
+ log.debug("Meta tag verification failed", {
+ domain,
+ foundPresent: Boolean($('meta[name="domainstack-verify"]').length),
+ });Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In server/services/domain-verification.ts around lines 121 to 126, the debug log
prints the full found verificationMeta which can leak tokens; change the log to
omit the token value — log only non-sensitive context such as domain and a flag
indicating whether a meta tag was present or masked (e.g., found:
!!verificationMeta or found: verificationMeta ? '<redacted>' : undefined) so the
actual token is never written to logs.
| // Calculate days until expiry | ||
| const getDaysUntilExpiry = (date?: Date | string | null) => { | ||
| if (!date) return null; | ||
| const expiryDate = date instanceof Date ? date : new Date(date); | ||
| const now = Date.now(); | ||
| return Math.floor((expiryDate.getTime() - now) / (1000 * 60 * 60 * 24)); | ||
| }; | ||
|
|
There was a problem hiding this comment.
Guard against Invalid Date to avoid false “Expired”.
new Date(invalidString) yields an Invalid Date; Math.floor(NaN) propagates, causing “Expired”. Return null when parse fails and use that to drive UI.
- const getDaysUntilExpiry = (date?: Date | string | null) => {
- if (!date) return null;
- const expiryDate = date instanceof Date ? date : new Date(date);
- const now = Date.now();
- return Math.floor((expiryDate.getTime() - now) / (1000 * 60 * 60 * 24));
- };
+ const getDaysUntilExpiry = (date?: Date | string | null) => {
+ if (!date) return null;
+ const d = date instanceof Date ? date : new Date(date);
+ const ms = d.getTime();
+ if (!Number.isFinite(ms)) return null; // invalid date string
+ const now = Date.now();
+ return Math.floor((ms - now) / 86_400_000);
+ };📝 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.
| // Calculate days until expiry | |
| const getDaysUntilExpiry = (date?: Date | string | null) => { | |
| if (!date) return null; | |
| const expiryDate = date instanceof Date ? date : new Date(date); | |
| const now = Date.now(); | |
| return Math.floor((expiryDate.getTime() - now) / (1000 * 60 * 60 * 24)); | |
| }; | |
| // Calculate days until expiry | |
| const getDaysUntilExpiry = (date?: Date | string | null) => { | |
| if (!date) return null; | |
| const d = date instanceof Date ? date : new Date(date); | |
| const ms = d.getTime(); | |
| if (!Number.isFinite(ms)) return null; // invalid date string | |
| const now = Date.now(); | |
| return Math.floor((ms - now) / 86_400_000); | |
| }; |
🤖 Prompt for AI Agents
In components/dashboard/domain-card.tsx around lines 34 to 41, the
getDaysUntilExpiry helper treats new Date(invalidString) as a Date and then
produces NaN (which renders as “Expired”); update the function to detect invalid
dates and return null: after constructing expiryDate (whether from a Date or
string) check if Number.isNaN(expiryDate.getTime()) and return null if so,
otherwise compute and return the floored day difference; ensure the Date
instance path also validates invalid Date objects.
| <ExternalLink className="size-3 opacity-0 transition-opacity group-hover:opacity-60" /> | ||
| </CardTitle> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Decorative icon should be hidden from assistive tech.
Add aria-hidden for the ExternalLink icon.
- <ExternalLink className="size-3 opacity-0 transition-opacity group-hover:opacity-60" />
+ <ExternalLink
+ aria-hidden="true"
+ className="size-3 opacity-0 transition-opacity group-hover:opacity-60"
+ />📝 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.
| <ExternalLink className="size-3 opacity-0 transition-opacity group-hover:opacity-60" /> | |
| </CardTitle> | |
| <ExternalLink | |
| aria-hidden="true" | |
| className="size-3 opacity-0 transition-opacity group-hover:opacity-60" | |
| /> | |
| </CardTitle> |
🤖 Prompt for AI Agents
In components/dashboard/domain-card.tsx around lines 71 to 72, the ExternalLink
icon is decorative but not hidden from assistive technologies; add
aria-hidden="true" to the ExternalLink element so screen readers ignore it (keep
the rest of props and className unchanged).
| const [activeMethod, setActiveMethod] = useState<string | null>(null); | ||
| const trpc = useTRPC(); | ||
| const trpcClient = useTRPCClient(); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Tighten types for method state.
Use a discriminated union for methods instead of string | null for better type-safety and autocomplete.
+type Method = "dns" | "meta" | "file";
...
- const [activeMethod, setActiveMethod] = useState<string | null>(null);
+ const [activeMethod, setActiveMethod] = useState<Method | null>(null);
...
- const verifyMutation = useMutation({
- mutationFn: ({ method }: { method: "dns" | "meta" | "file" }) =>
+ const verifyMutation = useMutation({
+ mutationFn: ({ method }: { method: Method }) =>
trpcClient.domains.verify.mutate({
domainId: userDomainId,
method,
}),
...
- const handleVerify = (method: "dns" | "meta" | "file") => {
+ const handleVerify = (method: Method) => {
setActiveMethod(method);
verifyMutation.mutate({ method });
};Also applies to: 35-41, 70-73, 181-184, 229-232, 288-291
🤖 Prompt for AI Agents
components/dashboard/verification-instructions.tsx lines 29-31 (and also update
occurrences at 35-41, 70-73, 181-184, 229-232, 288-291): replace the loose
useState<string | null> with a strict discriminated union for the possible
verification methods (e.g. declare type VerificationMethod = 'email' | 'sms' |
'document' | 'oauth' /* use actual method names from this file */;), change the
state to useState<VerificationMethod | null>(null), and update all related
handlers, props and switch/if branches to accept/return VerificationMethod
rather than string; ensure exhaustive narrowing (add a default/never case in
switches) and update any trpc call typings and refs to use the new union so
autocompletion and type-safety work across the listed line ranges.
| // Generate new verification token | ||
| const token = generateVerificationToken(); | ||
|
|
||
| // Insert or update user_domains record | ||
| if (existing) { | ||
| await db | ||
| .update(userDomains) | ||
| .set({ | ||
| verificationToken: token, | ||
| updatedAt: new Date(), | ||
| }) | ||
| .where(eq(userDomains.id, existing.id)); | ||
|
|
||
| return { | ||
| token, | ||
| domain: registrable, | ||
| domainId: domainRecord.id, | ||
| }; | ||
| } | ||
|
|
||
| // Insert new record | ||
| await db.insert(userDomains).values({ | ||
| userId: ctx.user.id, | ||
| domainId: domainRecord.id, | ||
| verificationToken: token, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Handle rare verification_token collisions (unique constraint 23505).
Token collisions are improbable but possible; on 23505, regenerate and retry a few times to avoid 500s.
- const token = generateVerificationToken();
+ let token = generateVerificationToken();For the update path:
- await db
- .update(userDomains)
- .set({
- verificationToken: token,
- updatedAt: new Date(),
- })
- .where(eq(userDomains.id, existing.id));
+ for (let i = 0; i < 3; i++) {
+ try {
+ await db
+ .update(userDomains)
+ .set({ verificationToken: token, updatedAt: new Date() })
+ .where(eq(userDomains.id, existing.id));
+ break;
+ } catch (err: unknown) {
+ if (typeof err === "object" && err && (err as any).code === "23505") {
+ token = generateVerificationToken();
+ continue;
+ }
+ throw err;
+ }
+ }For the insert path:
- await db.insert(userDomains).values({
- userId: ctx.user.id,
- domainId: domainRecord.id,
- verificationToken: token,
- });
+ for (let i = 0; i < 3; i++) {
+ try {
+ await db.insert(userDomains).values({
+ userId: ctx.user.id,
+ domainId: domainRecord.id,
+ verificationToken: token,
+ });
+ break;
+ } catch (err: unknown) {
+ if (typeof err === "object" && err && (err as any).code === "23505") {
+ token = generateVerificationToken();
+ continue;
+ }
+ throw err;
+ }
+ }Also applies to: 103-108
| .where( | ||
| sql`${certificates.domainId} IN ${domainIds} | ||
| AND ${certificates.fetchedAt} = ( | ||
| SELECT MAX(c2.fetched_at) | ||
| FROM certificates c2 | ||
| WHERE c2.domain_id = ${certificates.domainId} | ||
| )`, | ||
| ) |
There was a problem hiding this comment.
Use inArray() and a typed predicate instead of raw IN for correctness and type safety.
Replace the raw SQL IN with Drizzle’s inArray() and keep the correlated MAX subquery as a typed sql fragment.
Apply this diff (and add inArray to imports; see next comment):
- .where(
- sql`${certificates.domainId} IN ${domainIds}
- AND ${certificates.fetchedAt} = (
- SELECT MAX(c2.fetched_at)
- FROM certificates c2
- WHERE c2.domain_id = ${certificates.domainId}
- )`,
- )
+ .where(
+ and(
+ inArray(certificates.domainId, domainIds),
+ eq(
+ certificates.fetchedAt,
+ sql`(SELECT MAX(c2.fetched_at) FROM certificates c2 WHERE c2.domain_id = ${certificates.domainId})`,
+ ),
+ ),
+ )Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In server/routers/domains.ts around lines 239–246, the code uses a raw SQL IN
with template interpolation which is not type-safe; replace that condition with
Drizzle's inArray() predicate and keep the correlated MAX subquery as a typed
sql fragment: import inArray from Drizzle at the top, change the where clause to
use inArray(certificates.domainId, domainIds) combined with the existing
equality to a typed sql fragment for the correlated SELECT MAX(...) subquery
(keep the subquery as sql`...` using certificates.fetchedAt and
certificates.domainId bindings) so the IN is type-safe and the subquery remains
a typed raw fragment.
| return records.map((r) => ({ | ||
| ...r, | ||
| certificateExpiresAt: certMap.get(r.domainId) ?? null, | ||
| })); | ||
| }), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Response mixes Date objects and strings; pick one serialization.
registrationExpiresAt likely remains a Date while certificateExpiresAt is stringified. Standardize to ISO strings (or Dates), then align consumers.
If ISO strings are desired:
- return records.map((r) => ({
- ...r,
- certificateExpiresAt: certMap.get(r.domainId) ?? null,
- }));
+ return records.map((r) => ({
+ ...r,
+ registrationExpiresAt: r.registrationExpiresAt
+ ? r.registrationExpiresAt.toISOString()
+ : null,
+ certificateExpiresAt: certMap.get(r.domainId) ?? null,
+ }));Please confirm clients expect strings; otherwise we can keep Dates for both.
📝 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 records.map((r) => ({ | |
| ...r, | |
| certificateExpiresAt: certMap.get(r.domainId) ?? null, | |
| })); | |
| }), | |
| return records.map((r) => ({ | |
| ...r, | |
| registrationExpiresAt: r.registrationExpiresAt | |
| ? r.registrationExpiresAt.toISOString() | |
| : null, | |
| certificateExpiresAt: certMap.get(r.domainId) ?? null, | |
| })); | |
| }), |
🤖 Prompt for AI Agents
In server/routers/domains.ts around lines 255 to 259, the response mixes Date
objects and stringified dates (registrationExpiresAt vs certificateExpiresAt);
pick one serialization and make both consistent. Convert both fields to ISO
strings before returning (e.g., call toISOString() on registrationExpiresAt and
ensure certificateExpiresAt comes from certMap as an ISO string), or
alternatively parse certificateExpiresAt into a Date if you prefer Dates—also
update the response type/interface to reflect the chosen type so clients and
typings remain consistent.
…ts for consistency and improved maintainability.
…plication and improve maintainability.
…date database schema for timestamps
…mat for database lookup; update change detection to use inArray for snapshot cleanup and simplify auth middleware with destructured context
…nk and OAuth support; create LoginForm component for email submission and provider selection.
…ns across components; enhance toast notifications for better user feedback.
… delete account functionality and notification preferences
Summary by CodeRabbit
Release Notes