refactor(website): Improve server architecture with domain-driven design#803
refactor(website): Improve server architecture with domain-driven design#803
Conversation
This commit restructures the website server code to follow a domain-driven design pattern, improving maintainability and separation of concerns. Changes: - Create src/actions/ directory for API action handlers - Create src/domains/pack/ directory for pack-related business logic - Extract pack endpoint logic from index.ts into packAction.ts - Move processZipFile.ts and remoteRepo.ts to domains/pack/ - Update all import paths to reflect new directory structure - Simplify index.ts to focus on server configuration and routing The new structure separates API routing (index.ts), action handlers (actions/), and domain logic (domains/pack/) for better code organization.
This commit reorganizes the utilities directory by moving pack-specific utilities into the pack domain structure, following domain-driven design principles. Changes: - Move pack-specific utilities to domains/pack/utils/: - cache.ts: PackResult-specific caching functionality - fileUtils.ts: ZIP processing and pack-related file operations - validation.ts: Pack request validation logic - sharedInstance.ts: Pack domain cache and rate limiter instances - Keep generic utilities in utils/: - errorHandler.ts: Application-wide error handling - logger.ts: Cross-domain logging functionality - memory.ts: Cross-domain memory monitoring - network.ts: Cross-domain network utilities - processConcurrency.ts: System-wide CPU information - rateLimit.ts: Base rate limiting functionality - time.ts: Cross-domain time processing - Update all import paths to reflect new structure - Maintain separation of concerns between domain-specific and generic utilities This improves code organization by placing domain-specific logic within the appropriate domain boundaries while keeping truly generic utilities accessible across all domains.
This commit creates a dedicated middlewares directory and extracts Hono middleware configurations from the main server file for better organization and maintainability. Changes: - Create middlewares/ directory for middleware organization - Extract cloudLogger middleware to middlewares/logger.ts: - Move custom logging middleware with request tracking - Generate unique request IDs and track request lifecycle - Include memory usage and latency monitoring - Fix type errors with proper function signatures - Extract CORS configuration to middlewares/cors.ts: - Define allowed origins and headers - Handle development and production environments - Extract body limit middleware to middlewares/bodyLimit.ts: - Configure request size limits - Handle file size exceeded errors - Clean up utils/logger.ts: - Remove middleware-specific code - Export logger instance for reuse - Focus on pure logging utilities - Simplify index.ts: - Import middleware from dedicated modules - Remove inline middleware configurations - Improve code readability and maintainability This separation follows middleware best practices and makes the codebase more modular while maintaining all existing functionality.
… validation Major improvements to the website server architecture following domain-driven design principles: ## Validation & Request Handling - Move request validation from domain functions to packAction (action layer responsibility) - Consolidate request schemas from separate files into packAction.ts for better colocation - Create reusable utils/validation.ts for generic validation functionality - Remove redundant validation.ts from actions directory ## Rate Limiting & Client Information - Move rate limiting checks from domain functions to action layer - Create utils/clientInfo.ts utility for centralized client information extraction - Replace individual IP extraction with structured ClientInfo interface including IP, userAgent, and referer - Update cloudLogger middleware to use unified clientInfo utility ## Code Organization - Consolidate FILE_SIZE_LIMITS constants into domains/pack/utils/fileUtils.ts - Remove unused constants.ts and schemas/request.ts files - Rename middlewares/logger.ts to cloudLogger.ts with cloudLoggerMiddleware function for clarity - Clean up domain function signatures by removing clientIp parameters ## Architecture Benefits - Clear separation of concerns between action, domain, and utility layers - Centralized validation and rate limiting at appropriate architectural boundaries - Improved code reusability and maintainability - Better error handling and client information management This refactoring establishes a solid foundation for future feature development while maintaining backward compatibility.
…lidation - Inline schema definitions for better readability and reduced complexity - Remove intermediate variables (packOptionsSchema, fileSchema, isValidZipFile, ignorePatternRegex) - Eliminate redundant manual validation checks already covered by Zod schema: - Remove manual format enum validation (handled by z.enum) - Remove manual file/url existence check (handled by schema refinements) - Consolidate validation logic entirely within the Zod schema for consistency This simplification follows the DRY principle and ensures all validation is handled uniformly through the schema-first approach.
- Create dedicated rateLimitMiddleware for better separation of concerns - Remove rate limiting logic from packAction to focus on business logic - Apply rate limiting at middleware layer for early request blocking - Simplify packAction by removing rate limiting imports and checks - Improve middleware ordering in index.ts for optimal request processing This architectural improvement follows the middleware pattern and provides: - Better security through early rate limiting - Cleaner action layer focused on business logic - Consistent middleware-based approach for cross-cutting concerns - Improved reusability for future API endpoints
Deploying repomix with
|
| Latest commit: |
b492ebc
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bee6854c.repomix.pages.dev |
| Branch Preview URL: | https://feat-website-improve.repomix.pages.dev |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds a modular /api/pack HTTP action with middleware (CORS, body-limit, rate-limit, cloud logger). Refactors pack processing for ZIP uploads and remote repos to rely on direct options, new sanitization utility, and caching. Moves size constants to pack utils, introduces client info utility, and removes legacy schema/validation and network helpers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant App as Hono App
participant CORS as CORS MW
participant BL as BodyLimit MW
participant RL as RateLimit MW
participant CL as CloudLogger MW
participant Action as packAction
participant Zip as processZipFile
participant Repo as processRemoteRepo
participant Cache as RequestCache
Client->>App: POST /api/pack (form-data)
App->>CORS: apply
CORS->>BL: next
BL->>RL: next
RL->>CL: next
CL->>Action: next (requestId set)
rect rgb(245,245,255)
note right of Action: Parse form-data, sanitize patterns, build options
Action->>Cache: (via Zip/Repo) lookup by key
end
alt has ZIP file
Action->>Zip: processZipFile(file, format, options)
Zip-->>Action: PackResult
else has repo URL
Action->>Repo: processRemoteRepo(url, format, options)
Repo-->>Action: PackResult
else
Action-->>Client: 400 error (validation)
end
Action-->>Client: JSON PackResult
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @yamadashy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a significant refactoring of the website server's architecture, moving towards a more robust and scalable design based on Domain-Driven Design (DDD) principles. The primary goal is to improve the separation of concerns by creating distinct layers for actions, middleware, and domain-specific logic. This refactoring centralizes validation and rate limiting, streamlines utility functions, and simplifies the main server entry point, laying a solid foundation for future development while enhancing maintainability, security, and performance.
Highlights
- Domain-Driven Architecture Implementation: The server codebase has been restructured to follow Domain-Driven Design (DDD) principles. This includes creating a dedicated
domains/pack/directory to encapsulate all business logic related to the 'pack' functionality, moving relevant utilities (cache, fileUtils, sharedInstance) into this domain, and consolidating constants within domain-specific locations. - Action Layer Refinement: The core logic for the '/api/pack' endpoint has been extracted from
index.tsinto a new dedicated action file,actions/packAction.ts. Request validation is now centralized at this action layer using Zod schemas, with simplified schema definitions and direct integration for better colocation. - Modular Middleware Architecture: A new
middlewares/directory has been introduced to house modular middleware components. This includes dedicated middleware for body size limiting (bodyLimit.ts), structured cloud logging (cloudLogger.ts), CORS configuration (cors.ts), and rate limiting (rateLimit.ts). This improves modularity and ensures proper middleware ordering. - Utility and Infrastructure Streamlining: New utilities like
clientInfo.tshave been created for unified client information extraction. Redundant files such asconstants.ts,schemas/request.ts, andnetwork.tshave been removed, with their functionalities either absorbed into new structures or relocated. Imports across the codebase have been updated to reflect these changes. - Enhanced Maintainability, Security, and Performance: These architectural changes lead to a clearer separation of concerns, making the codebase easier to understand and navigate. Utilities and middleware are now more reusable, enhancing overall code maintainability. Security is improved through centralized validation and rate limiting at appropriate layers, and performance benefits from early request blocking and an optimized middleware chain.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #803 +/- ##
=======================================
Coverage 87.41% 87.41%
=======================================
Files 113 113
Lines 6493 6493
Branches 1331 1331
=======================================
Hits 5676 5676
Misses 817 817 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This is an excellent pull request that significantly improves the server architecture by implementing domain-driven design principles. The separation of concerns is much clearer with the introduction of actions, domain logic, and middleware layers. The code is now more modular, maintainable, and easier to follow. I have a few suggestions to further improve code quality and maintainability.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (3)
website/server/src/domains/pack/processZipFile.ts (2)
190-197: Fix directory traversal check: startsWith(destPath) is unsafe; use path.resolve + path.relativeUsing startsWith() on normalized paths can yield false positives (e.g., "/tmp/repomix" vs "/tmp/repomix-evil"). Switch to resolve/relative, consistent with fileUtils.extractZip().
Apply:
- const normalizedPath = path.normalize(path.join(destPath, entryPath)); - if (!normalizedPath.startsWith(destPath)) { + const fullPath = path.resolve(destPath, entryPath); + const relativePath = path.relative(destPath, fullPath); + if (relativePath.startsWith('..') || path.isAbsolute(relativePath)) { throw new AppError( `Security violation: Potential directory traversal attack detected in path: ${entryPath}`, 400, ); }
71-72: Deduplicate ZIP extraction logic; reuse extractZip() from fileUtilsThis file reimplements ZIP bomb/path traversal checks already present in domains/pack/utils/fileUtils.ts. Duplicating the logic risks drift and future security gaps. Call extractZip() and remove extractZipWithSecurity().
Proposed change:
-import { unzip } from 'fflate'; +import { unzip } from 'fflate'; // (can be removed if extractZipWithSecurity is deleted) +import { extractZip } from './utils/fileUtils.js' @@ - // Extract the ZIP file to the temporary directory with enhanced security checks - await extractZipWithSecurity(file, tempDirPath); + // Extract the ZIP file to the temporary directory with enhanced security checks + await extractZip(file, tempDirPath); @@ -/** - * Enhanced ZIP extraction with security checks using fflate - */ -async function extractZipWithSecurity(file: File, destPath: string): Promise<void> { - ... -} +// (No longer needed: extraction handled by extractZip in fileUtils)I can push a follow-up commit that removes the local function and dead imports.
Also applies to: 137-244
website/server/src/domains/pack/utils/fileUtils.ts (1)
147-153: Tighten temp directory validation to prevent accidental deletion outside os.tmpdir()Using includes('repomix-') is weak and could match unintended paths. Ensure the directory lives under os.tmpdir() and its basename starts with the expected prefix.
Apply:
export const cleanupTempDirectory = async (directory: string): Promise<void> => { try { - if (!directory.includes('repomix-')) { - throw new AppError('Invalid temporary directory path'); - } + const tmpRoot = path.resolve(os.tmpdir()); + const resolved = path.resolve(directory); + const rel = path.relative(tmpRoot, resolved); + const base = path.basename(resolved); + if (rel.startsWith('..') || path.isAbsolute(rel) || !base.startsWith('repomix-')) { + throw new AppError('Invalid temporary directory path'); + } await fs.rm(directory, { recursive: true, force: true }); } catch (error) { if (error instanceof AppError) { throw error; } console.error(`Failed to cleanup temporary directory: ${directory}`, error); } };
🧹 Nitpick comments (26)
website/server/src/domains/pack/utils/cache.ts (2)
17-22: Interval can keep the process alive; store and unref/clear it
setIntervalinside the constructor may prevent the process from exiting and can’t be cleared externally. Store the timer, call.unref()in Node, and expose adispose()to clear it when shutting down (e.g., during tests or server reloads).Apply within constructor and class fields:
export class RequestCache<T> { - private cache: Map<string, CacheEntry> = new Map(); + private cache: Map<string, CacheEntry> = new Map(); + private cleanupTimer?: ReturnType<typeof setInterval>; private readonly ttl: number; constructor(ttlInSeconds = 60) { this.ttl = ttlInSeconds * 1000; // Set up periodic cache cleanup - setInterval(() => this.cleanup(), ttlInSeconds * 1000); + this.cleanupTimer = setInterval(() => this.cleanup(), ttlInSeconds * 1000); + // Allow the event loop to exit even if the timer is pending (Node.js only) + (this.cleanupTimer as any).unref?.(); }And add a disposer outside this range:
// Call when tearing down the app or tests dispose(): void { if (this.cleanupTimer) { clearInterval(this.cleanupTimer); this.cleanupTimer = undefined; } }
74-86: Cache key should be stable and compact; avoid raw JSON of optionsUsing
JSON.stringify(options)in the key can be non-deterministic across equivalent objects (key order), produce very long keys, and may accidentally encode sensitive values. Prefer a stable, hashed key.Example minimal change (introduce a hash and keep inputs readable):
+import { createHash } from 'node:crypto'; ... export function generateCacheKey( identifier: string, format: string, options: PackOptions, type: 'url' | 'file', ): string { - return JSON.stringify({ - identifier, - format, - options, - type, - }); + const payload = { identifier, format, options, type }; + const json = JSON.stringify(payload, Object.keys(payload).sort()); + const hash = createHash('sha256').update(json).digest('hex'); + return `v1:${hash}`; }If you need full stability, consider a proper stable-stringify for
options(sorted recursively).website/server/src/middlewares/cors.ts (1)
11-13: Origin suffix check can fail with ports; parse URL hostname
origin.endsWith('.repomix.pages.dev')will fail forhttps://foo.repomix.pages.dev:443. Parse and check the hostname instead. Also, hoistallowedOriginsto module scope to avoid per-request allocation.-import { cors } from 'hono/cors'; +import { cors } from 'hono/cors'; -export const corsMiddleware = cors({ - origin: (origin) => { - const allowedOrigins = ['http://localhost:5173', 'https://repomix.com', 'https://api.repomix.com']; +const allowedOrigins = ['http://localhost:5173', 'https://repomix.com', 'https://api.repomix.com'] as const; + +export const corsMiddleware = cors({ + origin: (origin) => { if (!origin || allowedOrigins.includes(origin)) { return origin; } - if (origin.endsWith('.repomix.pages.dev')) { + try { + const { hostname } = new URL(origin); + if (hostname.endsWith('.repomix.pages.dev')) { + return origin; + } + } catch { + // Invalid origin; fall through and reject + } return origin; - } - - return null; + return null; }, allowMethods: ['GET', 'POST', 'OPTIONS'], allowHeaders: ['Content-Type'], maxAge: 86400, credentials: true, });website/server/src/utils/clientInfo.ts (1)
9-25: Trust model for client IPs: prefer trusted proxy headers and basic validationCurrent precedence trusts
x-forwarded-forfirst, which is client-controlled unless sanitized by a trusted proxy/CDN. In many setups (e.g., Cloudflare),cf-connecting-ipis the most trustworthy. Consider:
- Prefer
cf-connecting-ipwhen present.- Optionally gate
x-forwarded-forusage behind aTRUST_PROXY=trueenv.- Minimal validation to ensure the first token is a plausible IPv4/IPv6.
Example tweak (order and basic guard):
- const ip = - c.req.header('x-forwarded-for')?.split(',')[0]?.trim() || - c.req.header('x-real-ip') || - c.req.header('cf-connecting-ip') || - '0.0.0.0'; + const xf = c.req.header('x-forwarded-for')?.split(',')[0]?.trim(); + const cf = c.req.header('cf-connecting-ip'); + const xr = c.req.header('x-real-ip'); + const pick = cf || xr || (process.env.TRUST_PROXY === 'true' ? xf : undefined); + const ip = pick && pick.length <= 64 ? pick : '0.0.0.0';If you’d like, I can add a tiny
isValidIp()helper to keep this tidy.website/server/src/middlewares/rateLimit.ts (2)
4-4: Remove unused import
AppErroris not used.-import { AppError } from '../utils/errorHandler.js';
10-16: Return retry hints with 429Add
Retry-Afterto help clients back off gracefully and optionally expose remaining seconds.- if (!rateLimiter.isAllowed(clientInfo.ip)) { - const remainingTime = Math.ceil(rateLimiter.getRemainingTime(clientInfo.ip) / 1000); - return c.json({ - error: `Rate limit exceeded. Please try again in ${remainingTime} seconds.`, - }, 429); - } + if (!rateLimiter.isAllowed(clientInfo.ip)) { + const remainingTime = Math.ceil(rateLimiter.getRemainingTime(clientInfo.ip) / 1000); + return c.json( + { error: `Rate limit exceeded. Please try again in ${remainingTime} seconds.` }, + 429, + { + 'Retry-After': String(remainingTime), + 'X-RateLimit-Remaining-Seconds': String(remainingTime), + }, + ); + }website/server/src/utils/logger.ts (1)
6-23: Optional: export factory for testing/customizationIf you ever need to inject a test logger or add default meta (service/version), consider exporting
createLoggeras a named export.-function createLogger() { +export function createLogger() { const transports: winston.transport[] = [ new winston.transports.Console({ format: winston.format.combine(winston.format.timestamp(), winston.format.json()), }), ];website/server/src/domains/pack/processZipFile.ts (3)
74-80: Avoid writing artifacts to process.cwd(); read/delete directly from temp dirCopying the output file into CWD increases IO and broadens the blast radius if something goes wrong. Read from the temp directory and let cleanupTempDirectory remove it.
Apply:
- const outputFilePath = `repomix-output-${randomUUID()}.txt`; + const outputFileName = `repomix-output-${randomUUID()}.txt`; @@ - output: outputFilePath, + output: outputFileName, @@ - const result = await runDefaultAction([tempDirPath], tempDirPath, cliOptions); - await copyOutputToCurrentDirectory(tempDirPath, process.cwd(), result.config.output.filePath); + const result = await runDefaultAction([tempDirPath], tempDirPath, cliOptions); + const outputFullPath = path.join(tempDirPath, path.basename(result.config.output.filePath)); @@ - const content = await fs.readFile(outputFilePath, 'utf-8'); + const content = await fs.readFile(outputFullPath, 'utf-8'); @@ - // Clean up the output file - try { - await fs.unlink(outputFilePath); - } catch (err) { - // Ignore file deletion errors - console.warn('Failed to cleanup output file:', err); - } + // No explicit unlink needed; temp dir cleanup removes the fileAlso applies to: 125-131, 38-43
30-36: Cache key includes user-provided filename; consider hashing to reduce PII leakageIncluding file.name in the cache key can leak user information if keys are logged/observable. Use a hash of name/size/mtime instead, or omit the name entirely.
Apply:
- const cacheKey = generateCacheKey(`${file.name}-${file.size}-${file.lastModified}`, format, options, 'file'); + const identifier = crypto.createHash('sha256') + .update(`${file.name}|${file.size}|${file.lastModified}`) + .digest('hex'); + const cacheKey = generateCacheKey(identifier, format, options, 'file');Note: add
import crypto from 'node:crypto';or reuse randomUUID with a map if hashing feels heavy.
117-121: Use centralized logger instead of console.error/warnFor consistency with cloud logging and request correlation, route errors through the shared logger and include the requestId if available.
Also applies to: 128-130
website/server/src/domains/pack/utils/sharedInstance.ts (1)
6-8: Make rate limits configurable via environment variablesHardcoding 10/3 rpm is restrictive in production and too generous for some deployments. Parameterize via env (e.g., PACK_RATE_LIMIT_PER_MIN, PACK_RATE_LIMIT_WINDOW_MS) with sane defaults.
Apply:
-export const rateLimiter = new RateLimiter(60_000, process.env.NODE_ENV === 'development' ? 10 : 3); +const windowMs = Number(process.env.PACK_RATE_LIMIT_WINDOW_MS ?? 60_000); +const maxReq = Number(process.env.PACK_RATE_LIMIT_PER_MIN ?? (process.env.NODE_ENV === 'development' ? 10 : 3)); +export const rateLimiter = new RateLimiter(windowMs, maxReq);website/server/src/middlewares/bodyLimit.ts (1)
5-12: Harden onError: type-safe requestId and include allowed size in messagec.get returns unknown; pass a typed, nullable value and include a human-readable limit in the error.
Apply:
-import { FILE_SIZE_LIMITS } from '../domains/pack/utils/fileUtils.js'; +import { FILE_SIZE_LIMITS, formatFileSize } from '../domains/pack/utils/fileUtils.js'; @@ - onError: (c) => { - const requestId = c.get('requestId'); - const response = createErrorResponse('File size too large', requestId); + onError: (c) => { + const requestId = c.get<string>('requestId') ?? ''; + const response = createErrorResponse( + `Request body too large. Max allowed: ${formatFileSize(FILE_SIZE_LIMITS.MAX_REQUEST_SIZE)}`, + requestId + ); return c.json(response, 413); },website/server/src/domains/pack/utils/fileUtils.ts (2)
16-18: Normalize file size formattingformatFileSize currently returns many decimals. Round to two decimals for cleaner messages.
Apply:
-export const formatFileSize = (bytes: number): string => { - return `${bytes / 1024 / 1024}MB`; -}; +export const formatFileSize = (bytes: number): string => `${(bytes / 1024 / 1024).toFixed(2)}MB`;
20-27: Avoid ZIP limit duplication across filesZIP_SECURITY_LIMITS are defined here and in processZipFile.ts. Centralize in this module (exported) and reuse everywhere to prevent drift.
website/server/src/index.ts (4)
34-34: 30s timeout is likely too low for /api/pack; scope it to the route and raise to avoid premature timeouts.Remote repo packing and ZIP extraction can exceed 30s under load. Recommend applying a larger timeout only to /api/pack to avoid affecting other API routes.
Apply this diff:
-// Set timeout for API routes -app.use('/api', timeout(30000)); +// Set a more generous timeout only for the pack route +app.use('/api/pack', timeout(120000));If you’ve observed 504s/aborts in logs, consider 180–300s for worst-case large repos.
28-28: Expose X-Request-Id to browsers.You’re emitting requestId across logs; letting the client read it helps with support/debugging. Add CORS exposeHeaders for X-Request-Id.
Outside this file, update website/server/src/middlewares/cors.ts:
export const corsMiddleware = cors({ origin: (origin) => { @@ allowMethods: ['GET', 'POST', 'OPTIONS'], allowHeaders: ['Content-Type'], + exposeHeaders: ['X-Request-Id'], maxAge: 86400, credentials: true, });
36-38: Consider registering the logger first to capture pre-timeout/early middleware failures.Current order is fine, but placing cloudLogger before timeout gives you start/end/error logs even if the timeout middleware short-circuits.
40-40: Unify rate-limit error shape and communicate Retry-After.The rate limiter returns a plain { error } payload. For consistency with other failures (body limit, pack errors), return createErrorResponse() with requestId and set Retry-After.
Outside this file, update website/server/src/middlewares/rateLimit.ts:
export function rateLimitMiddleware() { return async function rateLimitMiddleware(c: Context, next: Next) { const clientInfo = getClientInfo(c); // Check rate limit if (!rateLimiter.isAllowed(clientInfo.ip)) { const remainingTime = Math.ceil(rateLimiter.getRemainingTime(clientInfo.ip) / 1000); - return c.json({ - error: `Rate limit exceeded. Please try again in ${remainingTime} seconds.`, - }, 429); + const requestId = c.get('requestId'); + c.header('Retry-After', remainingTime.toString()); + return c.json( + createErrorResponse(`Rate limit exceeded. Please try again in ${remainingTime} seconds.`, requestId), + 429 + ); } await next(); }; }Make sure to import createErrorResponse at the top.
website/server/src/middlewares/cloudLogger.ts (3)
15-17: Avoid deprecated String.prototype.substr; use slice.substr is deprecated; slice is clearer and standard.
-function generateRequestId(): string { - return `req-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; -} +function generateRequestId(): string { + return `req-${Date.now()}-${Math.random().toString(36).slice(2, 11)}`; +}
46-56: Surface X-Request-Id on all successful responses.Add the header after next() so it’s present on the final Response.
try { // Execute the request await next(); + // Ensure request id is observable to clients + c.header('X-Request-Id', requestId); + // Calculate response time and memory usage const responseTime = calculateLatency(startTime); const memoryUsage = getMemoryUsage(); // Log successful request completion logger.info({Also applies to: 58-69
70-98: Also set X-Request-Id for error responses.Setting the header before rethrowing increases the chance the app/global error handler keeps it.
} catch (error) { + // Attach request id for error responses, too + c.header('X-Request-Id', requestId); // Calculate response time even for errors const responseTime = calculateLatency(startTime); const memoryUsage = getMemoryUsage(); @@ // Re-throw the error to be handled by Hono's error handler throw error; }Note: If you later add app.onError in index.ts to standardize error JSON, it can also set X-Request-Id.
website/server/src/domains/pack/remoteRepo.ts (3)
10-12: Type the format parameter narrowly to prevent invalid styles.We already validate format upstream; narrow the type here for safety and self-documentation.
-export async function processRemoteRepo(repoUrl: string, format: string, options: PackOptions): Promise<PackResult> { +export async function processRemoteRepo( + repoUrl: string, + format: 'xml' | 'markdown' | 'plain', + options: PackOptions +): Promise<PackResult> {
104-118: Use structured logger instead of console for consistency.Replace console.error/warn with logError/logWarning to keep logs uniform and parseable.
- } catch (error) { - console.error('Error in remote action:', error); + } catch (error) { + logError('Error in remote action', error instanceof Error ? error : new Error('Unknown error'), { + repository: repoUrl, + }); @@ - } catch (err) { - // Ignore file deletion errors - console.warn('Failed to cleanup output file:', err); + } catch (err) { + // Ignore file deletion errors + logError('Failed to cleanup output file', err instanceof Error ? err : new Error(String(err)), { + repository: repoUrl, + });Make sure to import logError from ../../utils/logger.js.
75-83: Inconsistent topFiles sorting metric between remoteRepo.ts and processZipFile.tsIn remoteRepo.ts (lines 75–83), you sort topFiles by tokenCount, whereas in processZipFile.ts (lines 93–100) you sort by charCount. This discrepancy can lead to unexpected ordering for clients. I recommend choosing a single metric—tokenCount is likely more indicative of relevance—and updating the other implementation to match.
• remoteRepo.ts (lines 75–83):
topFiles: Object.entries(packResult.fileCharCounts) .map(([path, charCount]) => ({ path, charCount: charCount as number, tokenCount: packResult.fileTokenCounts[path] || 0, })) .sort((a, b) => b.tokenCount - a.tokenCount) // sorts by tokenCount .slice(0, cliOptions.topFilesLen),• processZipFile.ts (lines 93–100):
topFiles: Object.entries(packResult.fileCharCounts) .map(([path, charCount]) => ({ path, charCount: charCount as number, tokenCount: packResult.fileTokenCounts[path] || 0, })) .sort((a, b) => b.charCount - a.charCount) // sorts by charCount .slice(0, cliOptions.topFilesLen),To align both, update processZipFile.ts as follows:
- .sort((a, b) => b.charCount - a.charCount) + .sort((a, b) => b.tokenCount - a.tokenCount)website/server/src/actions/packAction.ts (2)
1-12: Avoid magic numbers: reuse central file size limits.Align the 10MB limit with the shared FILE_SIZE_LIMITS to keep constraints in one place.
import { sanitizePattern } from '../domains/pack/utils/validation.js'; +import { FILE_SIZE_LIMITS } from '../domains/pack/utils/fileUtils.js';- .refine((file) => file.size <= 10 * 1024 * 1024, { + .refine((file) => file.size <= FILE_SIZE_LIMITS.MAX_ZIP_SIZE, { // 10MB limit message: 'File size must be less than 10MB', })
91-99: Don’t pass empty pattern strings downstream; omit them.Empty strings can change CLI semantics vs. undefined. Only include keys when non-empty after sanitization.
- const sanitizedIncludePatterns = sanitizePattern(validatedData.options.includePatterns); - const sanitizedIgnorePatterns = sanitizePattern(validatedData.options.ignorePatterns); + const sanitizedIncludePatterns = sanitizePattern(validatedData.options.includePatterns); + const sanitizedIgnorePatterns = sanitizePattern(validatedData.options.ignorePatterns); @@ - const sanitizedOptions = { - ...validatedData.options, - includePatterns: sanitizedIncludePatterns, - ignorePatterns: sanitizedIgnorePatterns, - }; + const sanitizedOptions = { + ...validatedData.options, + ...(sanitizedIncludePatterns ? { includePatterns: sanitizedIncludePatterns } : {}), + ...(sanitizedIgnorePatterns ? { ignorePatterns: sanitizedIgnorePatterns } : {}), + };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
website/server/src/actions/packAction.ts(1 hunks)website/server/src/constants.ts(0 hunks)website/server/src/domains/pack/processZipFile.ts(3 hunks)website/server/src/domains/pack/remoteRepo.ts(2 hunks)website/server/src/domains/pack/utils/cache.ts(1 hunks)website/server/src/domains/pack/utils/fileUtils.ts(1 hunks)website/server/src/domains/pack/utils/sharedInstance.ts(1 hunks)website/server/src/domains/pack/utils/validation.ts(1 hunks)website/server/src/index.ts(2 hunks)website/server/src/middlewares/bodyLimit.ts(1 hunks)website/server/src/middlewares/cloudLogger.ts(1 hunks)website/server/src/middlewares/cors.ts(1 hunks)website/server/src/middlewares/rateLimit.ts(1 hunks)website/server/src/schemas/request.ts(0 hunks)website/server/src/utils/clientInfo.ts(1 hunks)website/server/src/utils/logger.ts(1 hunks)website/server/src/utils/network.ts(0 hunks)website/server/src/utils/validation.ts(0 hunks)
💤 Files with no reviewable changes (4)
- website/server/src/utils/network.ts
- website/server/src/constants.ts
- website/server/src/utils/validation.ts
- website/server/src/schemas/request.ts
🧰 Additional context used
🧬 Code graph analysis (7)
website/server/src/domains/pack/processZipFile.ts (3)
website/server/src/utils/errorHandler.ts (1)
AppError(3-11)website/server/src/domains/pack/utils/cache.ts (1)
generateCacheKey(74-86)website/server/src/domains/pack/utils/sharedInstance.ts (1)
cache(6-6)
website/server/src/middlewares/bodyLimit.ts (2)
website/server/src/domains/pack/utils/fileUtils.ts (1)
FILE_SIZE_LIMITS(8-13)website/server/src/utils/logger.ts (1)
createErrorResponse(36-42)
website/server/src/actions/packAction.ts (7)
website/server/src/utils/clientInfo.ts (1)
getClientInfo(9-25)website/server/src/utils/validation.ts (1)
validateRequest(4-14)website/server/src/domains/pack/utils/validation.ts (1)
sanitizePattern(1-23)website/server/src/domains/pack/processZipFile.ts (1)
processZipFile(25-132)website/server/src/utils/logger.ts (3)
createErrorResponse(36-42)logInfo(52-57)logError(66-77)website/server/src/domains/pack/remoteRepo.ts (1)
processRemoteRepo(10-120)website/server/src/utils/errorHandler.ts (1)
handlePackError(13-40)
website/server/src/middlewares/cloudLogger.ts (4)
website/server/src/utils/clientInfo.ts (1)
getClientInfo(9-25)website/server/src/utils/time.ts (1)
calculateLatency(17-23)website/server/src/utils/memory.ts (2)
getMemoryUsage(21-39)formatMemoryUsage(44-46)src/shared/logger.ts (1)
error(34-38)
website/server/src/middlewares/rateLimit.ts (2)
website/server/src/utils/clientInfo.ts (1)
getClientInfo(9-25)website/server/src/domains/pack/utils/sharedInstance.ts (1)
rateLimiter(7-7)
website/server/src/domains/pack/remoteRepo.ts (3)
website/server/src/utils/errorHandler.ts (1)
AppError(3-11)website/server/src/domains/pack/utils/cache.ts (1)
generateCacheKey(74-86)website/server/src/domains/pack/utils/sharedInstance.ts (1)
cache(6-6)
website/server/src/index.ts (5)
website/server/src/middlewares/cors.ts (1)
corsMiddleware(3-21)website/server/src/middlewares/cloudLogger.ts (1)
cloudLoggerMiddleware(20-98)website/server/src/middlewares/rateLimit.ts (1)
rateLimitMiddleware(6-20)website/server/src/middlewares/bodyLimit.ts (1)
bodyLimitMiddleware(5-12)website/server/src/actions/packAction.ts (1)
packAction(69-151)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Build and run (macos-latest, 20.x)
- GitHub Check: Build and run (windows-latest, 22.x)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (13)
website/server/src/domains/pack/utils/cache.ts (1)
3-3: Import path update aligns with centralized types moduleThe new
PackOptionsimport path matches the PR’s move tosrc/types.js. No issues.website/server/src/middlewares/cors.ts (2)
3-21: Solid CORS middleware; whitelist and credentials look correctThe origin callback correctly restricts to specific domains, and
credentials: truepairs with echoing the exact Origin. Methods/headers andmaxAgeare reasonable defaults.
18-19: CORS allowed headers verifiedI’ve searched the entire
website/server/srcdirectory for any reads ofAuthorization,X-Requested-With,X-API-Key(and related custom headers) and found no matches. That means none of the existing endpoints rely on those headers, so leavingallowHeaders: ['Content-Type'],is sufficient and no changes are required.
If in the future you introduce endpoints that expect other client-sent headers (e.g.
Authorization,X-Requested-With), simply extend the array at that time.website/server/src/middlewares/rateLimit.ts (1)
6-20: LGTM — middleware shape and IP extraction integration look goodThe middleware cleanly composes with
getClientInfoand a shared limiter instance. Naming and control flow are clear.website/server/src/utils/logger.ts (1)
25-26: Centralizing a single logger instance is a good simplificationExporting
loggerdirectly from here decouples transport config from middleware. Nice cleanup.website/server/src/domains/pack/processZipFile.ts (1)
41-56: Verify repomix’s CliOptions fieldsI wasn’t able to locate a local
node_modules/repomixto confirm the exact shape ofCliOptions. Please validate that the option names you’re passing here (and on lines 53–54) still match repomix’s current API. In your project root (or inwebsite/serverif that’s where dependencies live), run something like:•
rg -nP 'interface\s+CliOptions|type\s+CliOptions' -C2 node_modules/repomix
•rg -nP 'parsableStyle|fileSummary|directoryStructure|outputShowLineNumbers|compress' -C2 node_modules/repomixEnsure fields like
parsableStyle,fileSummary,directoryStructure, etc., are still defined exactly as named in repomix’sCliOptions.website/server/src/domains/pack/utils/sharedInstance.ts (1)
1-2: LGTM on import path realignmentTypes and RateLimiter paths now reference centralized locations; this matches the modularization elsewhere.
website/server/src/domains/pack/utils/fileUtils.ts (1)
84-88: Nice use of path.resolve/relative and duplicate-path guardThe traversal prevention and duplicate detection are solid and more robust than startsWith checks. Good job.
Also applies to: 105-111
website/server/src/index.ts (2)
5-10: Modularization and middleware wiring look solid.Imports cleanly switch the app to action + middleware layers. Good separation of concerns.
46-46: Route-level body limit on the pack endpoint is exactly right.Keeps uploads bounded without constraining unrelated endpoints.
website/server/src/middlewares/cloudLogger.ts (1)
21-27: LGTM: requestId stored in context for per-request tracing.This enables correlating logs and responses across the stack.
website/server/src/actions/packAction.ts (2)
101-112: LGTM: clear branching between ZIP and URL flows with unified result type.Validation guards the one-of input; good handoff to domain functions.
114-139: Good operational logging payload.Including requestId, client info, and summary metrics will make troubleshooting and telemetry much easier.
- Add calculateMemoryDiff function to utils/memory.ts for memory usage comparison - Replace logMemoryUsage with detailed before/after memory tracking in packAction - Include memory diff calculation in Pack operation completed logs - Add input type (file/url) information to operation logs for better monitoring
- Use FILE_SIZE_LIMITS constant instead of hardcoded value in packAction.ts - Remove redundant URL validation check (Zod schema already validates) - Use createErrorResponse for consistent error handling in rateLimit.ts - Add try-catch for JSON.parse with proper error handling - Enhance sanitizePattern to block Windows absolute and UNC paths for security - Replace non-null assertion with type casting for better linting compliance
- Move createErrorResponse function and ErrorResponse interface from logger.ts to utils/http.ts - Update imports across packAction.ts, rateLimit.ts, and bodyLimit.ts - Improve separation of concerns: logging utilities vs HTTP response utilities - Maintains consistent error response format across the application
This PR significantly improves the website server architecture by implementing domain-driven design principles and better separation of concerns.
Summary
Major architectural improvements:
Key Changes
🏗️ Domain-Driven Architecture
domains/pack/structure for pack-related business logiccache.ts,fileUtils.ts,sharedInstance.ts)🎯 Action Layer Improvements
index.tsinto dedicatedactions/packAction.ts🔒 Middleware Architecture
bodyLimit.ts- Request size limitingcloudLogger.ts- Structured logging (renamed from logger.ts)cors.ts- CORS configurationrateLimit.ts- Rate limiting (moved from action layer)🛠️ Utilities & Infrastructure
clientInfo.tsutility for unified client information extractionvalidation.tswith reusable validation functionsconstants.ts,schemas/request.ts,network.ts)Architecture Benefits
File Changes
New Files:
actions/packAction.ts- Consolidated pack endpoint logicmiddlewares/- Dedicated middleware components (4 files)domains/pack/- Pack domain structure (7 files)utils/clientInfo.ts- Unified client information utilityRemoved Files:
constants.ts- Consolidated into domain filesschemas/request.ts- Integrated into packActionnetwork.ts- Replaced with clientInfo utilityRefactored:
index.ts- Simplified to focus on server configurationChecklist
npm run test- All 743 tests passnpm run lint- No warnings or errorsThis refactoring establishes a solid, scalable foundation for future development while maintaining full backward compatibility.