-
Notifications
You must be signed in to change notification settings - Fork 0
chore: translate comments #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughMost edits translate comments from Chinese to English without changing behavior. Functional updates include: extending settings sync replaceData to dispatch lastSyncTime, implementing concrete subtodo filtering and visibility logic, and adjusting JWT verification to return the decoded payload with explicit error handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as UI
participant SM as SettingsSyncManagerNew
participant Store as Redux Store
participant API as Settings API
UI->>SM: Trigger sync (manual/interval)
SM->>API: POST /settings/sync (ops)
API-->>SM: { settings, lastSyncTime }
rect rgba(230, 245, 255, 0.6)
note right of SM: Updated interaction
SM->>Store: dispatch replaceSettings({ settings, lastSyncTime })
end
Store-->>UI: State updated
sequenceDiagram
autonumber
participant S as Server code
participant JWT as verifyJWT
S->>JWT: verifyJWT(token)
alt valid token
JWT-->>S: decoded payload
S-->>S: proceed with decoded claims
else invalid/expired
JWT-->>S: null (logged)
S-->>S: treat as unauthorized
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 3ce2493 in 45 seconds. Click for details.
- Reviewed
652lines of code in17files - Skipped
0files when reviewing. - Skipped posting
8draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. backgroundworker/prisma.config.ts:6
- Draft comment:
Comment translation is clear. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. middleware.ts:5
- Draft comment:
Accurate English translation for CORS preflight handling. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. prisma.config.ts:6
- Draft comment:
Consistent translation for environment variable loading. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. src/app/api/settings/route.ts:5
- Draft comment:
Comment translation is clear; note the TODO for database integration remains. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. src/app/api/settings/sync/route.ts:5
- Draft comment:
Translated comments and placeholder defaults are clear. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. src/app/lib/jwt.ts:64
- Draft comment:
Audience mismatch check is temporarily bypassed; ensure strict validation in production. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to ensure strict validation in production, which is similar to asking them to ensure behavior is intended or to double-check something. This violates the rules.
7. src/app/test-toast-simple/page.tsx:41
- Draft comment:
UI text remains in Chinese; confirm if this is intentional since only comments were targeted. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
8. tailwind.config.js:47
- Draft comment:
Comment translations in the config file are clear and maintain context. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_5Ah1fKYmePBGhkrV
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/app/lib/jwt.ts (3)
20-29: Constrain verification algorithms.Limit accepted algorithms to HS256 (matches signing) and consider small clock tolerance.
- const decoded = jwt.verify(token, JWT_SECRET) as JWTPayload; + const decoded = jwt.verify(token, JWT_SECRET, { + algorithms: ['HS256'], + clockTolerance: 5, // seconds + }) as JWTPayload;
41-53: Do not decode Google ID tokens without verifying the signature.Manual base64 decode skips signature and cert/issuer validation. Replace with google-auth-library verification.
Apply this diff to replace the entire function with a verified, async version:
-import type { GoogleUserInfo, JWTPayload } from '../types/api'; +import type { GoogleUserInfo, JWTPayload } from '../types/api'; +import { OAuth2Client } from 'google-auth-library'; +const oauthClient = new OAuth2Client(); -// Verify Google JWT token -export function verifyGoogleToken(credential: string, googleClientId: string): GoogleUserInfo | null { - // Check Google Client ID configuration - if (!googleClientId || googleClientId.trim() === '') { - console.error('🔴 [JWT] Google Client ID is not configured'); - console.error('🔴 [JWT] Please set GOOGLE_CLIENT_ID or NEXT_PUBLIC_GOOGLE_CLIENT_ID environment variable'); - return null; - } - - try { - // Decode JWT payload (signature should be verified in production) - const payload = JSON.parse(Buffer.from(credential.split('.')[1], 'base64').toString()) as GoogleUserInfo; - - console.log('Token payload:', { - aud: payload.aud, - expected: googleClientId, - exp: payload.exp, - currentTime: Date.now() / 1000, - iss: payload.iss, - sub: payload.sub, - email: payload.email - }); +// Verify Google ID token (signature + claims) +export async function verifyGoogleToken( + credential: string, + googleClientId: string +): Promise<GoogleUserInfo | null> { + if (!googleClientId || googleClientId.trim() === '') { + console.error('🔴 [JWT] Google Client ID is not configured'); + return null; + } + try { + const ticket = await oauthClient.verifyIdToken({ + idToken: credential, + audience: googleClientId, + }); + const payload = ticket.getPayload(); + if (!payload) return null; - // Check if the token has expired - if (payload.exp && payload.exp < Date.now() / 1000) { - throw new Error('Token expired'); - } - - // Validate issuer - if (payload.iss !== 'accounts.google.com' && payload.iss !== 'https://accounts.google.com') { - throw new Error('Invalid issuer'); - } - - // Validate audience (client ID) - supports array and string formats - let isValidAudience = false; - if (Array.isArray(payload.aud)) { - isValidAudience = payload.aud.includes(googleClientId); - } else { - isValidAudience = payload.aud === googleClientId; - } - - if (!isValidAudience) { - console.warn('Audience mismatch (allowing for now):', { - received: payload.aud, - expected: googleClientId - }); - // Temporarily do not throw an error for debugging - // throw new Error('Invalid audience') - } - - return payload; + // Map to our GoogleUserInfo shape + return { + sub: payload.sub ?? '', + email: payload.email ?? '', + name: payload.name ?? '', + picture: payload.picture ?? '', + iss: payload.iss ?? '', + aud: (payload.aud as string) ?? '', + exp: payload.exp ?? 0, + } as GoogleUserInfo; } catch (error) { console.error('Token verification failed:', error); return null; } }Note: Call sites must await this function after the change.
73-79: Do not allow audience mismatches. Fail closed.Permitting mismatched aud weakens security and can enable token replay from other apps.
Apply this diff:
- if (!isValidAudience) { - console.warn('Audience mismatch (allowing for now):', { - received: payload.aud, - expected: googleClientId - }); - // Temporarily do not throw an error for debugging - // throw new Error('Invalid audience') - } + if (!isValidAudience) { + throw new Error('Invalid audience'); + }Also avoid logging PII (email/sub) in payload logs; gate behind a DEBUG flag and mask if needed.
🧹 Nitpick comments (10)
src/app/api/settings/sync/route.ts (1)
11-22: Consider implementing the database operations soon.The TODO comments indicate that actual database operations are temporarily skipped. While this is acceptable for development, ensure this gets implemented before production deployment.
Would you like me to help generate the missing database operation implementation or create a tracking issue for this task?
src/app/componments/todo/TodoTypes.ts (4)
17-23: Simplify and null-safe subtodo status check.Current logic is correct but can be terser and null-safe via optional chaining.
-const hasSubtodosWithStatus = (todo: TodoItem, completed: boolean): boolean => { - if (!todo.subItems || todo.subItems.length === 0) { - return false; - } - return todo.subItems.some((subTodo: SubTodoItem) => subTodo.completed === completed && !subTodo.deleted); -}; +const hasSubtodosWithStatus = (todo: TodoItem, completed: boolean): boolean => + !!todo.subItems?.some((s: SubTodoItem) => s.completed === completed && !s.deleted);
43-59: Comment mismatch with actual logic (some vs all).The SHOW_COMPLETED comment says “all subtodos completed” but code uses a “has at least one completed subtodo” check.
If “all” is intended, change hasSubtodosWithStatus to an every-check; otherwise, update the comment:
-// Show todos with all subtodos completed, or if the main todo is completed +// Show todos with any completed subtodo, or if the main todo itself is completed
64-72: Coalesce originalSubItems to [] and consider memoization.
- If todo.subItems can be undefined, coalesce to [] to satisfy FilteredTodoItem and avoid runtime surprises.
- Optional: memoize getVisibleTodos with reselect to avoid re-creating objects on every render.
- return { + return { ...todo, subItems: filteredSubItems, - originalSubItems: todo.subItems // Save original data + originalSubItems: todo.subItems ?? [] // Save original data };If you want, I can sketch a selector with reselect keyed by filter to reduce recomputation.
25-41: Remove unreachable null check and consolidate filtering infilterSubtodos
TodoItem.subItemsis always defined (SubTodoItem[]), so theif (!subItems…)branch is never taken—drop that guard.- Pull out the common
.filter(s => !s.deleted)into a base array, then switch onfilterto apply only the remaining criteria (completed/!completed) and return that, eliminating duplicated deletion checks.src/app/componments/settings/settingsSyncManagerNew.ts (3)
41-71: Typed payload narrowing is OK; consider guarding unexpected shapes.If server or upstream code ever sends a payload without value, dispatches might receive undefined. A minimal runtime guard reduces risk.
applyOperation(operation: SyncOperation) { const { type, payload } = operation - // Type assertion since we know payload structures for different operation types - const typedPayload = payload as { value: number | boolean | string }; + // Type assertion since we know payload structures for different operation types + const typedPayload = payload as { value?: number | boolean | string }; + if (typedPayload?.value === undefined) { + console.warn('Settings operation missing value:', type, payload) + return + }
83-89: Make sync interval configurable.Hardcoding 10s is fine for now; consider an env/config override to tune without code changes.
-export const settingsSyncManager = new GenericSyncManager( +export const settingsSyncManager = new GenericSyncManager( 'settings', new SettingsAPIAdapter(), new SettingsOperationHandler(), - 10000 // 10-second sync interval for frequent synchronization + Number(import.meta.env.VITE_SETTINGS_SYNC_INTERVAL ?? 10000) // default 10s )
73-80: Add runtime shape validation in replaceData
Guard the incomingdata(typeunknown) with a lightweight type predicate—confirming it has{ settings: SettingsUpdate; lastSyncTime: string }—before callingstore.dispatch(replaceSettings), to prevent dispatching malformed payloads.src/app/api/user/tokens/route.ts (1)
88-94: Minor CORS nit: return 204 for preflight.204 is conventional for OPTIONS with no body.
Apply this diff:
-export async function OPTIONS(request: NextRequest) { - return new NextResponse(null, { - status: 200, - headers: getCorsHeaders(request), - }); -} +export async function OPTIONS(request: NextRequest) { + return new NextResponse(null, { + status: 204, + headers: getCorsHeaders(request), + }); +}src/app/lib/jwt.ts (1)
54-58: Redundant expiry check once signature verification is used.google-auth-library enforces exp; local check becomes unnecessary and risks clock skew divergence.
📜 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 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 (17)
backgroundworker/prisma.config.ts(1 hunks)middleware.ts(1 hunks)prisma.config.ts(1 hunks)src/app/api/settings/route.ts(1 hunks)src/app/api/settings/sync/route.ts(2 hunks)src/app/api/user/profile/route.ts(1 hunks)src/app/api/user/tokens/route.ts(5 hunks)src/app/componments/settings/Settings.css(2 hunks)src/app/componments/settings/settingsAPI.ts(4 hunks)src/app/componments/settings/settingsSyncManager.ts(4 hunks)src/app/componments/settings/settingsSyncManagerNew.ts(4 hunks)src/app/componments/todo/TodoTypes.ts(3 hunks)src/app/lib/cors.ts(2 hunks)src/app/lib/database.ts(2 hunks)src/app/lib/jwt.ts(4 hunks)src/app/test-toast-simple/page.tsx(1 hunks)tailwind.config.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
middleware.ts (1)
src/app/lib/cors.ts (1)
getCorsOptionsHeaders(29-38)
src/app/lib/database.ts (2)
backgroundworker/prisma.config.ts (1)
adapter(20-26)prisma.config.ts (1)
adapter(20-26)
src/app/componments/settings/settingsSyncManager.ts (1)
src/app/componments/settings/settingsSyncManagerNew.ts (1)
settingsSyncManager(84-89)
🔇 Additional comments (42)
src/app/test-toast-simple/page.tsx (2)
25-29: LGTM: Comment translation only.Wording is clear and matches behavior; no logic changes.
31-37: LGTM: Comment translation only.Clear and accurate; no functional impact.
backgroundworker/prisma.config.ts (1)
6-9: LGTM: Comment translation clarifies dotenv load order.Matches the existing behavior where .env.local variables take precedence over .env (no override).
middleware.ts (3)
5-11: LGTM: Clarified preflight handling comment.Accurately describes OPTIONS handling; no code changes.
13-15: LGTM: Clarified processing flow comment.NextResponse.next() behavior unchanged.
16-24: LGTM: Clarified CORS header application.Comment aligns with matcher '/api/:path*'.
prisma.config.ts (1)
6-9: LGTM: Comment translation keeps intent on env precedence.Matches current dotenv invocation order.
src/app/lib/database.ts (5)
6-7: LGTM: Comment clarifies usage context (dynamic routes).No functional changes.
10-12: LGTM: Comment clarifies D1 binding retrieval.Accurate and helpful.
18-20: LGTM: Dev-mode error context clarified.Keeps behavior intact.
24-37: LGTM: Comment clarifies static route usage (ISR/SSG).No logic changes introduced.
39-47: LGTM: Initialization check comment is clearer.No code changes.
src/app/componments/settings/settingsAPI.ts (4)
1-1: LGTM! Comment translation improves code readability.The translation from Chinese to English enhances accessibility for international developers.
10-10: LGTM! Clear and accurate comment translation.The English comment accurately describes the purpose of the optional ID field for deduplication.
164-164: LGTM! Comment translation maintains technical accuracy.The English translation properly describes the singleton pattern implementation.
174-174: LGTM! Backward compatibility comment is clear.The translation accurately explains the purpose of maintaining backward compatibility.
src/app/api/user/profile/route.ts (1)
5-5: LGTM! Comment translation improves code clarity.The English translation clearly describes the function's purpose for fetching user information.
src/app/api/settings/route.ts (2)
5-5: LGTM! Clear function description in English.The translation accurately describes the purpose of the settings retrieval function.
8-9: LGTM! TODO and implementation comments are well translated.Both the TODO comment for database integration and the temporary default settings comment are clearly expressed in English.
src/app/componments/settings/settingsSyncManager.ts (7)
15-15: LGTM! Clear adapter description.The English comment accurately describes the Settings API adapter class.
18-18: LGTM! Type conversion comment is clear.The translation clearly explains the purpose of converting to SettingsOperation type.
41-41: LGTM! Operation handler description is accurate.The English comment properly describes the Settings operation handler class.
46-46: LGTM! Type assertion comment is well explained.The translation clearly justifies the type assertion based on known payload structures.
74-74: LGTM! State update comment is clear.The English comment accurately describes updating local state with server settings.
83-83: LGTM! Sync manager instance comments are well translated.Both the instance creation comment and the sync interval comment provide clear technical details in English.
Also applies to: 88-88
91-91: LGTM! Backward compatibility wrapper is well documented.The English comment clearly explains the purpose of maintaining backward compatibility.
tailwind.config.js (3)
47-47: LGTM! CSS compatibility comment is clear.The English translation accurately describes ensuring CSS compatibility.
49-49: LGTM! Preflight configuration comment is well explained.The translation clearly describes the purpose of the base style reset.
51-51: LGTM! Browser compatibility comment is informative.The English comment provides clear guidance about disabling modern features for older browser support.
src/app/componments/settings/Settings.css (1)
7-7: LGTM! Comment translations improve code clarity.The translation of Chinese comments to English in CSS files makes the code more accessible to international developers. The actual CSS rules and styling remain unchanged.
Also applies to: 9-9, 218-219, 221-221
src/app/lib/cors.ts (1)
4-7: LGTM! JSDoc translations improve documentation quality.The translation of JSDoc comments from Chinese to English makes the API documentation more accessible to international developers while maintaining the same function signatures and behavior.
Also applies to: 25-27, 41-43, 46-46
src/app/api/settings/sync/route.ts (1)
5-5: LGTM! Comment translations improve code clarity.The translation of Chinese comments to English makes the code more accessible to international developers while maintaining the same functionality. The TODO comments appropriately indicate areas for future implementation.
Also applies to: 11-12, 20-20, 24-25, 40-41
src/app/componments/todo/TodoTypes.ts (1)
11-15: Docs clarity looks good.The comment clarifies the intent of FilteredTodoItem and preserving originalSubItems.
src/app/componments/settings/settingsSyncManagerNew.ts (2)
15-31: Adapter mapping is clear and correct.Mapping SyncOperation → SettingsOperation with a precise payload cast is appropriate, and passing lastSyncTime through is clean.
91-92: Wrapper for compatibility is fine.Thin alias keeps external callers stable while internals evolve.
src/app/api/user/tokens/route.ts (4)
8-8: Comment translation looks good.Wording matches the surrounding logic.
33-37: Comment translation looks good.Accurately describes the Prisma fetch by decoded.sub.
49-58: Confirm the intended semantics for isTokenExpired when expiry is null.Current logic treats a missing expiry as “expired = true.” If tokens are absent (both access/refresh null), do clients expect expired=true or a different signal?
70-71: Good privacy callout.Not returning raw tokens is correct.
src/app/lib/jwt.ts (3)
31-39: Client ID checks: LGTM.Clear errors and early return when misconfigured.
59-63: Issuer validation: OK.Covers both forms of Google issuer.
64-71: Audience handling: logic is correct.Supports both array and string formats.
| // Generate JWT token | ||
| export function generateJWT(userInfo: GoogleUserInfo): string { | ||
| const payload: JWTPayload = { | ||
| sub: userInfo.sub, | ||
| email: userInfo.email, | ||
| name: userInfo.name, | ||
| picture: userInfo.picture, | ||
| iat: Math.floor(Date.now() / 1000), | ||
| exp: Math.floor(Date.now() / 1000) + (24 * 60 * 60) // 24小时过期 | ||
| exp: Math.floor(Date.now() / 1000) + (24 * 60 * 60) // Expires in 24 hours | ||
| }; | ||
|
|
||
| return jwt.sign(payload, JWT_SECRET, { algorithm: 'HS256' }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
JWT secret handling and expiry: avoid insecure default and prefer library-managed exp.
- Don’t ship a fallback secret; fail fast if JWT_SECRET is unset.
- Prefer jwt.sign({ … }, { expiresIn: '24h' }) over manual iat/exp.
Apply this diff within the selected range to use expiresIn:
export function generateJWT(userInfo: GoogleUserInfo): string {
const payload: JWTPayload = {
sub: userInfo.sub,
email: userInfo.email,
name: userInfo.name,
picture: userInfo.picture,
- iat: Math.floor(Date.now() / 1000),
- exp: Math.floor(Date.now() / 1000) + (24 * 60 * 60) // Expires in 24 hours
};
-
- return jwt.sign(payload, JWT_SECRET, { algorithm: 'HS256' });
+ return jwt.sign(payload, JWT_SECRET, { algorithm: 'HS256', expiresIn: '24h' });
}Additional change outside the selected range (top of file): ensure no default secret and mark server-only.
// add at very top
import 'server-only';
// replace the secret declaration
const JWT_SECRET = process.env.JWT_SECRET;
if (!JWT_SECRET) {
throw new Error('JWT_SECRET is not set');
}🤖 Prompt for AI Agents
In src/app/lib/jwt.ts around lines 6 to 18, the code currently uses a fallback
JWT secret and manually sets iat/exp; update to fail fast if JWT_SECRET is
missing by adding import 'server-only' at the top and replacing any default
secret with const JWT_SECRET = process.env.JWT_SECRET; if (!JWT_SECRET) throw
new Error('JWT_SECRET is not set');, then remove iat and exp from the payload
and call jwt.sign(payload, JWT_SECRET, { algorithm: 'HS256', expiresIn: '24h' })
so the library manages expiration.
Summary
Testing
pnpm testhttps://chatgpt.com/codex/tasks/task_e_68b23aad6ba883278d75dc72d88059dc
Important
Translate comments from Chinese to English across multiple files to improve code readability.
prisma.config.tsfor environment variable loading.middleware.tsfor CORS handling.route.tsfiles for settings, sync, profile, and tokens API endpoints.Settings.cssfor component styles.settingsAPI.ts,settingsSyncManager.ts, andsettingsSyncManagerNew.tsfor settings synchronization.TodoTypes.tsfor todo filtering logic.cors.ts,database.ts, andjwt.tsfor utility functions.page.tsxfor toast testing.tailwind.config.jsfor configuration settings.This description was created by
for 3ce2493. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation