Conversation
* wip * feed types * better feed form * Update packages/shared-db/tsconfig.json Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update apps/app/src/routes/_layout/create/feed/settings.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update apps/app/src/routes/_layout/create/feed/index.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * set lock * nitpicks * fmt --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* better auth services * nit picks * nonce validation * nitpicks
* fix: toast colors not corrent * fmt
feat: add settings page UI
* feat: welcome route for new feed * fix: feed overflow in top feeds column of leaderboard * fix: coderabbit comments
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/app/src/contexts/auth-context.tsx (1)
62-79: Good implementation addressing previous feedback.The error handling improvement from the previous review has been properly implemented with user-facing toast notification instead of the TODO comment.
🧹 Nitpick comments (1)
apps/app/src/contexts/auth-context.tsx (1)
104-120: Consider optimizing the authentication flow.The sequential execution of
checkAuthorizationandinitiateLogincould be optimized sinceinitiateLogindoesn't depend on the authorization check result.Consider running these operations in parallel for better performance:
- checkAuthorization().then(() => { - if (currentAccountId) { - initiateLogin(currentAccountId); - } - }); + Promise.all([ + checkAuthorization(), + currentAccountId ? initiateLogin(currentAccountId) : Promise.resolve() + ]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/app/rsbuild.config.ts(1 hunks)apps/app/src/config/index.ts(1 hunks)apps/app/src/contexts/auth-context.tsx(4 hunks)packages/shared-queue/src/queues.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared-queue/src/queues.ts
🔇 Additional comments (6)
apps/app/src/contexts/auth-context.tsx (4)
42-50: State initialization looks well-structured.The initialization from
near.accountId()provides proper hydration of authentication state on mount, and the new state variables are correctly typed and initialized.
81-101: Account listener implementation is solid.The event listener properly handles account changes with appropriate state updates and cleanup. The toast notification for sign-out provides good user feedback.
145-153: Excellent validation for authentication prerequisites.The nonce and recipient validation prevents authorization attempts when the backend parameters aren't ready, with clear user feedback about the issue and recovery steps.
159-159: Good use of dynamic recipient value.Using the
recipientvariable instead of a hardcoded value properly implements the dynamic authentication flow.apps/app/src/config/index.ts (1)
1-1: I’ll re-run the search without the unsupported--typeflags to confirm there are truly no remaining references:#!/bin/bash echo "Re-verifying removal of configuration constants..." rg "evmWalletChains|OPEN_CROSSPOST_PROXY_API|AUTH_STORAGE_PREFIX|APP_NAME" -g '!node_modules' -g '!.git' --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' || echo "No matches found" echo "Re-verifying imports from config/index..." rg "from ['\"][^'\"]*config/index['\"]" -g '!node_modules' -g '!.git' --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' -A2 || echo "No matches found"apps/app/rsbuild.config.ts (1)
18-18: Good alignment with the simplified network configuration approach.This change consistently uses the
networkvariable directly, which aligns well with the simplified configuration approach in the config file. The removal of any conditional environment-based overrides creates a cleaner, more predictable configuration.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/app/src/contexts/auth-context.tsx (1)
105-122: Consider optimizing to avoid unnecessary API calls.The
initiateLogincall happens for every account change after authorization check, which may result in unnecessary API requests if the user is already properly initialized. Consider checking ifnonceandrecipientare already available before callinginitiateLogin.checkAuthorization().then(() => { - if (currentAccountId) { + if (currentAccountId && (!nonce || !recipient)) { initiateLogin(currentAccountId); } });packages/utils/src/logger.ts (1)
112-113: Verify redaction patterns cover all sensitive data.The current redaction patterns may not catch all sensitive fields. Consider adding more comprehensive patterns for API keys, JWTs, and other credentials.
...(isProduction && { - redact: ["*.password", "*.token", "*.key", "*.secret"], + redact: [ + "*.password", "*.token", "*.key", "*.secret", "*.apiKey", + "*.accessToken", "*.refreshToken", "*.jwt", "*.bearer", + "*.authorization", "*.auth", "*.credentials" + ], }),packages/core-services/src/service-provider.ts (1)
169-169: Remove TODO comment or address the initialization concern.The TODO comment suggests uncertainty about the
initialize()call. This should either be resolved or tracked in a proper issue.Either remove the TODO if the initialization is correct, or create a proper issue to track the concern:
- submissionService.initialize(); // TODO: remove + submissionService.initialize();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/api/src/app.ts(1 hunks)apps/app/src/components/BasicInformationForm.tsx(0 hunks)apps/app/src/components/FeedList.tsx(0 hunks)apps/app/src/contexts/auth-context.tsx(4 hunks)apps/worker/src/index.ts(1 hunks)packages/core-services/src/service-provider.ts(7 hunks)packages/utils/src/logger.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/app/src/components/FeedList.tsx
- apps/app/src/components/BasicInformationForm.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/worker/src/index.ts
- apps/api/src/app.ts
🔇 Additional comments (11)
apps/app/src/contexts/auth-context.tsx (6)
42-52: LGTM! Good state management improvements.The initialization from the
nearobject's current state ensures proper hydration, and the new state variables serve clear purposes for authentication flow optimization.
64-81: Well-implemented authentication initiation.The function properly separates login parameter fetching from authorization and includes appropriate error handling with user feedback.
83-103: Excellent reactive account management.The event-driven approach with proper cleanup prevents memory leaks and ensures consistent state management across account changes.
125-129: Good addition for tracking sign-in state.The ref-based state tracking enables better user feedback during the authentication flow.
147-155: Excellent defensive programming with user guidance.The validation prevents authorization attempts when required parameters aren't ready and provides clear guidance for users on how to resolve the issue.
161-161: Consistent use of cached authentication parameters.Using the cached
recipientvalue aligns well with the new authentication flow architecture.packages/utils/src/logger.ts (1)
83-121: Excellent logging architecture with proper environment handling.The logger implementation demonstrates good practices with environment-aware configuration, structured logging, and proper transport selection. The factory pattern allows for service-specific logger instances.
packages/core-services/src/service-provider.ts (4)
32-44: Well-designed dependency injection interface.The
ServiceProviderConfiginterface properly encapsulates all external dependencies, making the service provider testable and environment-agnostic.
268-278: Robust singleton pattern with required initialization.The getInstance method properly enforces that configuration must be provided on first instantiation, preventing runtime errors from missing dependencies.
83-84: Excellent use of structured logging with component-specific child loggers.All services now receive properly scoped child loggers, which will greatly improve observability and debugging across the system.
Also applies to: 87-88, 92-93, 97-98, 109-110, 121-122, 155-156, 166-167, 181-182
159-171: Verify SubmissionService constructor compatibility.The
SubmissionServiceconstructor now requires aModerationServiceparameter. Ensure all existing instantiations ofSubmissionServicehave been updated to provide this dependency.#!/bin/bash # Description: Find SubmissionService instantiations to verify constructor compatibility # Expected: All instantiations should provide ModerationService parameter # Search for SubmissionService constructor calls rg -A 10 "new SubmissionService\(" --type ts
| if (err.issues && Array.isArray(err.issues)) { | ||
| const zodIssues = err.issues | ||
| .map((issue: any) => { | ||
| const path = issue.path.join("."); | ||
| return `${path} (${issue.code}): ${issue.message}`; | ||
| }) | ||
| .join("; "); | ||
| return { | ||
| message: `ZodError: ${zodIssues}`, | ||
| name: err.name, | ||
| stack: err.stack, | ||
| issues: err.issues, // Keep original issues for full context if needed | ||
| }; | ||
| } |
There was a problem hiding this comment.
Potential runtime error in ZodError path handling.
The code assumes issue.path always has a join method, but this could fail if path is not an array or is undefined.
- const path = issue.path.join(".");
+ const path = Array.isArray(issue.path) ? issue.path.join(".") : String(issue.path || "unknown");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (err.issues && Array.isArray(err.issues)) { | |
| const zodIssues = err.issues | |
| .map((issue: any) => { | |
| const path = issue.path.join("."); | |
| return `${path} (${issue.code}): ${issue.message}`; | |
| }) | |
| .join("; "); | |
| return { | |
| message: `ZodError: ${zodIssues}`, | |
| name: err.name, | |
| stack: err.stack, | |
| issues: err.issues, // Keep original issues for full context if needed | |
| }; | |
| } | |
| if (err.issues && Array.isArray(err.issues)) { | |
| const zodIssues = err.issues | |
| .map((issue: any) => { | |
| - const path = issue.path.join("."); | |
| + const path = Array.isArray(issue.path) | |
| + ? issue.path.join(".") | |
| + : String(issue.path || "unknown"); | |
| return `${path} (${issue.code}): ${issue.message}`; | |
| }) | |
| .join("; "); | |
| return { | |
| message: `ZodError: ${zodIssues}`, | |
| name: err.name, | |
| stack: err.stack, | |
| issues: err.issues, // Keep original issues for full context if needed | |
| }; | |
| } |
🤖 Prompt for AI Agents
In packages/utils/src/logger.ts between lines 14 and 27, the code assumes
issue.path is always an array and calls join on it, which can cause a runtime
error if path is undefined or not an array. To fix this, add a check to ensure
issue.path is an array before calling join, and provide a fallback string if it
is not, to safely handle all cases without errors.
| if (err.original) { | ||
| serialized.original = errorSerializer(err.original); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Risk of infinite recursion in error serialization.
The recursive call to errorSerializer(err.original) could cause a stack overflow if there are circular error references.
Add a visited set to prevent infinite recursion:
-const errorSerializer = (err: any) => {
+const errorSerializer = (err: any, visited = new WeakSet()) => {
if (!err) return err;
+
+ // Prevent infinite recursion
+ if (typeof err === "object" && visited.has(err)) {
+ return { message: "[Circular Error Reference]" };
+ }
+ if (typeof err === "object") {
+ visited.add(err);
+ }
// ... rest of function
// Handle nested error objects
if (err.original) {
- serialized.original = errorSerializer(err.original);
+ serialized.original = errorSerializer(err.original, visited);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (err.original) { | |
| serialized.original = errorSerializer(err.original); | |
| } | |
| const errorSerializer = (err: any, visited = new WeakSet()) => { | |
| if (!err) return err; | |
| // Prevent infinite recursion | |
| if (typeof err === "object" && visited.has(err)) { | |
| return { message: "[Circular Error Reference]" }; | |
| } | |
| if (typeof err === "object") { | |
| visited.add(err); | |
| } | |
| // ... rest of function | |
| // Handle nested error objects | |
| if (err.original) { | |
| serialized.original = errorSerializer(err.original, visited); | |
| } | |
| return serialized; | |
| }; |
🤖 Prompt for AI Agents
In packages/utils/src/logger.ts around lines 49 to 51, the recursive call to
errorSerializer on err.original can cause infinite recursion if there are
circular references. To fix this, introduce a visited set that tracks errors
already serialized and check this set before recursively calling
errorSerializer. This prevents re-serializing the same error and avoids stack
overflow.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/utils/src/logger.ts (2)
18-18: The ZodError path handling issue remains unresolved.The code still assumes
issue.pathis always an array and callsjoinon it, which can cause a runtime error ifpathis undefined or not an array.Apply this fix to safely handle all cases:
- const path = issue.path.join("."); + const path = Array.isArray(issue.path) ? issue.path.join(".") : String(issue.path || "unknown");
44-46: The infinite recursion risk in error serialization remains unresolved.The recursive call to
errorSerializer(err.original)could still cause a stack overflow if there are circular error references.Add a visited set to prevent infinite recursion:
-const errorSerializer = (err: any) => { +const errorSerializer = (err: any, visited = new WeakSet()) => { if (!err) return err; + + // Prevent infinite recursion + if (typeof err === "object" && visited.has(err)) { + return { message: "[Circular Error Reference]" }; + } + if (typeof err === "object") { + visited.add(err); + } // ... rest of function // Handle nested error objects if (err.original) { - serialized.original = errorSerializer(err.original); + serialized.original = errorSerializer(err.original, visited); }
🧹 Nitpick comments (2)
packages/utils/src/logger.ts (1)
109-120: Consider expanding redaction patterns for better security.The current redaction patterns are good but could be more comprehensive for production security.
Consider adding these additional patterns:
pinoOptions.redact = { paths: [ "*.password", "*.token", "*.key", "*.secret", + "*.apiKey", + "*.privateKey", + "*.sessionId", + "*.accessToken", + "*.refreshToken", "req.headers.authorization", + "req.headers.cookie", + "req.headers['x-api-key']", "Authorization", ], censor: "[REDACTED]", };packages/core-services/src/service-provider.ts (1)
134-184: Consider documenting the two-phase initialization pattern.The split between constructor and
init()method for service creation creates a complex lifecycle that may not be immediately clear to developers.Consider adding documentation to explain why some services are created in the constructor vs. the
init()method:async init() { + // Phase 2: Initialize services that depend on Twitter service initialization + // or require async setup (moderation, submission, activity services) const { db, logger } = this.config;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/api/src/app.ts(1 hunks)apps/api/src/routes/api/feeds.ts(12 hunks)apps/api/src/routes/api/test.ts(2 hunks)apps/api/src/routes/api/trigger.ts(2 hunks)apps/api/src/routes/api/twitter.ts(2 hunks)apps/api/src/routes/api/users.ts(13 hunks)apps/worker/src/handlers/moderation-queue/processor.ts(1 hunks)apps/worker/src/handlers/processing-queue/processor.ts(1 hunks)apps/worker/src/index.ts(1 hunks)packages/core-services/src/mocks/twitter-service.mock.ts(7 hunks)packages/core-services/src/service-provider.ts(7 hunks)packages/core-services/src/services/plugin.service.ts(11 hunks)packages/core-services/src/services/processor.service.ts(8 hunks)packages/core-services/src/services/transformation.service.ts(2 hunks)packages/core-services/src/services/twitter/client.ts(10 hunks)packages/core-services/src/utils/plugin.ts(2 hunks)packages/utils/src/logger.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- apps/api/src/routes/api/trigger.ts
- packages/core-services/src/services/transformation.service.ts
- apps/api/src/routes/api/users.ts
- apps/api/src/routes/api/twitter.ts
- packages/core-services/src/services/processor.service.ts
- packages/core-services/src/utils/plugin.ts
- apps/worker/src/index.ts
- apps/api/src/app.ts
- apps/worker/src/handlers/moderation-queue/processor.ts
- apps/api/src/routes/api/feeds.ts
- apps/worker/src/handlers/processing-queue/processor.ts
- apps/api/src/routes/api/test.ts
- packages/core-services/src/services/plugin.service.ts
- packages/core-services/src/services/twitter/client.ts
- packages/core-services/src/mocks/twitter-service.mock.ts
🔇 Additional comments (6)
packages/utils/src/logger.ts (1)
121-121: Transport configuration is well-designed.The use of
sync: truefor container environments and explicit stdout redirection is appropriate for production deployments.packages/core-services/src/service-provider.ts (5)
32-44: Well-designed dependency injection interface.The
ServiceProviderConfiginterface clearly defines required dependencies and environment variables, improving testability and explicit dependency management.
52-56: Good use of constructor dependency injection.The constructor now requires explicit dependencies rather than accessing globals, which is a significant improvement for maintainability and testing.
159-171: SubmissionService dependency injection looks correct.The service now properly receives
ModerationServiceas a dependency, and the initialization flow ensures dependencies are available.
272-282: Excellent improvement to the singleton pattern.Requiring configuration on first initialization prevents the common anti-pattern of implicit global state and ensures explicit dependency management.
268-270: Good addition of logger getter method.Providing access to the configured logger instance enables consistent logging across the application.
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Chores