Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning Rate limit exceeded@elliotBraem has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 29 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 (1)
WalkthroughThis update introduces a comprehensive authentication system using JWT and nonce-based login, refactors feed creation into a multi-step, router-driven flow, modularizes leaderboard components, and centralizes feed configuration state. The backend adds an Changes
Sequence Diagram(s)Authentication Flow (Login/Authorization)sequenceDiagram
participant User
participant App (Frontend)
participant API (Backend)
participant DB
User->>App (Frontend): Click "Login"
App->>API (Backend): POST /auth/initiate-login (with accountId)
API->>DB: Create auth_request (nonce, expires)
API-->>App (Frontend): { nonce, recipient }
App->>User: Prompt to sign message (with nonce)
User-->>App (Frontend): Signed token
App->>API (Backend): POST /auth/verify-login (token, accountId)
API->>DB: Fetch latest auth_request
API->>API: Verify token, nonce, accountId
API->>DB: Delete used auth_request
API-->>App (Frontend): Set JWT cookie, return success
App->>User: User now authorized
Feed Creation Multi-Step FlowsequenceDiagram
participant User
participant App (Router)
participant FeedCreationStore
User->>App: Start feed creation
App->>FeedCreationStore: Initialize feedConfig
App->>User: Show step 1 (Basic Info)
User->>App: Submit step 1
App->>FeedCreationStore: Save basic info
App->>App: Navigate to step 2 (Settings)
User->>App: Submit step 2
App->>FeedCreationStore: Save settings
App->>App: Navigate to step 3 (Review)
User->>App: Submit review
App->>API: POST /feeds (with feedConfig)
API-->>App: Success/failure
App->>User: Show result, navigate home
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/app/src/components/leaderboard/LeaderboardTable.tsx (1)
78-105: Simplify complex conditional rendering logic.The nested conditional logic for rendering table rows is complex and hard to follow. Consider extracting it into a separate render method or component.
<TableBody className="overflow-x-auto"> {isLoading && <LeaderboardSkeleton rows={8} />} - - {error && ( - <TableRow> - <TableCell colSpan={table.getAllColumns().length} className="text-left py-8 text-red-500"> - <p>Error loading leaderboard: {error.message}</p> - </TableCell> - </TableRow> - )} - - {!hasData && !isLoading && !error && ( - <TableRow> - <TableCell colSpan={table.getAllColumns().length} className="text-left py-8"> - <p>No curator data available.</p> - </TableCell> - </TableRow> - )} - - {!isLoading && !error && table.getRowModel().rows.length > 0 - ? table.getRowModel().rows.map((row) => ( - <TableRow - key={row.id} - className="border-b border-[#e5e5e5] hover:bg-[#f9fafb]" - > - {row.getVisibleCells().map((cell) => ( - <TableCell - key={cell.id} - className="py-2 px-2 align-middle" - > - {flexRender( - cell.column.columnDef.cell, - cell.getContext(), - )} - </TableCell> - ))} - </TableRow> - )) - : !isLoading && - !error && - table.getRowModel().rows.length === 0 && ( - <TableRow> - <TableCell colSpan={table.getAllColumns().length} className="text-left py-8"> - <p>No matching results found.</p> - </TableCell> - </TableRow> - )} + {renderTableContent()} </TableBody>Then extract the rendering logic:
const renderTableContent = () => { if (error) { return ( <TableRow> <TableCell colSpan={table.getAllColumns().length} className="text-left py-8 text-red-500"> <p>Error loading leaderboard: {error.message}</p> </TableCell> </TableRow> ); } if (!hasData && !isLoading) { return ( <TableRow> <TableCell colSpan={table.getAllColumns().length} className="text-left py-8"> <p>No curator data available.</p> </TableCell> </TableRow> ); } const rows = table.getRowModel().rows; if (!isLoading && rows.length === 0) { return ( <TableRow> <TableCell colSpan={table.getAllColumns().length} className="text-left py-8"> <p>No matching results found.</p> </TableCell> </TableRow> ); } return rows.map((row) => ( <TableRow key={row.id} className="border-b border-[#e5e5e5] hover:bg-[#f9fafb]" > {row.getVisibleCells().map((cell) => ( <TableCell key={cell.id} className="py-2 px-2 align-middle"> {flexRender(cell.column.columnDef.cell, cell.getContext())} </TableCell> ))} </TableRow> )); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
apps/app/package.json(1 hunks)apps/app/src/components/Leaderboard.tsx(2 hunks)apps/app/src/components/leaderboard/LeaderboardColumns.tsx(1 hunks)apps/app/src/components/leaderboard/LeaderboardFilters.tsx(1 hunks)apps/app/src/components/leaderboard/LeaderboardSkeleton.tsx(1 hunks)apps/app/src/components/leaderboard/LeaderboardTable.tsx(1 hunks)apps/app/src/components/leaderboard/index.ts(1 hunks)apps/app/src/hooks/useLeaderboard.ts(1 hunks)packages/shared-db/migrations/0015_ambiguous_the_watchers.sql(1 hunks)pnpm-workspace.yaml(1 hunks)
🔇 Additional comments (8)
packages/shared-db/migrations/0015_ambiguous_the_watchers.sql (1)
7-7: LGTM: Formatting improvement for readability.The blank line addition improves visual separation between the DROP INDEX operations and the subsequent ALTER TABLE operations, enhancing the migration script's readability.
pnpm-workspace.yaml (1)
1-8: LGTM: Standard pnpm workspace configuration.The changes are appropriate:
- Unquoted package globs are valid and commonly used
- The
onlyBuiltDependenciesconfiguration forcore-js,esbuild, andsecp256k1is a standard practice to ensure compatibility and security by building these packages from sourceapps/app/package.json (1)
34-34: Verify the package version is current and secure.Please ensure that
@tanstack/react-table@^8.21.3is the latest stable version and check for any security advisories.What is the latest version of @tanstack/react-table and are there any known security vulnerabilities in version 8.21.3?apps/app/src/components/leaderboard/index.ts (1)
1-8: LGTM: Clean barrel export pattern.The index file provides a well-organized central entry point for leaderboard components and utilities, following modular design best practices.
apps/app/src/components/leaderboard/LeaderboardTable.tsx (1)
14-19: LGTM: Well-defined component interface.The props interface is clean and provides good separation of concerns by accepting a pre-configured table instance and state flags.
apps/app/src/components/leaderboard/LeaderboardSkeleton.tsx (1)
1-61: Well-structured skeleton component!The implementation provides a clean loading state with appropriate dimensions and animations for each column. Good use of semantic table elements and TypeScript.
apps/app/src/hooks/useLeaderboard.ts (1)
1-179: Excellent hook implementation!Well-structured custom hook with proper separation of concerns, performance optimizations (useCallback, useMemo), and clean state management. The debouncing and click-outside handling are implemented correctly.
apps/app/src/components/Leaderboard.tsx (1)
1-75: Excellent refactoring!The component has been successfully modularized with clear separation of concerns. State management is properly delegated to the custom hook, and UI is split into focused child components. Good use of React.memo for performance optimization.
| <TableCell colSpan={5} className="text-left py-8 text-red-500"> | ||
| <p>Error loading leaderboard: {error.message}</p> | ||
| </TableCell> | ||
| </TableRow> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hard-coded colSpan values.
The hard-coded colSpan={5} will break if the number of columns changes. Use dynamic column count instead.
- <TableCell colSpan={5} className="text-left py-8 text-red-500">
+ <TableCell colSpan={table.getAllColumns().length} className="text-left py-8 text-red-500">
<p>Error loading leaderboard: {error.message}</p>
</TableCell>Apply the same pattern to the other colSpan={5} instances at lines 72 and 101.
Also applies to: 72-75, 101-104
🤖 Prompt for AI Agents
In apps/app/src/components/leaderboard/LeaderboardTable.tsx around lines 64 to
67, 72 to 75, and 101 to 104, the colSpan attribute is hard-coded to 5, which
can cause layout issues if the number of columns changes. Replace the hard-coded
colSpan values with a dynamic calculation based on the current number of
columns, such as using the length of the columns array or a variable
representing the column count, to ensure the colspan adapts automatically to any
changes in the table structure.
| <TableHead | ||
| key={header.id} | ||
| className="text-left py-3 px-2 font-medium text-sm whitespace-nowrap cursor-pointer" | ||
| onClick={header.column.getToggleSortingHandler()} | ||
| > | ||
| <div className="flex items-center gap-1"> | ||
| {flexRender( | ||
| header.column.columnDef.header, | ||
| header.getContext(), | ||
| )} | ||
| {header.column.getIsSorted() === "asc" ? ( | ||
| <ChevronUp className="h-4 w-4 text-gray-500" /> | ||
| ) : header.column.getIsSorted() === "desc" ? ( | ||
| <ChevronDown className="h-4 w-4 text-gray-500" /> | ||
| ) : null} | ||
| </div> | ||
| </TableHead> | ||
| ))} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes for sortable table headers.
The sortable headers are missing important accessibility attributes that screen readers need to understand the sorting functionality.
<TableHead
key={header.id}
className="text-left py-3 px-2 font-medium text-sm whitespace-nowrap cursor-pointer"
onClick={header.column.getToggleSortingHandler()}
+ role="columnheader"
+ tabIndex={0}
+ aria-sort={
+ header.column.getIsSorted() === "asc" ? "ascending" :
+ header.column.getIsSorted() === "desc" ? "descending" : "none"
+ }
+ onKeyDown={(e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ e.preventDefault();
+ header.column.getToggleSortingHandler()?.(e);
+ }
+ }}
>📝 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.
| <TableHead | |
| key={header.id} | |
| className="text-left py-3 px-2 font-medium text-sm whitespace-nowrap cursor-pointer" | |
| onClick={header.column.getToggleSortingHandler()} | |
| > | |
| <div className="flex items-center gap-1"> | |
| {flexRender( | |
| header.column.columnDef.header, | |
| header.getContext(), | |
| )} | |
| {header.column.getIsSorted() === "asc" ? ( | |
| <ChevronUp className="h-4 w-4 text-gray-500" /> | |
| ) : header.column.getIsSorted() === "desc" ? ( | |
| <ChevronDown className="h-4 w-4 text-gray-500" /> | |
| ) : null} | |
| </div> | |
| </TableHead> | |
| ))} | |
| <TableHead | |
| key={header.id} | |
| className="text-left py-3 px-2 font-medium text-sm whitespace-nowrap cursor-pointer" | |
| onClick={header.column.getToggleSortingHandler()} | |
| role="columnheader" | |
| tabIndex={0} | |
| aria-sort={ | |
| header.column.getIsSorted() === "asc" ? "ascending" : | |
| header.column.getIsSorted() === "desc" ? "descending" : "none" | |
| } | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); | |
| header.column.getToggleSortingHandler()?.(e); | |
| } | |
| }} | |
| > | |
| <div className="flex items-center gap-1"> | |
| {flexRender( | |
| header.column.columnDef.header, | |
| header.getContext(), | |
| )} | |
| {header.column.getIsSorted() === "asc" ? ( | |
| <ChevronUp className="h-4 w-4 text-gray-500" /> | |
| ) : header.column.getIsSorted() === "desc" ? ( | |
| <ChevronDown className="h-4 w-4 text-gray-500" /> | |
| ) : null} | |
| </div> | |
| </TableHead> |
🤖 Prompt for AI Agents
In apps/app/src/components/leaderboard/LeaderboardTable.tsx between lines 38 and
55, the sortable table headers lack accessibility attributes. Add appropriate
ARIA attributes such as aria-sort to indicate the current sorting state
(ascending, descending, or none) on each header element. Also, ensure the
headers have role="button" or similar to convey interactivity to screen readers.
This will improve accessibility by making the sorting functionality clear to
assistive technologies.
| timeframe: time.value, | ||
| }} | ||
| role="option" | ||
| aria-selected={search.timeframe === time.label} |
There was a problem hiding this comment.
Fix aria-selected comparison logic
The comparison should use time.value instead of time.label to properly indicate the selected state.
- aria-selected={search.timeframe === time.label}
+ aria-selected={search.timeframe === time.value}📝 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.
| aria-selected={search.timeframe === time.label} | |
| aria-selected={search.timeframe === time.value} |
🤖 Prompt for AI Agents
In apps/app/src/components/leaderboard/LeaderboardFilters.tsx at line 129, the
aria-selected attribute is incorrectly comparing search.timeframe with
time.label. Update the comparison to use time.value instead of time.label to
correctly reflect the selected state for accessibility.
| key={time.value} | ||
| to="/leaderboard" | ||
| search={{ | ||
| feed: search.feed.toLowerCase(), |
There was a problem hiding this comment.
Avoid unnecessary toLowerCase() on feed value
The search.feed might already be in the correct case. Applying toLowerCase() could cause mismatches if feed IDs are case-sensitive.
- feed: search.feed.toLowerCase(),
+ feed: search.feed,📝 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.
| feed: search.feed.toLowerCase(), | |
| feed: search.feed, |
🤖 Prompt for AI Agents
In apps/app/src/components/leaderboard/LeaderboardFilters.tsx at line 125,
remove the unnecessary toLowerCase() call on search.feed because feed IDs may be
case-sensitive and altering the case could cause mismatches. Use search.feed
directly without converting its case.
| {feedSubmissions && feedSubmissions.length > 1 && ( | ||
| <button | ||
| onClick={() => toggleRow(rowIndex)} | ||
| className="w-8 h-8 flex items-center justify-center border border-neutral-400 rounded-md transition-colors" | ||
| > | ||
| {expandedRows.includes(rowIndex) ? ( | ||
| <ChevronUp className="h-4 w-4" /> | ||
| ) : ( | ||
| <span className="text-xs"> | ||
| +{feedSubmissions.length - 1} | ||
| </span> | ||
| )} | ||
| </button> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes to the expand button
The expand button should include proper ARIA attributes for better screen reader support.
<button
onClick={() => toggleRow(rowIndex)}
className="w-8 h-8 flex items-center justify-center border border-neutral-400 rounded-md transition-colors"
+ aria-expanded={expandedRows.includes(rowIndex)}
+ aria-label={expandedRows.includes(rowIndex) ? "Collapse additional feeds" : `Show ${feedSubmissions.length - 1} more feeds`}
>📝 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.
| {feedSubmissions && feedSubmissions.length > 1 && ( | |
| <button | |
| onClick={() => toggleRow(rowIndex)} | |
| className="w-8 h-8 flex items-center justify-center border border-neutral-400 rounded-md transition-colors" | |
| > | |
| {expandedRows.includes(rowIndex) ? ( | |
| <ChevronUp className="h-4 w-4" /> | |
| ) : ( | |
| <span className="text-xs"> | |
| +{feedSubmissions.length - 1} | |
| </span> | |
| )} | |
| </button> | |
| )} | |
| {feedSubmissions && feedSubmissions.length > 1 && ( | |
| <button | |
| onClick={() => toggleRow(rowIndex)} | |
| className="w-8 h-8 flex items-center justify-center border border-neutral-400 rounded-md transition-colors" | |
| aria-expanded={expandedRows.includes(rowIndex)} | |
| aria-label={ | |
| expandedRows.includes(rowIndex) | |
| ? "Collapse additional feeds" | |
| : `Show ${feedSubmissions.length - 1} more feeds` | |
| } | |
| > | |
| {expandedRows.includes(rowIndex) ? ( | |
| <ChevronUp className="h-4 w-4" /> | |
| ) : ( | |
| <span className="text-xs"> | |
| +{feedSubmissions.length - 1} | |
| </span> | |
| )} | |
| </button> | |
| )} |
🤖 Prompt for AI Agents
In apps/app/src/components/leaderboard/LeaderboardColumns.tsx around lines 99 to
112, the expand button lacks ARIA attributes for accessibility. Add appropriate
ARIA attributes such as aria-expanded to indicate the toggle state and
aria-controls to reference the expandable content. This will improve screen
reader support and make the button's function clear to assistive technologies.
* wip * feed types * better feed form * Update packages/shared-db/tsconfig.json Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update apps/app/src/routes/_layout/create/feed/settings.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update apps/app/src/routes/_layout/create/feed/index.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * set lock * nitpicks * fmt --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (4)
packages/shared-db/src/utils.ts (3)
31-35: Add type guards for error property accessThe function accesses
error.codeanderror.originalwithout type checking, which could cause runtime errors.export function isRetryableError(error: unknown): boolean { + if (!error || typeof error !== 'object') { + return false; + } + const err = error as any; // Type assertion after object check return RETRYABLE_ERROR_CODES.some( (code) => - error.code === code || (error.original && error.original.code === code), + err.code === code || (err.original && err.original.code === code), ); }
50-58: Add type guards for error loggingThe logging accesses
error.messageanderror.codewithout proper type checking.console.warn( // TODO: Logger `Database operation failed (attempt ${attempt}/${maxRetries})`, { - error: error.message, - code: error.code, + error: error instanceof Error ? error.message : String(error), + code: (error as any)?.code, isLastAttempt, }, );
111-116: Add type guards for error logging in withErrorHandlingSimilar to the previous comment, error property access needs type checking.
console.error(`Failed to ${operationName}:`, { // TODO: Logger - error: error.message, - code: error.code, + error: error instanceof Error ? error.message : String(error), + code: (error as any)?.code, ...additionalContext, });apps/app/src/components/PublishIntegrations.tsx (1)
40-41: Fix SVG attributes for React compatibilityThe SVG gradient uses
stop-colorwhich should bestopColorin React/JSX.- <stop stop-color="#37AEE2" /> - <stop offset="1" stop-color="#1E96C8" /> + <stop stopColor="#37AEE2" /> + <stop offset="1" stopColor="#1E96C8" />
🧹 Nitpick comments (12)
apps/api/src/utils/plugin.ts (1)
238-238: Good type safety improvement.Changing from
anytounknownenhances type safety in error handling. Consider adding a type guard or safe property access forerror.messageto handle cases where the error might not have a message property.packages/types/src/config.ts (1)
79-79: Consider validation for empty string edge case.The image field now accepts empty strings or URLs. Ensure the consuming code properly handles the empty string case to avoid broken image displays.
#!/bin/bash # Check how the image field is used in components to ensure empty string handling rg -A 5 -B 5 "image.*=|\.image" apps/app/src/components/apps/app/src/components/PublishIntegrations.tsx (1)
74-107: Consider adding validation for Telegram configurationThe Telegram channel ID field should have validation to ensure valid format.
Add validation rules in the parent form schema:
telegramChannelId: z.string().regex(/^@?[a-zA-Z0-9_]{5,32}$/, "Invalid channel username").optional(), telegramThreadId: z.string().regex(/^[0-9]+$/, "Thread ID must be numeric").optional(),apps/app/src/routes/_layout/create/feed/index.tsx (2)
71-73: Remove unused parameterThe
_ipfsHashparameter is not used in the callback function.- onUploadSuccess={(_ipfsHash, ipfsUrl) => { + onUploadSuccess={(_, ipfsUrl) => { field.onChange(ipfsUrl); }}
23-23: Consider validating hashtag uniquenessThe hashtag is described as a unique identifier but there's no uniqueness validation.
Consider adding async validation to check if the hashtag is already in use:
id: z.string() .min(1, "Please provide at least one hashtag") .regex(/^[a-zA-Z0-9_]+$/, "Hashtag can only contain letters, numbers, and underscores") .refine(async (value) => { // Check if hashtag is unique const exists = await checkHashtagExists(value); return !exists; }, "This hashtag is already in use"),Also applies to: 117-129
apps/app/src/routes/_layout/create/feed/review.tsx (2)
99-99: Implement user name fetching for approversThe TODO comment indicates missing functionality for displaying user names.
Would you like me to implement a solution to fetch and display Twitter user names for the approvers? This could be done via a batch API call or by storing user data when approvers are added.
30-36: Provide more specific error feedback to usersThe generic error message doesn't help users understand what went wrong.
} catch (error) { console.error("Error creating feed:", error); + const errorMessage = error instanceof Error ? error.message : "Unknown error occurred"; toast({ title: "Error Creating Feed", - description: "There was an error creating your feed. Please try again.", + description: `Failed to create feed: ${errorMessage}`, variant: "destructive", });apps/app/src/components/ContentApprovers.tsx (1)
28-30: Consider optimizing the useEffect dependency.The current pattern syncs form state to the external store on every change, which could cause unnecessary renders. Consider debouncing or using a more selective dependency.
+import { useDebouncedCallback } from 'use-debounce'; // or similar +const debouncedSetApprovers = useDebouncedCallback(setApprovers, 300); useEffect(() => { - setApprovers(approvers); -}, [approvers, setApprovers]); + debouncedSetApprovers(approvers); +}, [approvers, debouncedSetApprovers]);apps/app/src/lib/api/feeds.ts (1)
135-140: Consider using the actual API response type.The TODO comment suggests this interface should match the API. Consider importing the actual response type from the API definitions instead of maintaining a separate interface.
-interface CanModerateResponse { - // TODO: should match with API - canModerate: boolean; - reason?: string; - error?: string; -} +import type { CanModerateResponse } from "@curatedotfun/types";apps/app/src/components/ui/tanstack-form.tsx (1)
23-28: Consider forward declaration for form components.The components
FormLabel,FormControl, etc. are referenced before they're defined. While this works due to JavaScript hoisting, consider reordering or using forward declarations for better clarity.// Either move the createFormHook call after component definitions // or add forward declarations at the top +const FormLabel: React.FC<any>; +const FormControl: React.FC<any>; +// ... other components const { useAppForm, withForm } = createFormHook({ fieldContext, formContext, fieldComponents: { FormLabel, FormControl, FormDescription, FormMessage, FormItem, }, formComponents: {}, });apps/api/src/services/feed.service.ts (1)
168-285: Consider refactoring this method for better maintainability.The
processFeedmethod is quite long (117 lines) and handles multiple responsibilities. Consider extracting the following into separate private methods:
- Distributor selection logic (lines 219-268)
- Submission processing loop (lines 204-279)
Example refactor:
private selectDistributors( streamConfig: StreamConfig, distributorsParam?: string ): { validDistributors: string[]; streamConfig: StreamConfig } { // Extract lines 219-268 logic here } private async processSubmissions( submissions: RichSubmission[], streamConfig: StreamConfig, feedId: string ): Promise<{ processedCount: number; usedDistributors: Set<string> }> { // Extract lines 204-279 logic here }apps/app/src/store/feed-creation-store.ts (1)
58-59: Consider making the AI transform prompt more platform-agnostic.The prompt specifically mentions Twitter/X and @mentions format, which may not be appropriate if the system supports multiple social media platforms. Consider using a more generic approach or making the prompt configurable per platform.
- "Summarize the content into a concise news flash, incorporating relevant details from the curator's notes. Maintain a neutral, third-person tone. Mention the author if relevant, or simply convey the information. When processing social media-style content, convert @mentions into markdown links in the format: [@username](https://x.com/username). Ensure all mentions are accurately linked and preserve their original intent.", + "Summarize the content into a concise news flash, incorporating relevant details from the curator's notes. Maintain a neutral, third-person tone. Mention the author if relevant, or simply convey the information. When processing social media-style content, convert mentions into appropriate markdown links based on the source platform. Ensure all mentions are accurately linked and preserve their original intent.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (42)
apps/api/src/routes/api/feeds.ts(1 hunks)apps/api/src/routes/api/moderation.ts(4 hunks)apps/api/src/routes/api/plugins.ts(1 hunks)apps/api/src/routes/api/users.ts(2 hunks)apps/api/src/services/feed.service.ts(6 hunks)apps/api/src/services/moderation.service.ts(2 hunks)apps/api/src/services/users.service.ts(7 hunks)apps/api/src/types/errors.ts(1 hunks)apps/api/src/utils/plugin.ts(1 hunks)apps/api/src/utils/service-provider.ts(1 hunks)apps/app/package.json(2 hunks)apps/app/rsbuild.config.ts(3 hunks)apps/app/src/components/ContentApprovers.tsx(2 hunks)apps/app/src/components/CurationFormSteps.tsx(0 hunks)apps/app/src/components/CurationSettingsForm.tsx(0 hunks)apps/app/src/components/DistributorBadges.tsx(1 hunks)apps/app/src/components/FeedItem.tsx(1 hunks)apps/app/src/components/FeedList.tsx(1 hunks)apps/app/src/components/PublishIntegrations.tsx(2 hunks)apps/app/src/components/TopCurators.tsx(1 hunks)apps/app/src/components/profile/ProfileHeader.tsx(1 hunks)apps/app/src/components/ui/tanstack-form.tsx(1 hunks)apps/app/src/lib/api/feed.ts(0 hunks)apps/app/src/lib/api/feeds.ts(4 hunks)apps/app/src/routeTree.gen.ts(14 hunks)apps/app/src/routes/_layout/create/feed.tsx(1 hunks)apps/app/src/routes/_layout/create/feed/index.tsx(1 hunks)apps/app/src/routes/_layout/create/feed/review.tsx(5 hunks)apps/app/src/routes/_layout/create/feed/settings.tsx(1 hunks)apps/app/src/routes/_layout/feed/$feedId.tsx(1 hunks)apps/app/src/store/feed-creation-store.ts(1 hunks)packages/shared-db/package.json(1 hunks)packages/shared-db/scripts/seed-remote.ts(2 hunks)packages/shared-db/src/repositories/feed.repository.ts(4 hunks)packages/shared-db/src/repositories/user.repository.ts(1 hunks)packages/shared-db/src/schema/feeds.ts(3 hunks)packages/shared-db/src/utils.ts(4 hunks)packages/shared-db/src/validators.ts(0 hunks)packages/shared-db/tsconfig.json(1 hunks)packages/types/src/api/feeds.ts(1 hunks)packages/types/src/config.ts(2 hunks)packages/types/src/index.ts(1 hunks)
💤 Files with no reviewable changes (4)
- packages/shared-db/src/validators.ts
- apps/app/src/lib/api/feed.ts
- apps/app/src/components/CurationSettingsForm.tsx
- apps/app/src/components/CurationFormSteps.tsx
✅ Files skipped from review due to trivial changes (12)
- apps/app/src/components/FeedItem.tsx
- apps/app/src/components/FeedList.tsx
- packages/shared-db/package.json
- apps/app/src/components/TopCurators.tsx
- apps/api/src/services/users.service.ts
- packages/shared-db/tsconfig.json
- apps/api/src/routes/api/users.ts
- packages/types/src/index.ts
- apps/app/src/components/profile/ProfileHeader.tsx
- apps/app/src/routes/_layout/feed/$feedId.tsx
- apps/api/src/routes/api/moderation.ts
- packages/types/src/api/feeds.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/app/package.json
🔇 Additional comments (34)
apps/api/src/utils/service-provider.ts (1)
242-244: LGTM! Consistent with established patterns.The new
getModerationService()method follows the same pattern as other service getter methods in the class and is properly typed.apps/api/src/routes/api/plugins.ts (1)
31-31: Consistent type safety improvement.The change from
anytounknownaligns with the broader pattern of improving error handling type safety across the codebase.packages/shared-db/src/repositories/user.repository.ts (1)
112-112: Consistent with codebase-wide type safety improvements.The error type change from
anytounknownaligns with similar improvements across the codebase while maintaining the existing error handling logic.packages/shared-db/scripts/seed-remote.ts (1)
106-106: Consistent type safety improvement.The error type change from
anytounknownaligns with similar improvements across the codebase.apps/app/rsbuild.config.ts (3)
27-27: Good addition of source alias.The "@" alias pointing to "./src" will improve import readability and maintainability throughout the codebase.
49-51: Verify plugin configuration consistency.The plugin invocation was updated to use the new import name and route generation was enabled. Ensure this configuration works with the expanded routing structure mentioned in the AI summary.
#!/bin/bash # Check if there are route files in the expected directory structure fd -t f -e tsx -e ts . apps/app/src/routes | head -20
3-3: Verify the import change aligns with the package API.The import was changed from a default import to a named import. Ensure this matches the actual export from
@tanstack/router-plugin/rspack.#!/bin/bash # Check the actual exports from the TanStack Router rspack plugin rg -A 5 -B 5 "export.*tanstackRouter|export.*TanStackRouterRspack" node_modules/@tanstack/router-plugin/apps/api/src/services/moderation.service.ts (2)
262-262: Excellent type safety improvement.Changing from
error: anytoerror: unknownfollows TypeScript best practices and improves type safety without affecting the existing error handling logic.
336-336: Consistent type safety improvement.Good consistency in applying the same
unknowntype annotation throughout the error handling code.apps/app/src/components/DistributorBadges.tsx (3)
9-9: Good replacement of fragment with proper container.Using a flex container with proper CSS classes provides better layout control and visual presentation compared to the previous fragment wrapper.
14-14: Improved key uniqueness.Combining the plugin string with the index creates more unique keys, which is better for React's reconciliation process, especially if the same plugin appears multiple times.
15-15: Enhanced badge styling for better UX.The comprehensive styling with
whitespace-nowrap,flex-shrink-0, and smaller text size provides better visual hierarchy and prevents layout issues with long plugin names.apps/app/src/routes/_layout/create/feed/settings.tsx (1)
8-30: Well-structured reusable StepHeader component.The StepHeader component is properly typed and styled with responsive design considerations. Good separation of concerns.
packages/types/src/config.ts (2)
35-38: Good flexibility improvement for RecapConfig.Making the
id,name, andschedulefields optional provides better flexibility for partial configurations and incremental setup flows.
81-81: Good addition of admins field.The optional
adminsfield as an array of strings aligns well with the feed administration features mentioned in the AI summary.packages/shared-db/src/utils.ts (1)
30-30: Good improvement for type safetyChanging error types from
anytounknownis a great practice that enforces proper type checking.Also applies to: 46-46, 79-79, 108-108
apps/app/src/components/ContentApprovers.tsx (1)
16-21: Well-implemented react-hook-form integration.The migration from local state to
useFieldArrayis correctly implemented and follows react-hook-form patterns.packages/shared-db/src/repositories/feed.repository.ts (3)
40-53: Excellent refactor to use ORM queries.The migration from raw query builder to
query.feeds.findFirstimproves readability and type safety. The method rename tofindFeedByIdbetter reflects the nullable return type.
77-79: Good addition of runtime validation.The check ensures that the insert operation actually created a record, preventing silent failures.
110-132: Well-implemented cascading delete with proper return value.The method now properly handles related data cleanup before deletion and returns the deleted record instead of just a count. This is more useful for API responses and logging.
packages/shared-db/src/schema/feeds.ts (2)
1-1: Excellent use of drizzle-zod for schema generation.Importing the
FeedConfigtype and drizzle-zod functions sets up proper schema generation that will stay in sync with the database schema.Also applies to: 18-22
102-116: Great refactor to eliminate schema duplication.Using
createInsertSchema,createUpdateSchema, andcreateSelectSchemaensures that validation schemas stay consistent with the database schema automatically. The field overrides are appropriately minimal.apps/app/src/lib/api/feeds.ts (3)
1-8: Good migration to comprehensive type definitions.The import of detailed API types from
@curatedotfun/typesensures type safety across the API layer.
24-31: Proper response data unwrapping.The
selectfunctions correctly unwrap the.dataproperty from wrapped API responses, providing cleaner data access for components.Also applies to: 35-42
147-160: Well-implemented moderation permission hook.The
useCanModerateFeedhook has proper guards for authentication state and includes appropriate caching withstaleTime. The conditional enabling prevents unnecessary requests.apps/app/src/components/ui/tanstack-form.tsx (3)
12-30: Well-structured form hook and context setup.The use of
createFormHookandcreateFormHookContextsprovides a solid foundation for form state management with proper context isolation.
54-70: Excellent error handling and validation.The
useFormFieldhook properly validates context usage and provides comprehensive field metadata including error states and accessibility IDs.
92-108: Outstanding accessibility implementation.The
FormControlcomponent properly implements ARIA attributes for error states and descriptions, ensuring screen reader compatibility.apps/api/src/services/feed.service.ts (2)
102-108: Good error handling pattern!The change to throw
NotFoundErrorinstead of returning null makes the API more predictable and consistent with other methods in the service.
110-146: Excellent permission enforcement and data merging!The method correctly:
- Enforces permissions before allowing updates
- Preserves existing configuration while merging new changes
- Provides clear error messages for both permission and not found scenarios
Note: The deep merge behavior means nested objects will be merged rather than replaced, which is typically the desired behavior for configuration updates.
apps/app/src/store/feed-creation-store.ts (1)
86-139: Well-structured state management with immer!The store implementation demonstrates good practices:
- Immutable updates with immer
- Flexible path-based updates with
setValue- Clean distributor configuration management
- Proper transformation of approvers data
The use of environment variable placeholders (e.g.,
{{TELEGRAM_BOT_TOKEN}}) is a good security practice.apps/app/src/routeTree.gen.ts (1)
1-10: Auto-generated file - changes look correct.The route tree correctly reflects the new nested structure under
/create/feedwith index, settings, and review routes.apps/api/src/types/errors.ts (1)
3-69: Excellent error handling standardization!The simplified error class hierarchy with consistent status codes and the
ApiErrorResponseSchemaprovides a clean, predictable API error interface. This is a significant improvement for API consumers.apps/api/src/routes/api/feeds.ts (1)
1-312: Exceptional API route implementation!This file demonstrates excellent API design practices:
- ✅ Comprehensive request validation with Zod schemas
- ✅ Consistent error handling and response formats
- ✅ Proper HTTP status codes (201 for creation, 204 for deletion)
- ✅ Clear separation of authentication, authorization, and business logic errors
- ✅ Type-safe responses with schema parsing
The error handling pattern with specific catches for
NotFoundErrorandForbiddenErrorprovides clear, actionable error messages to API consumers.
| } catch (cleanuperror: unknown) { | ||
| console.error("Error cleaning up dump file:", cleanupError.message); | ||
| } |
There was a problem hiding this comment.
Fix variable name mismatch causing runtime error.
There's a critical variable name mismatch - the catch parameter is cleanuperror but line 118 references cleanupError.
Apply this diff to fix the variable name:
- } catch (cleanuperror: unknown) {
- console.error("Error cleaning up dump file:", cleanupError.message);
+ } catch (cleanupError: unknown) {
+ console.error("Error cleaning up dump file:", cleanupError.message);📝 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.
| } catch (cleanuperror: unknown) { | |
| console.error("Error cleaning up dump file:", cleanupError.message); | |
| } | |
| } catch (cleanupError: unknown) { | |
| console.error("Error cleaning up dump file:", cleanupError.message); | |
| } |
🤖 Prompt for AI Agents
In packages/shared-db/scripts/seed-remote.ts around lines 117 to 119, the catch
block parameter is named 'cleanuperror' but the error logging references
'cleanupError', causing a runtime error. Rename the variable in the
console.error statement to match the catch parameter 'cleanuperror' to fix the
mismatch.
| {steps.map((step, index) => ( | ||
| <div | ||
| key={index} | ||
| className="flex flex-col items-center min-w-[100px] px-2" | ||
| role="button" | ||
| tabIndex={index <= currentStep ? 0 : -1} | ||
| > | ||
| <span | ||
| className={`text-sm text-center ${ | ||
| index === currentStep ? "font-bold" : "" | ||
| }`} | ||
| > | ||
| {step.title} | ||
| </span> | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve accessibility for step indicators
The step indicators have role="button" but lack proper keyboard event handlers and aria labels for screen readers.
{steps.map((step, index) => (
<div
key={index}
className="flex flex-col items-center min-w-[100px] px-2"
- role="button"
+ role="presentation"
tabIndex={index <= currentStep ? 0 : -1}
+ aria-current={index === currentStep ? "step" : undefined}
+ aria-label={`Step ${index + 1}: ${step.title}`}
>
<span
className={`text-sm text-center ${
index === currentStep ? "font-bold" : ""
}`}
>
{step.title}
</span>
</div>
))}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/app/src/routes/_layout/create/feed.tsx around lines 38 to 53, the step
indicators use role="button" but lack keyboard event handlers and aria labels,
which reduces accessibility. Add onKeyDown handlers to support keyboard
interaction (e.g., triggering the button action on Enter or Space keys) and
include appropriate aria-label attributes describing each step for screen
readers. This will ensure the step indicators are fully accessible via keyboard
and assistive technologies.
| <Button type="submit" disabled={form.formState.isSubmitting}> | ||
| {form.formState.isSubmitting ? "..." : "Next"} | ||
| </Button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve loading state UX
The submit button shows "..." which provides poor user feedback.
<Button type="submit" disabled={form.formState.isSubmitting}>
- {form.formState.isSubmitting ? "..." : "Next"}
+ {form.formState.isSubmitting ? "Saving..." : "Next"}
</Button>🤖 Prompt for AI Agents
In apps/app/src/routes/_layout/create/feed/index.tsx around lines 133 to 135,
the submit button displays "..." during form submission, which is not
user-friendly. Replace the "..." text with a more descriptive loading indicator
such as "Submitting..." or a spinner component to clearly communicate the
loading state to users.
| // @ts-expect-error TODO: feed validation | ||
| await createFeedMutation.mutateAsync(feedConfig); |
There was a problem hiding this comment.
Address TypeScript error instead of ignoring it
The @ts-expect-error comment indicates a type mismatch that should be properly resolved.
The feedConfig object doesn't match the expected mutation input type. Would you like me to help create a proper type mapping function or update the feed configuration types?
🤖 Prompt for AI Agents
In apps/app/src/routes/_layout/create/feed/review.tsx around lines 22 to 23, the
use of @ts-expect-error hides a TypeScript type mismatch for the feedConfig
passed to createFeedMutation.mutateAsync. To fix this, identify the expected
input type for the mutation and either transform feedConfig to match that type
or update the type definitions of feedConfig accordingly. Remove the
@ts-expect-error comment once the types align correctly.
| {fields.map((field, index) => { | ||
| const typedField = field as Approver & { id: string }; | ||
| return ( | ||
| <div | ||
| key={typedField.id} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety for field iteration.
The type casting to Approver & { id: string } could be unsafe if the field structure changes. Consider using proper typing or validation.
-{fields.map((field, index) => {
- const typedField = field as Approver & { id: string };
+{fields.map((field, index) => {
+ // Ensure proper typing without casting
+ if (!field || typeof field !== 'object' || !('handle' in field) || !('platform' in field)) {
+ return null;
+ }
+ const typedField = field as Approver & { id: string };📝 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.
| {fields.map((field, index) => { | |
| const typedField = field as Approver & { id: string }; | |
| return ( | |
| <div | |
| key={typedField.id} | |
| {fields.map((field, index) => { | |
| // Ensure proper typing without casting | |
| if (!field || typeof field !== 'object' || !('handle' in field) || !('platform' in field)) { | |
| return null; | |
| } | |
| const typedField = field as Approver & { id: string }; | |
| return ( | |
| <div | |
| key={typedField.id} |
🤖 Prompt for AI Agents
In apps/app/src/components/ContentApprovers.tsx around lines 130 to 134, the
current type casting of field to Approver & { id: string } is unsafe and may
cause runtime issues if the field structure changes. To fix this, define a
proper TypeScript type or interface for the fields array that includes the id
property, and use that type directly in the map function instead of casting.
Alternatively, add runtime validation to ensure each field has the expected
structure before rendering.
* better auth services * nit picks * nonce validation * nitpicks
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
docker-compose.yml (1)
43-43: Address static analysis warning about credential exposure.The static analysis tool flagged the hardcoded database credentials in the
DATABASE_URL. While this is acceptable for development environments, consider using environment variables for better security practices.- DATABASE_URL: postgresql://postgres:postgres@postgres_dev:5432/curatedotfun + DATABASE_URL: postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@postgres_dev:5432/curatedotfunapps/api/src/middlewares/auth.middleware.ts (1)
7-30: Solid JWT authentication implementation.The middleware correctly:
- Extracts JWT from cookies (secure approach)
- Verifies against environment secret
- Gracefully handles invalid tokens as anonymous users
- Sets
accountIdfrom token'ssubclaimThe error handling is appropriate for middleware - invalid tokens don't block requests but proceed anonymously.
Consider validating
JWT_SECRETat application startup rather than in middleware to fail fast:// In app initialization if (!process.env.JWT_SECRET) { throw new Error("JWT_SECRET environment variable is required"); }apps/api/src/services/auth.service.ts (1)
52-56: Consider making the recipient configurable.The recipient "curatefun.near" is hardcoded, which limits flexibility across different environments or use cases.
Consider making this configurable via environment variables:
+ const recipient = process.env.AUTH_RECIPIENT || "curatefun.near"; + return { nonce, - recipient: "curatefun.near", + recipient, };packages/shared-db/src/repositories/authRequest.repository.ts (1)
31-56: Consider making the time window configurable to match service layer constants.The hardcoded 5-minute window should match the
AUTH_REQUEST_EXPIRY_MSconstant used in the AuthService to ensure consistency.Consider accepting the time window as a parameter:
- async findLatestByAccountId(accountId: string) { + async findLatestByAccountId(accountId: string, timeWindowMs: number = 5 * 60 * 1000) { return withErrorHandling( async () => { return executeWithRetry(async (dbInstance) => { - const fiveMinutesAgo = new Date(Date.now() - 5 * 60 * 1000); + const cutoffTime = new Date(Date.now() - timeWindowMs); const result = await dbInstance .select() .from(authRequests) .where( and( eq(authRequests.accountId, accountId), - gte(authRequests.createdAt, fiveMinutesAgo), + gte(authRequests.createdAt, cutoffTime), ), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
apps/api/package.json(2 hunks)apps/api/src/middlewares/auth.middleware.ts(1 hunks)apps/api/src/routes/api/auth.ts(1 hunks)apps/api/src/routes/api/index.ts(2 hunks)apps/api/src/services/auth.service.ts(1 hunks)apps/api/src/services/users.service.ts(8 hunks)apps/api/src/utils/service-provider.ts(5 hunks)apps/app/package.json(2 hunks)apps/app/src/components/UserMenu.tsx(2 hunks)apps/app/src/contexts/auth-context.tsx(2 hunks)apps/app/src/lib/api-client.ts(0 hunks)docker-compose.yml(1 hunks)packages/shared-db/Dockerfile.dev(1 hunks)packages/shared-db/migrations/0017_warm_cerebro.sql(1 hunks)packages/shared-db/migrations/meta/0017_snapshot.json(1 hunks)packages/shared-db/migrations/meta/_journal.json(1 hunks)packages/shared-db/src/index.ts(1 hunks)packages/shared-db/src/repositories/authRequest.repository.ts(1 hunks)packages/shared-db/src/schema/auth.ts(1 hunks)packages/shared-db/src/schema/index.ts(1 hunks)packages/utils/src/errors.ts(1 hunks)packages/utils/src/index.ts(0 hunks)packages/utils/src/plugins/index.ts(0 hunks)scripts/dev.sh(1 hunks)
💤 Files with no reviewable changes (3)
- packages/utils/src/index.ts
- packages/utils/src/plugins/index.ts
- apps/app/src/lib/api-client.ts
✅ Files skipped from review due to trivial changes (7)
- packages/shared-db/migrations/meta/_journal.json
- scripts/dev.sh
- apps/api/package.json
- packages/shared-db/src/schema/index.ts
- packages/shared-db/src/index.ts
- apps/api/src/routes/api/index.ts
- packages/shared-db/migrations/0017_warm_cerebro.sql
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/api/src/services/users.service.ts
- apps/api/src/utils/service-provider.ts
- apps/app/package.json
🧰 Additional context used
🪛 Checkov (3.2.334)
docker-compose.yml
[MEDIUM] 43-44: Basic Auth Credentials
(CKV_SECRET_4)
🔇 Additional comments (15)
packages/utils/src/errors.ts (1)
1-4: LGTM! Improved TypeScript type-only imports.Converting to
import typeis correct since bothPluginErrorInterfaceandPluginErrorContextare used purely for type checking (interface implementation and type annotations). This change reduces bundle size and follows TypeScript best practices.docker-compose.yml (2)
39-41: LGTM! Build configuration aligns with new Dockerfile.The change from a prebuilt image to a build configuration using the new
Dockerfile.devis well-structured and allows for better control over the migration environment.
51-51: Good improvement: Array command format is cleaner.Using array format for the command is more explicit and avoids potential shell parsing issues.
apps/app/src/components/UserMenu.tsx (3)
24-31: Excellent authentication state management.The separation of
isSignedInandisAuthorizedprovides a cleaner authentication flow, allowing for proper OAuth-like authorization steps.
50-67: Well-structured progressive authentication UI.The three-state authentication flow (not signed in → signed in but not authorized → fully authorized) provides clear user guidance and improves the overall authentication experience.
69-124: Clean dropdown menu implementation.The dropdown menu correctly renders only when the user is fully authorized, maintaining good security boundaries in the UI.
packages/shared-db/Dockerfile.dev (1)
1-41: Excellent multi-stage Dockerfile design.This Dockerfile follows best practices for monorepo builds:
- Pruner stage: Uses
turbo pruneto optimize build context for the specific package- Builder stage: Efficiently installs dependencies using frozen lockfile
- Production stage: Creates minimal runtime image focused on migration tasks
The structure is well-optimized for build caching and reduces final image size.
apps/api/src/middlewares/auth.middleware.ts (1)
2-3: Good choice of JWT utilities.Using
hono/jwtfor verification andhono/cookiefor token extraction provides secure, framework-integrated JWT handling.packages/shared-db/src/schema/auth.ts (3)
14-30: Well-designed authentication requests schema.The table structure effectively supports nonce-based authentication:
- Unique nonce: Prevents replay attacks
- Optional state: Supports OAuth-like flows
- Expiration timestamp: Prevents stale requests
- Efficient indexing:
(accountId, createdAt)supports common query patternsThe schema follows database best practices for authentication flows.
32-37: Proper relational integrity.The relation to the
userstable throughnearAccountIdmaintains referential integrity and enables efficient joins.
39-45: Good validation schema design.The Zod schema properly excludes auto-generated fields (
id,createdAt,updatedAt) from insert operations, preventing common validation errors.apps/api/src/routes/api/auth.ts (1)
58-67: LGTM!The logout endpoint correctly clears the JWT cookie by setting maxAge to 0 while maintaining the same security settings.
apps/app/src/contexts/auth-context.tsx (2)
77-119: LGTM!The authorization flow is well-implemented with proper error handling, user feedback via toasts, and secure nonce-based challenge-response authentication.
121-139: LGTM!The sign-out implementation correctly prioritizes user experience by ensuring local sign-out succeeds even if the backend call fails, while still attempting to clear server-side session data.
packages/shared-db/migrations/meta/0017_snapshot.json (1)
1404-1493: LGTM!The
auth_requeststable schema is well-designed with:
- Appropriate unique constraints on
nonceandstate- Efficient composite index on
account_idandcreated_atfor query performance- Proper timestamp tracking with
expires_atfor time-based validation
| } catch (error: unknown) { | ||
| c.status(401); | ||
| return c.json({ | ||
| success: false, | ||
| error: error instanceof Error ? error.message : "Authentication failed", | ||
| }); | ||
| } |
There was a problem hiding this comment.
Avoid exposing internal error details in authentication responses.
For security reasons, authentication endpoints should return generic error messages to prevent information leakage about the system's internal state.
Apply this diff to return a generic error message:
} catch (error: unknown) {
c.status(401);
return c.json({
success: false,
- error: error instanceof Error ? error.message : "Authentication failed",
+ error: "Authentication failed",
});
}📝 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.
| } catch (error: unknown) { | |
| c.status(401); | |
| return c.json({ | |
| success: false, | |
| error: error instanceof Error ? error.message : "Authentication failed", | |
| }); | |
| } | |
| } catch (error: unknown) { | |
| c.status(401); | |
| return c.json({ | |
| success: false, | |
| error: "Authentication failed", | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/routes/api/auth.ts around lines 48 to 54, the current error
response returns the actual error message which may expose internal details.
Modify the catch block to always return a generic error message like
"Authentication failed" instead of the specific error.message, ensuring no
internal error details are leaked in the response.
| const secret = process.env.JWT_SECRET; | ||
| if (!secret) { | ||
| throw new Error("JWT_SECRET is not set."); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Check JWT_SECRET earlier in the service initialization.
The JWT_SECRET environment variable should be validated during service initialization rather than during request processing.
Consider adding a constructor validation:
constructor(
authRequestRepository: AuthRequestRepository,
userService: UserService,
) {
if (!process.env.JWT_SECRET) {
throw new Error("JWT_SECRET environment variable is required");
}
this.authRequestRepository = authRequestRepository;
this.userService = userService;
}Then simplify the JWT signing:
- const secret = process.env.JWT_SECRET;
- if (!secret) {
- throw new Error("JWT_SECRET is not set.");
- }
+ const secret = process.env.JWT_SECRET!;🤖 Prompt for AI Agents
In apps/api/src/services/auth.service.ts around lines 95 to 98, the JWT_SECRET
environment variable is currently checked during request processing, which is
too late. Move the validation of JWT_SECRET to the service class constructor to
ensure it is set during initialization. Add a constructor that throws an error
if JWT_SECRET is missing, then remove the runtime check and directly use the
secret for JWT signing.
| if (!latestRequest) { | ||
| throw new Error("No recent auth request found for this account."); | ||
| } | ||
|
|
||
| if (latestRequest.expiresAt < new Date()) { | ||
| await this.authRequestRepository.deleteById(latestRequest.id); | ||
| throw new Error("Auth request has expired."); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use generic error messages for authentication failures.
The detailed error messages could leak information about the system's state to potential attackers.
Apply this diff to use generic authentication error messages:
if (!latestRequest) {
- throw new Error("No recent auth request found for this account.");
+ throw new Error("Authentication failed");
}
if (latestRequest.expiresAt < new Date()) {
await this.authRequestRepository.deleteById(latestRequest.id);
- throw new Error("Auth request has expired.");
+ throw new Error("Authentication failed");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!latestRequest) { | |
| throw new Error("No recent auth request found for this account."); | |
| } | |
| if (latestRequest.expiresAt < new Date()) { | |
| await this.authRequestRepository.deleteById(latestRequest.id); | |
| throw new Error("Auth request has expired."); | |
| } | |
| if (!latestRequest) { | |
| throw new Error("Authentication failed"); | |
| } | |
| if (latestRequest.expiresAt < new Date()) { | |
| await this.authRequestRepository.deleteById(latestRequest.id); | |
| throw new Error("Authentication failed"); | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/services/auth.service.ts around lines 64 to 71, replace the
specific error messages "No recent auth request found for this account." and
"Auth request has expired." with a single generic error message like
"Authentication failed." to avoid leaking system state details. This means
throwing the same generic error for both cases to enhance security by not
revealing whether the account has no recent request or if the request expired.
| if (verificationResult.accountId !== accountId) { | ||
| throw new Error("Account ID mismatch."); | ||
| } |
There was a problem hiding this comment.
Use generic error message for account mismatch.
Apply this diff:
if (verificationResult.accountId !== accountId) {
- throw new Error("Account ID mismatch.");
+ throw new Error("Authentication failed");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (verificationResult.accountId !== accountId) { | |
| throw new Error("Account ID mismatch."); | |
| } | |
| if (verificationResult.accountId !== accountId) { | |
| throw new Error("Authentication failed"); | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/services/auth.service.ts around lines 84 to 86, the error
message for account ID mismatch is too specific. Replace the current error
message "Account ID mismatch." with a more generic error message to avoid
revealing sensitive information. Use a general message like "Invalid
credentials" or "Authentication failed" instead.
* fix: toast colors not corrent * fmt
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores