refactor: apply DRY principles and eliminate code duplication#381
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@thedaviddias has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 18 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 ignored due to path filters (1)
📒 Files selected for processing (22)
WalkthroughReplaces local rate-limiting with a shared rate-limiting package, adds api-utils and security packages (validation/sanitization/constants), centralizes CSRF handling across middleware/actions/forms, removes duplicate hooks/components, introduces a shared form-state hook, and updates tests/mocks and package manifests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Route as API Route
participant RL as packages/rate-limiting
participant Handler as Route Handler
Client->>Route: HTTP request
Route->>RL: withRateLimit(request, ENDPOINT, handler)
alt within limit
RL->>Handler: invoke handler
Handler-->>RL: Response/data
RL-->>Route: Response (+ rate-limit headers)
Route-->>Client: 200 OK (+ headers)
else exceeded
RL-->>Route: 429 Response (+ Retry-After)
Route-->>Client: 429 Too Many Requests
end
sequenceDiagram
autonumber
actor User
participant Form as SubmitForm (client)
participant API as /api/fetch-metadata
participant MW as middleware (CSRF)
participant Action as submitLlmsTxt (server action)
Note over Form: Read meta[name="csrf-token"]
User->>Form: Fill & submit
Form->>API: fetch(..., headers x-csrf-token?)
API->>MW: Request passes through
alt token valid or exempt
MW-->>API: allow
API-->>Form: Response
else invalid
MW-->>Form: 403 CSRF_INVALID
end
Form->>Action: FormData (+ _csrf if present)
Action->>Action: timingSafeEqual(stored, submitted)
alt valid
Action-->>User: Success
else invalid
Action-->>User: Error "Security validation failed."
end
sequenceDiagram
autonumber
actor Client
participant Route as API Route
participant Utils as withApiHandler (api-utils)
participant Impl as Business Logic
Client->>Route: HTTP request
Route->>Utils: withApiHandler(handler, options)
Utils->>Impl: handler(request)
alt handler returns Response
Impl-->>Utils: Response
Utils-->>Client: Response
else handler returns data
Impl-->>Utils: Data
Utils-->>Client: JSON 200
else throws ApiException
Impl-->>Utils: throw ApiException
Utils-->>Client: JSON error (statusCode)
else throws Error
Impl-->>Utils: throw Error
Utils-->>Client: JSON 500 (sanitized)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Major refactoring to improve code quality and maintainability: ## Consolidated Duplicate Code - Unified useDebounce hook implementations - Merged duplicate FavoriteButton components - Extracted common patterns into reusable packages ## New Reusable Packages Created - **@thedaviddias/api-utils**: Centralized API error handling with withApiHandler wrapper - **@thedaviddias/rate-limiting**: Unified Upstash Redis rate limiting implementation - **@thedaviddias/hooks/use-form-state**: Abstracted form state management patterns - **@thedaviddias/security**: Centralized validation and sanitization utilities ## Code Quality Improvements - Replaced type assertions with proper type guards - Added comprehensive JSDoc documentation - Split large modules into smaller, focused files - Fixed all biome linting issues ## Impact - Reduced code duplication across 100+ files - Improved type safety and maintainability - Standardized error handling and security validation - Better code organization and reusability
thedaviddias
force-pushed
the
feature/refactor-dry-code
branch
from
0a8d59e to
e9f0d83
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/web/app/members/members-search.tsx (1)
31-53: Guard navigation and use replace; remove searchParams from deps to avoid re-run loops/history spam.
Including searchParams in deps can cause redundant transitions; pushing the same URL repeatedly harms UX.Apply:
useEffect(() => { - const params = new URLSearchParams(searchParams) + const params = new URLSearchParams(searchParams) if (debouncedSearch) { params.set('search', debouncedSearch) } else { params.delete('search') } if (filter !== 'all') { params.set('filter', filter) } else { params.delete('filter') } // Reset page when search/filter changes params.delete('page') - startTransition(() => { - router.push(`${pathname}?${params.toString()}`) - }) -}, [debouncedSearch, filter, pathname, router, searchParams]) + const nextUrl = `${pathname}?${params.toString()}` + const currUrl = `${pathname}?${searchParams.toString()}` + if (nextUrl === currUrl) return + startTransition(() => { + router.replace(nextUrl) + }) +}, [debouncedSearch, filter, pathname, router])packages/rate-limiting/index.ts (1)
193-199: Prefix inconsistency (“next-forge”)Default prefix should match this repo to avoid key collisions with other projects.
export const createRateLimiter = (props: Omit<RatelimitConfig, 'redis'>) => new Ratelimit({ redis: getRedisClient(), limiter: props.limiter ?? Ratelimit.slidingWindow(10, '10 s'), - prefix: props.prefix ?? 'next-forge' + prefix: props.prefix ?? 'llms-txt-hub' })apps/web/app/api/members/route.ts (1)
51-55: Update callsite if withRateLimit adopts object-parameter signatureIf you apply the suggested API change, adjust the call accordingly and simplify the return type to Promise.
-export async function GET( - request: NextRequest -): Promise<NextResponse<MembersResponse | { error: string }> | Response> { - return await withRateLimit(request, 'MEMBERS_API', async () => { +export async function GET( + request: NextRequest +): Promise<Response> { + return await withRateLimit({ + request, + endpoint: 'MEMBERS_API', + handler: async () => {…and close the brace at the end:
- }) + } + })apps/web/app/api/members/contributions/route.ts (1)
40-52: Validate/normalize GitHub usernames and deduplicate to avoid wasted calls and cache fragmentationEnforce GitHub’s username rules, normalize to lowercase for consistent cache keys, and remove duplicates before batching.
- // Limit the number of usernames to prevent abuse - if (body.usernames.length > 50) { - return NextResponse.json({ error: 'Too many usernames requested' }, { status: 400 }) - } - - // Filter out invalid usernames - const validUsernames = body.usernames - .filter(username => typeof username === 'string' && username.trim().length > 0) - .slice(0, 50) + // Limit, validate and normalize usernames to prevent abuse and cache fragmentation + const MAX = 50 + const USERNAME_RE = /^[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}$/i + if (body.usernames.length > MAX) { + return NextResponse.json({ error: `Too many usernames requested (max ${MAX})` }, { status: 400 }) + } + const validUsernames = Array.from( + new Set( + body.usernames + .filter(u => typeof u === 'string') + .map(u => u.trim().toLowerCase()) + .filter(u => USERNAME_RE.test(u)) + ) + ).slice(0, MAX)packages/api-utils/index.ts.bak (1)
1-308: Remove packages/api-utils/index.ts.bak
This stray backup duplicates the real entry (packages/api-utils/index.ts) and isn’t referenced by package.json’smainorexports; delete it to avoid accidental publication.
🧹 Nitpick comments (33)
apps/web/lib/security-utils-helpers.ts (1)
63-90: Avoid unbounded Map growth; consider eviction or external storerateLimitMap can grow without bound with many unique identifiers. Either (a) add a simple TTL-based eviction pass, or (b) delegate to the new @thedaviddias/rate-limiting (Upstash Redis) package for durability and multi-instance correctness.
Happy to provide a minimal LRU/TTL eviction helper if you want to keep in-memory for local/dev.
apps/web/lib/security-utils.ts (1)
9-9: Gate server-only re-exports to avoid client bundle bloatRe-exporting checkRateLimit/clearRateLimiting/getRateLimitKey from a common module may pull rate-limiting code into client bundles if this file is imported on the client. Consider re-exporting these from a server-only entry (or add a "server-only" barrier) to keep client bundles lean.
If you’re on Next.js, placing rate-limit exports in a server-only module (or using the "server-only" package) is sufficient.
apps/web/app/members/members-search.tsx (2)
15-18: Complete the JSDoc per repo rules (add @param and @example).Apply:
/** - * Search component for filtering and navigating members - * @returns Member search component with filters + * Members search UI with debounced query + filter navigation. + * @param props - Component props (none). + * @returns JSX.Element + * @example + * <MembersSearch /> */
26-27: Narrow types for filter to a string-literal union.
Improves type-safety and autocompletion.Apply:
-const [filter, setFilter] = useState(searchParams.get('filter') || 'all') +type MemberFilter = 'all' | 'contributors' | 'community' +const [filter, setFilter] = useState<MemberFilter>((searchParams.get('filter') as MemberFilter) || 'all') @@ -const handleFilterChange = useCallback((newFilter: string) => { +const handleFilterChange = useCallback((newFilter: MemberFilter) => { setFilter(newFilter) }, [])Also applies to: 59-61
packages/security/package.json (2)
6-9: Point exports/main to built JS and add types for robust consumption.Referencing .ts in exports/main can break Node resolution outside Next/Turbopack. Prefer compiled dist with types.
Apply:
- "exports": { - ".": "./index.ts" - }, - "main": "./index.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js", + "default": "./dist/index.js" + } + }, + "types": "./dist/index.d.ts", + "main": "./dist/index.js",
10-13: Add a build step and restrict published files.Ensure artifacts exist and keep package lean.
Apply:
"scripts": { - "clean": "rm -rf .turbo && rm -rf node_modules && rm -rf dist", - "lint": "biome check ." + "clean": "rm -rf .turbo node_modules dist", + "build": "tsc -p tsconfig.json", + "lint": "biome check .", + "typecheck": "tsc -p tsconfig.json --noEmit" }, + "files": ["dist"],packages/api-utils/package.json (2)
6-9: Export built JS with types; avoid exporting .ts.Same rationale as the security package for broader compatibility.
Apply:
- "exports": { - ".": "./index.ts" - }, - "main": "./index.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js", + "default": "./dist/index.js" + } + }, + "types": "./dist/index.d.ts", + "main": "./dist/index.js",
10-13: Add build/typecheck scripts and engine constraint.Ensure artifacts and Node version alignment with Next.
Apply:
"scripts": { - "clean": "rm -rf .turbo && rm -rf node_modules && rm -rf dist", - "lint": "biome check ." + "clean": "rm -rf .turbo node_modules dist", + "build": "tsc -p tsconfig.json", + "lint": "biome check .", + "typecheck": "tsc -p tsconfig.json --noEmit" }, + "files": ["dist"], + "engines": { "node": ">=18.18" },packages/security/types.ts (1)
4-10: Prefer readonly to signal immutability of options.Helps avoid accidental mutation and clarifies intent.
Apply:
-export interface URLValidationOptions { - protocols?: string[] - requireProtocol?: boolean - requireTLD?: boolean - allowAuth?: boolean - maxLength?: number -} +export interface URLValidationOptions { + readonly protocols?: readonly string[] + readonly requireProtocol?: boolean + readonly requireTLD?: boolean + readonly allowAuth?: boolean + readonly maxLength?: number +}packages/security/utils.ts (2)
3-9: Add complete JSDoc with @param and @returns tags.Guidelines require JSDoc for utilities; include tags for tooling.
Apply:
-/** - * Batch validate multiple validation functions - */ -export function batchValidate(validations: Array<() => ValidationResult>): { +/** + * Batch validate multiple validators and aggregate their errors. + * @param validations - List of zero-arg validators returning a ValidationResult. + * @returns Object with overall validity and collected error messages. + */ +export function batchValidate(validations: ReadonlyArray<() => ValidationResult>): { valid: boolean errors: string[] } {
12-17: Collect thrown errors from validators instead of crashing the batch.A single throwing validator currently aborts the whole process.
Apply:
- for (const validate of validations) { - const result = validate() - if (!result.valid && result.error) { - errors.push(result.error) - } - } + for (const validate of validations) { + try { + const result = validate() + if (!result.valid && result.error) errors.push(result.error) + } catch (err) { + errors.push(err instanceof Error ? err.message : 'Unknown validation error') + } + }packages/security/constants.ts (1)
4-4: Favor an allowlist-driven URL protocol check.Blocklists miss edge cases; ensure validateURL defaults to an allowlist of safe protocols (e.g., http:, https:, mailto:, tel:).
You can add:
export const SAFE_PROTOCOLS = ['http:', 'https:', 'mailto:', 'tel:'] as constand have validators prefer SAFE_PROTOCOLS over MALICIOUS_PROTOCOLS when options are not provided.
sessions/tasks/m-refactor-dry-code.md (1)
45-52: Minor grammar and clarity nitsTighten wording and hyphenation.
Apply:
-**Rate Limiting Pattern Duplication:** +**Rate‑limiting Pattern Duplication:** @@ -- `/apps/web/lib/security-utils.ts` contains another rate limiting implementation using a shared Map +- `/apps/web/lib/security-utils.ts` contains another rate‑limiting implementation using a shared Map @@ -This creates maintenance burden and inconsistency in rate limiting behavior across endpoints. +This creates a maintenance burden and inconsistency in rate‑limiting behavior across endpoints.packages/rate-limiting/index.ts (1)
25-41: Add full JSDoc blocks with @param/@returns/@throws on exported utilitiesMeets repo guidelines for .ts utilities.
/** * Rate limiting configuration for different API endpoints */ export const RATE_LIMITS = { @@ -/** - * Check rate limit for a client and endpoint - */ +/** + * Check rate limit for a client and endpoint. + * @param {CheckRateLimitInput} input - The request and endpoint to check. + * @returns {Promise<RateLimitResult>} Result including limit, remaining, and reset timestamp (ms). + * @throws If Redis configuration is missing; otherwise fails open and logs the error. + */ export async function checkRateLimit( @@ -/** - * Middleware-style rate limiter that returns appropriate HTTP response - */ +/** + * Middleware-style rate limiter that returns appropriate HTTP response. + * @param {WithRateLimitInput} input - Request, endpoint key, and async handler returning a Response. + * @returns {Promise<Response>} A Response with rate-limit headers attached. + */ export async function withRateLimit<T>( @@ -/** - * Creates a custom rate limiter with specified configuration - * @param props - Rate limiter configuration without redis client - * @returns Configured Ratelimit instance - */ +/** + * Creates a custom rate limiter with specified configuration. + * @param {Omit<RatelimitConfig, 'redis'>} props - Limiter, prefix, and analytics options (redis is injected). + * @returns {Ratelimit} Configured Upstash Ratelimit instance. + */ export const createRateLimiter = (props: Omit<RatelimitConfig, 'redis'>) =>Also applies to: 90-97, 141-146, 193-200
apps/web/app/api/members/contributions/route.ts (2)
61-73: Cache key stability: use normalized usernamesAfter normalization, ensure the cache key uses the normalized value passed to getUserContributions to avoid split caches.
- const batchPromises = batch.map(async (username): Promise<UserContributionStatus> => { - const cacheKey = `${CACHE_KEYS.GITHUB_CONTRIBUTIONS}${username}` + const batchPromises = batch.map(async (username): Promise<UserContributionStatus> => { + const cacheKey = `${CACHE_KEYS.GITHUB_CONTRIBUTIONS}${username}`No logic change needed if you apply the normalization above, but keep the key derived from the normalized username exclusively.
26-31: Optional: standardize error handling via withApiHandlerConsider composing the rate limiter with the new @thedaviddias/api-utils withApiHandler to unify error shapes and logging across routes.
-export async function POST( - request: NextRequest -): Promise<NextResponse<ContributionsResponse | { error: string }> | Response> { - return await withRateLimit(request, 'CONTRIBUTIONS_API', async () => { +export async function POST( + request: NextRequest +): Promise<NextResponse<ContributionsResponse | { error: string }> | Response> { + return await withRateLimit(request, 'CONTRIBUTIONS_API', withApiHandler(async () => { try { const body: ContributionsRequest = await request.json() - ... + ... } catch (error) { logger.error('Error fetching member contributions:', { data: error, tags: { type: 'api' } }) return NextResponse.json({ error: 'Internal Server Error' }, { status: 500 }) } - }) + }, { tags: { endpoint: 'members/contributions' } })) }packages/hooks/src/use-form-state.tsx (3)
164-189: Prevent double-submits and setState-after-unmount with an in-flight/mounted refAvoid concurrent submissions and state updates on unmounted components.
const submit = useCallback( async (handler: (data: T) => Promise<unknown>): Promise<unknown> => { - setStatus('loading') - setError(null) + if (status === 'loading') return + const mounted = { current: true } + setStatus('loading') + setError(null) try { - const result = await handler(data) - setStatus('success') - setSuccessData(result) + const result = await handler(data) + if (!mounted.current) return result + setStatus('success') + setSuccessData(result) ... - } catch (err) { + } catch (err) { const errorMessage = getErrorMessage(err) - setStatus('error') - setError(errorMessage) + if (mounted.current) { + setStatus('error') + setError(errorMessage) + } onError?.(errorMessage) throw err } }, - [data, initialData, resetOnSuccess, onSuccess, onError, getErrorMessage] + [status, data, initialData, resetOnSuccess, onSuccess, onError, getErrorMessage] )Add a useEffect cleanup to flip mounted.current = false if you adopt this pattern.
69-70: Improve submit typing to propagate the handler’s result typeLet submit return the handler’s resolved type for better ergonomics.
- submit: (handler: (data: T) => Promise<unknown>) => Promise<unknown> + submit: <R>(handler: (data: T) => Promise<R>) => Promise<R>And in implementation:
- const submit = useCallback( - async (handler: (data: T) => Promise<unknown>): Promise<unknown> => { + const submit = useCallback( + async <R>(handler: (data: T) => Promise<R>): Promise<R> => {
1-1: Nit: use .ts since no JSX is usedRename to use-form-state.ts to reduce unnecessary TSX handling in tooling.
packages/api-utils/index.ts.bak (2)
216-225: Clarify handling of empty-string query paramssearchParams.get returns '' for “?key=”. Current logic treats '' as missing; ensure this is intended, or accept '' when optional with default.
- if (!value) { + if (value === null || value === undefined) { if (config.optional) { continue } if (config.default !== undefined) { result[key] = config.default continue } throw new ApiException(`Missing required parameter: ${String(key)}`, 400) }If you want to treat '' as missing too, keep as-is and add a comment.
302-307: CORS helper: add Vary: Origin and optional credentialsImprove cache correctness and interoperability.
export function withCors(response: NextResponse, origin = '*'): NextResponse { response.headers.set('Access-Control-Allow-Origin', origin) response.headers.set('Access-Control-Allow-Methods', 'GET, POST, PUT, DELETE, OPTIONS') response.headers.set('Access-Control-Allow-Headers', 'Content-Type, Authorization') + response.headers.append('Vary', 'Origin') + // Uncomment when needed: + // response.headers.set('Access-Control-Allow-Credentials', 'true') return response }packages/api-utils/index.ts (4)
202-271: Type-safety: remove ts-expect-error and tighten genericsAvoid ts-expect-error by iterating keys as keyof T and assigning with proper casts. Also handle boolean parsing case-insensitively and validate invalid boolean tokens.
-export function parseQueryParams<T extends Record<string, unknown>>( - searchParams: URLSearchParams, - schema: { - [K in keyof T]: { +export function parseQueryParams<T extends Record<string, unknown>>( + searchParams: URLSearchParams, + schema: { + [K in keyof T]: { type: 'string' | 'number' | 'boolean' default?: T[K] optional?: boolean min?: number max?: number enum?: readonly T[K][] } - } -): T { - const result: Partial<T> = {} - - for (const [key, config] of Object.entries(schema)) { - const value = searchParams.get(key) + } +): T { + const result = {} as T + + for (const key of Object.keys(schema) as Array<keyof T>) { + const config = schema[key] + const value = searchParams.get(String(key)) @@ - if (!value) { + if (!value) { @@ - throw new ApiException(`Missing required parameter: ${String(key)}`, 400) + throw new ApiException(`Missing required parameter: ${String(key)}`, 400) } @@ - let parsedValue: any = value + let parsedValue: unknown = value @@ case 'number': parsedValue = Number(value) if (Number.isNaN(parsedValue)) { throw new ApiException(`Invalid number for parameter: ${String(key)}`, 400) } @@ case 'boolean': - parsedValue = value === 'true' || value === '1' + if (/^(true|1)$/i.test(value)) parsedValue = true + else if (/^(false|0)$/i.test(value)) parsedValue = false + else + throw new ApiException(`Invalid boolean for parameter: ${String(key)}`, 400) break @@ - if (config.enum && !config.enum.includes(parsedValue)) { + if (config.enum && !config.enum.includes(parsedValue as T[keyof T])) { @@ - // @ts-expect-error - Safe assignment with validated key - result[key] = parsedValue + result[key] = parsedValue as T[keyof T] } @@ - // @ts-expect-error - Result is fully populated after validation - return result + return result
138-155: Optional: use centralized safe error messagingTo avoid leaking details in non-dev while remaining informative, consider using createSafeErrorMessage from @thedaviddias/security in the generic error path.
- const errorMessage = error instanceof Error ? error.message : defaultErrorMessage - const sanitizedMessage = includeErrorDetails ? errorMessage : defaultErrorMessage + const errorMessage = error instanceof Error ? error.message : defaultErrorMessage + const sanitizedMessage = includeErrorDetails + ? errorMessage + : /* prefer centralized sanitizer */ errorMessageIf desired, replace the comment with an actual call after importing the helper.
309-314: CORS: add Vary header and optional credentialsSet Vary: Origin to keep caches correct. Optionally support credentials via options object.
-export function withCors(response: NextResponse, origin = '*'): NextResponse { +export function withCors(response: NextResponse, origin = '*', allowCredentials = false): NextResponse { response.headers.set('Access-Control-Allow-Origin', origin) response.headers.set('Access-Control-Allow-Methods', 'GET, POST, PUT, DELETE, OPTIONS') response.headers.set('Access-Control-Allow-Headers', 'Content-Type, Authorization') + response.headers.set('Vary', 'Origin') + if (allowCredentials) response.headers.set('Access-Control-Allow-Credentials', 'true') return response }
31-31: Nit: JSDoc wordingApiResponse is a union of success or error; the comment “Standard API success response” is misleading. Consider “Standard API response union (success or error)”.
packages/security/sanitization.ts (2)
10-17: API shape: define and export an options interfaceFor consistency with the codebase guidelines (interfaces, named Input objects), define SanitizeTextOptions and use it here.
-export function sanitizeText( - input: string | null | undefined, - options?: { - allowedTags?: string[] - allowedAttributes?: string[] - keepContent?: boolean - } -): string | null { +export interface SanitizeTextOptions { + allowedTags?: string[] + allowedAttributes?: string[] + keepContent?: boolean +} + +export function sanitizeText( + input: string | null | undefined, + options?: SanitizeTextOptions +): string | null {
79-91: Parity: include “Resource not found” mappingapps/web/lib/security-utils.ts maps “Resource not found” as well. Add for parity across packages.
const errorMappings: Array<{ pattern: RegExp; message: string }> = [ @@ - { pattern: /Access denied/i, message: 'Access denied' } + { pattern: /Access denied/i, message: 'Access denied' }, + { pattern: /Resource not found/i, message: 'Resource not found' } ]packages/security/validation.ts (6)
9-12: Precision: restrict malicious protocol check to the schemeincludes() can false-positive when the string appears in query/fragment. Check only the beginning of the URL.
-export function hasMaliciousProtocol(url: string): boolean { - const lowerUrl = url.toLowerCase().trim() - return MALICIOUS_PROTOCOLS.some(protocol => lowerUrl.includes(protocol)) -} +export function hasMaliciousProtocol(url: string): boolean { + const lower = url.toLowerCase().trim() + return MALICIOUS_PROTOCOLS.some(protocol => lower.startsWith(protocol)) +}
163-178: Phone: preserve leading + for international formatsStripping non-digits removes “+”, hurting non-US locales. Preserve a single leading +.
- const cleaned = phone.replace(/\D/g, '') + const cleaned = phone.trim().replace(/(?!^\+)[^\d]/g, '')
183-209: LinkedIn: case-insensitive check and normalizationMake the profile path check case-insensitive; keep sanitized URL with https:// prefix.
- if (!trimmed.includes('linkedin.com/in/')) { + if (!/linkedin\.com\/in\//i.test(trimmed)) { return { valid: false, error: 'Please enter a valid LinkedIn profile URL (e.g., linkedin.com/in/yourname)' } }
214-242: Origin validation: advise method gatingBrowsers often omit Origin on same-site GETs. Consider applying this check only to state-changing methods and documenting that expectation.
1-247: JSDoc completeness for utilitiesPer repo guidelines, add @param/@returns (and @throws where applicable) to all exported functions in this file.
244-247: Remove orphaned comment blockThere’s a dangling “Batch validate multiple inputs” doc block without implementation.
-/** - * Batch validate multiple inputs - */ -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
apps/web/app/api/members/contributions/route.ts(1 hunks)apps/web/app/api/members/route.ts(1 hunks)apps/web/app/members/members-search.tsx(1 hunks)apps/web/components/favorites/favorite-button.tsx(0 hunks)apps/web/hooks/use-debounce.ts(0 hunks)apps/web/lib/persistent-rate-limit.ts(0 hunks)apps/web/lib/rate-limiter.ts(0 hunks)apps/web/lib/security-utils-helpers.ts(1 hunks)apps/web/lib/security-utils.ts(1 hunks)configs/typescript/package.json(1 hunks)packages/api-utils/index.ts(1 hunks)packages/api-utils/index.ts.bak(1 hunks)packages/api-utils/package.json(1 hunks)packages/content/package.json(1 hunks)packages/hooks/package.json(1 hunks)packages/hooks/src/use-form-state.tsx(1 hunks)packages/newsletter/package.json(1 hunks)packages/rate-limiting/index.ts(2 hunks)packages/security/constants.ts(1 hunks)packages/security/package.json(1 hunks)packages/security/sanitization.ts(1 hunks)packages/security/types.ts(1 hunks)packages/security/utils.ts(1 hunks)packages/security/validation.ts(1 hunks)sessions/tasks/m-refactor-dry-code.md(1 hunks)
💤 Files with no reviewable changes (4)
- apps/web/hooks/use-debounce.ts
- apps/web/lib/rate-limiter.ts
- apps/web/components/favorites/favorite-button.tsx
- apps/web/lib/persistent-rate-limit.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/functions.mdc)
**/*.{ts,tsx}: For functions with more than one parameter, use a single object parameter instead of multiple positional parameters
Name the input type for such object parameters using the pattern {FunctionName}Input
For single-parameter functions, direct positional arguments are acceptable
**/*.{ts,tsx}: Write concise, technical TypeScript code
Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Use TypeScript for all code; prefer interfaces over types
Avoid enums; use object maps instead
Use thefunctionkeyword for pure functions
Avoid unnecessary curly braces in simple conditionals; use concise syntax
**/*.{ts,tsx}: Use interfaces over types when possible in TypeScript
Avoid enums; prefer const objects (as const) instead
Use proper error handling with custom error types
Favor functional programming patterns
Avoid classes
Use pure functions where possible
Implement proper error handling
Use meaningful variable names
Keep functions small and focused
Files:
packages/security/utils.tsapps/web/app/api/members/route.tspackages/security/types.tsapps/web/app/members/members-search.tsxpackages/security/constants.tsapps/web/lib/security-utils-helpers.tspackages/security/sanitization.tspackages/api-utils/index.tspackages/security/validation.tspackages/rate-limiting/index.tspackages/hooks/src/use-form-state.tsxapps/web/app/api/members/contributions/route.tsapps/web/lib/security-utils.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/jsdoc-comments.mdc)
Add JSDoc blocks for utility functions, documenting purpose, @param entries, @returns, and @throws where applicable
Files:
packages/security/utils.tsapps/web/app/api/members/route.tspackages/security/types.tspackages/security/constants.tsapps/web/lib/security-utils-helpers.tspackages/security/sanitization.tspackages/api-utils/index.tspackages/security/validation.tspackages/rate-limiting/index.tsapps/web/app/api/members/contributions/route.tsapps/web/lib/security-utils.ts
apps/web/app/**/route.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/nextjs.mdc)
Use API Route Handlers (route.ts/tsx) to manage backend logic within the app structure
Files:
apps/web/app/api/members/route.tsapps/web/app/api/members/contributions/route.ts
apps/web/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
apps/web/app/**/*.{ts,tsx}: Follow Next.js 15 App Router best practices in the web app
Use React Server Components (RSC) by default; only add 'use client' when necessary
Implement type-safe Server Actions using next-safe-action
Files:
apps/web/app/api/members/route.tsapps/web/app/members/members-search.tsxapps/web/app/api/members/contributions/route.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
apps/web/**/*.{ts,tsx}: Use Shadcn UI components with Radix UI primitives sourced from packages/design-system
Optimize for Core Web Vitals (images, fonts, script strategy, code-splitting)
Use camelCase for function and variable names
Use descriptive names with auxiliary verbs (e.g., isLoading, hasError)
Files:
apps/web/app/api/members/route.tsapps/web/app/members/members-search.tsxapps/web/lib/security-utils-helpers.tsapps/web/app/api/members/contributions/route.tsapps/web/lib/security-utils.ts
apps/web/**
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
Use kebab-case for directories
Files:
apps/web/app/api/members/route.tsapps/web/app/members/members-search.tsxapps/web/lib/security-utils-helpers.tsapps/web/app/api/members/contributions/route.tsapps/web/lib/security-utils.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional React components typed with TypeScript interfaces
Write declarative JSX
Use Shadcn UI, Radix UI, and Tailwind CSS for components and styling
Implement responsive, mobile-first design with Tailwind CSS
When using next/image, include explicit sizes and enable lazy loadingAdd JSDoc blocks for React components, including brief description, @param props with individual prop fields, @returns, and an @example usage
Files:
apps/web/app/members/members-search.tsxpackages/hooks/src/use-form-state.tsx
apps/web/app/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/nextjs.mdc)
apps/web/app/**/*.tsx: Prioritize using React Server Components (RSC) in the Next.js App Router for performance, SEO, and data fetching
Use client components sparingly—add 'use client' only when interactivity is required
Files:
apps/web/app/members/members-search.tsx
apps/web/{**/*.{tsx,css},tailwind.config.{js,ts},postcss.config.{js,ts}}
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
Follow mobile-first responsive design using Tailwind CSS 4
Files:
apps/web/app/members/members-search.tsx
**/index.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/shared.mdc)
Do not use barrel files (avoid index files that only re-export)
Files:
packages/api-utils/index.tspackages/rate-limiting/index.ts
**/use*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/jsdoc-comments.mdc)
Add JSDoc blocks for custom React hooks, including purpose, parameters, return value, and an example of hook usage
Files:
packages/hooks/src/use-form-state.tsx
🧠 Learnings (14)
📚 Learning: 2025-08-26T11:18:35.229Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:35.229Z
Learning: Applies to **/use*.{ts,tsx} : Add JSDoc comments for custom React hooks, including description, param entries, returns, and an example usage snippet
Applied to files:
packages/hooks/package.jsonpackages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:39.811Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:39.811Z
Learning: Applies to **/use*.{ts,tsx} : Document React hooks with JSDoc: include a brief description, param entries, returns, and an example of hook usage.
Applied to files:
packages/hooks/package.jsonpackages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:53.092Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-08-26T11:18:53.092Z
Learning: Applies to apps/web/app/**/route.ts : Use Next.js API route handlers within the App Router via 'route.ts'
Applied to files:
packages/api-utils/index.ts.bak
📚 Learning: 2025-08-26T11:18:59.984Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-08-26T11:18:59.984Z
Learning: Applies to apps/web/app/**/error.tsx : Implement route-level error handling with error.tsx
Applied to files:
packages/api-utils/index.ts.bak
📚 Learning: 2025-08-26T11:19:09.148Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-08-26T11:19:09.148Z
Learning: Applies to apps/web/app/**/error.tsx : Implement custom error handling with error.tsx in route segments
Applied to files:
packages/api-utils/index.ts.bak
📚 Learning: 2025-08-26T11:18:47.247Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-08-26T11:18:47.247Z
Learning: Applies to apps/web/app/**/error.tsx : Implement route-level error boundaries with error.tsx to handle errors gracefully
Applied to files:
packages/api-utils/index.ts.bak
📚 Learning: 2025-08-26T11:18:44.245Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:44.245Z
Learning: Applies to **/use*.{ts,tsx} : Add JSDoc blocks for custom React hooks, including purpose, parameters, return value, and an example of hook usage
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:40.806Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:40.806Z
Learning: Applies to **/use*.{ts,tsx} : Document React hooks with a JSDoc block including a brief description, param, returns, and an example
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:10.612Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:10.612Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Test custom React hooks with renderHook and wrap state changes in act
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:18.780Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:18.780Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : For custom hooks, use renderHook and wrap state changes in act when needed
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:30.246Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:30.246Z
Learning: Applies to **/use*.{ts,tsx} : Add JSDoc to custom React hooks with a brief purpose, param for inputs, returns for outputs, and an example of usage.
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:24.941Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:24.941Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : For custom hooks, use renderHook and wrap state changes in act when asserting updates
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:49.726Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:49.726Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Test custom React hooks using renderHook and wrap state changes in act
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:58.532Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:58.532Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}} : Test custom hooks with renderHook and wrap state changes in act
Applied to files:
packages/hooks/src/use-form-state.tsx
🧬 Code graph analysis (5)
packages/security/utils.ts (1)
packages/security/types.ts (1)
ValidationResult(15-19)
packages/security/sanitization.ts (1)
apps/web/lib/security-utils.ts (2)
sanitizeText(14-29)createSafeErrorMessage(122-166)
packages/api-utils/index.ts (1)
packages/logging/index.ts (2)
error(106-122)logger(168-168)
packages/security/validation.ts (3)
packages/security/constants.ts (2)
MALICIOUS_PROTOCOLS(4-4)RESERVED_USERNAMES(9-30)packages/security/types.ts (2)
URLValidationOptions(4-10)ValidationResult(15-19)packages/security/sanitization.ts (1)
sanitizeText(10-34)
packages/rate-limiting/index.ts (2)
apps/web/lib/security-utils.ts (1)
checkRateLimit(9-9)apps/web/middleware.ts (1)
config(389-396)
🪛 LanguageTool
sessions/tasks/m-refactor-dry-code.md
[grammar] ~15-~15: There might be a mistake here.
Context: ...ean, and efficient. ## Success Criteria - [ ] Identify all significant code duplic...
(QB_NEW_EN)
[uncategorized] ~45-~45: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ses a different context import path. Rate Limiting Pattern Duplication: Rate limiting l...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~47-~47: There might be a mistake here.
Context: ...with similar but not identical patterns: - /apps/web/app/api/check-url/route.ts contains inline rate limiting with in-me...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ...a shared Map The inline implementation in check-url route duplicates the core rat...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ...the dedicated rate-limiter module. This creates maintenance burden and inconsistency in...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...tem contains sophisticated patterns for: - Debounced input handling (using the dupl...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ... (using the duplicated useDebounce hook) - Loading state management during async op...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ...state management during async operations - Keyboard navigation (ArrowDown, ArrowUp,...
(QB_NEW_EN)
[grammar] ~109-~109: There might be a mistake here.
Context: ...tion (ArrowDown, ArrowUp, Enter, Escape) - Click outside handling to close dropdown...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...lick outside handling to close dropdowns - Recent searches localStorage management ...
(QB_NEW_EN)
[grammar] ~117-~117: There might be a mistake here.
Context: ...rity 1 - Utility Function Consolidation:** The useDebounce hook duplication should ...
(QB_NEW_EN)
[grammar] ~120-~120: There might be a mistake here.
Context: ... Priority 2 - Component Abstraction: The FavoriteButton components need to be...
(QB_NEW_EN)
[grammar] ~123-~123: There might be a mistake here.
Context: ...Priority 3 - API Pattern Extraction: Create a higher-order function or wrappe...
(QB_NEW_EN)
[grammar] ~124-~124: There might be a mistake here.
Context: ...rapper for API routes that standardizes: - Error logging with consistent format - N...
(QB_NEW_EN)
[grammar] ~125-~125: There might be a mistake here.
Context: ...: - Error logging with consistent format - NextResponse.json error responses - Ra...
(QB_NEW_EN)
[grammar] ~126-~126: There might be a mistake here.
Context: ...rmat - NextResponse.json error responses - Rate limiting application - CSRF validat...
(QB_NEW_EN)
[uncategorized] ~127-~127: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...- NextResponse.json error responses - Rate limiting application - CSRF validation where nee...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~127-~127: There might be a mistake here.
Context: ... responses - Rate limiting application - CSRF validation where needed **Priority...
(QB_NEW_EN)
[grammar] ~130-~130: There might be a mistake here.
Context: ...Priority 4 - Form State Abstraction: Extract the common form submission patte...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ...pattern into a custom hook that handles: - Loading/submitting state - Error message...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...that handles: - Loading/submitting state - Error message management - Success sta...
(QB_NEW_EN)
[grammar] ~133-~133: There might be a mistake here.
Context: ...mitting state - Error message management - Success state handling - CSRF token mana...
(QB_NEW_EN)
[grammar] ~134-~134: There might be a mistake here.
Context: ...ge management - Success state handling - CSRF token management - Async submission...
(QB_NEW_EN)
[grammar] ~135-~135: There might be a mistake here.
Context: ...s state handling - CSRF token management - Async submission with try-catch **Prior...
(QB_NEW_EN)
[grammar] ~138-~138: There might be a mistake here.
Context: ...5 - Security Validation Consolidation:** Centralize URL validation, rate limiting...
(QB_NEW_EN)
[grammar] ~145-~145: There might be a mistake here.
Context: ... Current Duplication Inventory Hooks: - useDebounce: 2 implementations (should use packages...
(QB_NEW_EN)
[grammar] ~148-~148: There might be a mistake here.
Context: ...e packages/hooks version) Components: - FavoriteButton: 2 implementations with different APIs ...
(QB_NEW_EN)
[grammar] ~149-~149: There might be a mistake here.
Context: ...`: 2 implementations with different APIs - Loading states: Manual useState pattern ...
(QB_NEW_EN)
[grammar] ~150-~150: There might be a mistake here.
Context: ...State pattern repeated across components - Form submission: Manual async/await patt...
(QB_NEW_EN)
[grammar] ~153-~153: There might be a mistake here.
Context: ...await pattern repeated API Utilities: - Rate limiting: 3 different implementatio...
(QB_NEW_EN)
[grammar] ~154-~154: There might be a mistake here.
Context: ...te limiting: 3 different implementations - Error handling: Manual try-catch repeate...
(QB_NEW_EN)
[grammar] ~155-~155: There might be a mistake here.
Context: ...nual try-catch repeated across 7+ routes - URL validation: 2 implementations with d...
(QB_NEW_EN)
[grammar] ~158-~158: There might be a mistake here.
Context: ...different coverage Security Patterns: - Input sanitization: Scattered across mul...
(QB_NEW_EN)
[grammar] ~159-~159: There might be a mistake here.
Context: ...ization: Scattered across multiple files - CSRF handling: Inline implementation in ...
(QB_NEW_EN)
[grammar] ~160-~160: There might be a mistake here.
Context: ...handling: Inline implementation in forms - Origin validation: Single implementation...
(QB_NEW_EN)
[grammar] ~201-~201: There might be a mistake here.
Context: ...r Implementation Remove Duplications: - Delete: `/apps/web/hooks/use-debounce.ts...
(QB_NEW_EN)
[grammar] ~202-~202: There might be a mistake here.
Context: ...ion Remove Duplications: - Delete: /apps/web/hooks/use-debounce.ts - Delete: One of the FavoriteButton implem...
(QB_NEW_EN)
🔇 Additional comments (14)
apps/web/lib/security-utils.ts (1)
6-6: LGTM: import source consolidatedImporting sanitizeErrorMessage from helpers removes duplication and keeps sanitization logic single-sourced.
configs/typescript/package.json (1)
6-8: LGTM — formatting-only change.
No functional impact; keeping base.json included via "files" is fine even though the package is private.packages/hooks/package.json (1)
7-8: Exports and downstream dependency verified. The filepackages/hooks/src/use-form-state.tsxprovidesuseFormStateanduseSimpleFormState, andapps/webdeclares a dependency on@thedaviddias/hooks.packages/newsletter/package.json (1)
5-12: LGTM — keywords pretty-print only.
No behavioral change.packages/content/package.json (1)
5-9: LGTM — formatting-only change.
No impact on consumers.apps/web/app/members/members-search.tsx (1)
10-10: Dependency declared; no action needed. Confirmed@thedaviddias/hooksis listed asworkspace:*in apps/web/package.json.packages/security/package.json (1)
14-21: Dependencies valid and consistent
All specified versions (isomorphic-dompurify@2.26.0, validator@13.15.15, typescript@^5.8.2) exist on npm and no conflicting TypeScript versions were found across the workspace.packages/api-utils/package.json (1)
1-21: Next version consistency verified. All packages across the monorepo use Next v15.5.2.packages/security/types.ts (1)
15-19: LGTM on the result shape.Interfaces over types and optional fields are appropriate here.
sessions/tasks/m-refactor-dry-code.md (1)
31-36: Inventory is outdated after this PRThe duplicate useDebounce and one FavoriteButton have been removed/centralized in this PR. Update the task status and “Remove Duplications” to reflect current state.
Suggested tweaks:
-The `useDebounce` hook exists in TWO identical locations: +The `useDebounce` hook duplication has been eliminated; all imports now point to the shared package: - `/apps/web/hooks/use-debounce.ts` (17 lines, minimal implementation) - `/packages/hooks/src/use-debounce.tsx` (31 lines, with comprehensive JSDoc) +→ Keep `/packages/hooks/src/use-debounce.tsx` as the canonical source.-**Remove Duplications:** -- Delete: `/apps/web/hooks/use-debounce.ts` -- Delete: One of the FavoriteButton implementations (TBD based on feature analysis) +**Remove Duplications (updated):** +- Deleted: `/apps/web/hooks/use-debounce.ts` +- Deleted: `/apps/web/components/favorites/favorite-button.tsx` (favor the unified component)Also applies to: 201-204
apps/web/app/api/members/route.ts (1)
2-2: Import path migration LGTMSwitching to the centralized rate-limiting package is correct and aligns with the DRY refactor.
packages/api-utils/index.ts (1)
91-98: OK: error object passthrough behavior is consistentReturning ApiError as JSON with status code is clear and consistent.
packages/security/sanitization.ts (1)
42-57: LGTM: escapeHTML covers core entities and neutralizes </script>Solid default escaping for server-rendered contexts.
packages/security/validation.ts (1)
42-53: Validator.isURL supports the used options
The optionsdisallow_auth,allow_query_components,allow_fragments,allow_underscores,require_protocol,require_valid_protocolandrequire_tldare all supported by validator.isURL; no changes needed. (github.com, unpkg.com)
| export async function withRateLimit<T>( | ||
| request: NextRequest, | ||
| endpoint: keyof typeof RATE_LIMITS, | ||
| handler: () => Promise<T> | ||
| ): Promise<T | Response> { | ||
| const rateLimitResult = await checkRateLimit(request, endpoint) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Conform to codebase rule: single object parameter for multi-arg functions; tighten types to Response
Aligns with repo guidelines and simplifies call sites.
-export async function withRateLimit<T>(
- request: NextRequest,
- endpoint: keyof typeof RATE_LIMITS,
- handler: () => Promise<T>
-): Promise<T | Response> {
- const rateLimitResult = await checkRateLimit(request, endpoint)
+export interface WithRateLimitInput<T extends Response = Response> {
+ request: NextRequest
+ endpoint: keyof typeof RATE_LIMITS
+ handler: () => Promise<T>
+}
+export async function withRateLimit<T extends Response = Response>({
+ request,
+ endpoint,
+ handler
+}: WithRateLimitInput<T>): Promise<Response> {
+ const rateLimitResult = await checkRateLimit(request, endpoint)Also apply the same pattern to checkRateLimit:
-export async function checkRateLimit(
- request: NextRequest,
- endpoint: keyof typeof RATE_LIMITS
-): Promise<RateLimitResult> {
+export interface CheckRateLimitInput {
+ request: NextRequest
+ endpoint: keyof typeof RATE_LIMITS
+}
+export async function checkRateLimit({
+ request,
+ endpoint
+}: CheckRateLimitInput): Promise<RateLimitResult> {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/rate-limiting/index.ts around lines 141 to 146, change withRateLimit
to accept a single object parameter (e.g. { request, endpoint, handler })
instead of multiple positional args and tighten the return/type annotations to
use Response where appropriate; update the function body to destructure that
object and keep await checkRateLimit call accordingly. Do the same refactor for
checkRateLimit: switch its signature to a single options object (e.g. { request,
endpoint }) and adjust internal usage and return type to Response; update all
local call sites and imports to pass a single object and adjust any type imports
so signatures match the new types. Ensure no runtime behavior changes beyond the
signature and that TypeScript types reflect Response and the handler returning
Promise<T>.
| // Add rate limit headers to response | ||
| const headers = new Headers() | ||
| headers.set('X-RateLimit-Limit', rateLimitResult.limit.toString()) | ||
| headers.set('X-RateLimit-Remaining', rateLimitResult.remaining.toString()) | ||
| headers.set('X-RateLimit-Reset', rateLimitResult.reset.toString()) | ||
|
|
||
| if (!rateLimitResult.success) { | ||
| if (rateLimitResult.retryAfter) { | ||
| headers.set('Retry-After', rateLimitResult.retryAfter.toString()) | ||
| } | ||
|
|
||
| return new Response( | ||
| JSON.stringify({ | ||
| error: 'Rate limit exceeded', | ||
| message: `Too many requests. Try again in ${rateLimitResult.retryAfter} seconds.`, | ||
| retryAfter: rateLimitResult.retryAfter | ||
| }), | ||
| { | ||
| status: 429, | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| ...Object.fromEntries(headers.entries()) | ||
| } | ||
| } | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Header semantics: add standardized RateLimit header and clamp Retry-After
Set RateLimit (RFC draft) and keep X- headers for compatibility. Ensure reset is seconds (not a timestamp) and clamp to ≥1. (datatracker.ietf.org, ietf.org)
- // Add rate limit headers to response
- const headers = new Headers()
- headers.set('X-RateLimit-Limit', rateLimitResult.limit.toString())
- headers.set('X-RateLimit-Remaining', rateLimitResult.remaining.toString())
- headers.set('X-RateLimit-Reset', rateLimitResult.reset.toString())
+ // Compute seconds-until-reset for header semantics
+ const secondsUntilReset = Math.max(
+ 1,
+ Math.ceil((rateLimitResult.reset - Date.now()) / 1000)
+ )
+ // Add rate limit headers to response
+ const headers = new Headers()
+ // Standard header (draft): RateLimit: limit=…, remaining=…, reset=…
+ headers.set(
+ 'RateLimit',
+ `limit=${rateLimitResult.limit}, remaining=${rateLimitResult.remaining}, reset=${secondsUntilReset}`
+ )
+ // Back-compat X-* headers
+ headers.set('X-RateLimit-Limit', rateLimitResult.limit.toString())
+ headers.set('X-RateLimit-Remaining', rateLimitResult.remaining.toString())
+ headers.set('X-RateLimit-Reset', secondsUntilReset.toString())
@@
- if (!rateLimitResult.success) {
- if (rateLimitResult.retryAfter) {
- headers.set('Retry-After', rateLimitResult.retryAfter.toString())
- }
+ if (!rateLimitResult.success) {
+ const retryAfter = Math.max(
+ 1,
+ rateLimitResult.retryAfter ?? secondsUntilReset
+ )
+ headers.set('Retry-After', retryAfter.toString())
@@
- message: `Too many requests. Try again in ${rateLimitResult.retryAfter} seconds.`,
- retryAfter: rateLimitResult.retryAfter
+ message: `Too many requests. Try again in ${retryAfter} seconds.`,
+ retryAfterNote: Upstash “reset” for sliding windows is the start of the next window; treat it as a hint. (upstash.com)
📝 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.
| // Add rate limit headers to response | |
| const headers = new Headers() | |
| headers.set('X-RateLimit-Limit', rateLimitResult.limit.toString()) | |
| headers.set('X-RateLimit-Remaining', rateLimitResult.remaining.toString()) | |
| headers.set('X-RateLimit-Reset', rateLimitResult.reset.toString()) | |
| if (!rateLimitResult.success) { | |
| if (rateLimitResult.retryAfter) { | |
| headers.set('Retry-After', rateLimitResult.retryAfter.toString()) | |
| } | |
| return new Response( | |
| JSON.stringify({ | |
| error: 'Rate limit exceeded', | |
| message: `Too many requests. Try again in ${rateLimitResult.retryAfter} seconds.`, | |
| retryAfter: rateLimitResult.retryAfter | |
| }), | |
| { | |
| status: 429, | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| ...Object.fromEntries(headers.entries()) | |
| } | |
| } | |
| ) | |
| } | |
| // Compute seconds-until-reset for header semantics (clamped ≥1s) | |
| const secondsUntilReset = Math.max( | |
| 1, | |
| Math.ceil((rateLimitResult.reset - Date.now()) / 1000) | |
| ) | |
| // Add rate limit headers to response | |
| const headers = new Headers() | |
| // Standard header (RFC draft): RateLimit: limit=<n>, remaining=<n>, reset=<n> | |
| headers.set( | |
| 'RateLimit', | |
| `limit=${rateLimitResult.limit}, remaining=${rateLimitResult.remaining}, reset=${secondsUntilReset}` | |
| ) | |
| // Back-compat X-* headers | |
| headers.set('X-RateLimit-Limit', rateLimitResult.limit.toString()) | |
| headers.set('X-RateLimit-Remaining', rateLimitResult.remaining.toString()) | |
| headers.set('X-RateLimit-Reset', secondsUntilReset.toString()) | |
| if (!rateLimitResult.success) { | |
| // Clamp Retry-After to ≥1s, defaulting to secondsUntilReset if unset | |
| const retryAfter = Math.max( | |
| 1, | |
| rateLimitResult.retryAfter ?? secondsUntilReset | |
| ) | |
| headers.set('Retry-After', retryAfter.toString()) | |
| return new Response( | |
| JSON.stringify({ | |
| error: 'Rate limit exceeded', | |
| message: `Too many requests. Try again in ${retryAfter} seconds.`, | |
| retryAfter | |
| }), | |
| { | |
| status: 429, | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| ...Object.fromEntries(headers.entries()) | |
| } | |
| } | |
| ) | |
| } |
🤖 Prompt for AI Agents
In packages/rate-limiting/index.ts around lines 148-173, the response headers
currently only set X- headers and treat reset blindly; update to also set the
standardized RateLimit headers while retaining the X- headers for compatibility,
ensure the reset value is expressed as seconds (not an absolute timestamp) and
clamp it to a minimum of 1 second, and clamp Retry-After to ≥1 as well; compute
secondsReset = Math.max(1, /* convert timestamp->seconds or coerce existing
seconds */) and use that value for both RateLimit-Reset and for Retry-After and
response body retryAfter, while still setting X-RateLimit-* equivalents.
| export const RESERVED_USERNAMES = [ | ||
| 'admin', | ||
| 'api', | ||
| 'root', | ||
| 'system', | ||
| 'support', | ||
| 'help', | ||
| 'about', | ||
| 'login', | ||
| 'signup', | ||
| 'www', | ||
| 'mail', | ||
| 'ftp', | ||
| 'app', | ||
| 'dashboard', | ||
| 'settings', | ||
| 'profile', | ||
| 'user', | ||
| 'users', | ||
| 'member', | ||
| 'members' | ||
| ] as const |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure case-insensitive and normalized username validation.
Confirm validation lowercases input and normalizes separators before comparing with RESERVED_USERNAMES.
Run:
🏁 Script executed:
#!/bin/bash
# Inspect username validator implementation for normalization and case handling
rg -nP 'validateUsername|RESERVED_USERNAMES|username' packages/security -C3Length of output: 2528
🏁 Script executed:
#!/bin/bash
sed -n '60,120p' packages/security/validation.tsLength of output: 1565
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' packages/security/sanitization.tsLength of output: 3218
Normalize separators before reserved username comparison. The validation lowercases sanitized but still allows variants like -admin- or ad_min. Strip or collapse separators (e.g. sanitized.replace(/[-_]/g, '')) before comparing to RESERVED_USERNAMES so all reserved variants are blocked.
🤖 Prompt for AI Agents
In packages/security/constants.ts around lines 9 to 30, the reserved-username
check only lowercases `sanitized` but still lets variants like `-admin-` or
`ad_min` pass; normalize the candidate before comparing by removing/collapsing
common separators (e.g. strip hyphens and underscores, optionally dots) to
produce a plain alphanumeric string, then compare that normalized string (and
the existing lowercased form if needed) against RESERVED_USERNAMES so all
separator-variant forms of reserved names are rejected.
5a95acc to
2cfb55f
Compare
thedaviddias
force-pushed
the
feature/refactor-dry-code
branch
from
2cfb55f to
79ac0d5
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
packages/rate-limiting/index.ts (2)
107-150: Cache Ratelimit per endpoint; avoid constructing on every callReusing instances reduces overhead and connection churn.
- const rateLimiter = new Ratelimit({ - redis: getRedisClient(), - limiter: Ratelimit.slidingWindow(config.requests, `${config.window} s`), - prefix: `llms-txt-hub:${endpoint}` - }) + // Reuse a limiter per endpoint (module-local cache) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const g = globalThis as any + g.__rateLimiterCache ??= new Map<string, Ratelimit>() + const cacheKey = `__limiter:${endpoint}` + let rateLimiter = g.__rateLimiterCache.get(cacheKey) as Ratelimit | undefined + if (!rateLimiter) { + rateLimiter = new Ratelimit({ + redis: getRedisClient(), + limiter: Ratelimit.slidingWindow(config.requests, `${config.window} s`), + prefix: `llms-txt-hub:${endpoint}` + }) + g.__rateLimiterCache.set(cacheKey, rateLimiter) + }
155-200: Conform to repo rule: single object parameter for multi-arg functions; standardize headersAdopt object inputs and emit standard RateLimit headers while keeping X-* for compatibility.
-export async function withRateLimit<T>( - request: NextRequest, - endpoint: keyof typeof RATE_LIMITS, - handler: () => Promise<T> -): Promise<T | Response> { - const rateLimitResult = await checkRateLimit(request, endpoint) +export interface WithRateLimitInput<T extends Response = Response> { + request: NextRequest + endpoint: keyof typeof RATE_LIMITS + handler: () => Promise<T> +} +export async function withRateLimit<T extends Response = Response>({ + request, + endpoint, + handler +}: WithRateLimitInput<T>): Promise<Response> { + const rateLimitResult = await checkRateLimit({ request, endpoint }) @@ - headers.set('X-RateLimit-Reset', rateLimitResult.reset.toString()) + const secondsUntilReset = Math.max(1, Math.ceil((rateLimitResult.reset - Date.now()) / 1000)) + headers.set('RateLimit', `limit=${rateLimitResult.limit}, remaining=${rateLimitResult.remaining}, reset=${secondsUntilReset}`) + headers.set('X-RateLimit-Reset', secondsUntilReset.toString()) @@ - if (!rateLimitResult.success) { - if (rateLimitResult.retryAfter) { - headers.set('Retry-After', rateLimitResult.retryAfter.toString()) - } + if (!rateLimitResult.success) { + const retryAfter = Math.max(1, rateLimitResult.retryAfter ?? secondsUntilReset) + headers.set('Retry-After', retryAfter.toString()) @@ - message: `Too many requests. Try again in ${rateLimitResult.retryAfter} seconds.`, - retryAfter: rateLimitResult.retryAfter + message: `Too many requests. Try again in ${retryAfter} seconds.`, + retryAfter }), @@ - const result = await handler() + const result = await handler() // If result is a Response, add rate limit headers if (result instanceof Response) { headers.forEach((value, key) => { result.headers.set(key, value) }) } - return result + return result }Also update checkRateLimit signature to a single object:
-export async function checkRateLimit( - request: NextRequest, - endpoint: keyof typeof RATE_LIMITS -): Promise<RateLimitResult> { +export interface CheckRateLimitInput { + request: NextRequest + endpoint: keyof typeof RATE_LIMITS +} +export async function checkRateLimit({ + request, + endpoint +}: CheckRateLimitInput): Promise<RateLimitResult> {
🧹 Nitpick comments (7)
packages/rate-limiting/index.ts (2)
25-41: Per-endpoint limits: looks good; add JSDoc with @param/@returns where exportedAdd JSDoc on public exports to satisfy repo rules for **/*.ts utilities.
export const RATE_LIMITS = { @@ } as const + +/** + * @public Per-endpoint quotas (requests per window in seconds). + */
46-91: Client ID extraction: solid; minor hardeningTrim/ignore “unknown” tokens from proxies and normalize whitespace to reduce spoofing.
- const forwardedFor = request.headers.get('x-forwarded-for') + const forwardedFor = request.headers.get('x-forwarded-for') if (forwardedFor) { - const firstIp = forwardedFor.split(',')[0]?.trim() - if (firstIp && isValidIp(firstIp)) return firstIp + const firstIp = forwardedFor.split(',')[0]?.trim() + if (firstIp && firstIp.toLowerCase() !== 'unknown' && isValidIp(firstIp)) return firstIp }packages/security/validation.ts (2)
17-58: Add JSDoc with @param/@returns and consider parsing to compare protocolCurrent logic is fine; adding JSDoc satisfies repo rules. Optionally parse once via new URL for stricter protocol checks.
+/** + * Validate and sanitize a URL string. + * @param url - Input URL. + * @param options - Validation options (protocols, TLD, auth, maxLength). + * @returns ValidationResult with sanitized value on success. + */ export function validateURL( url: string | null | undefined, options: URLValidationOptions = {} ): ValidationResult {
63-109: Username validation: OK; micro-optimization for reserved lookupUse a Set for O(1) checks; preserves semantics.
- const lowerUsername = sanitized.toLowerCase() - if (reservedUsernames.some(reserved => reserved === lowerUsername)) { + const lowerUsername = sanitized.toLowerCase() + const reservedSet = new Set(reservedUsernames) + if (reservedSet.has(lowerUsername)) { return { valid: false, error: 'Username is reserved' } }packages/hooks/src/use-form-state.tsx (3)
1-1: Prevent state updates after unmount during async submit (optional hardening)Avoids “setState on unmounted” warnings when a pending submit resolves post-unmount.
- import { useCallback, useState } from 'react' + import { useCallback, useEffect, useRef, useState } from 'react' @@ - const [successData, setSuccessData] = useState<unknown>() + const [successData, setSuccessData] = useState<unknown>() + const mountedRef = useRef(true) + useEffect(() => { + return () => { + mountedRef.current = false + } + }, []) @@ const submit = useCallback( async (handler: (data: T) => Promise<unknown>): Promise<unknown> => { setStatus('loading') setError(null) try { const result = await handler(data) - setStatus('success') - setSuccessData(result) + if (!mountedRef.current) return result + setStatus('success') + setSuccessData(result) if (resetOnSuccess) { - setData(initialData) + setData(initialData) } onSuccess?.(result) return result } catch (err) { - const errorMessage = getErrorMessage(err) - setStatus('error') - setError(errorMessage) - onError?.(errorMessage) + const errorMessage = getErrorMessage(err) + if (!mountedRef.current) throw err + setStatus('error') + setError(errorMessage) + onError?.(errorMessage) throw err } }, [data, initialData, resetOnSuccess, onSuccess, onError, getErrorMessage] )Also applies to: 135-139, 173-198
263-266: Remove unnecessary type assertion
undefined as undefinedis redundant; passundefineddirectly.- const [state, actions] = useFormState({ - initialData: undefined as undefined, - ...options - }) + const [state, actions] = useFormState({ + initialData: undefined, + ...options + })
124-134: Optional: Expose generic result type for successData/submitIf you want typed
successData, consider parameterizing result:useFormState<T, R = unknown>,submit: (handler: (data: T) => Promise<R>) => Promise<R>, andsuccessData?: R. This is a nice-to-have for DX; can be deferred.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.gitignore(1 hunks)apps/web/actions/submit-llms-xxt.ts(3 hunks)apps/web/app/api/members/route.ts(1 hunks)apps/web/components/forms/submit-form-step2-content-fields.tsx(1 hunks)apps/web/components/forms/submit-form.tsx(2 hunks)apps/web/lib/security-utils-helpers.ts(2 hunks)apps/web/lib/security-utils.ts(1 hunks)apps/web/middleware.ts(2 hunks)packages/api-utils/index.ts(1 hunks)packages/api-utils/package.json(1 hunks)packages/hooks/src/use-form-state.tsx(1 hunks)packages/rate-limiting/index.ts(2 hunks)packages/security/validation.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/web/actions/submit-llms-xxt.ts
- .gitignore
- apps/web/components/forms/submit-form-step2-content-fields.tsx
- packages/api-utils/index.ts
- apps/web/components/forms/submit-form.tsx
- apps/web/app/api/members/route.ts
- apps/web/lib/security-utils-helpers.ts
- apps/web/lib/security-utils.ts
- packages/api-utils/package.json
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/functions.mdc)
**/*.{ts,tsx}: For functions with more than one parameter, use a single object parameter instead of multiple positional parameters
Name the input type for such object parameters using the pattern {FunctionName}Input
For single-parameter functions, direct positional arguments are acceptable
**/*.{ts,tsx}: Write concise, technical TypeScript code
Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Use TypeScript for all code; prefer interfaces over types
Avoid enums; use object maps instead
Use thefunctionkeyword for pure functions
Avoid unnecessary curly braces in simple conditionals; use concise syntax
**/*.{ts,tsx}: Use interfaces over types when possible in TypeScript
Avoid enums; prefer const objects (as const) instead
Use proper error handling with custom error types
Favor functional programming patterns
Avoid classes
Use pure functions where possible
Implement proper error handling
Use meaningful variable names
Keep functions small and focused
Files:
packages/rate-limiting/index.tspackages/hooks/src/use-form-state.tsxapps/web/middleware.tspackages/security/validation.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/jsdoc-comments.mdc)
Add JSDoc blocks for utility functions, documenting purpose, @param entries, @returns, and @throws where applicable
Files:
packages/rate-limiting/index.tsapps/web/middleware.tspackages/security/validation.ts
**/index.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/shared.mdc)
Do not use barrel files (avoid index files that only re-export)
Files:
packages/rate-limiting/index.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional React components typed with TypeScript interfaces
Write declarative JSX
Use Shadcn UI, Radix UI, and Tailwind CSS for components and styling
Implement responsive, mobile-first design with Tailwind CSS
When using next/image, include explicit sizes and enable lazy loadingAdd JSDoc blocks for React components, including brief description, @param props with individual prop fields, @returns, and an @example usage
Files:
packages/hooks/src/use-form-state.tsx
**/use*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/jsdoc-comments.mdc)
Add JSDoc blocks for custom React hooks, including purpose, parameters, return value, and an example of hook usage
Files:
packages/hooks/src/use-form-state.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
apps/web/**/*.{ts,tsx}: Use Shadcn UI components with Radix UI primitives sourced from packages/design-system
Optimize for Core Web Vitals (images, fonts, script strategy, code-splitting)
Use camelCase for function and variable names
Use descriptive names with auxiliary verbs (e.g., isLoading, hasError)
Files:
apps/web/middleware.ts
apps/web/**
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
Use kebab-case for directories
Files:
apps/web/middleware.ts
🧠 Learnings (11)
📚 Learning: 2025-08-26T11:18:35.229Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:35.229Z
Learning: Applies to **/use*.{ts,tsx} : Add JSDoc comments for custom React hooks, including description, param entries, returns, and an example usage snippet
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:44.245Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:44.245Z
Learning: Applies to **/use*.{ts,tsx} : Add JSDoc blocks for custom React hooks, including purpose, parameters, return value, and an example of hook usage
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:39.811Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:39.811Z
Learning: Applies to **/use*.{ts,tsx} : Document React hooks with JSDoc: include a brief description, param entries, returns, and an example of hook usage.
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:40.806Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:40.806Z
Learning: Applies to **/use*.{ts,tsx} : Document React hooks with a JSDoc block including a brief description, param, returns, and an example
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:18.780Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:18.780Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : For custom hooks, use renderHook and wrap state changes in act when needed
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:10.612Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:10.612Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Test custom React hooks with renderHook and wrap state changes in act
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:30.246Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:30.246Z
Learning: Applies to **/use*.{ts,tsx} : Add JSDoc to custom React hooks with a brief purpose, param for inputs, returns for outputs, and an example of usage.
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:24.941Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:24.941Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : For custom hooks, use renderHook and wrap state changes in act when asserting updates
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:58.532Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:58.532Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}} : Test custom hooks with renderHook and wrap state changes in act
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:49.726Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:49.726Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Test custom React hooks using renderHook and wrap state changes in act
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:21.095Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/functions.mdc:0-0
Timestamp: 2025-08-26T11:18:21.095Z
Learning: Applies to **/*.{ts,tsx} : For functions with more than one parameter, use a single object parameter instead of multiple positional parameters
Applied to files:
packages/hooks/src/use-form-state.tsx
🧬 Code graph analysis (3)
packages/rate-limiting/index.ts (2)
apps/web/lib/security-utils-helpers.ts (1)
checkRateLimit(80-117)apps/web/middleware.ts (1)
config(422-429)
apps/web/middleware.ts (2)
packages/logging/index.ts (2)
error(106-122)logger(168-168)apps/web/lib/server-crypto.ts (1)
hashSensitiveData(8-10)
packages/security/validation.ts (3)
packages/security/constants.ts (2)
MALICIOUS_PROTOCOLS(4-4)RESERVED_USERNAMES(9-30)packages/security/types.ts (2)
URLValidationOptions(4-10)ValidationResult(15-19)packages/security/sanitization.ts (1)
sanitizeText(10-34)
🔇 Additional comments (5)
apps/web/middleware.ts (1)
368-384: Good: richer CSRF failure logging with IP hashing and tagsStronger signal, safer PII handling, and appropriate severity. LGTM.
packages/security/validation.ts (1)
114-166: Email validation looks correct with conditional host listsOption names align with validator v13.15.15; conditional whitelist/blacklist avoids accidental blanket rejects. LGTM.
packages/hooks/src/use-form-state.tsx (3)
98-123: LGTM: Strong JSDoc and examples for both hooksDocs meet our hook guidelines (description, params, returns, example). Keep this pattern.
Also applies to: 246-262
222-236: LGTM: Defensive default error extractionCovers Error, string, and
{ message }objects with a sane fallback.
45-54: Refactor setField signature and implementation to use single-object paramVerified no internal positional call sites for
setField; safe to update API and hook implementation inpackages/hooks/src/use-form-state.tsx(lines 45–54 & 144–158):@@ interface FormActions<T = unknown> (lines 45–54) - setField: <K extends keyof T>(field: K, value: T[K]) => void + setField: <K extends keyof T>(input: { field: K; value: T[K] }) => void @@ implementation (lines 144–158) - const setField = useCallback(<K extends keyof T>(field: K, value: T[K]) => { - setData(prev => { - // Runtime guard: check if prev is object-like - if (typeof prev !== 'object' || prev === null) { - const error = `setField requires object-like form data, but received: ${typeof prev}` - console.error(error) - throw new Error(error) - } - - return { - ...prev, - [field]: value - } - }) - }, []) + const setField = useCallback(<K extends keyof T>({ field, value }: { field: K; value: T[K] }) => { + setData(prev => { + if (typeof prev !== 'object' || prev === null) { + throw new Error('useFormState.setField requires object-like form data') + } + return { + ...(prev as Record<PropertyKey, unknown>), + [field as PropertyKey]: value + } as T + }) + }, [])
| // Skip validation for Server Actions (Next.js internal endpoints) | ||
| // These will be handled by the Server Action itself | ||
| if (request.nextUrl.pathname.startsWith('/_next/static/chunks/')) { | ||
| return true | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix “Server Actions” skip: current path check targets static chunks, not actions
Skip based on the Server Action request header instead of a static asset path. The current condition is effectively a no-op (safe methods are already bypassed) and is misleading.
- // Skip validation for Server Actions (Next.js internal endpoints)
- // These will be handled by the Server Action itself
- if (request.nextUrl.pathname.startsWith('/_next/static/chunks/')) {
- return true
- }
+ // Skip validation for Server Actions (Next.js internal posts)
+ // Next adds "Next-Action" header to action invocations
+ if (request.headers.has('next-action')) {
+ return true
+ }📝 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.
| // Skip validation for Server Actions (Next.js internal endpoints) | |
| // These will be handled by the Server Action itself | |
| if (request.nextUrl.pathname.startsWith('/_next/static/chunks/')) { | |
| return true | |
| } | |
| // Skip validation for Server Actions (Next.js internal posts) | |
| // Next adds "Next-Action" header to action invocations | |
| if (request.headers.has('next-action')) { | |
| return true | |
| } |
🤖 Prompt for AI Agents
In apps/web/middleware.ts around lines 173 to 178, the current skip logic checks
for a static chunks path which is misleading and ineffective for Server Actions;
replace that pathname check with a header-based check for the Server Action
request (e.g. if request.headers.get('x-action') exists) and return true when
that header is present, and update/remove the misleading comment accordingly.
| export function validatePhoneNumber( | ||
| phone: string | null | undefined, | ||
| locale: validator.MobilePhoneLocale = 'en-US' | ||
| ): ValidationResult { | ||
| if (!phone) { | ||
| return { valid: false, error: 'Phone number is required' } | ||
| } | ||
|
|
||
| const cleaned = phone.replace(/\D/g, '') | ||
|
|
||
| if (!validator.isMobilePhone(cleaned, locale, { strictMode: false })) { | ||
| return { valid: false, error: 'Invalid phone number format' } | ||
| } | ||
|
|
||
| return { valid: true, sanitized: cleaned } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Conform to repo rule: single object parameter for multi-arg functions
Switch to an input object and export its interface.
-export function validatePhoneNumber(
- phone: string | null | undefined,
- locale: validator.MobilePhoneLocale = 'en-US'
-): ValidationResult {
+export interface ValidatePhoneNumberInput {
+ phone: string | null | undefined
+ locale?: validator.MobilePhoneLocale
+}
+export function validatePhoneNumber({
+ phone,
+ locale = 'en-US'
+}: ValidatePhoneNumberInput): ValidationResult {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function validatePhoneNumber( | |
| phone: string | null | undefined, | |
| locale: validator.MobilePhoneLocale = 'en-US' | |
| ): ValidationResult { | |
| if (!phone) { | |
| return { valid: false, error: 'Phone number is required' } | |
| } | |
| const cleaned = phone.replace(/\D/g, '') | |
| if (!validator.isMobilePhone(cleaned, locale, { strictMode: false })) { | |
| return { valid: false, error: 'Invalid phone number format' } | |
| } | |
| return { valid: true, sanitized: cleaned } | |
| } | |
| export interface ValidatePhoneNumberInput { | |
| phone: string | null | undefined | |
| locale?: validator.MobilePhoneLocale | |
| } | |
| export function validatePhoneNumber({ | |
| phone, | |
| locale = 'en-US' | |
| }: ValidatePhoneNumberInput): ValidationResult { | |
| if (!phone) { | |
| return { valid: false, error: 'Phone number is required' } | |
| } | |
| const cleaned = phone.replace(/\D/g, '') | |
| if (!validator.isMobilePhone(cleaned, locale, { strictMode: false })) { | |
| return { valid: false, error: 'Invalid phone number format' } | |
| } | |
| return { valid: true, sanitized: cleaned } | |
| } |
🤖 Prompt for AI Agents
In packages/security/validation.ts around lines 171 to 186, change
validatePhoneNumber to accept a single parameter object and export its input
interface: create and export an interface (e.g. ValidatePhoneNumberArgs) with
phone: string | null | undefined and optional locale:
validator.MobilePhoneLocale = 'en-US', update the function signature to
validatePhoneNumber(args: ValidatePhoneNumberArgs): ValidationResult and
destructure args inside the function, and update any call sites to pass an
object instead of positional args; keep the existing validation logic and return
shape unchanged.
| /** | ||
| * Validate origin for CSRF protection | ||
| */ | ||
| export function validateOrigin(request: Request, allowedOrigins: string[] = []): ValidationResult { | ||
| const origin = request.headers.get('origin') | ||
|
|
||
| if (!origin) { | ||
| return { valid: false, error: 'Origin header required' } | ||
| } | ||
|
|
||
| if (allowedOrigins.length > 0) { | ||
| if (allowedOrigins.includes(origin)) { | ||
| return { valid: true } | ||
| } | ||
| return { valid: false, error: 'Origin not allowed' } | ||
| } | ||
|
|
||
| const host = request.headers.get('host') | ||
| if (!host) { | ||
| return { valid: false, error: 'Host header missing' } | ||
| } | ||
|
|
||
| try { | ||
| const originUrl = new URL(origin) | ||
| if (originUrl.host === host) { | ||
| return { valid: true } | ||
| } | ||
| return { valid: false, error: 'Origin does not match host' } | ||
| } catch { | ||
| return { valid: false, error: 'Invalid origin URL' } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Origin validation: support proxies (x-forwarded-*) and use object param
Improve robustness behind proxies and satisfy single-object-arg rule.
-export function validateOrigin(request: Request, allowedOrigins: string[] = []): ValidationResult {
+export interface ValidateOriginInput {
+ request: Request
+ allowedOrigins?: string[]
+}
+export function validateOrigin({
+ request,
+ allowedOrigins = []
+}: ValidateOriginInput): ValidationResult {
const origin = request.headers.get('origin')
@@
- const host = request.headers.get('host')
+ const host = request.headers.get('x-forwarded-host') || request.headers.get('host')
+ const proto = request.headers.get('x-forwarded-proto') || 'https'
@@
- const originUrl = new URL(origin)
- if (originUrl.host === host) {
+ const originUrl = new URL(origin)
+ if (originUrl.host === host && originUrl.protocol.replace(':', '') === proto) {
return { valid: true }
}📝 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.
| /** | |
| * Validate origin for CSRF protection | |
| */ | |
| export function validateOrigin(request: Request, allowedOrigins: string[] = []): ValidationResult { | |
| const origin = request.headers.get('origin') | |
| if (!origin) { | |
| return { valid: false, error: 'Origin header required' } | |
| } | |
| if (allowedOrigins.length > 0) { | |
| if (allowedOrigins.includes(origin)) { | |
| return { valid: true } | |
| } | |
| return { valid: false, error: 'Origin not allowed' } | |
| } | |
| const host = request.headers.get('host') | |
| if (!host) { | |
| return { valid: false, error: 'Host header missing' } | |
| } | |
| try { | |
| const originUrl = new URL(origin) | |
| if (originUrl.host === host) { | |
| return { valid: true } | |
| } | |
| return { valid: false, error: 'Origin does not match host' } | |
| } catch { | |
| return { valid: false, error: 'Invalid origin URL' } | |
| } | |
| } | |
| /** | |
| * Validate origin for CSRF protection | |
| */ | |
| export interface ValidateOriginInput { | |
| request: Request | |
| allowedOrigins?: string[] | |
| } | |
| export function validateOrigin({ | |
| request, | |
| allowedOrigins = [] | |
| }: ValidateOriginInput): ValidationResult { | |
| const origin = request.headers.get('origin') | |
| if (!origin) { | |
| return { valid: false, error: 'Origin header required' } | |
| } | |
| if (allowedOrigins.length > 0) { | |
| if (allowedOrigins.includes(origin)) { | |
| return { valid: true } | |
| } | |
| return { valid: false, error: 'Origin not allowed' } | |
| } | |
| const host = request.headers.get('x-forwarded-host') || request.headers.get('host') | |
| if (!host) { | |
| return { valid: false, error: 'Host header missing' } | |
| } | |
| const proto = request.headers.get('x-forwarded-proto') || 'https' | |
| try { | |
| const originUrl = new URL(origin) | |
| if (originUrl.host === host && originUrl.protocol.replace(':', '') === proto) { | |
| return { valid: true } | |
| } | |
| return { valid: false, error: 'Origin does not match host' } | |
| } catch { | |
| return { valid: false, error: 'Invalid origin URL' } | |
| } | |
| } |
thedaviddias
force-pushed
the
feature/refactor-dry-code
branch
from
79ac0d5 to
6506fca
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/lib/__tests__/security-utils.test.ts (1)
83-96: Fix flakey async test:setTimeoutassertions never run.This test finishes before the callback runs; assertions inside
setTimeoutare skipped. Make time deterministic by stubbingDate.now(or using fake timers). This avoids flakiness and follows our testing learnings to keep tests isolated from real timers.Apply:
- // Wait for window to expire - setTimeout(() => { - const result2 = checkRateLimit({ identifier, ...options }) - expect(result2.allowed).toBe(true) - }, 10) + // Simulate window expiry deterministically + const nowSpy = jest.spyOn(Date, 'now').mockReturnValue(Date.now() + 10) + const result2 = checkRateLimit({ identifier, ...options }) + expect(result2.allowed).toBe(true) + nowSpy.mockRestore()
♻️ Duplicate comments (5)
packages/hooks/src/use-form-state.tsx (1)
144-158: setField: add a type assertion to avoid TS intersection typing pitfalls.Without an assertion,
{ ...prev, [field]: value }can inferT & { ... }and be rejected bysetDataunder stricter TS configs. Keep your runtime guard, but assert the spread toT.return { - ...prev, - [field]: value - } + ...(prev as Record<string, unknown>), + [field as string]: value + } as Tpackages/rate-limiting/index.ts (4)
107-110: Conform to repo rule: use a single object parameter for multi-arg functions.Refactor checkRateLimit to accept an options object.
Apply:
-export async function checkRateLimit( - request: NextRequest, - endpoint: keyof typeof RATE_LIMITS -): Promise<RateLimitResult> { +export interface CheckRateLimitInput { + request: NextRequest + endpoint: keyof typeof RATE_LIMITS +} +export async function checkRateLimit({ + request, + endpoint +}: CheckRateLimitInput): Promise<RateLimitResult> {
115-121: Avoid constructing a new Upstash Ratelimit per call; cache per endpoint.Cuts cold-start overhead and reduces Redis chatter.
- const rateLimiter = new Ratelimit({ - redis: getRedisClient(), - limiter: Ratelimit.slidingWindow(config.requests, `${config.window} s`), - prefix: `llms-txt-hub:${endpoint}` - }) + // Reuse one limiter per endpoint across invocations + // @ts-expect-error module-local cache + globalThis.__rateLimiterCache ??= new Map<string, Ratelimit>() + // @ts-expect-error see above + let rateLimiter = globalThis.__rateLimiterCache.get(endpoint) + if (!rateLimiter) { + rateLimiter = new Ratelimit({ + redis: getRedisClient(), + limiter: Ratelimit.slidingWindow(config.requests, `${config.window} s`), + prefix: `llms-txt-hub:${endpoint}` + }) + // @ts-expect-error see above + globalThis.__rateLimiterCache.set(endpoint, rateLimiter) + }
155-160: Align with guidelines: withRateLimit should accept a single options object and return Response.Tighten generics to T extends Response; callers always return Response/NextResponse.
-export async function withRateLimit<T>( - request: NextRequest, - endpoint: keyof typeof RATE_LIMITS, - handler: () => Promise<T> -): Promise<T | Response> { - const rateLimitResult = await checkRateLimit(request, endpoint) +export interface WithRateLimitInput<T extends Response = Response> { + request: NextRequest + endpoint: keyof typeof RATE_LIMITS + handler: () => Promise<T> +} +export async function withRateLimit<T extends Response = Response>({ + request, + endpoint, + handler +}: WithRateLimitInput<T>): Promise<Response> { + const rateLimitResult = await checkRateLimit({ request, endpoint })Note: update all call sites accordingly (e.g., withRateLimit({ request, endpoint: 'MEMBERS_API', handler: ... })).
162-167: Header semantics: add standard RateLimit header and normalize reset to seconds; clamp Retry-After.Improves client compatibility and fixes incorrect X-RateLimit-Reset units.
- // Add rate limit headers to response - const headers = new Headers() - headers.set('X-RateLimit-Limit', rateLimitResult.limit.toString()) - headers.set('X-RateLimit-Remaining', rateLimitResult.remaining.toString()) - headers.set('X-RateLimit-Reset', rateLimitResult.reset.toString()) + // Compute seconds-until-reset (normalize ms/seconds) + const nowSec = Math.floor(Date.now() / 1000) + const resetSec = + rateLimitResult.reset > 1e12 + ? Math.floor(rateLimitResult.reset / 1000) + : rateLimitResult.reset + const secondsUntilReset = Math.max(1, resetSec - nowSec) + // Add rate limit headers (standard + X- for back-compat) + const headers = new Headers() + headers.set( + 'RateLimit', + `limit=${rateLimitResult.limit}, remaining=${rateLimitResult.remaining}, reset=${secondsUntilReset}` + ) + headers.set('X-RateLimit-Limit', rateLimitResult.limit.toString()) + headers.set('X-RateLimit-Remaining', rateLimitResult.remaining.toString()) + headers.set('X-RateLimit-Reset', secondsUntilReset.toString()) @@ - if (!rateLimitResult.success) { - if (rateLimitResult.retryAfter) { - headers.set('Retry-After', rateLimitResult.retryAfter.toString()) - } + if (!rateLimitResult.success) { + const retryAfter = Math.max( + 1, + rateLimitResult.retryAfter ?? secondsUntilReset + ) + headers.set('Retry-After', retryAfter.toString()) @@ - message: `Too many requests. Try again in ${rateLimitResult.retryAfter} seconds.`, - retryAfter: rateLimitResult.retryAfter + message: `Too many requests. Try again in ${retryAfter} seconds.`, + retryAfterAlso applies to: 168-172, 174-186
🧹 Nitpick comments (5)
packages/hooks/src/use-form-state.tsx (4)
160-166: Reset baseline drifts if options.initialData changes after mount.If consumers pass a new
initialData,reset()restores the old one. Track the latest value via a ref.- const reset = useCallback(() => { - setData(initialData) + // Keep latest initialData for reset() + // (considered optional; enables dynamic baselines) + // Imports needed: useEffect, useRef + // const initialRef = useRef(initialData) + // useEffect(() => { initialRef.current = initialData }, [initialData]) + const reset = useCallback(() => { + setData(initialRef.current) setStatus('idle') setError(null) setSuccessData(undefined) - }, [initialData]) + }, [])- import { useCallback, useState } from 'react' + import { useCallback, useEffect, useRef, useState } from 'react'const [successData, setSuccessData] = useState<unknown>() + const initialRef = useRef(initialData) + useEffect(() => { + initialRef.current = initialData + }, [initialData])
173-198: Prevent state updates after unmount and stale “older submit wins”.Add a request id + isMounted guard so only the latest in‑flight submit updates state.
+ const requestIdRef = useRef(0) + const isMountedRef = useRef(true) + useEffect(() => () => { isMountedRef.current = false }, [])const submit = useCallback( async (handler: (data: T) => Promise<unknown>): Promise<unknown> => { - setStatus('loading') + const requestId = ++requestIdRef.current + setStatus('loading') setError(null) try { const result = await handler(data) - setStatus('success') - setSuccessData(result) + if (isMountedRef.current && requestId === requestIdRef.current) { + setStatus('success') + setSuccessData(result) + } if (resetOnSuccess) { - setData(initialData) + if (isMountedRef.current && requestId === requestIdRef.current) { + setData(initialData) + } } - onSuccess?.(result) + if (requestId === requestIdRef.current) onSuccess?.(result) return result } catch (err) { const errorMessage = getErrorMessage(err) - setStatus('error') - setError(errorMessage) - onError?.(errorMessage) + if (isMountedRef.current && requestId === requestIdRef.current) { + setStatus('error') + setError(errorMessage) + onError?.(errorMessage) + } throw err } }, - [data, initialData, resetOnSuccess, onSuccess, onError, getErrorMessage] + [data, initialData, resetOnSuccess, onSuccess, onError, getErrorMessage] )
1-1: Rename to .ts (no JSX).Using .ts avoids unnecessary TSX transforms and slightly improves build perf.
124-220: Add tests for hook behavior (renderHook + act).Cover: setField happy/guard paths, submit success/error transitions, reset vs resetStatus, resetOnSuccess, and “ignore updates after unmount”. I can scaffold tests if helpful.
apps/web/lib/__tests__/security-utils.test.ts (1)
12-38: Don’t test your mock—use the real rate limiter (partial mock instead).Here you fully replace
../security-utils-helpers, includingcheckRateLimit, so the tests validate your mock logic rather than the implementation. Prefer a partial mock: keep the realcheckRateLimitand only mocksanitizeErrorMessage(and useclearRateLimiting()to reset state). This keeps tests meaningful and aligns with our testing guidelines to avoid flakiness by isolating dependencies sensibly.Apply this refactor:
-// Mock the helpers module -jest.mock('../security-utils-helpers', () => { - const mockRateLimitMap = new Map() - - return { - rateLimitMap: mockRateLimitMap, - sanitizeErrorMessage: jest.fn(msg => msg), - clearRateLimiting: jest.fn(() => mockRateLimitMap.clear()), - getRateLimitKey: jest.fn((ip, action) => `${ip}:${action}`), - checkRateLimit: jest.fn(({ identifier, windowMs = 60000, maxRequests = 10 }) => { - const now = Date.now() - const record = mockRateLimitMap.get(identifier) - - if (!record || now > record.resetTime) { - const resetTime = now + windowMs - mockRateLimitMap.set(identifier, { count: 1, resetTime }) - return { allowed: true, resetTime } - } - - if (record.count < maxRequests) { - record.count++ - return { allowed: true, resetTime: record.resetTime } - } - - return { allowed: false, resetTime: record.resetTime } - }) - } -}) +// Partially mock helpers: keep real rate limiter, only mock sanitization +jest.mock('../security-utils-helpers', () => { + const actual = jest.requireActual('../security-utils-helpers') + return { + ...actual, + sanitizeErrorMessage: jest.fn((msg: string) => msg) + } +})And adjust imports/use to reset state via the public API:
-import { - checkRateLimit, - createSafeErrorMessage, - escapeHtml, - sanitizeText, - sanitizeUrl, - validateOrigin, - validateUsername -} from '../security-utils' +import { + checkRateLimit, + clearRateLimiting, + createSafeErrorMessage, + escapeHtml, + sanitizeText, + sanitizeUrl, + validateOrigin, + validateUsername +} from '../security-utils'-const { rateLimitMap, sanitizeErrorMessage } = require('../security-utils-helpers') +const { sanitizeErrorMessage } = require('../security-utils-helpers')- beforeEach(() => { - rateLimitMap.clear() - jest.clearAllMocks() - }) + beforeEach(() => { + clearRateLimiting() + jest.clearAllMocks() + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.gitignore(1 hunks)apps/web/actions/submit-llms-xxt.ts(3 hunks)apps/web/app/api/members/route.ts(1 hunks)apps/web/components/forms/submit-form-step2-content-fields.tsx(1 hunks)apps/web/components/forms/submit-form.tsx(2 hunks)apps/web/lib/__tests__/security-utils.test.ts(2 hunks)apps/web/lib/security-utils-helpers.ts(2 hunks)apps/web/lib/security-utils.ts(1 hunks)apps/web/middleware.ts(2 hunks)packages/api-utils/index.ts(1 hunks)packages/api-utils/package.json(1 hunks)packages/hooks/src/use-form-state.tsx(1 hunks)packages/rate-limiting/index.ts(2 hunks)packages/security/validation.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- .gitignore
- packages/api-utils/package.json
- apps/web/actions/submit-llms-xxt.ts
- apps/web/components/forms/submit-form-step2-content-fields.tsx
- apps/web/lib/security-utils.ts
- apps/web/middleware.ts
- apps/web/lib/security-utils-helpers.ts
- packages/api-utils/index.ts
- apps/web/components/forms/submit-form.tsx
- packages/security/validation.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/functions.mdc)
**/*.{ts,tsx}: For functions with more than one parameter, use a single object parameter instead of multiple positional parameters
Name the input type for such object parameters using the pattern {FunctionName}Input
For single-parameter functions, direct positional arguments are acceptable
**/*.{ts,tsx}: Write concise, technical TypeScript code
Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Use TypeScript for all code; prefer interfaces over types
Avoid enums; use object maps instead
Use thefunctionkeyword for pure functions
Avoid unnecessary curly braces in simple conditionals; use concise syntax
**/*.{ts,tsx}: Use interfaces over types when possible in TypeScript
Avoid enums; prefer const objects (as const) instead
Use proper error handling with custom error types
Favor functional programming patterns
Avoid classes
Use pure functions where possible
Implement proper error handling
Use meaningful variable names
Keep functions small and focused
Files:
apps/web/app/api/members/route.tsapps/web/lib/__tests__/security-utils.test.tspackages/hooks/src/use-form-state.tsxpackages/rate-limiting/index.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/jsdoc-comments.mdc)
Add JSDoc blocks for utility functions, documenting purpose, @param entries, @returns, and @throws where applicable
Files:
apps/web/app/api/members/route.tsapps/web/lib/__tests__/security-utils.test.tspackages/rate-limiting/index.ts
apps/web/app/**/route.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/nextjs.mdc)
Use API Route Handlers (route.ts/tsx) to manage backend logic within the app structure
Files:
apps/web/app/api/members/route.ts
apps/web/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
apps/web/app/**/*.{ts,tsx}: Follow Next.js 15 App Router best practices in the web app
Use React Server Components (RSC) by default; only add 'use client' when necessary
Implement type-safe Server Actions using next-safe-action
Files:
apps/web/app/api/members/route.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
apps/web/**/*.{ts,tsx}: Use Shadcn UI components with Radix UI primitives sourced from packages/design-system
Optimize for Core Web Vitals (images, fonts, script strategy, code-splitting)
Use camelCase for function and variable names
Use descriptive names with auxiliary verbs (e.g., isLoading, hasError)
Files:
apps/web/app/api/members/route.tsapps/web/lib/__tests__/security-utils.test.ts
apps/web/**
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
Use kebab-case for directories
Files:
apps/web/app/api/members/route.tsapps/web/lib/__tests__/security-utils.test.ts
**/*.{spec,test}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/shared.mdc)
Add meaningful tests
Files:
apps/web/lib/__tests__/security-utils.test.ts
{**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}}
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
{**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}}: Place tests next to the file being tested using the .test.ts or .test.tsx extension
Alternatively, place tests in a tests directory located alongside the source files
Match the test folder structure to the source code structure
In tests, import from '@/test/test-utils' and use the custom render to get consistent providers/utilities
Structure tests with describe blocks and it cases using clear scenario names
Follow the Arrange, Act, Assert (AAA) pattern within test cases
Prefer @testing-library/user-event over fireEvent for user interactions
Follow React Testing Library query priority: getByRole, getByLabelText, getByPlaceholderText, getByText, getByDisplayValue, getByAltText, getByTitle, then getByTestId as a last resort
Mock external modules and APIs in tests using jest.mock and replace global.fetch with a jest.fn where needed
For Next.js API route tests, construct req/res using node-mocks-http createMocks
For Next.js server components, render with data from server-side props and assert rendered output
Test custom hooks with renderHook and wrap state changes in act
Always test both success and error paths for features and API calls
Focus tests on business logic, user interactions, error states, and edge cases
Use meaningful, descriptive test names
Keep tests focused and isolated; mock external dependencies as needed
Use proper cleanup in afterEach when needed
Use async/await properly for asynchronous tests
Maintain test directory structure that mirrors component/module structure
Files:
apps/web/lib/__tests__/security-utils.test.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional React components typed with TypeScript interfaces
Write declarative JSX
Use Shadcn UI, Radix UI, and Tailwind CSS for components and styling
Implement responsive, mobile-first design with Tailwind CSS
When using next/image, include explicit sizes and enable lazy loadingAdd JSDoc blocks for React components, including brief description, @param props with individual prop fields, @returns, and an @example usage
Files:
packages/hooks/src/use-form-state.tsx
**/use*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/jsdoc-comments.mdc)
Add JSDoc blocks for custom React hooks, including purpose, parameters, return value, and an example of hook usage
Files:
packages/hooks/src/use-form-state.tsx
**/index.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/shared.mdc)
Do not use barrel files (avoid index files that only re-export)
Files:
packages/rate-limiting/index.ts
🧠 Learnings (26)
📚 Learning: 2025-08-26T11:20:24.941Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:24.941Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Keep tests focused and isolated; mock external dependencies where appropriate
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:27.126Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:27.126Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Keep tests focused and isolated; mock external dependencies as needed
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:01.309Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:01.309Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Keep tests focused and isolated; mock external dependencies to avoid flakiness
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:58.532Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:58.532Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}} : Keep tests focused and isolated; mock external dependencies as needed
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:20.240Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:20.240Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Keep tests focused and isolated, mocking external dependencies as needed
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:18.780Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:18.780Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and API calls with jest.mock and stub globals like fetch when needed
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:49.726Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:49.726Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and APIs with jest.mock and mock fetch calls as needed
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:15.656Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:15.656Z
Learning: Applies to {*.test.ts,*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Mock modules and external calls (e.g., jest.mock, global.fetch) as needed to isolate tests
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:28.226Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:28.226Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and APIs using jest.mock and mock global.fetch as needed
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:20.905Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:20.905Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and APIs with jest.mock and stub network calls (e.g., global.fetch) in tests
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:10.612Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:10.612Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Mock modules and external dependencies using jest.mock and mock global.fetch for API calls as needed
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:20.240Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:20.240Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock modules with jest.mock and stub external APIs (e.g., global.fetch) as needed in tests
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:58.532Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:58.532Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}} : Mock external modules and APIs in tests using jest.mock and replace global.fetch with a jest.fn where needed
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:27.126Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:27.126Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Mock external modules and network calls with jest.mock and stubbed globals (e.g., global.fetch)
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:20:21.081Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:21.081Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Mock external modules and APIs using jest.mock and stub global.fetch when needed
Applied to files:
apps/web/lib/__tests__/security-utils.test.ts
📚 Learning: 2025-08-26T11:18:35.229Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:35.229Z
Learning: Applies to **/use*.{ts,tsx} : Add JSDoc comments for custom React hooks, including description, param entries, returns, and an example usage snippet
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:44.245Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:44.245Z
Learning: Applies to **/use*.{ts,tsx} : Add JSDoc blocks for custom React hooks, including purpose, parameters, return value, and an example of hook usage
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:39.811Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:39.811Z
Learning: Applies to **/use*.{ts,tsx} : Document React hooks with JSDoc: include a brief description, param entries, returns, and an example of hook usage.
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:40.806Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:40.806Z
Learning: Applies to **/use*.{ts,tsx} : Document React hooks with a JSDoc block including a brief description, param, returns, and an example
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:18.780Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:18.780Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : For custom hooks, use renderHook and wrap state changes in act when needed
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:10.612Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:10.612Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Test custom React hooks with renderHook and wrap state changes in act
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:30.246Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/jsdoc-comments.mdc:0-0
Timestamp: 2025-08-26T11:18:30.246Z
Learning: Applies to **/use*.{ts,tsx} : Add JSDoc to custom React hooks with a brief purpose, param for inputs, returns for outputs, and an example of usage.
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:24.941Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:24.941Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : For custom hooks, use renderHook and wrap state changes in act when asserting updates
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:58.532Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:58.532Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}} : Test custom hooks with renderHook and wrap state changes in act
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:20:49.726Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:49.726Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Test custom React hooks using renderHook and wrap state changes in act
Applied to files:
packages/hooks/src/use-form-state.tsx
📚 Learning: 2025-08-26T11:18:21.095Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/functions.mdc:0-0
Timestamp: 2025-08-26T11:18:21.095Z
Learning: Applies to **/*.{ts,tsx} : For functions with more than one parameter, use a single object parameter instead of multiple positional parameters
Applied to files:
packages/hooks/src/use-form-state.tsx
🧬 Code graph analysis (2)
packages/hooks/src/use-form-state.tsx (1)
packages/logging/index.ts (1)
error(106-122)
packages/rate-limiting/index.ts (3)
apps/web/lib/security-utils-helpers.ts (1)
checkRateLimit(80-117)apps/web/lib/security-utils.ts (1)
checkRateLimit(9-9)apps/web/middleware.ts (1)
config(422-429)
🪛 GitHub Actions: PR Review
apps/web/app/api/members/route.ts
[error] 12-12: Jest encountered an unexpected token: SyntaxError: Cannot use import statement outside a module (env-nextjs) while loading rate-limiting dependencies.
packages/rate-limiting/index.ts
[error] 30-30: SyntaxError: Cannot use import statement outside a module (env-nextjs) when importing rate-limiting dependencies in tests.
🔇 Additional comments (5)
packages/hooks/src/use-form-state.tsx (3)
98-123: Solid API + JSDoc — aligns with our hook guidelines.Single-object param, clear examples, and typed state/actions look good.
Also applies to: 124-126
225-236: Robust error normalization.Covers Error, string, and objects with message; sensible fallback.
241-244: useSimpleFormState matches guidelines (object input, concise API, JSDoc).Good separation for primitive/no‑data forms; return shape is pragmatic.
Also applies to: 262-282
apps/web/lib/__tests__/security-utils.test.ts (1)
53-54: LGTM: assertresetTimeshape for API contract.These checks strengthen the public contract of
checkRateLimit.apps/web/app/api/members/route.ts (1)
2-2: Ignore ESM/CJS mismatch suggestion
The monorepo’s Jest setup maps@thedaviddias/rate-limitingdirectly to its TypeScript source and uses NextJest’s built-in transformer for.tsfiles, so there is no un-transpiled ESM module being imported. The package.json has no"type":"module"and there is nodist/CJS build in packages/rate-limiting, making both the dual-build and custom Jest ESM options unnecessary.Likely an incorrect or invalid review comment.
| import { Ratelimit, type RatelimitConfig } from '@upstash/ratelimit' | ||
| import { Redis } from '@upstash/redis' | ||
| import type { NextRequest } from 'next/server' | ||
| import { keys } from './keys' |
There was a problem hiding this comment.
CI failure: package is consumed as ESM but tests run in CJS; provide dual outputs or update Jest transforms.
Same root cause as the route import failure. Ship both ESM/CJS or configure Jest to transpile this package.
See suggested package.json/Jest snippets in my comment on apps/web/app/api/members/route.ts (Option A/B). Want a tsup build config too?
| const { success, limit, remaining, reset } = await rateLimiter.limit(clientId) | ||
|
|
||
| if (!success) { | ||
| const retryAfter = Math.ceil((reset - Date.now()) / 1000) | ||
| return { | ||
| success: false, | ||
| limit, | ||
| remaining, | ||
| reset, | ||
| retryAfter | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
retryAfter can be negative if reset is seconds (Upstash) and you subtract Date.now() (ms). Normalize units and clamp ≥ 1.
- if (!success) {
- const retryAfter = Math.ceil((reset - Date.now()) / 1000)
+ if (!success) {
+ const nowSec = Math.floor(Date.now() / 1000)
+ // Upstash reset is seconds (if ms, normalize)
+ const resetSec = reset > 1e12 ? Math.floor(reset / 1000) : reset
+ const retryAfter = Math.max(1, resetSec - nowSec)
return {
success: false,
limit,
remaining,
reset,
retryAfter
}📝 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.
| const { success, limit, remaining, reset } = await rateLimiter.limit(clientId) | |
| if (!success) { | |
| const retryAfter = Math.ceil((reset - Date.now()) / 1000) | |
| return { | |
| success: false, | |
| limit, | |
| remaining, | |
| reset, | |
| retryAfter | |
| } | |
| } | |
| const { success, limit, remaining, reset } = await rateLimiter.limit(clientId) | |
| if (!success) { | |
| const nowSec = Math.floor(Date.now() / 1000) | |
| // Normalize reset to seconds (handles both ms and sec values) | |
| const resetSec = reset > 1e12 ? Math.floor(reset / 1000) : reset | |
| const retryAfter = Math.max(1, resetSec - nowSec) | |
| return { | |
| success: false, | |
| limit, | |
| remaining, | |
| reset, | |
| retryAfter | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/rate-limiting/index.ts around lines 121 to 132, the retryAfter
calculation subtracts Date.now() (ms) from reset which may be in seconds
(Upstash) and can produce negative values; normalize units first (convert reset
to milliseconds if it's in seconds, or convert Date.now() to seconds
consistently), compute the seconds until reset using a ceil for partial seconds,
and clamp the result to a minimum of 1 (e.g., use Math.ceil and Math.max(1,
...)) before returning retryAfter.
6506fca to
5bb6ffc
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (5)
packages/security/validation.ts (2)
172-187: Adopt single input object for validatePhoneNumber; preserve leading '+' when sanitizingMatches repo style and avoids stripping E.164 plus prefix.
-/** - * Validate phone number - */ -export function validatePhoneNumber( - phone: string | null | undefined, - locale: validator.MobilePhoneLocale = 'en-US' -): ValidationResult { +/** + * Validate phone number. + * @param input - Phone number and locale + * @param input.phone - Raw phone string + * @param input.locale - Validator locale (default 'en-US') + */ +export interface ValidatePhoneNumberInput { + phone: string | null | undefined + locale?: validator.MobilePhoneLocale +} +export function validatePhoneNumber({ + phone, + locale = 'en-US' +}: ValidatePhoneNumberInput): ValidationResult { if (!phone) { return { valid: false, error: 'Phone number is required' } } - const cleaned = phone.replace(/\D/g, '') + const trimmed = phone.trim() + const cleaned = trimmed.startsWith('+') + ? `+${trimmed.slice(1).replace(/\D/g, '')}` + : trimmed.replace(/\D/g, '') if (!validator.isMobilePhone(cleaned, locale, { strictMode: false })) { return { valid: false, error: 'Invalid phone number format' } } return { valid: true, sanitized: cleaned } }
223-251: Origin validation: support proxies (x-forwarded-*), compare protocol, and follow single-object param ruleImproves correctness behind reverse proxies/CDNs and aligns with repo API style.
-/** - * Validate origin for CSRF protection - */ -export function validateOrigin(request: Request, allowedOrigins: string[] = []): ValidationResult { - const origin = request.headers.get('origin') +/** + * Validate Origin header for CSRF protection. + * - If allowedOrigins provided, require exact origin match (normalized). + * - Otherwise, require origin host/proto to match request host/proto (proxy-aware). + * @param input - Request and policy + * @param input.request - Incoming Request + * @param input.allowedOrigins - Explicitly allowed origins + */ +export interface ValidateOriginInput { + request: Request + allowedOrigins?: string[] +} +export function validateOrigin({ + request, + allowedOrigins = [] +}: ValidateOriginInput): ValidationResult { + const origin = request.headers.get('origin') if (!origin) { return { valid: false, error: 'Origin header required' } } - if (allowedOrigins.length > 0) { - if (allowedOrigins.includes(origin)) { + if (allowedOrigins.length > 0) { + const normalize = (o: string) => { + try { const u = new URL(o); return `${u.protocol}//${u.host}`.toLowerCase() } catch { return o.replace(/\/$/, '').toLowerCase() } + } + const allowed = allowedOrigins.map(normalize) + const incoming = normalize(origin) + if (allowed.includes(incoming)) { return { valid: true } } return { valid: false, error: 'Origin not allowed' } } - const host = request.headers.get('host') + const host = request.headers.get('x-forwarded-host') || request.headers.get('host') + const proto = request.headers.get('x-forwarded-proto') || 'https' if (!host) { return { valid: false, error: 'Host header missing' } } try { - const originUrl = new URL(origin) - if (originUrl.host === host) { + const originUrl = new URL(origin) + if (originUrl.host === host && originUrl.protocol.replace(':', '') === proto) { return { valid: true } } - return { valid: false, error: 'Origin does not match host' } + return { valid: false, error: 'Origin does not match host/protocol' } } catch { return { valid: false, error: 'Invalid origin URL' } } }apps/web/middleware.ts (1)
173-177: Fix Server Actions skip: check header instead of static chunks pathHeader-based detection avoids false positives; matches Next’s action requests.
- // Skip validation for Server Actions (Next.js internal endpoints) - // These will be handled by the Server Action itself - if (request.nextUrl.pathname.startsWith('/_next/static/chunks/')) { + // Skip validation for Server Actions (Next.js internal posts) + // Next adds "Next-Action" header to action invocations + if (request.headers.has('next-action')) { return true }apps/web/lib/security-utils-helpers.ts (2)
80-84: Harden identifier validation (trim) and improve errorReject whitespace-only identifiers and normalize the message.
- if (!input.identifier || typeof input.identifier !== 'string') { - throw new Error('Rate limit identifier is required and must be a string') - } + if (typeof input.identifier !== 'string' || input.identifier.trim().length === 0) { + throw new Error('checkRateLimit: "identifier" must be a non-empty string') + }
86-93: Coerce numeric options instead of requiring integersAccept floats/strings, floor to ints, and guard with Number.isFinite.
- const windowMs = Number(input.windowMs) - const validWindowMs = Number.isInteger(windowMs) && windowMs > 0 ? windowMs : 60000 + const windowMsRaw = Number(input.windowMs) + const validWindowMs = + Number.isFinite(windowMsRaw) && windowMsRaw > 0 ? Math.floor(windowMsRaw) : 60000 - - const maxRequests = Number(input.maxRequests) - const validMaxRequests = Number.isInteger(maxRequests) && maxRequests > 0 ? maxRequests : 10 + const maxRequestsRaw = Number(input.maxRequests) + const validMaxRequests = + Number.isFinite(maxRequestsRaw) && maxRequestsRaw > 0 ? Math.floor(maxRequestsRaw) : 10
🧹 Nitpick comments (23)
apps/web/components/forms/submit-form-step2-content-fields.tsx (5)
144-144: Use design tokens instead of hard-coded colors on SelectContent.Align with the design system and dark mode by using semantic tokens.
- <SelectContent className="bg-white dark:bg-gray-800 border border-gray-200 dark:border-gray-700"> + <SelectContent className="bg-popover text-popover-foreground border border-border">
138-138: Make Select controlled to support form.reset and external updates.Using value keeps the field in sync with RHF changes.
- <Select onValueChange={field.onChange} defaultValue={field.value}> + <Select onValueChange={field.onChange} value={field.value}>
29-33: Expand JSDoc to meet repo guidelines (props, returns, example).Keeps component docs consistent with apps/web standards.
-/** - * Content fields for Step 2 of the submit form - */ -export function ContentFields({ form }: ContentFieldsProps) { +/** + * Content fields for Step 2 of the submit form. + * @param props - Component props. + * @param props.form - React Hook Form instance for Step 2. + * @returns JSX element rendering the controlled fields. + * @example + * <ContentFields form={form} /> + */ +export function ContentFields({ form }: ContentFieldsProps) {
145-181: Prefer SelectGroup/SelectLabel for semantic grouping inside Select.Improves a11y semantics and keyboard navigation; avoids ad‑hoc div headings.
- {/* Tool Categories */} - <div className="px-2 py-1.5 text-sm font-semibold text-muted-foreground"> - Tools & Platforms - </div> - {toolCategories.map(category => ( - <SelectItem - key={category.slug} - value={category.slug} - className="pl-4 text-sm cursor-pointer" - > - <div className="flex items-center gap-2"> - <category.icon className="h-4 w-4" /> - {category.name} - </div> - </SelectItem> - ))} - - {/* Separator */} - <div className="my-1 h-px bg-border" /> - - {/* Non-Tool Categories */} - <div className="px-2 py-1.5 text-sm font-semibold text-muted-foreground"> - Other Sites - </div> - {nonToolCategories.map(category => ( - <SelectItem - key={category.slug} - value={category.slug} - className="pl-4 text-sm cursor-pointer" - > - <div className="flex items-center gap-2"> - <category.icon className="h-4 w-4" /> - {category.name} - </div> - </SelectItem> - ))} + <SelectGroup> + <SelectLabel className="px-2 py-1.5 text-sm text-muted-foreground"> + Tools & Platforms + </SelectLabel> + {toolCategories.map(category => ( + <SelectItem + key={category.slug} + value={category.slug} + className="pl-4 text-sm cursor-pointer" + > + <div className="flex items-center gap-2"> + <category.icon className="h-4 w-4" /> + {category.name} + </div> + </SelectItem> + ))} + </SelectGroup> + + <div className="my-1 h-px bg-border" /> + + <SelectGroup> + <SelectLabel className="px-2 py-1.5 text-sm text-muted-foreground"> + Other Sites + </SelectLabel> + {nonToolCategories.map(category => ( + <SelectItem + key={category.slug} + value={category.slug} + className="pl-4 text-sm cursor-pointer" + > + <div className="flex items-center gap-2"> + <category.icon className="h-4 w-4" /> + {category.name} + </div> + </SelectItem> + ))} + </SelectGroup>If SelectGroup/SelectLabel aren’t exported by the design‑system, I can adapt this to use the system’s Separator/Label equivalents.
13-19: Import SelectGroup/SelectLabel (and Separator if available).Required for the semantic grouping change above.
import { Select, SelectContent, SelectItem, SelectTrigger, - SelectValue + SelectValue, + SelectGroup, + SelectLabel, + SelectSeparator } from '@thedaviddias/design-system/select'If SelectSeparator isn’t available, we’ll keep the div-based separator or switch to the design system’s Separator component.
packages/api-utils/index.ts (5)
269-271: Boolean parsing should validate inputs (don’t silently coerce unknown strings to false).Currently,
?active=foobecomesfalse. Enforce accepted literals.- case 'boolean': - parsedValue = value === 'true' || value === '1' - break + case 'boolean': + if (value === 'true' || value === '1') parsedValue = true + else if (value === 'false' || value === '0') parsedValue = false + else throw new ApiException(`Invalid boolean for parameter: ${String(key)} (expected true/false or 1/0)`, 400) + break
240-250: Empty/whitespace-only string handling for query params.
!valuetreats only empty string as missing;" "passes through untrimmed. Consider trimming fortype: 'string'and optionally rejecting empty-after-trim unlessoptional: trueor an explicitallowEmptyString: true.Minimal tweak:
- if (!value) { + if (!value) { ... } @@ - default: - parsedValue = value + default: + parsedValue = value.trim() + if (parsedValue === '' && !config.optional) { + throw new ApiException(`Missing required parameter: ${String(key)}`, 400) + }If you want configurability, add
allowEmptyString?: booleanto the schema and gate the empty-check accordingly.Also applies to: 273-276
245-247: Remove@ts-expect-errorwith stronger typing of the schema/result.The casts hide potential mistakes. Use a typed schema and derived result type to keep it sound.
Example approach (outside-diff, for clarity):
interface ParamConfig<T> { type: 'string' | 'number' | 'boolean' default?: T optional?: boolean min?: number max?: number enum?: readonly T[] } type Schema = Record<string, ParamConfig<any>> type InferParam<C extends ParamConfig<any>> = C['type'] extends 'string' ? string : C['type'] extends 'number' ? number : C['type'] extends 'boolean' ? boolean : never type ParsedFromSchema<S extends Schema> = { [K in keyof S]: InferParam<S[K]> } export function parseQueryParams<S extends Schema>( { searchParams, schema }: { searchParams: URLSearchParams; schema: S } ): ParsedFromSchema<S> { /* ...no ts-expect-error needed... */ }Also applies to: 285-291
301-309: Standardize error payload shape (ApiError vs helpers).
ApiErrorincludes optionalmessage/statusCode, buterrorResponsereturns{ error, details? }only; ApiException path also omitsstatusCodein body. Pick one shape and use helpers everywhere to keep clients consistent (e.g., always{ error: string; details? }in body with status in HTTP code).Also applies to: 318-320
329-334: CORS headers: add Vary and optional credentials/max-age.Add
Vary: Originto avoid cache poisoning; optionally supportAccess-Control-Allow-CredentialsandAccess-Control-Max-Age.export function withCors({ response, origin = '*' }: WithCorsInput): NextResponse { response.headers.set('Access-Control-Allow-Origin', origin) response.headers.set('Access-Control-Allow-Methods', 'GET, POST, PUT, DELETE, OPTIONS') response.headers.set('Access-Control-Allow-Headers', 'Content-Type, Authorization') + response.headers.set('Vary', 'Origin') + // Optional: + // response.headers.set('Access-Control-Max-Age', '86400') + // if (origin !== '*') response.headers.set('Access-Control-Allow-Credentials', 'true') return response }packages/security/validation.ts (2)
9-13: Prefer centralized list over hardcoded regex for malicious schemesUse MALICIOUS_PROTOCOLS to avoid drift and keep a single source of truth. Also add a brief JSDoc.
-/** - * Check if URL contains malicious protocols - */ -export function hasMaliciousProtocol(url: string): boolean { - const trimmed = url.trim() - // Match malicious protocols only at the start (case-insensitive) - return /^(javascript|data|vbscript|file):/i.test(trimmed) -} +/** + * Check if URL starts with a known dangerous protocol. + * @param url - Raw URL to inspect + * @returns true if the URL scheme is malicious; otherwise false. + */ +export function hasMaliciousProtocol(url: string): boolean { + const lower = url.trim().toLowerCase() + return MALICIOUS_PROTOCOLS.some(protocol => lower.startsWith(`${protocol}:`)) +}
1-256: Add complete JSDoc across all exported utilitiesA few functions have header comments but are missing @param/@returns. Applies to hasMaliciousProtocol, validateURL, validateUsername, validateEmail, validatePhoneNumber, validateLinkedInURL, validateOrigin.
I can push a docs-only commit if you prefer.
packages/rate-limiting/keys.ts (1)
4-14: Add BC fallback to legacy envs and document the helperSafer migration: fall back to legacy UPSTASH_* names, and add JSDoc for the factory.
export const keys = () => - createEnv({ + createEnv({ server: { - STORAGE_REDIS_URL: z.string().min(1).url().optional(), - STORAGE_KV_REST_API_TOKEN: z.string().min(1).optional() + STORAGE_REDIS_URL: z.string().min(1).url().optional(), + STORAGE_KV_REST_API_TOKEN: z.string().min(1).optional() }, runtimeEnv: { - STORAGE_REDIS_URL: process.env.STORAGE_REDIS_URL, - STORAGE_KV_REST_API_TOKEN: process.env.STORAGE_KV_REST_API_TOKEN + STORAGE_REDIS_URL: + process.env.STORAGE_REDIS_URL ?? + process.env.UPSTASH_REDIS_REST_URL, + STORAGE_KV_REST_API_TOKEN: + process.env.STORAGE_KV_REST_API_TOKEN ?? + process.env.UPSTASH_REST_TOKEN } }) + +/** + * Build typed env for rate-limiting package. + * @returns Parsed environment variables (server-only) + */apps/web/__mocks__/@t3-oss/env-nextjs.ts (1)
3-9: Avoid empty-string sentinels and type the mock returnReturn undefined when not set to better mirror real library behavior; add a minimal return type.
-export function createEnv(config: any) { +export function createEnv(config: any): Record<string, string | undefined> { // Return a mock object with the expected environment variables return { - STORAGE_REDIS_URL: process.env.STORAGE_REDIS_URL || '', - STORAGE_KV_REST_API_TOKEN: process.env.STORAGE_KV_REST_API_TOKEN || '' + STORAGE_REDIS_URL: + process.env.STORAGE_REDIS_URL ?? + process.env.UPSTASH_REDIS_REST_URL, + STORAGE_KV_REST_API_TOKEN: + process.env.STORAGE_KV_REST_API_TOKEN ?? + process.env.UPSTASH_REST_TOKEN } }apps/web/lib/env-config.ts (1)
24-26: Add BC fallback for legacy UPSTASH vars in apps/web/lib/env-config.ts
Fall back toUPSTASH_REDIS_REST_URLandUPSTASH_REDIS_REST_TOKENif the new vars are unset:@@ apps/web/lib/env-config.ts:24-26 - STORAGE_REDIS_URL: process.env.STORAGE_REDIS_URL, - STORAGE_KV_REST_API_TOKEN: process.env.STORAGE_KV_REST_API_TOKEN, + STORAGE_REDIS_URL: + process.env.STORAGE_REDIS_URL ?? + process.env.UPSTASH_REDIS_REST_URL, + STORAGE_KV_REST_API_TOKEN: + process.env.STORAGE_KV_REST_API_TOKEN ?? + process.env.UPSTASH_REDIS_REST_TOKEN,Run
rg -n "UPSTASH_REDIS_REST_URL|UPSTASH_REDIS_REST_TOKEN"to verify no remaining legacy references, or remove this fallback once infra is fully migrated.
apps/web/__mocks__/@thedaviddias/rate-limiting.ts (1)
3-21: Light typing for mocks; freeze message constantKeep API shape, but add minimal types and const-assert the message.
-export const withRateLimit = async (request: any, rateLimitType: string, handler: any) => { +export const withRateLimit = async ( + request: unknown, + rateLimitType: string, + handler: () => unknown | Promise<unknown> +) => { // In tests, just call the handler directly without rate limiting return await handler() } -export const checkRateLimit = jest.fn(() => ({ +export const checkRateLimit = jest.fn((): { + allowed: boolean + remaining: number + resetTime: number +} => ({ allowed: true, remaining: 10, resetTime: Date.now() + 60000 })) -export const RATE_LIMIT_EXCEEDED_MESSAGE = 'Too many requests' +export const RATE_LIMIT_EXCEEDED_MESSAGE = 'Too many requests' as constapps/web/middleware.ts (1)
181-183: CSRF hardening: accept common header aliases and fallback to Referer for same-originSmall robustness wins without changing behavior. Consider centralizing origin checks via @thedaviddias/security if available.
// 1. Try to get from headers (for AJAX requests) - csrfToken = request.headers.get('x-csrf-token') + csrfToken = + request.headers.get('x-csrf-token') ?? + request.headers.get('x-xsrf-token') ?? + request.headers.get('csrf-token') @@ - // Basic same-origin check - const origin = request.headers.get('origin') + // Basic same-origin check (Origin if present, else fallback to Referer) + const origin = request.headers.get('origin') ?? request.headers.get('referer') if (origin) { try { - const { host, protocol } = new URL(origin) - const reqHost = request.headers.get('host') + const { host, protocol } = new URL(origin) + const reqHost = request.headers.get('host') || '' const reqProto = request.headers.get('x-forwarded-proto') || 'https' if (host !== reqHost || (protocol && protocol.replace(':', '') !== reqProto)) return false } catch { return false } }Optional (centralize origin check):
// import { validateOrigin } from '@thedaviddias/security' // if (!validateOrigin(request)) return falseAlso applies to: 217-231
apps/web/lib/security-utils-helpers.ts (2)
20-23: Expose optional limit/remaining to help callers set standard headersAdd limit and remaining to the result (optional, backward-compatible) so routes can emit X-RateLimit-* and Retry-After easily.
export interface CheckRateLimitResult { allowed: boolean resetTime: number + /** Optional: total allowed in the window */ + limit?: number + /** Optional: remaining allowed in the current window */ + remaining?: number }
99-107: Return limit/remaining alongside resetTime (matches optional fields above)Makes it trivial for API handlers to set headers without re-computing.
- return { allowed: true, resetTime } + return { allowed: true, resetTime, limit: validMaxRequests, remaining: validMaxRequests - 1 } @@ - return { allowed: true, resetTime: record.resetTime } + return { + allowed: true, + resetTime: record.resetTime, + limit: validMaxRequests, + remaining: Math.max(0, validMaxRequests - record.count) + } @@ - return { allowed: false, resetTime: record.resetTime } + return { allowed: false, resetTime: record.resetTime, limit: validMaxRequests, remaining: 0 }Also applies to: 109-116
apps/web/__tests__/lib/security-rate-limiting.test.ts (3)
12-18: Stabilize time across testsSet a fixed system time to avoid flakiness when comparing/resetting windows.
beforeEach(() => { - jest.useFakeTimers() + jest.useFakeTimers() + jest.setSystemTime(0) })
58-62: Advance mocked clock and system time togetherBe explicit to ensure Date.now aligns with timer advancement in all Jest modes.
- jest.advanceTimersByTime(1001) + jest.advanceTimersByTime(1001) + jest.setSystemTime(1001)
20-87: Add edge-case tests: invalid identifier and option coercionCover empty/whitespace identifiers and non-integer options to lock behavior.
describe('checkRateLimit', () => { + it('throws on empty (trimmed) identifier', () => { + expect(() => checkRateLimit({ identifier: ' ' })).toThrow(/identifier/i) + }) + + it('coerces numeric options (floors and defaults)', () => { + const identifier = 'test-coerce' + const result = checkRateLimit({ identifier, windowMs: 999.9 as unknown as number, maxRequests: 2.7 as unknown as number }) + expect(result.allowed).toBe(true) + // next call should still be allowed (maxRequests floored to 2) + const result2 = checkRateLimit({ identifier, windowMs: 999.9 as unknown as number, maxRequests: 2.7 as unknown as number }) + expect(result2.allowed).toBe(true) + // third should be blocked + const result3 = checkRateLimit({ identifier, windowMs: 999.9 as unknown as number, maxRequests: 2.7 as unknown as number }) + expect(result3.allowed).toBe(false) + })I can push a patch with these tests if you want.
apps/web/jest.config.ts (1)
2-2: Use canonical import path for next/jestMinor nit to match Next.js docs and avoid potential resolution quirks.
-import nextJest from 'next/jest.js' +import nextJest from 'next/jest'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.gitignore(1 hunks)apps/web/__mocks__/@t3-oss/env-nextjs.ts(1 hunks)apps/web/__mocks__/@thedaviddias/rate-limiting.ts(1 hunks)apps/web/__tests__/lib/security-rate-limiting.test.ts(1 hunks)apps/web/actions/submit-llms-xxt.ts(3 hunks)apps/web/app/api/members/route.ts(1 hunks)apps/web/components/forms/submit-form-step2-content-fields.tsx(1 hunks)apps/web/components/forms/submit-form.tsx(2 hunks)apps/web/jest.config.ts(1 hunks)apps/web/lib/__tests__/security-utils.test.ts(2 hunks)apps/web/lib/env-config.ts(1 hunks)apps/web/lib/security-utils-helpers.ts(2 hunks)apps/web/lib/security-utils.ts(1 hunks)apps/web/middleware.ts(2 hunks)packages/api-utils/index.ts(1 hunks)packages/api-utils/package.json(1 hunks)packages/hooks/src/use-form-state.tsx(1 hunks)packages/rate-limiting/index.ts(1 hunks)packages/rate-limiting/keys.ts(1 hunks)packages/security/validation.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/api-utils/package.json
- apps/web/lib/tests/security-utils.test.ts
- apps/web/app/api/members/route.ts
- .gitignore
- packages/rate-limiting/index.ts
- apps/web/actions/submit-llms-xxt.ts
- apps/web/lib/security-utils.ts
- packages/hooks/src/use-form-state.tsx
- apps/web/components/forms/submit-form.tsx
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/functions.mdc)
**/*.{ts,tsx}: For functions with more than one parameter, use a single object parameter instead of multiple positional parameters
Name the input type for such object parameters using the pattern {FunctionName}Input
For single-parameter functions, direct positional arguments are acceptable
**/*.{ts,tsx}: Write concise, technical TypeScript code
Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Use TypeScript for all code; prefer interfaces over types
Avoid enums; use object maps instead
Use thefunctionkeyword for pure functions
Avoid unnecessary curly braces in simple conditionals; use concise syntax
**/*.{ts,tsx}: Use interfaces over types when possible in TypeScript
Avoid enums; prefer const objects (as const) instead
Use proper error handling with custom error types
Favor functional programming patterns
Avoid classes
Use pure functions where possible
Implement proper error handling
Use meaningful variable names
Keep functions small and focused
Files:
apps/web/__mocks__/@thedaviddias/rate-limiting.tsapps/web/jest.config.tsapps/web/__tests__/lib/security-rate-limiting.test.tsapps/web/lib/env-config.tsapps/web/lib/security-utils-helpers.tsapps/web/__mocks__/@t3-oss/env-nextjs.tspackages/rate-limiting/keys.tsapps/web/middleware.tsapps/web/components/forms/submit-form-step2-content-fields.tsxpackages/api-utils/index.tspackages/security/validation.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/jsdoc-comments.mdc)
Add JSDoc blocks for utility functions, documenting purpose, @param entries, @returns, and @throws where applicable
Files:
apps/web/__mocks__/@thedaviddias/rate-limiting.tsapps/web/jest.config.tsapps/web/__tests__/lib/security-rate-limiting.test.tsapps/web/lib/env-config.tsapps/web/lib/security-utils-helpers.tsapps/web/__mocks__/@t3-oss/env-nextjs.tspackages/rate-limiting/keys.tsapps/web/middleware.tspackages/api-utils/index.tspackages/security/validation.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
apps/web/**/*.{ts,tsx}: Use Shadcn UI components with Radix UI primitives sourced from packages/design-system
Optimize for Core Web Vitals (images, fonts, script strategy, code-splitting)
Use camelCase for function and variable names
Use descriptive names with auxiliary verbs (e.g., isLoading, hasError)
Files:
apps/web/__mocks__/@thedaviddias/rate-limiting.tsapps/web/jest.config.tsapps/web/__tests__/lib/security-rate-limiting.test.tsapps/web/lib/env-config.tsapps/web/lib/security-utils-helpers.tsapps/web/__mocks__/@t3-oss/env-nextjs.tsapps/web/middleware.tsapps/web/components/forms/submit-form-step2-content-fields.tsx
apps/web/**
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
Use kebab-case for directories
Files:
apps/web/__mocks__/@thedaviddias/rate-limiting.tsapps/web/jest.config.tsapps/web/__tests__/lib/security-rate-limiting.test.tsapps/web/lib/env-config.tsapps/web/lib/security-utils-helpers.tsapps/web/__mocks__/@t3-oss/env-nextjs.tsapps/web/middleware.tsapps/web/components/forms/submit-form-step2-content-fields.tsx
**/*.{spec,test}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/shared.mdc)
Add meaningful tests
Files:
apps/web/__tests__/lib/security-rate-limiting.test.ts
{**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}}
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
{**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}}: Place tests next to the file being tested using the .test.ts or .test.tsx extension
Alternatively, place tests in a tests directory located alongside the source files
Match the test folder structure to the source code structure
In tests, import from '@/test/test-utils' and use the custom render to get consistent providers/utilities
Structure tests with describe blocks and it cases using clear scenario names
Follow the Arrange, Act, Assert (AAA) pattern within test cases
Prefer @testing-library/user-event over fireEvent for user interactions
Follow React Testing Library query priority: getByRole, getByLabelText, getByPlaceholderText, getByText, getByDisplayValue, getByAltText, getByTitle, then getByTestId as a last resort
Mock external modules and APIs in tests using jest.mock and replace global.fetch with a jest.fn where needed
For Next.js API route tests, construct req/res using node-mocks-http createMocks
For Next.js server components, render with data from server-side props and assert rendered output
Test custom hooks with renderHook and wrap state changes in act
Always test both success and error paths for features and API calls
Focus tests on business logic, user interactions, error states, and edge cases
Use meaningful, descriptive test names
Keep tests focused and isolated; mock external dependencies as needed
Use proper cleanup in afterEach when needed
Use async/await properly for asynchronous tests
Maintain test directory structure that mirrors component/module structure
Files:
apps/web/__tests__/lib/security-rate-limiting.test.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional React components typed with TypeScript interfaces
Write declarative JSX
Use Shadcn UI, Radix UI, and Tailwind CSS for components and styling
Implement responsive, mobile-first design with Tailwind CSS
When using next/image, include explicit sizes and enable lazy loadingAdd JSDoc blocks for React components, including brief description, @param props with individual prop fields, @returns, and an @example usage
Files:
apps/web/components/forms/submit-form-step2-content-fields.tsx
apps/web/{**/*.{tsx,css},tailwind.config.{js,ts},postcss.config.{js,ts}}
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
Follow mobile-first responsive design using Tailwind CSS 4
Files:
apps/web/components/forms/submit-form-step2-content-fields.tsx
**/index.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/shared.mdc)
Do not use barrel files (avoid index files that only re-export)
Files:
packages/api-utils/index.ts
🧠 Learnings (27)
📚 Learning: 2025-08-26T11:20:24.941Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:24.941Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Keep tests focused and isolated; mock external dependencies where appropriate
Applied to files:
apps/web/__mocks__/@thedaviddias/rate-limiting.ts
📚 Learning: 2025-08-26T11:20:27.126Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:27.126Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Keep tests focused and isolated; mock external dependencies as needed
Applied to files:
apps/web/__mocks__/@thedaviddias/rate-limiting.ts
📚 Learning: 2025-08-26T11:20:10.612Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:10.612Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Mock modules and external dependencies using jest.mock and mock global.fetch for API calls as needed
Applied to files:
apps/web/__mocks__/@thedaviddias/rate-limiting.tsapps/web/jest.config.tsapps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:15.656Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:15.656Z
Learning: Applies to {*.test.ts,*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Mock modules and external calls (e.g., jest.mock, global.fetch) as needed to isolate tests
Applied to files:
apps/web/__mocks__/@thedaviddias/rate-limiting.tsapps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:01.309Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:01.309Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and APIs (e.g., using jest.mock and mocking global.fetch) in tests
Applied to files:
apps/web/__mocks__/@thedaviddias/rate-limiting.tsapps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:58.532Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:58.532Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}} : Mock external modules and APIs in tests using jest.mock and replace global.fetch with a jest.fn where needed
Applied to files:
apps/web/__mocks__/@thedaviddias/rate-limiting.tsapps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:01.309Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:01.309Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Keep tests focused and isolated; mock external dependencies to avoid flakiness
Applied to files:
apps/web/__mocks__/@thedaviddias/rate-limiting.ts
📚 Learning: 2025-08-26T11:20:20.240Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:20.240Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock modules with jest.mock and stub external APIs (e.g., global.fetch) as needed in tests
Applied to files:
apps/web/__mocks__/@thedaviddias/rate-limiting.tsapps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:18.780Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:18.780Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and API calls with jest.mock and stub globals like fetch when needed
Applied to files:
apps/web/__mocks__/@thedaviddias/rate-limiting.tsapps/web/jest.config.tsapps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:20.240Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:20.240Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Keep tests focused and isolated, mocking external dependencies as needed
Applied to files:
apps/web/__mocks__/@thedaviddias/rate-limiting.ts
📚 Learning: 2025-08-26T11:20:18.780Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:18.780Z
Learning: Applies to {**/*.test.ts,**/__tests__/*.ts} : For Next.js API routes, use node-mocks-http createMocks to simulate req/res and assert handler behavior
Applied to files:
apps/web/__mocks__/@thedaviddias/rate-limiting.tsapps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:49.726Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:49.726Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and APIs with jest.mock and mock fetch calls as needed
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:24.941Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:24.941Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Mock modules with jest.mock and mock fetch (e.g., global.fetch as jest.Mock) for external dependencies
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:20.905Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:20.905Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and APIs with jest.mock and stub network calls (e.g., global.fetch) in tests
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:28.226Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:28.226Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and APIs using jest.mock and mock global.fetch as needed
Applied to files:
apps/web/jest.config.tsapps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:49.726Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:49.726Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : For Next.js API routes, use node-mocks-http createMocks to create req/res objects in tests
Applied to files:
apps/web/jest.config.tsapps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:01.309Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:01.309Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Test Next.js API routes using node-mocks-http createMocks to simulate req/res objects
Applied to files:
apps/web/jest.config.tsapps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:28.226Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:28.226Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Test Next.js API routes using node-mocks-http createMocks to construct req/res objects
Applied to files:
apps/web/jest.config.tsapps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:20.905Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:20.905Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : For Next.js API routes, use node-mocks-http createMocks to construct req/res and assert status codes
Applied to files:
apps/web/jest.config.tsapps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:21.081Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:21.081Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Test Next.js API routes using node-mocks-http (createMocks) to simulate req/res and assert handler behavior
Applied to files:
apps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:24.941Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:24.941Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : For Next.js API routes, use node-mocks-http createMocks to construct req/res and assert on the response
Applied to files:
apps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:27.126Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:27.126Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Test Next.js API routes using node-mocks-http createMocks and assert on res
Applied to files:
apps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:20.456Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:20.456Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Test Next.js API routes using node-mocks-http createMocks to construct req/res and assert on handler results
Applied to files:
apps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:58.532Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:58.532Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}} : For Next.js API route tests, construct req/res using node-mocks-http createMocks
Applied to files:
apps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:20:27.126Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:27.126Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Mock external modules and network calls with jest.mock and stubbed globals (e.g., global.fetch)
Applied to files:
apps/web/__mocks__/@t3-oss/env-nextjs.ts
📚 Learning: 2025-08-26T11:21:06.811Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/website.mdc:0-0
Timestamp: 2025-08-26T11:21:06.811Z
Learning: Applies to apps/web/app/**/*.{ts,tsx} : Implement type-safe Server Actions using next-safe-action
Applied to files:
apps/web/middleware.ts
📚 Learning: 2025-08-26T11:21:03.221Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/website.mdc:0-0
Timestamp: 2025-08-26T11:21:03.221Z
Learning: Applies to apps/web/app/**/*.{ts,tsx,js,jsx} : Implement type-safe server actions using next-safe-action
Applied to files:
apps/web/middleware.ts
🧬 Code graph analysis (4)
apps/web/lib/security-utils-helpers.ts (2)
apps/web/lib/security-utils.ts (1)
checkRateLimit(9-9)packages/rate-limiting/index.ts (1)
checkRateLimit(107-150)
apps/web/middleware.ts (2)
packages/logging/index.ts (2)
error(106-122)logger(168-168)apps/web/lib/server-crypto.ts (1)
hashSensitiveData(8-10)
packages/api-utils/index.ts (1)
packages/logging/index.ts (2)
error(106-122)logger(168-168)
packages/security/validation.ts (3)
packages/security/types.ts (2)
URLValidationOptions(4-10)ValidationResult(15-19)packages/security/constants.ts (1)
RESERVED_USERNAMES(9-30)packages/security/sanitization.ts (1)
sanitizeText(10-34)
🔇 Additional comments (4)
packages/security/validation.ts (1)
1-256: No internal call sites detected; confirm external usage
Search across.ts/.tsxfiles found only the function definitions and tests—no consumer calls. Ensure any external callers are updated to the new object-arg signature to avoid breaking changes.apps/web/middleware.ts (1)
397-414: CSRF failure logging: LGTMGood use of IP hashing, structured tags, and upgraded severity.
apps/web/__tests__/lib/security-rate-limiting.test.ts (1)
24-26: LGTM: asserting resetTime presence and typeThe expectations now align with the helper always returning a numeric resetTime.
apps/web/jest.config.ts (1)
74-75: LGTM: targeted mocks for env and rate-limitingSpecific mappers precede the generic @thedaviddias pattern, so they won’t be shadowed.
| export interface ApiError { | ||
| error: string | ||
| message?: string | ||
| details?: unknown | ||
| statusCode?: number | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid misclassifying successful payloads that happen to include an error field; add a proper type guard.
Using 'error' in result is too loose and can flag valid payloads. Introduce an isApiError guard and use it.
@@
export interface ApiError {
error: string
message?: string
details?: unknown
statusCode?: number
}
+
+export function isApiError(value: unknown): value is ApiError {
+ return !!value && typeof value === 'object' && 'error' in (value as any)
+ && typeof (value as any).error === 'string'
+}
@@
- // Check if result is an error response
- if (result && typeof result === 'object' && 'error' in result) {
+ // Check if result is an ApiError payload
+ if (isApiError(result)) {
const errorResponse = result
const statusCode =
'statusCode' in errorResponse && typeof errorResponse.statusCode === 'number'
? errorResponse.statusCode
: 500
return NextResponse.json(errorResponse, { status: statusCode })
}Also applies to: 90-98
🤖 Prompt for AI Agents
In packages/api-utils/index.ts around lines 7 to 12 (and also update the check
at lines 90-98), the current use of "'error' in result" is too permissive and
can misclassify valid payloads; implement a proper type guard function
isApiError(obj: unknown): obj is ApiError that verifies obj is a non-null object
and that obj.error is a string (and optionally validate other fields like
statusCode as a number if present), then replace all "'error' in result" checks
with calls to isApiError(result) so only genuine ApiError shapes are treated as
errors.
| export function withApiHandler<T = unknown>( | ||
| handler: (request: NextRequest) => Promise<ApiResponse<T> | Response>, | ||
| options: ApiHandlerOptions = {} | ||
| ) { | ||
| return async (request: NextRequest): Promise<Response> => { | ||
| const { | ||
| logErrors = true, | ||
| defaultErrorMessage = 'An error occurred processing your request', | ||
| tags = {}, | ||
| includeErrorDetails = process.env.NODE_ENV !== 'production' | ||
| } = options | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Adopt single object parameter for exported functions (align with repo TS guidelines).
Per coding_guidelines, functions with >1 param should accept a single {FunctionName}Input object. Update signatures for withApiHandler, validateRequiredFields, parseQueryParams, errorResponse, successResponse, and withCors.
Apply diffs below (examples shown; replicate pattern for the rest):
@@
-export function withApiHandler<T = unknown>(
- handler: (request: NextRequest) => Promise<ApiResponse<T> | Response>,
- options: ApiHandlerOptions = {}
-) {
- return async (request: NextRequest): Promise<Response> => {
+export interface WithApiHandlerInput<T = unknown> {
+ handler: (request: NextRequest) => Promise<ApiResponse<T> | Response>
+ options?: ApiHandlerOptions
+}
+export function withApiHandler<T = unknown>({ handler, options = {} }: WithApiHandlerInput<T>) {
+ return async (request: NextRequest): Promise<Response> => {@@
-export function validateRequiredFields(
- body: Record<string, unknown>,
- requiredFields: string[]
-): void {
+export interface ValidateRequiredFieldsInput {
+ body: Record<string, unknown>
+ requiredFields: readonly string[]
+}
+export function validateRequiredFields({ body, requiredFields }: ValidateRequiredFieldsInput): void {@@
-export function parseQueryParams<T extends Record<string, unknown>>(
- searchParams: URLSearchParams,
- schema: {
- [K in keyof T]: {
- type: 'string' | 'number' | 'boolean'
- default?: T[K]
- optional?: boolean
- min?: number
- max?: number
- enum?: readonly T[K][]
- }
- }
-): T {
+export interface ParseQueryParamsInput<T extends Record<string, unknown>> {
+ searchParams: URLSearchParams
+ schema: {
+ [K in keyof T]: {
+ type: 'string' | 'number' | 'boolean'
+ default?: T[K]
+ optional?: boolean
+ min?: number
+ max?: number
+ enum?: readonly T[K][]
+ }
+ }
+}
+export function parseQueryParams<T extends Record<string, unknown>>(
+ { searchParams, schema }: ParseQueryParamsInput<T>
+): T {@@
-export function errorResponse(message: string, statusCode = 500, details?: unknown): NextResponse {
+export interface ErrorResponseInput {
+ message: string
+ statusCode?: number
+ details?: unknown
+}
+export function errorResponse({ message, statusCode = 500, details }: ErrorResponseInput): NextResponse {@@
-export function successResponse<T>(data: T, statusCode = 200): NextResponse {
- return NextResponse.json(data, { status: statusCode })
-}
+export interface SuccessResponseInput<T> {
+ data: T
+ statusCode?: number
+}
+export function successResponse<T>({ data, statusCode = 200 }: SuccessResponseInput<T>): NextResponse {
+ return NextResponse.json(data, { status: statusCode })
+}@@
-export function withCors(response: NextResponse, origin = '*'): NextResponse {
+export interface WithCorsInput {
+ response: NextResponse
+ origin?: string
+}
+export function withCors({ response, origin = '*' }: WithCorsInput): NextResponse {Also applies to: 173-176, 222-234, 301-301, 318-318, 329-329
thedaviddias
force-pushed
the
feature/refactor-dry-code
branch
from
5bb6ffc to
50c717c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (17)
packages/rate-limiting/index.ts (2)
121-131: retryAfter calculation mixes seconds vs milliseconds; can go negativeNormalize units and clamp to ≥ 1s to avoid confusing headers and messages.
- if (!success) { - const retryAfter = Math.ceil((reset - Date.now()) / 1000) + if (!success) { + const nowSec = Math.floor(Date.now() / 1000) + const resetSec = reset > 1e12 ? Math.floor(reset / 1000) : reset + const retryAfter = Math.max(1, resetSec - nowSec) return { success: false, limit, remaining, reset, retryAfter }
162-187: Set standard RateLimit header and normalize X-RateLimit-Reset/Retry-AfterExpose seconds-until-reset (not absolute timestamp) and add the standard RateLimit header; clamp values.
- // Add rate limit headers to response - const headers = new Headers() - headers.set('X-RateLimit-Limit', rateLimitResult.limit.toString()) - headers.set('X-RateLimit-Remaining', rateLimitResult.remaining.toString()) - headers.set('X-RateLimit-Reset', rateLimitResult.reset.toString()) + // Add rate limit headers to response (normalized) + const headers = new Headers() + const resetMs = + rateLimitResult.reset > 1e12 + ? rateLimitResult.reset + : rateLimitResult.reset * 1000 + const secondsUntilReset = Math.max( + 1, + Math.ceil((resetMs - Date.now()) / 1000) + ) + // Standard header (draft) + headers.set( + 'RateLimit', + `limit=${rateLimitResult.limit}, remaining=${rateLimitResult.remaining}, reset=${secondsUntilReset}` + ) + // Back-compat X-* headers + headers.set('X-RateLimit-Limit', rateLimitResult.limit.toString()) + headers.set('X-RateLimit-Remaining', rateLimitResult.remaining.toString()) + headers.set('X-RateLimit-Reset', secondsUntilReset.toString()) @@ - if (!rateLimitResult.success) { - if (rateLimitResult.retryAfter) { - headers.set('Retry-After', rateLimitResult.retryAfter.toString()) - } + if (!rateLimitResult.success) { + const retryAfter = Math.max( + 1, + rateLimitResult.retryAfter ?? secondsUntilReset + ) + headers.set('Retry-After', retryAfter.toString()) @@ - message: `Too many requests. Try again in ${rateLimitResult.retryAfter} seconds.`, - retryAfter: rateLimitResult.retryAfter + message: `Too many requests. Try again in ${retryAfter} seconds.`, + retryAfterpackages/security/validation.ts (6)
18-33: Conform to single object param + add input interface for validateURL.Aligns with repo TS guidelines; keep existing behavior.
-/** - * Validate and sanitize URL - */ -export function validateURL( - url: string | null | undefined, - options: URLValidationOptions = {} -): ValidationResult { +/** + * Validate and sanitize a URL. + * @param input - URL and options + */ +export interface ValidateURLInput { + url: string | null | undefined + options?: URLValidationOptions +} +export function validateURL( + { url, options = {} }: ValidateURLInput +): ValidationResult {
64-83: Flatten options and use a single input object for validateUsername; keep defaults.Improves discoverability and follows the repo rule.
-/** - * Validate username format - */ -export function validateUsername( - username: string | null | undefined, - options?: { - minLength?: number - maxLength?: number - allowedChars?: RegExp - reservedUsernames?: readonly string[] - } -): ValidationResult { +/** + * Validate username format. + */ +export interface ValidateUsernameInput { + username: string | null | undefined + minLength?: number + maxLength?: number + allowedChars?: RegExp + reservedUsernames?: readonly string[] +} +export function validateUsername({ + username, + minLength = 3, + maxLength = 30, + allowedChars = /^[a-zA-Z0-9_-]+$/, + reservedUsernames = RESERVED_USERNAMES +}: ValidateUsernameInput): ValidationResult { @@ - const { - minLength = 3, - maxLength = 30, - allowedChars = /^[a-zA-Z0-9_-]+$/, - reservedUsernames = RESERVED_USERNAMES - } = options || {} + // defaults taken from input destructuring aboveAlso applies to: 84-110
115-167: Switch validateEmail to single input object and strongly type options; keep whitelist/blacklist behavior.Also replace any with validator.IsEmailOptions.
-/** - * Validate email address - */ -export function validateEmail( - email: string | null | undefined, - options?: { - allowDisplayName?: boolean - requireDisplayName?: boolean - allowUtf8LocalPart?: boolean - requireTld?: boolean - ignoreMaxLength?: boolean - domainWhitelist?: string[] - domainBlacklist?: string[] - } -): ValidationResult { +/** + * Validate email address. + */ +export interface ValidateEmailInput { + email: string | null | undefined + allowDisplayName?: boolean + requireDisplayName?: boolean + allowUtf8LocalPart?: boolean + requireTld?: boolean + ignoreMaxLength?: boolean + domainWhitelist?: string[] + domainBlacklist?: string[] +} +export function validateEmail({ + email, + allowDisplayName = false, + requireDisplayName = false, + allowUtf8LocalPart = false, + requireTld = true, + ignoreMaxLength = false, + domainWhitelist = [], + domainBlacklist = [] +}: ValidateEmailInput): ValidationResult { @@ - const { - allowDisplayName = false, - requireDisplayName = false, - allowUtf8LocalPart = false, - requireTld = true, - ignoreMaxLength = false, - domainWhitelist = [], - domainBlacklist = [] - } = options || {} - - const emailOptions: any = { + const emailOptions: validator.IsEmailOptions = {
172-187: Adopt single object parameter for validatePhoneNumber; export input interface.Follows repo guideline and keeps the same behavior.
-/** - * Validate phone number - */ -export function validatePhoneNumber( - phone: string | null | undefined, - locale: validator.MobilePhoneLocale = 'en-US' -): ValidationResult { +export interface ValidatePhoneNumberInput { + phone: string | null | undefined + locale?: validator.MobilePhoneLocale +} +export function validatePhoneNumber({ + phone, + locale = 'en-US' +}: ValidatePhoneNumberInput): ValidationResult {
192-218: Fix LinkedIn validation: avoid substring matches; enforce https + host + path.Current includes() accepts evil-linkedin.com and non-HTTPS. Parse URL, check host ends with linkedin.com, require https, and enforce /in/{slug}.
-/** - * Validate LinkedIn URL - */ -export function validateLinkedInURL(url: string | null | undefined): ValidationResult { +/** + * Validate LinkedIn profile URL. + */ +export function validateLinkedInURL(url: string | null | undefined): ValidationResult { @@ - if (!trimmed.includes('linkedin.com/in/')) { - return { - valid: false, - error: 'Please enter a valid LinkedIn profile URL (e.g., linkedin.com/in/yourname)' - } - } - - const urlToCheck = trimmed.startsWith('http') ? trimmed : `https://${trimmed}` - try { - new URL(urlToCheck) - return { valid: true, sanitized: urlToCheck } + const candidate = trimmed.startsWith('http') ? trimmed : `https://${trimmed}` + const u = new URL(candidate) + const host = u.hostname.toLowerCase() + const isLinkedInHost = host === 'linkedin.com' || host.endsWith('.linkedin.com') + if (!isLinkedInHost) { + return { valid: false, error: 'Please use a linkedin.com profile URL' } + } + if (u.protocol !== 'https:') { + return { valid: false, error: 'LinkedIn URL must use HTTPS' } + } + if (!/^\/in\/[A-Za-z0-9._%-]+\/?$/.test(u.pathname)) { + return { + valid: false, + error: 'Please enter a valid LinkedIn profile URL (e.g., https://www.linkedin.com/in/yourname)' + } + } + const sanitized = `https://${host}${u.pathname.replace(/\/+$/, '')}` + return { valid: true, sanitized } } catch { return { valid: false, error: 'Please enter a valid LinkedIn URL' } } }
223-251: validateOrigin: support proxies and switch to single object parameter.Use x-forwarded-host/proto and object arg per guidelines.
-/** - * Validate origin for CSRF protection - */ -export function validateOrigin(request: Request, allowedOrigins: string[] = []): ValidationResult { +export interface ValidateOriginInput { + request: Request + allowedOrigins?: string[] +} +export function validateOrigin({ request, allowedOrigins = [] }: ValidateOriginInput): ValidationResult { @@ - const host = request.headers.get('host') + const host = request.headers.get('x-forwarded-host') || request.headers.get('host') + const proto = request.headers.get('x-forwarded-proto') || 'https' @@ - const originUrl = new URL(origin) - if (originUrl.host === host) { + const originUrl = new URL(origin) + if (originUrl.host === host && originUrl.protocol.replace(':', '') === proto) { return { valid: true } }packages/api-utils/index.ts (8)
7-12: Add an isApiError type guard to avoid misclassifying payloads.export interface ApiError { error: string message?: string details?: unknown statusCode?: number } + +export function isApiError(value: unknown): value is ApiError { + return !!value && typeof value === 'object' && typeof (value as any).error === 'string' +}
70-81: Adopt single object parameter for withApiHandler.Align with repo rule; no behavior change.
-export function withApiHandler<T = unknown>( - handler: (request: NextRequest) => Promise<ApiResponse<T> | Response>, - options: ApiHandlerOptions = {} -) { - return async (request: NextRequest): Promise<Response> => { +export interface WithApiHandlerInput<T = unknown> { + handler: (request: NextRequest) => Promise<ApiResponse<T> | Response> + options?: ApiHandlerOptions +} +export function withApiHandler<T = unknown>({ handler, options = {} }: WithApiHandlerInput<T>) { + return async (request: NextRequest): Promise<Response> => {
90-98: Use the type guard when detecting error payloads.- // Check if result is an error response - if (result && typeof result === 'object' && 'error' in result) { + // Check if result is an ApiError payload + if (isApiError(result)) {
173-204: validateRequiredFields: object param + own-property check.Use a single input object and avoid prototype-chain surprises; keep improved null/empty handling.
-export function validateRequiredFields( - body: Record<string, unknown>, - requiredFields: string[] -): void { +export interface ValidateRequiredFieldsInput { + body: Record<string, unknown> + requiredFields: readonly string[] +} +export function validateRequiredFields({ body, requiredFields }: ValidateRequiredFieldsInput): void { const missingFields = requiredFields.filter(field => { - // Field not present in object - if (!(field in body)) { + // Field not present in object (own property only) + if (!Object.prototype.hasOwnProperty.call(body, field)) { return true } @@ - const value = body[field] + const value = (body as Record<string, unknown>)[field]
222-291: parseQueryParams: switch to single input object.Matches repo guideline; behavior unchanged.
-export function parseQueryParams<T extends Record<string, unknown>>( - searchParams: URLSearchParams, - schema: { +export interface ParseQueryParamsInput<T extends Record<string, unknown>> { + searchParams: URLSearchParams + schema: { [K in keyof T]: { type: 'string' | 'number' | 'boolean' default?: T[K] optional?: boolean min?: number max?: number enum?: readonly T[K][] } } -): T { +} +export function parseQueryParams<T extends Record<string, unknown>>( + { searchParams, schema }: ParseQueryParamsInput<T> +): T {
301-309: errorResponse: adopt single object parameter.-export function errorResponse(message: string, statusCode = 500, details?: unknown): NextResponse { +export interface ErrorResponseInput { + message: string + statusCode?: number + details?: unknown +} +export function errorResponse({ message, statusCode = 500, details }: ErrorResponseInput): NextResponse {
318-320: successResponse: adopt single object parameter.-export function successResponse<T>(data: T, statusCode = 200): NextResponse { - return NextResponse.json(data, { status: statusCode }) -} +export interface SuccessResponseInput<T> { + data: T + statusCode?: number +} +export function successResponse<T>({ data, statusCode = 200 }: SuccessResponseInput<T>): NextResponse { + return NextResponse.json(data, { status: statusCode }) +}
329-334: withCors: adopt single object parameter.-export function withCors(response: NextResponse, origin = '*'): NextResponse { +export interface WithCorsInput { + response: NextResponse + origin?: string +} +export function withCors({ response, origin = '*' }: WithCorsInput): NextResponse {apps/web/middleware.ts (1)
174-178: Fix Server Actions skip: check request header, not static chunk path.Next sets the next-action header for action posts.
- // Skip validation for Server Actions (Next.js internal endpoints) - // These will be handled by the Server Action itself - if (request.nextUrl.pathname.startsWith('/_next/static/chunks/')) { + // Skip validation for Server Actions (Next.js internal posts) + if (request.headers.has('next-action')) { return true }
🧹 Nitpick comments (7)
apps/web/jest.config.ts (1)
68-85: Optional: co-locate workspace source mapping after all explicit mocksNot required, but keeping all explicit mocks first avoids accidental shadowing as more workspace packages are added.
moduleNameMapper: { '^@/(.*)$': '<rootDir>/$1', 'lucide-react': require.resolve('lucide-react'), 'react-markdown': '<rootDir>/__mocks__/react-markdown.tsx', nuqs: '<rootDir>/__mocks__/nuqs.ts', '@t3-oss/env-nextjs': '<rootDir>/__mocks__/@t3-oss/env-nextjs.ts', '@thedaviddias/rate-limiting': '<rootDir>/__mocks__/@thedaviddias/rate-limiting.ts', - '^@thedaviddias/([^/]+)$': ['<rootDir>/../../packages/$1/src', '<rootDir>/../../packages/$1'], + '^@thedaviddias/([^/]+)$': [ + '<rootDir>/../../packages/$1/src', + '<rootDir>/../../packages/$1' + ],packages/api-utils/package.json (1)
6-13: Emit explicit “types” entry for TS toolingHelps editors/consumers resolve declarations without guessing through exports.
"exports": { - ".": "./index.ts" + ".": "./index.ts" }, - "main": "./index.ts", + "main": "./index.ts", + "types": "./index.ts",apps/web/lib/security-utils.ts (1)
6-6: Deprecate transitional helper export insecurity-utils-helpers.ts
ThesanitizeErrorMessagefunction is exported directly inapps/web/lib/security-utils-helpers.ts(line 31); if this import is temporary (now maintained in@thedaviddias/rate-limiting), annotate the export with@deprecated(e.g. JSDoc) to guide callers.packages/rate-limiting/index.ts (3)
114-120: Avoid constructing a new Ratelimit per call; cache per-endpointReduces connection/instantiation overhead, especially under load.
+ // Cache a limiter per endpoint to avoid recreating each call + // Module-level cache + // eslint-disable-next-line no-var + // @ts-ignore - attach once per module instance + globalThis.__rlCache ??= new Map<string, Ratelimit>() + // @ts-ignore + const cache: Map<string, Ratelimit> = globalThis.__rlCache + let rateLimiter = cache.get(endpoint) - const rateLimiter = new Ratelimit({ - redis: getRedisClient(), - limiter: Ratelimit.slidingWindow(config.requests, `${config.window} s`), - prefix: `llms-txt-hub:${endpoint}` - }) + if (!rateLimiter) { + rateLimiter = new Ratelimit({ + redis: getRedisClient(), + limiter: Ratelimit.slidingWindow(config.requests, `${config.window} s`), + prefix: `llms-txt-hub:${endpoint}` + }) + cache.set(endpoint, rateLimiter) + }
61-66: Minor: ignore 'unknown' tokens in forwarded-forSome proxies set "unknown"; filter it out.
- if (forwardedFor) { - const firstIp = forwardedFor.split(',')[0]?.trim() - if (firstIp && isValidIp(firstIp)) return firstIp - } + if (forwardedFor) { + const firstIp = forwardedFor.split(',')[0]?.trim() + if (firstIp && firstIp.toLowerCase() !== 'unknown' && isValidIp(firstIp)) { + return firstIp + } + }
140-149: Prefer centralized logger over console.errorKeeps logs consistent across packages.
- console.error('Rate limit check failed:', error) + // TODO: inject @thedaviddias/logging or a noop logger for libraries + console.error('Rate limit check failed:', error)I can wire a lightweight logger interface and adapt usage with minimal footprint.
apps/web/middleware.ts (1)
182-187: Optional: accept common CSRF header aliases.Broaden header extraction to x-xsrf-token/csrf-token.
- // 1. Try to get from headers (for AJAX requests) - csrfToken = request.headers.get('x-csrf-token') + // 1. Try to get from headers (for AJAX requests) + csrfToken = + request.headers.get('x-csrf-token') || + request.headers.get('x-xsrf-token') || + request.headers.get('csrf-token')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.gitignore(1 hunks)apps/web/__mocks__/@t3-oss/env-nextjs.ts(1 hunks)apps/web/__mocks__/@thedaviddias/rate-limiting.ts(1 hunks)apps/web/__tests__/lib/security-rate-limiting.test.ts(1 hunks)apps/web/actions/submit-llms-xxt.ts(3 hunks)apps/web/app/api/members/route.ts(1 hunks)apps/web/components/forms/submit-form-step2-content-fields.tsx(1 hunks)apps/web/components/forms/submit-form.tsx(2 hunks)apps/web/jest.config.ts(1 hunks)apps/web/lib/__tests__/security-utils.test.ts(2 hunks)apps/web/lib/env-config.ts(1 hunks)apps/web/lib/security-utils-helpers.ts(2 hunks)apps/web/lib/security-utils.ts(1 hunks)apps/web/middleware.ts(3 hunks)packages/api-utils/index.ts(1 hunks)packages/api-utils/package.json(1 hunks)packages/hooks/src/use-form-state.tsx(1 hunks)packages/rate-limiting/index.ts(1 hunks)packages/rate-limiting/keys.ts(1 hunks)packages/security/validation.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- apps/web/lib/security-utils-helpers.ts
- apps/web/mocks/@thedaviddias/rate-limiting.ts
- packages/rate-limiting/keys.ts
- apps/web/lib/env-config.ts
- apps/web/mocks/@t3-oss/env-nextjs.ts
- apps/web/components/forms/submit-form-step2-content-fields.tsx
- apps/web/actions/submit-llms-xxt.ts
- apps/web/app/api/members/route.ts
- apps/web/components/forms/submit-form.tsx
- .gitignore
- apps/web/tests/lib/security-rate-limiting.test.ts
- apps/web/lib/tests/security-utils.test.ts
- packages/hooks/src/use-form-state.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/functions.mdc)
**/*.{ts,tsx}: For functions with more than one parameter, use a single object parameter instead of multiple positional parameters
Name the input type for such object parameters using the pattern {FunctionName}Input
For single-parameter functions, direct positional arguments are acceptable
**/*.{ts,tsx}: Write concise, technical TypeScript code
Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Use TypeScript for all code; prefer interfaces over types
Avoid enums; use object maps instead
Use thefunctionkeyword for pure functions
Avoid unnecessary curly braces in simple conditionals; use concise syntax
**/*.{ts,tsx}: Use interfaces over types when possible in TypeScript
Avoid enums; prefer const objects (as const) instead
Use proper error handling with custom error types
Favor functional programming patterns
Avoid classes
Use pure functions where possible
Implement proper error handling
Use meaningful variable names
Keep functions small and focused
Files:
apps/web/middleware.tspackages/security/validation.tsapps/web/lib/security-utils.tsapps/web/jest.config.tspackages/rate-limiting/index.tspackages/api-utils/index.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/jsdoc-comments.mdc)
Add JSDoc blocks for utility functions, documenting purpose, @param entries, @returns, and @throws where applicable
Files:
apps/web/middleware.tspackages/security/validation.tsapps/web/lib/security-utils.tsapps/web/jest.config.tspackages/rate-limiting/index.tspackages/api-utils/index.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
apps/web/**/*.{ts,tsx}: Use Shadcn UI components with Radix UI primitives sourced from packages/design-system
Optimize for Core Web Vitals (images, fonts, script strategy, code-splitting)
Use camelCase for function and variable names
Use descriptive names with auxiliary verbs (e.g., isLoading, hasError)
Files:
apps/web/middleware.tsapps/web/lib/security-utils.tsapps/web/jest.config.ts
apps/web/**
📄 CodeRabbit inference engine (.cursor/rules/website.mdc)
Use kebab-case for directories
Files:
apps/web/middleware.tsapps/web/lib/security-utils.tsapps/web/jest.config.ts
**/index.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/shared.mdc)
Do not use barrel files (avoid index files that only re-export)
Files:
packages/rate-limiting/index.tspackages/api-utils/index.ts
🧠 Learnings (26)
📚 Learning: 2025-08-26T11:21:06.811Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/website.mdc:0-0
Timestamp: 2025-08-26T11:21:06.811Z
Learning: Applies to apps/web/app/**/*.{ts,tsx} : Implement type-safe Server Actions using next-safe-action
Applied to files:
apps/web/middleware.ts
📚 Learning: 2025-08-26T11:21:03.221Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/website.mdc:0-0
Timestamp: 2025-08-26T11:21:03.221Z
Learning: Applies to apps/web/app/**/*.{ts,tsx,js,jsx} : Implement type-safe server actions using next-safe-action
Applied to files:
apps/web/middleware.ts
📚 Learning: 2025-08-26T11:20:10.612Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:10.612Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Mock modules and external dependencies using jest.mock and mock global.fetch for API calls as needed
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:20.240Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:20.240Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock modules with jest.mock and stub external APIs (e.g., global.fetch) as needed in tests
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:49.726Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:49.726Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and APIs with jest.mock and mock fetch calls as needed
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:18.780Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:18.780Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and API calls with jest.mock and stub globals like fetch when needed
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:01.309Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:01.309Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and APIs (e.g., using jest.mock and mocking global.fetch) in tests
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:24.941Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:24.941Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Mock modules with jest.mock and mock fetch (e.g., global.fetch as jest.Mock) for external dependencies
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:15.656Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:15.656Z
Learning: Applies to {*.test.ts,*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Mock modules and external calls (e.g., jest.mock, global.fetch) as needed to isolate tests
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:58.532Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:58.532Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}} : Mock external modules and APIs in tests using jest.mock and replace global.fetch with a jest.fn where needed
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:20.905Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:20.905Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and APIs with jest.mock and stub network calls (e.g., global.fetch) in tests
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:28.226Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:28.226Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Mock external modules and APIs using jest.mock and mock global.fetch as needed
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:49.726Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:49.726Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : For Next.js API routes, use node-mocks-http createMocks to create req/res objects in tests
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:01.309Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:01.309Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Test Next.js API routes using node-mocks-http createMocks to simulate req/res objects
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:28.226Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:28.226Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Test Next.js API routes using node-mocks-http createMocks to construct req/res objects
Applied to files:
apps/web/jest.config.ts
📚 Learning: 2025-08-26T11:20:20.905Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:20.905Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : For Next.js API routes, use node-mocks-http createMocks to construct req/res and assert status codes
Applied to files:
apps/web/jest.config.tspackages/rate-limiting/index.ts
📚 Learning: 2025-08-26T11:20:58.532Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:58.532Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx},*.test.{ts,tsx}} : Always test both success and error paths for features and API calls
Applied to files:
packages/rate-limiting/index.ts
📚 Learning: 2025-08-26T11:20:18.780Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:18.780Z
Learning: Applies to {**/*.test.ts,**/__tests__/*.ts} : For Next.js API routes, use node-mocks-http createMocks to simulate req/res and assert handler behavior
Applied to files:
packages/rate-limiting/index.ts
📚 Learning: 2025-08-26T11:20:28.226Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:28.226Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Always test both success and error paths for features and side effects
Applied to files:
packages/rate-limiting/index.ts
📚 Learning: 2025-08-26T11:20:24.941Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:24.941Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : For Next.js API routes, use node-mocks-http createMocks to construct req/res and assert on the response
Applied to files:
packages/rate-limiting/index.ts
📚 Learning: 2025-08-26T11:20:27.126Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:27.126Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Always test both success and error cases for features and API interactions
Applied to files:
packages/rate-limiting/index.ts
📚 Learning: 2025-08-26T11:20:21.081Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:21.081Z
Learning: Applies to {**/*.test.ts,**/*.test.tsx,**/__tests__/*.ts,**/__tests__/*.tsx} : Always test both success and error paths, including edge cases
Applied to files:
packages/rate-limiting/index.ts
📚 Learning: 2025-08-26T11:20:20.905Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:20.905Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Always test both success and error paths, including edge cases
Applied to files:
packages/rate-limiting/index.ts
📚 Learning: 2025-08-26T11:20:18.780Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:18.780Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Always test both success and error paths in your tests
Applied to files:
packages/rate-limiting/index.ts
📚 Learning: 2025-08-26T11:20:01.309Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:20:01.309Z
Learning: Applies to {**/*.test.{ts,tsx},**/__tests__/*.{ts,tsx}} : Keep tests focused and isolated; mock external dependencies to avoid flakiness
Applied to files:
packages/rate-limiting/index.ts
📚 Learning: 2025-08-26T11:18:21.095Z
Learnt from: CR
PR: thedaviddias/llms-txt-hub#0
File: .cursor/rules/functions.mdc:0-0
Timestamp: 2025-08-26T11:18:21.095Z
Learning: Applies to **/*.{ts,tsx} : For functions with more than one parameter, use a single object parameter instead of multiple positional parameters
Applied to files:
packages/api-utils/index.ts
🧬 Code graph analysis (4)
apps/web/middleware.ts (2)
packages/logging/index.ts (2)
error(106-122)logger(168-168)apps/web/lib/server-crypto.ts (1)
hashSensitiveData(8-10)
packages/security/validation.ts (3)
packages/security/types.ts (2)
URLValidationOptions(4-10)ValidationResult(15-19)packages/security/constants.ts (1)
RESERVED_USERNAMES(9-30)packages/security/sanitization.ts (1)
sanitizeText(10-34)
packages/rate-limiting/index.ts (3)
packages/rate-limiting/keys.ts (1)
keys(4-14)packages/caching/keys.ts (1)
keys(4-14)apps/web/__mocks__/@thedaviddias/rate-limiting.ts (2)
checkRateLimit(8-12)withRateLimit(3-6)
packages/api-utils/index.ts (1)
packages/logging/index.ts (2)
error(106-122)logger(168-168)
🪛 GitHub Actions: PR Review
apps/web/middleware.ts
[error] Jest test suite failed for apps/web. Step 'jest' exited with code 1.
apps/web/lib/security-utils.ts
[error] Jest test suite failed for apps/web. Step 'jest' exited with code 1.
apps/web/jest.config.ts
[error] Jest test suite failed for apps/web. Step 'jest' exited with code 1.
🔇 Additional comments (11)
apps/web/jest.config.ts (2)
74-76: Good: explicit mocks mapped before the generic workspace resolverPlacing the two moduleNameMapper entries ahead of the broad ^@thedaviddias/... rules ensures the mocks win resolution during tests.
101-103: Allow Jest to transpile Upstash ESM packages
- In apps/web/jest.config.ts (around line 101), add
@upstashto thetransformIgnorePatternsallowlist so Babel/SWC will transpile ESM code under@upstash/*.packages/api-utils/package.json (1)
14-23: Packaging looks good; Next moved to peerDependencyThanks for making Next a peerDependency and keeping it as dev for local builds.
apps/web/lib/security-utils.ts (1)
9-9: Re-export is fine; keep names stableMaintains backwards compatibility while refactor lands across the app.
packages/security/validation.ts (2)
9-13: Anchored protocol check looks good.Anchoring the protocol regex at start prevents false positives in query/fragment.
18-33: No local call sites to update This package declares but doesn’t callvalidateURL,validateUsername,validateEmail,validatePhoneNumber, orvalidateOrigin; remove this follow-up here and apply object-parameter updates in downstream consumers instead.Likely an incorrect or invalid review comment.
packages/api-utils/index.ts (2)
85-88: Good: return native Response directly.Avoids unsafe Response → NextResponse wrapping.
70-81: Remove follow-up on updating call sites — no external invocations found.Likely an incorrect or invalid review comment.
apps/web/middleware.ts (3)
12-12: LGTM: expose /api/csrf as public.Consistent with token fetch endpoints.
414-431: LGTM: detailed CSRF failure logging with hashing and tags.Good structure and redaction.
12-12: Remove pipeline hint regarding logger.warn
The Logger class in packages/logging/index.ts defines a working warn() method (lines 98–104), so no replacements or additions are needed.Likely an incorrect or invalid review comment.
- Add missing NextResponse import in members route - Restore checkRateLimit function to security-utils-helpers for backward compatibility - Apply automatic lint formatting fixes across modified files - Ensure all tests and builds pass successfully
thedaviddias
force-pushed
the
feature/refactor-dry-code
branch
from
50c717c to
3de7cb9
Compare
1 participant
Major refactoring to improve code quality and maintainability:
Consolidated Duplicate Code
New Reusable Packages Created
Code Quality Improvements
Impact
Summary by CodeRabbit