feat: expand item types and improve API response handling #25
Conversation
- Added new interfaces and types for ItemsPublic, ItemResponse, and ItemsResponse to enhance type safety and clarity in API interactions. - Updated the index file to export the new types for broader accessibility. - Refactored the ItemsTable component to utilize the new response structure, improving data handling and reducing type assertions.
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces a major refactor and feature expansion for the Nexus browser extension, focusing on AI-powered content clipping, summarization, and user experience improvements. It replaces the previous popup and routing system with a new popup UI, sidebar content script, and robust background logic for context menus, badge management, and tab/session tracking. The extension now features React-based components for popup actions, recent items, and settings, with new utility modules for API interaction, class name merging, and data formatting. The update also removes legacy files related to the old architecture and documentation, and introduces a new Tailwind CSS setup for consistent theming. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Popup
participant ContentScript
participant Background
participant API
participant Storage
User->>Popup: Click "Clip" or AI action
Popup->>Background: Request active tab info
Background->>Popup: Return tab info
Popup->>ContentScript: Send clip/summarize/extract message
ContentScript->>ContentScript: Extract content/selection
ContentScript->>API: Save clipping or request AI insight
API-->>ContentScript: Return result
ContentScript->>Popup: Update UI with result
Popup->>Storage: Save recent items
Popup->>Background: Update badge count
Background->>Popup: Badge updated
User->>Popup: Open settings
Popup->>OptionsPage: Render settings UI
OptionsPage->>Storage: Load/save user settings
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
PR Review 🔍
|
PR Code Suggestions ✨
|
- Removed unused exports from the index file to streamline the module interface. - Updated type definitions in `types.ts` for `ItemPublic`, `ItemsPublic`, `ItemResponse`, and `ItemsResponse` to enhance clarity and maintainability. - Refactored utility functions in `utils.ts` for consistent formatting and improved readability.
…able component - Simplified the handling of API responses in the ItemsTable component by using type assertions for better clarity and safety. - Updated the way items and count are derived from the API response to ensure robustness against unexpected data structures. - Removed unused imports to streamline the codebase.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
backend/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
admin/src/client/index.ts(0 hunks)admin/src/client/types.ts(1 hunks)admin/src/client/utils.ts(2 hunks)admin/src/routes/_layout/items.tsx(1 hunks)backend/app/core/config.py(1 hunks)backend/pyproject.toml(1 hunks)frontend/openapi-ts.config.ts(1 hunks)frontend/openapi.json(1 hunks)
💤 Files with no reviewable changes (1)
- admin/src/client/index.ts
✅ Files skipped from review due to trivial changes (2)
- backend/pyproject.toml
- admin/src/client/utils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- admin/src/routes/_layout/items.tsx
- admin/src/client/types.ts
🧰 Additional context used
🪛 Checkov (3.2.334)
frontend/openapi.json
[MEDIUM] 1: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-docker-compose
- GitHub Check: test-backend
🔇 Additional comments (5)
backend/app/core/config.py (2)
125-136: Good enhancement: Explicit string conversion for PostgresDsn.The addition of explicit string conversion (
str()) around theURL.build()result is a good improvement. This makes the type conversion explicit and prevents potential type compatibility issues when passing the URL object to thePostgresDsnconstructor.
144-155: Good consistency: Applied same pattern to standard PostgreSQL connection.The same explicit string conversion pattern is correctly applied to the standard PostgreSQL connection URI construction, maintaining consistency throughout the codebase.
frontend/openapi-ts.config.ts (3)
3-3: Good addition of the path module.Adding the path module helps ensure cross-platform path compatibility, which is a good practice for projects that might run on different operating systems.
7-8: Great defensive programming with fallback value.The fallback mechanism for
openapiFileimproves robustness by ensuring a valid path even when the environment variable is not set. This prevents potential runtime errors and makes the configuration more resilient.
12-12: Clean type handling.Removing the explicit type assertion is appropriate since TypeScript can now infer that
openapiFileis always a string from both possible values.
frontend/openapi.json
Outdated
| } | ||
| } | ||
| } | ||
| {"openapi": "3.1.0", "info": {"title": "nexus", "version": "0.1.0"}, "paths": {"/health": {"get": {"tags": ["health"], "summary": "Get Health Root", "description": "\u517c\u5bb9\u524d\u7aef\u7684\u6839\u7ea7\u522b\u5065\u5eb7\u68c0\u67e5\u8def\u7531", "operationId": "health-get_health_root", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}}}}, "/api/v1/health": {"get": {"tags": ["health"], "summary": "Get Health Api", "description": "\u517c\u5bb9\u524d\u7aef\u7684API\u7ea7\u522b\u5065\u5eb7\u68c0\u67e5\u8def\u7531", "operationId": "health-get_health_api", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}}}}, "/api/v1/login/access-token": {"post": {"tags": ["login"], "summary": "Login Access Token", "description": "OAuth2 compatible token login, get an access token for future requests", "operationId": "login-login_access_token", "requestBody": {"content": {"application/x-www-form-urlencoded": {"schema": {"$ref": "#/components/schemas/Body_login-login_access_token"}}}, "required": true}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/Token"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/api/v1/login/test-token": {"post": {"tags": ["login"], "summary": "Test Token", "description": "Test access token", "operationId": "login-test_token", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/UserPublic"}}}}}, "security": [{"OAuth2PasswordBearer": []}]}}, "/api/v1/logout": {"post": {"tags": ["login"], "summary": "Logout", "description": "Logout current user\n\nThis endpoint invalidates the current token by adding it to a blacklist.\nThe frontend should still remove the tokens from local storage after calling this endpoint.", "operationId": "login-logout", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/Message"}}}}}, "security": [{"OAuth2PasswordBearer": []}]}}, "/api/v1/password-recovery/{email}": {"post": {"tags": ["login"], "summary": "Recover Password", "description": "Password Recovery", "operationId": "login-recover_password", "parameters": [{"name": "email", "in": "path", "required": true, "schema": {"type": "string", "title": "Email"}}], "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/Message"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/api/v1/reset-password/": {"post": {"tags": ["login"], "summary": "Reset Password", "description": "Reset password", "operationId": "login-reset_password", "requestBody": {"content": {"application/json": {"schema": {"$ref": "#/components/schemas/NewPassword"}}}, "required": true}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/Message"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/api/v1/password-recovery-html-content/{email}": {"post": {"tags": ["login"], "summary": "Recover Password Html Content", "description": "HTML Content for Password Recovery", "operationId": "login-recover_password_html_content", "security": [{"OAuth2PasswordBearer": []}], "parameters": [{"name": "email", "in": "path", "required": true, "schema": {"type": "string", "title": "Email"}}], "responses": {"200": {"description": "Successful Response", "content": {"text/html": {"schema": {"type": "string"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/api/v1/users/": {"get": {"tags": ["users"], "summary": "Read Users", "description": "Retrieve users.", "operationId": "users-read_users", "security": [{"OAuth2PasswordBearer": []}], "parameters": [{"name": "skip", "in": "query", "required": false, "schema": {"type": "integer", "default": 0, "title": "Skip"}}, {"name": "limit", "in": "query", "required": false, "schema": {"type": "integer", "default": 100, "title": "Limit"}}], "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/UsersPublic"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}, "post": {"tags": ["users"], "summary": "Create User", "description": "Create new user.", "operationId": "users-create_user", "security": [{"OAuth2PasswordBearer": []}], "requestBody": {"required": true, "content": {"application/json": {"schema": {"$ref": "#/components/schemas/UserCreate"}}}}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/UserPublic"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/api/v1/users/me": {"get": {"tags": ["users"], "summary": "Read User Me", "description": "Get current user.", "operationId": "users-read_user_me", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/UserPublic"}}}}}, "security": [{"OAuth2PasswordBearer": []}]}, "delete": {"tags": ["users"], "summary": "Delete User Me", "description": "Delete own user.", "operationId": "users-delete_user_me", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/Message"}}}}}, "security": [{"OAuth2PasswordBearer": []}]}, "patch": {"tags": ["users"], "summary": "Update User Me", "description": "Update own user.", "operationId": "users-update_user_me", "requestBody": {"content": {"application/json": {"schema": {"$ref": "#/components/schemas/UserUpdateMe"}}}, "required": true}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/UserPublic"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}, "security": [{"OAuth2PasswordBearer": []}]}}, "/api/v1/users/me/password": {"patch": {"tags": ["users"], "summary": "Update Password Me", "description": "Update own password.", "operationId": "users-update_password_me", "requestBody": {"content": {"application/json": {"schema": {"$ref": "#/components/schemas/UpdatePassword"}}}, "required": true}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/Message"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}, "security": [{"OAuth2PasswordBearer": []}]}}, "/api/v1/users/signup": {"post": {"tags": ["users"], "summary": "Register User", "description": "Create new user without the need to be logged in.", "operationId": "users-register_user", "requestBody": {"content": {"application/json": {"schema": {"$ref": "#/components/schemas/UserRegister"}}}, "required": true}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/UserPublic"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/api/v1/users/{user_id}": {"get": {"tags": ["users"], "summary": "Read User By Id", "description": "Get a specific user by id.", "operationId": "users-read_user_by_id", "security": [{"OAuth2PasswordBearer": []}], "parameters": [{"name": "user_id", "in": "path", "required": true, "schema": {"type": "string", "format": "uuid", "title": "User Id"}}], "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/UserPublic"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}, "patch": {"tags": ["users"], "summary": "Update User", "description": "Update a user.", "operationId": "users-update_user", "security": [{"OAuth2PasswordBearer": []}], "parameters": [{"name": "user_id", "in": "path", "required": true, "schema": {"type": "string", "format": "uuid", "title": "User Id"}}], "requestBody": {"required": true, "content": {"application/json": {"schema": {"$ref": "#/components/schemas/UserUpdate"}}}}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/UserPublic"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}, "delete": {"tags": ["users"], "summary": "Delete User", "description": "Delete a user.", "operationId": "users-delete_user", "security": [{"OAuth2PasswordBearer": []}], "parameters": [{"name": "user_id", "in": "path", "required": true, "schema": {"type": "string", "format": "uuid", "title": "User Id"}}], "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/Message"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/api/v1/utils/test-email/": {"post": {"tags": ["utils"], "summary": "Test Email", "description": "Test emails.", "operationId": "utils-test_email", "security": [{"OAuth2PasswordBearer": []}], "parameters": [{"name": "email_to", "in": "query", "required": true, "schema": {"type": "string", "format": "email", "title": "Email To"}}], "responses": {"201": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/Message"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/api/v1/utils/health-check/": {"get": {"tags": ["utils"], "summary": "Health Check", "operationId": "utils-health_check", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"type": "boolean", "title": "Response Utils-Health Check"}}}}}}}, "/api/v1/items/": {"get": {"tags": ["items"], "summary": "Read Items", "description": "Retrieve items.", "operationId": "items-read_items", "security": [{"OAuth2PasswordBearer": []}], "parameters": [{"name": "skip", "in": "query", "required": false, "schema": {"type": "integer", "default": 0, "title": "Skip"}}, {"name": "limit", "in": "query", "required": false, "schema": {"type": "integer", "default": 100, "title": "Limit"}}], "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/ApiResponse_ItemsPublic_"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}, "post": {"tags": ["items"], "summary": "Create Item", "description": "Create new item.", "operationId": "items-create_item", "security": [{"OAuth2PasswordBearer": []}], "requestBody": {"required": true, "content": {"application/json": {"schema": {"$ref": "#/components/schemas/ItemCreate"}}}}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/ApiResponse_ItemPublic_"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/api/v1/items/{id}": {"get": {"tags": ["items"], "summary": "Read Item", "description": "Get item by ID.", "operationId": "items-read_item", "security": [{"OAuth2PasswordBearer": []}], "parameters": [{"name": "id", "in": "path", "required": true, "schema": {"type": "string", "format": "uuid", "title": "Id"}}], "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/ApiResponse_ItemPublic_"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}, "put": {"tags": ["items"], "summary": "Update Item", "description": "Update an item.", "operationId": "items-update_item", "security": [{"OAuth2PasswordBearer": []}], "parameters": [{"name": "id", "in": "path", "required": true, "schema": {"type": "string", "format": "uuid", "title": "Id"}}], "requestBody": {"required": true, "content": {"application/json": {"schema": {"$ref": "#/components/schemas/ItemUpdate"}}}}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/ApiResponse_ItemPublic_"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}, "delete": {"tags": ["items"], "summary": "Delete Item", "description": "Delete an item.", "operationId": "items-delete_item", "security": [{"OAuth2PasswordBearer": []}], "parameters": [{"name": "id", "in": "path", "required": true, "schema": {"type": "string", "format": "uuid", "title": "Id"}}], "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/ApiResponse_NoneType_"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/api/v1/auth/google-callback": {"post": {"tags": ["google_oauth"], "summary": "Google Callback Api", "description": "Handle Google OAuth callback from frontend", "operationId": "google_oauth-google_callback_api", "requestBody": {"content": {"application/json": {"schema": {"$ref": "#/components/schemas/GoogleCallbackRequest"}}}, "required": true}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/api/v1/login/google": {"get": {"tags": ["google_oauth"], "summary": "Google Login", "description": "Initiate Google OAuth2 authentication flow", "operationId": "google_oauth-google_login", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}}}}, "/api/v1/login/google/callback": {"get": {"tags": ["google_oauth"], "summary": "Google Callback", "description": "Handle the callback from Google OAuth", "operationId": "google_oauth-google_callback", "parameters": [{"name": "code", "in": "query", "required": false, "schema": {"anyOf": [{"type": "string"}, {"type": "null"}], "title": "Code"}}, {"name": "state", "in": "query", "required": false, "schema": {"anyOf": [{"type": "string"}, {"type": "null"}], "title": "State"}}, {"name": "error", "in": "query", "required": false, "schema": {"anyOf": [{"type": "string"}, {"type": "null"}], "title": "Error"}}], "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/api/v1/private/users/": {"post": {"tags": ["private"], "summary": "Create User", "description": "Create a new user.", "operationId": "private-create_user", "requestBody": {"content": {"application/json": {"schema": {"$ref": "#/components/schemas/PrivateUserCreate"}}}, "required": true}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/UserPublic"}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}}, "components": {"schemas": {"ApiResponse_ItemPublic_": {"properties": {"data": {"anyOf": [{}, {"type": "null"}], "title": "Data"}, "meta": {"anyOf": [{"type": "object"}, {"type": "null"}], "title": "Meta"}, "error": {"anyOf": [{"type": "string"}, {"type": "null"}], "title": "Error"}}, "type": "object", "title": "ApiResponse[ItemPublic]"}, "ApiResponse_ItemsPublic_": {"properties": {"data": {"anyOf": [{}, {"type": "null"}], "title": "Data"}, "meta": {"anyOf": [{"type": "object"}, {"type": "null"}], "title": "Meta"}, "error": {"anyOf": [{"type": "string"}, {"type": "null"}], "title": "Error"}}, "type": "object", "title": "ApiResponse[ItemsPublic]"}, "ApiResponse_NoneType_": {"properties": {"data": {"anyOf": [{}, {"type": "null"}], "title": "Data"}, "meta": {"anyOf": [{"type": "object"}, {"type": "null"}], "title": "Meta"}, "error": {"anyOf": [{"type": "string"}, {"type": "null"}], "title": "Error"}}, "type": "object", "title": "ApiResponse[NoneType]"}, "Body_login-login_access_token": {"properties": {"grant_type": {"anyOf": [{"type": "string", "pattern": "password"}, {"type": "null"}], "title": "Grant Type"}, "username": {"type": "string", "title": "Username"}, "password": {"type": "string", "title": "Password"}, "scope": {"type": "string", "title": "Scope", "default": ""}, "client_id": {"anyOf": [{"type": "string"}, {"type": "null"}], "title": "Client Id"}, "client_secret": {"anyOf": [{"type": "string"}, {"type": "null"}], "title": "Client Secret"}}, "type": "object", "required": ["username", "password"], "title": "Body_login-login_access_token"}, "GoogleCallbackRequest": {"properties": {"token": {"type": "string", "title": "Token"}, "user_info": {"type": "object", "title": "User Info"}}, "type": "object", "required": ["token", "user_info"], "title": "GoogleCallbackRequest"}, "HTTPValidationError": {"properties": {"detail": {"items": {"$ref": "#/components/schemas/ValidationError"}, "type": "array", "title": "Detail"}}, "type": "object", "title": "HTTPValidationError"}, "ItemCreate": {"properties": {"title": {"type": "string", "maxLength": 255, "minLength": 1, "title": "Title"}, "description": {"anyOf": [{"type": "string", "maxLength": 255}, {"type": "null"}], "title": "Description"}}, "type": "object", "required": ["title"], "title": "ItemCreate"}, "ItemUpdate": {"properties": {"title": {"anyOf": [{"type": "string", "maxLength": 255, "minLength": 1}, {"type": "null"}], "title": "Title"}, "description": {"anyOf": [{"type": "string", "maxLength": 255}, {"type": "null"}], "title": "Description"}}, "type": "object", "title": "ItemUpdate"}, "Message": {"properties": {"message": {"type": "string", "title": "Message"}}, "type": "object", "required": ["message"], "title": "Message"}, "NewPassword": {"properties": {"token": {"type": "string", "title": "Token"}, "new_password": {"type": "string", "maxLength": 40, "minLength": 8, "title": "New Password"}}, "type": "object", "required": ["token", "new_password"], "title": "NewPassword"}, "PrivateUserCreate": {"properties": {"email": {"type": "string", "title": "Email"}, "password": {"type": "string", "title": "Password"}, "full_name": {"type": "string", "title": "Full Name"}, "is_verified": {"type": "boolean", "title": "Is Verified", "default": false}}, "type": "object", "required": ["email", "password", "full_name"], "title": "PrivateUserCreate"}, "Token": {"properties": {"access_token": {"type": "string", "title": "Access Token"}, "token_type": {"type": "string", "title": "Token Type", "default": "bearer"}}, "type": "object", "required": ["access_token"], "title": "Token"}, "UpdatePassword": {"properties": {"current_password": {"type": "string", "maxLength": 40, "minLength": 8, "title": "Current Password"}, "new_password": {"type": "string", "maxLength": 40, "minLength": 8, "title": "New Password"}}, "type": "object", "required": ["current_password", "new_password"], "title": "UpdatePassword"}, "UserCreate": {"properties": {"email": {"type": "string", "maxLength": 255, "format": "email", "title": "Email"}, "is_active": {"type": "boolean", "title": "Is Active", "default": true}, "is_superuser": {"type": "boolean", "title": "Is Superuser", "default": false}, "full_name": {"anyOf": [{"type": "string", "maxLength": 255}, {"type": "null"}], "title": "Full Name"}, "password": {"type": "string", "maxLength": 40, "minLength": 8, "title": "Password"}}, "type": "object", "required": ["password"], "title": "UserCreate"}, "UserPublic": {"properties": {"email": {"type": "string", "maxLength": 255, "format": "email", "title": "Email"}, "is_active": {"type": "boolean", "title": "Is Active", "default": true}, "is_superuser": {"type": "boolean", "title": "Is Superuser", "default": false}, "full_name": {"anyOf": [{"type": "string", "maxLength": 255}, {"type": "null"}], "title": "Full Name"}, "id": {"type": "string", "format": "uuid", "title": "Id"}}, "type": "object", "required": ["id"], "title": "UserPublic"}, "UserRegister": {"properties": {"email": {"type": "string", "maxLength": 255, "title": "Email"}, "password": {"type": "string", "maxLength": 40, "minLength": 8, "title": "Password"}, "full_name": {"anyOf": [{"type": "string", "maxLength": 255}, {"type": "null"}], "title": "Full Name"}}, "type": "object", "required": ["email", "password"], "title": "UserRegister"}, "UserUpdate": {"properties": {"email": {"anyOf": [{"type": "string", "maxLength": 255}, {"type": "null"}], "title": "Email"}, "is_active": {"type": "boolean", "title": "Is Active", "default": true}, "is_superuser": {"type": "boolean", "title": "Is Superuser", "default": false}, "full_name": {"anyOf": [{"type": "string", "maxLength": 255}, {"type": "null"}], "title": "Full Name"}, "password": {"anyOf": [{"type": "string", "maxLength": 40, "minLength": 8}, {"type": "null"}], "title": "Password"}}, "type": "object", "title": "UserUpdate"}, "UserUpdateMe": {"properties": {"full_name": {"anyOf": [{"type": "string", "maxLength": 255}, {"type": "null"}], "title": "Full Name"}, "email": {"anyOf": [{"type": "string", "maxLength": 255}, {"type": "null"}], "title": "Email"}}, "type": "object", "title": "UserUpdateMe"}, "UsersPublic": {"properties": {"data": {"items": {"$ref": "#/components/schemas/UserPublic"}, "type": "array", "title": "Data"}, "count": {"type": "integer", "title": "Count"}}, "type": "object", "required": ["data", "count"], "title": "UsersPublic"}, "ValidationError": {"properties": {"loc": {"items": {"anyOf": [{"type": "string"}, {"type": "integer"}]}, "type": "array", "title": "Location"}, "msg": {"type": "string", "title": "Message"}, "type": {"type": "string", "title": "Error Type"}}, "type": "object", "required": ["loc", "msg", "type"], "title": "ValidationError"}}, "securitySchemes": {"OAuth2PasswordBearer": {"type": "oauth2", "flows": {"password": {"scopes": {}, "tokenUrl": "/api/v1/login/access-token"}}}}}} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Minified format is appropriate for production.
The minification of the OpenAPI JSON file is good for reducing file size and potentially improving parse performance without changing functionality.
Consider addressing the array size limitation issue.
The static analysis tool has identified that arrays in the OpenAPI specification don't have maximum size limits defined, which could potentially lead to DoS vulnerabilities.
Consider adding maxItems properties to array definitions in the schema to address this security best practice. You can verify which arrays need this with:
🏁 Script executed:
#!/bin/bash
# Find arrays without maxItems property in the OpenAPI schema
cat frontend/openapi.json | jq -c 'paths(..|select(.type? == "array" and .maxItems == null))' | grep -v nullLength of output: 717
✅ Production-Ready Minification
Minifying frontend/openapi.json is a good practice for reducing payload size without changing functionality.
maxItems to Array Schemas
Several schema definitions declare type: "array" but lack a maxItems constraint, which can lead to DoS risks. Please add appropriate limits to each of the following paths:
components.schemas.HTTPValidationError.properties.detailcomponents.schemas.UsersPublic.properties.datacomponents.schemas.ValidationError.properties.loc
For example, to limit the number of validation errors returned:
"HTTPValidationError": {
"properties": {
- "detail": {
- "type": "array",
- "items": { "$ref": "#/components/schemas/ValidationError" },
- "title": "Detail"
- }
+ "detail": {
+ "type": "array",
+ "items": { "$ref": "#/components/schemas/ValidationError" },
+ "maxItems": 50,
+ "title": "Detail"
+ }
}
}Adjust each maxItems value according to realistic use-case expectations.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 1: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🤖 Prompt for AI Agents
In frontend/openapi.json at line 1, several array schemas lack maxItems
constraints, which can lead to DoS vulnerabilities. Add a maxItems property to
the arrays at components.schemas.HTTPValidationError.properties.detail,
components.schemas.UsersPublic.properties.data, and
components.schemas.ValidationError.properties.loc. Set maxItems to reasonable
limits based on expected maximum array sizes for validation errors and user
lists to improve security.
…omponents - Improved error handling in the DashboardPage to accommodate various error response formats. - Updated the login endpoint to the correct API path and enhanced error messages for better user feedback. - Localized UI text in the LoginPage to support Chinese language, ensuring a more accessible user experience.
- Upgraded pnpm version to 9.9.0 in GitHub workflows for improved package management. - Added type definitions for API error responses in the DashboardPage and LoginPage components to enhance error handling. - Refactored the LoginPage to include a SearchParamsHandler for better email handling and added debug information for easier troubleshooting.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
frontend/app/dashboard/page.tsx (1)
51-61: Provide a fallback error message to improve UXIf none of the three checked properties exist the user will see a blank
red box. Add a default message so that unexpected error shapes are still
communicated.-} else if (errorResponse.meta && "message" in errorResponse.meta) { - errorMessage = String(errorResponse.meta.message); -} +} else if (errorResponse.meta && "message" in errorResponse.meta) { + errorMessage = String(errorResponse.meta.message); +} else { + errorMessage = "Unknown error occurred"; +}frontend/components/actions/items-action.ts (1)
73-77: Use discriminated union for easier downstream checksReturning
{ error: ... }or{ message: ... }makes callers perform
three separate property checks. Standardise on a singleerrorkey:-return { error: typeof error === 'string' ? error : JSON.stringify(error) }; +return { error: typeof error === "string" ? error : JSON.stringify(error) };…and in the catch-block:
-return { message: "Failed to fetch items" }; +return { error: "Failed to fetch items" };Also update
ApiErrorResponseso"error"is the discriminant.
Call-sites can then testif ("error" in result)instead of three
conditionals.frontend/app/login/page.tsx (2)
61-65: Conditionalise debug logging for production safety
setDebugInfo(JSON.stringify(data, null, 2));may surface access tokens
or other sensitive data in the UI.
Protect this behind a development flag to avoid exposing secrets.-if (process.env.NODE_ENV !== "production") { - setDebugInfo(JSON.stringify(data, null, 2)); -} +if (process.env.NODE_ENV !== "production") { + setDebugInfo(JSON.stringify(data, null, 2)); +}
239-253: Hide test user & debug panels outside developmentBoth the “测试用户信息” and “调试信息” panels are invaluable during
development but should be omitted in production bundles.-{debugInfo && ( +{process.env.NODE_ENV !== "production" && debugInfo && ( ... -)} +)}Apply a similar guard to the test-user panel above.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
.github/workflows/generate-client.yml(1 hunks).github/workflows/playwright.yml(1 hunks)frontend/app/dashboard/page.tsx(2 hunks)frontend/app/login/page.tsx(2 hunks)frontend/components/actions/items-action.ts(4 hunks)frontend/lib/auth.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/generate-client.yml
- .github/workflows/playwright.yml
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/app/login/page.tsx (3)
frontend/lib/auth.ts (1)
useAuth(26-157)frontend/components/actions/login-action.ts (1)
login(10-87)website/src/components/ui/card-hover-effect.tsx (1)
Card(9-32)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-backend
- GitHub Check: test-docker-compose
frontend/lib/auth.ts
Outdated
| // 添加调试信息，解析JWT令牌（不验证签名） | ||
| try { | ||
| const base64Url = token.split('.')[1]; | ||
| const base64 = base64Url.replace(/-/g, '+').replace(/_/g, '/'); | ||
| const jsonPayload = decodeURIComponent(atob(base64).split('').map(function(c) { | ||
| return '%' + ('00' + c.charCodeAt(0).toString(16)).slice(-2); | ||
| }).join('')); | ||
|
|
||
| console.log("JWT Payload:", JSON.parse(jsonPayload)); | ||
| } catch (e) { | ||
| console.error("Failed to decode JWT:", e); | ||
| } | ||
|
|
There was a problem hiding this comment.
Avoid logging sensitive JWT payloads in production
Decoding the JWT purely for console output is helpful during development, but risks exposing PII (email, user id, roles, etc.) in production logs or in the browser console of end-users.
Gate this block behind a development check so it never runs in production builds.
-// 添加调试信息，解析JWT令牌（不验证签名）
-try {
- const base64Url = token.split('.')[1];
- ...
- console.log("JWT Payload:", JSON.parse(jsonPayload));
-} catch (e) {
- console.error("Failed to decode JWT:", e);
-}
+// Dev-only: decode JWT without verifying signature
+if (process.env.NODE_ENV !== "production") {
+ try {
+ const base64Url = token.split(".")[1];
+ const base64 = base64Url.replace(/-/g, "+").replace(/_/g, "/");
+ const jsonPayload = decodeURIComponent(
+ atob(base64)
+ .split("")
+ .map((c) => "%" + ("00" + c.charCodeAt(0).toString(16)).slice(-2))
+ .join(""),
+ );
+ // eslint-disable-next-line no-console
+ console.log("JWT Payload:", JSON.parse(jsonPayload));
+ } catch (e) {
+ console.error("Failed to decode JWT:", e);
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 添加调试信息，解析JWT令牌（不验证签名） | |
| try { | |
| const base64Url = token.split('.')[1]; | |
| const base64 = base64Url.replace(/-/g, '+').replace(/_/g, '/'); | |
| const jsonPayload = decodeURIComponent(atob(base64).split('').map(function(c) { | |
| return '%' + ('00' + c.charCodeAt(0).toString(16)).slice(-2); | |
| }).join('')); | |
| console.log("JWT Payload:", JSON.parse(jsonPayload)); | |
| } catch (e) { | |
| console.error("Failed to decode JWT:", e); | |
| } | |
| // Dev-only: decode JWT without verifying signature | |
| if (process.env.NODE_ENV !== "production") { | |
| try { | |
| const base64Url = token.split(".")[1]; | |
| const base64 = base64Url.replace(/-/g, "+").replace(/_/g, "/"); | |
| const jsonPayload = decodeURIComponent( | |
| atob(base64) | |
| .split("") | |
| .map((c) => "%" + ("00" + c.charCodeAt(0).toString(16)).slice(-2)) | |
| .join(""), | |
| ); | |
| // eslint-disable-next-line no-console | |
| console.log("JWT Payload:", JSON.parse(jsonPayload)); | |
| } catch (e) { | |
| console.error("Failed to decode JWT:", e); | |
| } | |
| } |
🤖 Prompt for AI Agents
In frontend/lib/auth.ts around lines 44 to 56, the code logs the decoded JWT
payload which may expose sensitive user information in production. Wrap this
entire JWT decoding and console logging block inside a condition that checks if
the environment is development (e.g., using process.env.NODE_ENV or a similar
flag) so that it only runs during development and is skipped in production
builds.
frontend/app/dashboard/page.tsx
Outdated
| // 定义可能的API响应类型 | ||
| interface ApiErrorResponse { | ||
| message?: string; | ||
| error?: string; | ||
| meta?: { | ||
| message?: string; | ||
| [key: string]: unknown; | ||
| }; | ||
| [key: string]: unknown; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Deduplicate ApiErrorResponse – extract into a shared types module
ApiErrorResponse is now declared here and in
frontend/components/actions/items-action.ts.
Keeping separate but identical interfaces invites drift and increases
maintenance effort. Consider moving this definition to
@/lib/types, re-exporting it, and importing it in both places.
🤖 Prompt for AI Agents
In frontend/app/dashboard/page.tsx around lines 29 to 38, the ApiErrorResponse
interface is duplicated in another file. To avoid duplication and maintenance
issues, move the ApiErrorResponse interface definition into a shared module at
@/lib/types. Then, re-export it from there and update both this file and
frontend/components/actions/items-action.ts to import ApiErrorResponse from the
shared module instead of declaring it locally.
| interface ApiErrorResponse { | ||
| message?: string; | ||
| error?: string; | ||
| meta?: { | ||
| message?: string; | ||
| [key: string]: unknown; | ||
| }; | ||
| [key: string]: unknown; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Centralise ApiErrorResponse to avoid type drift
This interface duplicates the one defined in dashboard/page.tsx. Move
it to a common module (e.g. @/lib/types/api.ts) and import it here.
That guarantees one source of truth and avoids silent mismatches when
API error contracts change.
🤖 Prompt for AI Agents
In frontend/components/actions/items-action.ts around lines 27 to 35, the
ApiErrorResponse interface duplicates the one in dashboard/page.tsx. To avoid
type drift, move the ApiErrorResponse interface to a shared module such as
@/lib/types/api.ts, then import it in this file instead of redefining it. This
centralizes the type definition and ensures consistency across the codebase.
frontend/app/login/page.tsx
Outdated
| const [email, setEmail] = useState("[email protected]"); // 预填测试用户 | ||
| const [password, setPassword] = useState("password"); // 预填测试用户密码 | ||
| const [isLoading, setIsLoading] = useState(false); | ||
| const [error, setError] = useState(""); | ||
| const [debugInfo, setDebugInfo] = useState(""); | ||
|
|
There was a problem hiding this comment.
Remove hard-coded test credentials before shipping
Defaulting the email/password state to real test credentials increases
the risk of accidental credential leakage and may confuse users.
-const [email, setEmail] = useState("[email protected]"); // 预填测试用户
-const [password, setPassword] = useState("password"); // 预填测试用户密码
+const [email, setEmail] = useState("");
+const [password, setPassword] = useState("");📝 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 [email, setEmail] = useState("[email protected]"); // 预填测试用户 | |
| const [password, setPassword] = useState("password"); // 预填测试用户密码 | |
| const [isLoading, setIsLoading] = useState(false); | |
| const [error, setError] = useState(""); | |
| const [debugInfo, setDebugInfo] = useState(""); | |
| const [email, setEmail] = useState(""); | |
| const [password, setPassword] = useState(""); | |
| const [isLoading, setIsLoading] = useState(false); | |
| const [error, setError] = useState(""); | |
| const [debugInfo, setDebugInfo] = useState(""); |
🤖 Prompt for AI Agents
In frontend/app/login/page.tsx around lines 33 to 38, remove the hard-coded
default values for the email and password state variables to avoid exposing test
credentials. Initialize both email and password states as empty strings instead,
so users must input their own credentials.
- Made the item description optional in the Item interface to accommodate cases where it may not be provided. - Updated the rendering of item descriptions in the DashboardPage to display an empty string when the description is null or undefined, improving UI consistency.
…dling - Changed the frontend host in `.env.example` to reflect the new development port. - Updated the README to modify the command for running the frontend development server. - Removed deprecated Google OAuth route files and adjusted the callback handling in the Google OAuth process for improved error logging and user feedback. - Added session management middleware to the backend for better state handling during OAuth flows. - Enhanced the frontend login and callback pages to improve user experience and error handling during authentication.
…ware and components - Added detailed logging in the middleware to track access token validation and redirect logic. - Improved error handling in the Google login callback by utilizing the auth hook for token management. - Removed the deprecated set-token API route to streamline authentication processes. - Enhanced cookie management and debugging information in the customers page for better user experience during authentication checks.
- Introduced new Makefile targets for running extension unit and E2E tests. - Updated the Jest configuration to ignore E2E test paths. - Added Playwright as a development dependency for E2E testing. - Enhanced the Makefile to include extension tests in the overall testing process.
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (1)
frontend/app/login/page.tsx (1)
34-35: Remove hard-coded test credentialsPre-filling email and password fields with actual test credentials poses a security risk and could lead to unintended access in production environments.
- const [email, setEmail] = useState("[email protected]"); // 预填测试用户 - const [password, setPassword] = useState("password"); // 预填测试用户密码 + const [email, setEmail] = useState(""); + const [password, setPassword] = useState("");
🧹 Nitpick comments (18)
backend/app/core/config.py (3)
207-207: Remove unnecessary f-string prefix.This string doesn't contain any placeholders, so the 'f' prefix is unnecessary.
- logger.info(f"Make sure this matches your Google Console configuration") + logger.info("Make sure this matches your Google Console configuration")🧰 Tools
🪛 Ruff (0.8.2)
207-207: f-string without any placeholders
Remove extraneous
fprefix(F541)
214-214: Remove whitespace from blank line.There's unnecessary whitespace in this blank line which triggers a linting error.
- +🧰 Tools
🪛 Ruff (0.8.2)
214-214: Blank line contains whitespace
Remove whitespace from blank line
(W293)
258-258: Add newline at end of file.Add a trailing newline to fix the linting error.
-settings = Settings() +settings = Settings() +🧰 Tools
🪛 Ruff (0.8.2)
258-258: No newline at end of file
Add trailing newline
(W292)
extension/__tests__/e2e/popup.spec.ts (2)
4-28: Well-structured test setup with room for improvementThe test setup looks good overall, but I have some suggestions for making it more robust:
The implementation extracts the extension ID from the first background page, which assumes it's always available at index 0. Consider adding error handling for cases where no background pages exist.
The comment on line 4 is in Chinese. Consider translating it to English for consistency.
-// 测试扩展程序的弹出界面 +// Test for extension popup interface
68-92: Enhance login test with more robust assertionsThe login test could be improved by verifying more specific UI changes after login and adding error handling for form element detection.
test('should display login form and handle login', async () => { // 检查登录表单 const loginForm = await page.$('form'); if (loginForm) { // 填写表单 await page.fill('input[type="email"]', '[email protected]'); await page.fill('input[type="password"]', 'password123'); // 模拟服务器响应 await page.route('**/api/auth/login', route => { route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify({ success: true, token: 'fake-token' }) }); }); // 提交表单 await page.click('button[type="submit"]'); // 验证登录成功后的状态 - await page.waitForSelector('.home-page, .dashboard'); + await page.waitForSelector('.home-page, .dashboard', { timeout: 5000 }); + + // Verify that user-specific elements are visible + const userElements = await page.$('.user-info, .profile-section'); + expect(userElements).not.toBeNull(); + } else { + console.warn('Login form not found, skipping login test'); } });backend/app/api/routes/google_oauth.py (7)
18-19: Translate Chinese comments to English for consistencyThe comment on line 18 appears to be in Chinese. For consistency and maintainability, translate it to English.
-# u8bbeu7f6eu65e5u5fd7 +# Set up logging logger = logging.getLogger("app.google_oauth")
96-106: Clean up f-string and whitespace issuesThere are several issues with whitespace in blank lines and an f-string without placeholders. Also, it would be beneficial to translate the Chinese comment.
# Check if credentials are configured if not settings.GOOGLE_CLIENT_ID or not settings.GOOGLE_CLIENT_SECRET: logger.error("Google OAuth credentials not configured properly") return RedirectResponse( f"{settings.FRONTEND_HOST}/login?error=oauth_config_error" ) - -# u6253u5370u914du7f6eu4fe1u606fu4fbfu4e8eu8c03u8bd5 + +# Print configuration info for debugging logger.info(f"Google OAuth using redirect_uri: {settings.google_oauth_redirect_uri}") -logger.info(f"Make sure this matches the Authorized redirect URIs in Google Console") +logger.info("Make sure this matches the Authorized redirect URIs in Google Console")🧰 Tools
🪛 Ruff (0.8.2)
102-102: Blank line contains whitespace
Remove whitespace from blank line
(W293)
105-105: f-string without any placeholders
Remove extraneous
fprefix(F541)
106-106: Blank line contains whitespace
Remove whitespace from blank line
(W293)
110-113: Translate Chinese comment and clean up whitespaceTranslate the Chinese comment to English and remove unnecessary whitespace.
- -# u6253u5370u5f53u524du7684 session u72b6u6001 + +# Print the current session state logger.info(f"Generated OAuth state: {state}")🧰 Tools
🪛 Ruff (0.8.2)
110-110: Blank line contains whitespace
Remove whitespace from blank line
(W293)
141-142: Fix whitespace issueThere's unnecessary whitespace in the blank line.
logger.info(f"Received Google OAuth callback: code={bool(code)}, state={bool(state)}, error={error or 'None'}") - +🧰 Tools
🪛 Ruff (0.8.2)
142-142: Blank line contains whitespace
Remove whitespace from blank line
(W293)
152-153: Fix whitespace issueThere's unnecessary whitespace in the blank line.
logger.info(f"Validating state: received={state}, stored={stored_state}") - +🧰 Tools
🪛 Ruff (0.8.2)
153-153: Blank line contains whitespace
Remove whitespace from blank line
(W293)
175-176: Fix whitespace issueThere's unnecessary whitespace in the blank line.
- + logger.info(f"Exchanging code for token with redirect_uri: {settings.google_oauth_redirect_uri}")🧰 Tools
🪛 Ruff (0.8.2)
175-175: Blank line contains whitespace
Remove whitespace from blank line
(W293)
227-231: Improve error handling for better debuggingWhen logging exceptions, consider including more context to help debugging.
except Exception as e: - logger.exception(f"Google OAuth callback error: {str(e)}") + logger.exception(f"Google OAuth callback error during token exchange or user creation: {str(e)}") return RedirectResponse( f"{settings.FRONTEND_HOST}/login?error=server_error&message={str(e)}" )frontend/components/actions/google-auth-action.ts (1)
22-26: Remove redundant return statement after redirectThe
redirectfunction in Next.js throws an error to stop the execution, so the return statement after it is unnecessary.export async function processGoogleAuthToken(token: string) { if (!token) { redirect("/login?error=no_token"); - return; }extension/__tests__/routes/ui/button.spec.tsx (2)
7-17: Translate Chinese comments to English for consistencyThe comments in Chinese should be translated to English for better maintainability.
it('renders correctly with default props', () => { render(<Button>Click me</Button>); const button = screen.getByRole('button', { name: /click me/i }); expect(button).toBeInTheDocument(); - // 检查默认变体样式类是否应用 + // Check if default variant style classes are applied expect(button).toHaveClass('bg-primary'); expect(button).toHaveClass('text-primary-foreground'); });
90-96: Translate Chinese comments to English for consistencyThe comment in Chinese should be translated to English for better maintainability.
const button = screen.getByRole('button', { name: /custom class/i }); expect(button).toHaveClass('custom-class'); -expect(button).toHaveClass('bg-primary'); // 默认变体也应存在 +expect(button).toHaveClass('bg-primary'); // Default variant should also existextension/__tests__/utils/commons.spec.ts (1)
99-118: Test re-implements production algorithm – weakens the assertionThe expectation reproduces
toIsoString’s exact logic.
When a test duplicates the implementation it is supposed to verify, both can be wrong yet the test still passes.Consider validating only the externally observable contract instead, e.g. ISO-8601 structure and correct TZ sign:
-const expected = /* fully-computed string */ -expect(toIsoString(testDate)).toBe(expected); +const iso = toIsoString(testDate); +expect(iso).toMatch( + /^2023-05-15T\d{2}:\d{2}:\d{2}[+-]\d{2}:\d{2}$/ +);This keeps the test robust if the internal padding or concatenation changes, while still flagging behaviour regressions.
extension/__tests__/background/index.spec.ts (1)
56-66: Tests duplicate production logic instead of importing the real background scriptBy re-implementing the handlers (
onCreatedHandler,onUpdatedHandler, …) inside the test file, the suite can diverge from actual code and give false confidence.
Prefer importing the production module and triggering its side-effects; then you only stub external dependencies (chrome,Storage, etc.).This keeps tests aligned with real behaviour and reduces maintenance.
frontend/app/login/page.tsx (1)
62-76: Simplify error handling logic and improve security checksThe current implementation has multiple error conditions that could be consolidated. Also, consider validating token structure before storing.
const data = await response.json(); // 添加调试信息 - setDebugInfo(JSON.stringify(data, null, 2)); + if (process.env.NODE_ENV === "development") { + setDebugInfo(JSON.stringify(data, null, 2)); + } if (!response.ok) { throw new Error(data.detail || "登录失败，请检查用户名和密码"); } console.log("Login successful:", data); - // 确保响应包含access_token - if (!data.access_token) { - throw new Error("登录响应缺少访问令牌"); - } + // 验证访问令牌的存在和格式 + if (!data.access_token || typeof data.access_token !== "string" || data.access_token.length < 10) { + throw new Error("登录响应返回的访问令牌无效"); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (2)
backend/uv.lockis excluded by!**/*.lockextension/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
.cursor/rules/mcp-operation.mdc(2 hunks).env.example(1 hunks).github/workflows/release.yml(1 hunks)README.md(1 hunks)backend/app/api/routes/google_oauth.py(7 hunks)backend/app/core/config.py(3 hunks)backend/app/main.py(2 hunks)backend/pyproject.toml(2 hunks)extension/__tests__/background/index.spec.ts(1 hunks)extension/__tests__/basic.spec.ts(1 hunks)extension/__tests__/e2e/popup.spec.ts(1 hunks)extension/__tests__/routes/ui/button.spec.tsx(1 hunks)extension/__tests__/ui-basic.spec.tsx(1 hunks)extension/__tests__/utils/commons.spec.ts(1 hunks)extension/background/messages/google-auth.ts(3 hunks)extension/content-scripts/oauth-receiver.ts(1 hunks)extension/jest.config.js(1 hunks)extension/jest.setup.js(1 hunks)extension/package.json(2 hunks)extension/playwright.config.js(1 hunks)extension/tsconfig.test.json(1 hunks)frontend/.env.example(1 hunks)frontend/__tests__/google-auth.test.ts(0 hunks)frontend/app/api/auth/google/callback/route.ts(0 hunks)frontend/app/api/auth/google/route.ts(0 hunks)frontend/app/customers/page.tsx(1 hunks)frontend/app/login/google/callback/page.tsx(3 hunks)frontend/app/login/page.tsx(2 hunks)frontend/app/page.tsx(1 hunks)frontend/app/register/page.tsx(3 hunks)frontend/components/actions/google-auth-action.ts(1 hunks)frontend/components/actions/items-action.ts(4 hunks)frontend/lib/auth.ts(5 hunks)frontend/lib/clientConfig.ts(1 hunks)frontend/middleware.ts(1 hunks)
💤 Files with no reviewable changes (3)
- frontend/app/api/auth/google/route.ts
- frontend/tests/google-auth.test.ts
- frontend/app/api/auth/google/callback/route.ts
✅ Files skipped from review due to trivial changes (10)
- extension/tests/basic.spec.ts
- .cursor/rules/mcp-operation.mdc
- frontend/.env.example
- README.md
- backend/pyproject.toml
- .env.example
- extension/playwright.config.js
- extension/jest.setup.js
- extension/tsconfig.test.json
- extension/jest.config.js
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/components/actions/items-action.ts
- frontend/lib/auth.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
frontend/app/register/page.tsx (1)
frontend/components/actions/google-auth-action.ts (1)
initiateGoogleLogin(10-16)
extension/__tests__/background/index.spec.ts (1)
extension/utils/commons.ts (2)
initWebHistory(63-94)initQueues(6-52)
extension/__tests__/utils/commons.spec.ts (1)
extension/utils/commons.ts (4)
initQueues(6-52)initWebHistory(63-94)toIsoString(96-120)webhistoryToLangChainDocument(122-144)
backend/app/core/config.py (1)
frontend/lib/logger.ts (1)
logger(17-33)
🪛 Ruff (0.8.2)
backend/app/api/routes/google_oauth.py
102-102: Blank line contains whitespace
Remove whitespace from blank line
(W293)
105-105: f-string without any placeholders
Remove extraneous f prefix
(F541)
106-106: Blank line contains whitespace
Remove whitespace from blank line
(W293)
110-110: Blank line contains whitespace
Remove whitespace from blank line
(W293)
142-142: Blank line contains whitespace
Remove whitespace from blank line
(W293)
153-153: Blank line contains whitespace
Remove whitespace from blank line
(W293)
175-175: Blank line contains whitespace
Remove whitespace from blank line
(W293)
backend/app/core/config.py
207-207: f-string without any placeholders
Remove extraneous f prefix
(F541)
214-214: Blank line contains whitespace
Remove whitespace from blank line
(W293)
258-258: No newline at end of file
Add trailing newline
(W292)
🪛 GitHub Actions: Lint Backend
backend/app/core/config.py
[error] 196-196: mypy: Name "FRONTEND_HOST" already defined on line 47 [no-redef]
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-backend
- GitHub Check: test-docker-compose
🔇 Additional comments (32)
backend/app/core/config.py (3)
125-136: LGTM: Improved PostgresDsn construction.The refactoring of the PostgresDsn construction is a good practice, making the code more explicit about the string conversion step within the constructor.
Also applies to: 144-155
197-199: LGTM: Added backend API URL configuration.Adding the BACKEND_API_URL configuration variable is a good addition for centralizing the API URL configuration.
200-208: LGTM: Dynamic Google OAuth redirect URI with logging.The new property dynamically constructs the Google OAuth redirect URI using the BACKEND_API_URL, which is a good improvement over a static URI. The added logging helps with debugging configuration issues.
🧰 Tools
🪛 Ruff (0.8.2)
207-207: f-string without any placeholders
Remove extraneous
fprefix(F541)
frontend/app/register/page.tsx (3)
17-17: Good enhancement with server-side action for Google loginThe addition of server-side action handling via
initiateGoogleLoginimproves the implementation by centralizing OAuth flow logic on the server side.
89-89: Well-implemented Google login delegationThe click handler now properly delegates to the server action instead of handling redirection directly in the client. This improves security and maintainability.
119-119: Appropriate internationalization of button textThe button text is now in Chinese ("使用 Google 账号注册" - "Sign up with Google account"), which improves the user experience for Chinese-speaking users.
extension/__tests__/ui-basic.spec.tsx (1)
1-11: Good basic test setup for React componentsThis is a well-structured baseline test that verifies the React Testing Library setup. It correctly tests component rendering and content verification.
The test follows best practices:
- Uses proper testing-library patterns with screen queries
- Tests both element presence and text content
- Uses clear test descriptions
A solid foundation for more complex UI tests.
backend/app/main.py (2)
28-28: Appropriate import for session managementThe SessionMiddleware import from Starlette complements the OAuth flow improvements mentioned in the PR objectives.
63-64: Well-implemented session middleware with secure key usageThe SessionMiddleware is correctly added with the app's SECRET_KEY from settings, which is a secure approach.
Note that the Chinese comment appropriately explains the purpose: "Add SessionMiddleware, secret_key is recommended to use settings.SECRET_KEY".
extension/package.json (2)
12-14: Good test script setupThe addition of standard Jest testing scripts enables proper CI integration and developer workflows.
42-45: Comprehensive testing dependenciesThe dependencies added form a robust testing ecosystem:
- React Testing Library for component testing
- Jest with TypeScript support
- MSW for API mocking
- Playwright for end-to-end testing
This enables thorough testing of the extension across different levels.
Also applies to: 46-47, 52-55, 58-59
frontend/app/login/google/callback/page.tsx (3)
10-10: Improved code organization.Moving the router declaration below searchParams is a good practice for organizing hook calls.
19-46: Great enhancement to error handling and token processing.The implementation now has comprehensive error handling with:
- Clear error routing with meaningful query parameters
- Try-catch block for token processing
- Timeout to ensure token is properly set before redirect
- Detailed console logging for debugging
These changes significantly improve the robustness of the authentication flow.
51-52: Improved localization for Chinese users.The UI text has been localized from English to Chinese, enhancing the user experience for Chinese-speaking users.
Also applies to: 63-63
extension/background/messages/google-auth.ts (3)
18-19: Explicit storage initialization is more maintainable.Explicitly initializing the storage with area: "local" clarifies where the token is being stored, making the code more maintainable.
35-48: Excellent fallback mechanism for token verification.The enhanced fallback logic:
- Logs warnings when verification has issues
- Stores the original token as a fallback
- Implements a recovery mode for 400+ status codes
- Provides clearer error handling
This increases resilience by allowing operation to continue even when backend verification encounters problems.
62-80: Robust error recovery strategy.The improved error handling:
- Stores both the original token and error information
- Uses an optimistic response strategy to allow the content script to continue
- Only returns failure when all recovery attempts fail
This makes the extension more resilient in varying network conditions and authentication scenarios.
frontend/app/customers/page.tsx (4)
26-36: Enhanced observability for authentication debugging.The detailed logging of cookie information and authentication state significantly improves observability, which is crucial for debugging authentication issues in production.
44-44: Important credentials inclusion for cross-domain requests.Adding
credentials: 'include'ensures cookies are sent with the request, which is essential for authentication when making cross-domain API calls.
47-60: Improved API response handling with detailed error logging.The enhanced response handling:
- Logs API response status codes
- Retrieves and logs error text from failed responses
- Sets comprehensive debug information for troubleshooting
This makes API errors much easier to diagnose and resolve.
67-85: Comprehensive debug information for authentication states.The expanded debug information now includes:
- Partially masked tokens (good security practice)
- Full cookie context
- Timestamps for error scenarios
- Structured error data
This provides a complete picture for troubleshooting authentication issues without exposing sensitive information.
frontend/middleware.ts (4)
6-11: Enhanced logging improves middleware observability.The added logging for request paths and token presence provides valuable context for debugging authentication flows.
13-17: User-friendly redirect with callback URL.Saving the original URL as a callback parameter allows users to be redirected back to their intended destination after login, enhancing user experience.
20-38: Robust asynchronous token validation.The refactored validation logic:
- Uses proper try-catch error handling
- Performs asynchronous token validation with readUserMe
- Provides detailed logging throughout the process
- Handles success and error paths explicitly
This significantly improves the reliability of the authentication middleware.
46-46: Complete route protection for authenticated paths.Updating the matcher to include both dashboard and customers paths ensures consistent authentication protection across all secured routes.
frontend/components/actions/google-auth-action.ts (1)
10-16: LGTM!The implementation of
initiateGoogleLoginis clean and well-documented. It correctly retrieves the backend API URL from environment variables and redirects to the backend Google OAuth endpoint.extension/__tests__/routes/ui/button.spec.tsx (3)
29-52: LGTM!The test for button variants is well-implemented, using rerender to efficiently test multiple variants. The assertions check for appropriate class names for each variant.
54-75: LGTM!The test for button sizes is well-structured and checks the appropriate class names for each size variant.
98-104: LGTM!The test for the disabled state is well-implemented, checking both the disabled attribute and the appropriate CSS class.
frontend/app/login/page.tsx (3)
16-29: Good implementation of SearchParamsHandler with useEffectThe SearchParamsHandler component is well implemented with proper dependency handling in useEffect. This is a good pattern for handling URL parameters in a server component context using suspense.
126-162: Well-designed Google OAuth button with proper accessibilityThe Google OAuth button implementation is visually appealing with good hover effects and proper SVG rendering. The component follows best practices for accessibility with appropriate element types and relative positioning.
178-238: Form implementation is well-structured with proper validationThe login form is well-structured with proper input validation, clear labeling, and effective state management. The loading state indicator on the button provides good user feedback during form submission.
| @@ -30,7 +30,7 @@ export default function Home() { | |||
| > | |||
| <FaGithub className="w-5 h-5 text-black dark:text-white" /> | |||
| <Link | |||
| href="https://github.com/vintasoftware/nextjs-fastapi-template" | |||
| href="https://github.com/https://github.com/telepace" | |||
There was a problem hiding this comment.
Fix the malformed GitHub URL.
The GitHub URL contains a duplication of "https://github.com/" which will result in a broken link.
- href="https://github.com/https://github.com/telepace"
+ href="https://github.com/telepace"📝 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.
| href="https://github.com/https://github.com/telepace" | |
| href="https://github.com/telepace" |
🤖 Prompt for AI Agents
In frontend/app/page.tsx at line 33, the GitHub URL is malformed due to
duplication of "https://github.com/". Remove the extra "https://github.com/" so
the href attribute correctly points to "https://github.com/telepace" to fix the
broken link.
| @@ -57,5 +57,5 @@ jobs: | |||
| run: | | |||
| echo "## 🚀 Release Summary" >> $GITHUB_STEP_SUMMARY | |||
| echo "Release draft created for version ${{ github.event.inputs.version }}." >> $GITHUB_STEP_SUMMARY | |||
| echo "Visit the [Releases section](https://github.com/vintasoftware/nextjs-fastapi-template/releases) to review and publish the release." >> $GITHUB_STEP_SUMMARY | |||
| echo "Visit the [Releases section](https://github.com/https://github.com/telepace/releases) to review and publish the release." >> $GITHUB_STEP_SUMMARY | |||
There was a problem hiding this comment.
Fix the malformed GitHub URL in the release summary.
The GitHub URL contains a duplication of "https://github.com/" which will result in a broken link.
- echo "Visit the [Releases section](https://github.com/https://github.com/telepace/releases) to review and publish the release." >> $GITHUB_STEP_SUMMARY
+ echo "Visit the [Releases section](https://github.com/telepace/releases) to review and publish the release." >> $GITHUB_STEP_SUMMARY📝 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.
| echo "Visit the [Releases section](https://github.com/https://github.com/telepace/releases) to review and publish the release." >> $GITHUB_STEP_SUMMARY | |
| echo "Visit the [Releases section](https://github.com/telepace/releases) to review and publish the release." >> $GITHUB_STEP_SUMMARY |
🤖 Prompt for AI Agents
In .github/workflows/release.yml at line 60, the GitHub URL in the release
summary is malformed due to a duplicated "https://github.com/". Remove the extra
"https://github.com/" so the URL correctly points to
"https://github.com/telepace/releases" to fix the broken link.
| "http://localhost:8000/api/v1/login/google/callback" | ||
| ) | ||
| FRONTEND_URL: str = "http://localhost:3000" | ||
| FRONTEND_HOST: str = "http://localhost:3000" |
There was a problem hiding this comment.
Remove the duplicate FRONTEND_HOST declaration.
This variable is already defined on line 47, causing a pipeline failure with the error: "Name 'FRONTEND_HOST' already defined on line 47".
- FRONTEND_HOST: str = "http://localhost:3000"📝 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.
| FRONTEND_HOST: str = "http://localhost:3000" |
🧰 Tools
🪛 GitHub Actions: Lint Backend
[error] 196-196: mypy: Name "FRONTEND_HOST" already defined on line 47 [no-redef]
🤖 Prompt for AI Agents
In backend/app/core/config.py at line 196, remove the duplicate declaration of
FRONTEND_HOST since it is already defined at line 47. Keep only the original
declaration at line 47 and delete the one at line 196 to resolve the naming
conflict and pipeline failure.
| test('should navigate between pages', async () => { | ||
| // 如果有导航元素，测试导航功能 | ||
| const hasNavigation = await page.$('nav, .navigation'); | ||
|
|
||
| if (hasNavigation) { | ||
| // 点击导航项 | ||
| await page.click('nav a:first-child'); | ||
|
|
||
| // 检查URL或页面内容变化 | ||
| await page.waitForNavigation(); | ||
|
|
||
| // 导航回主页 | ||
| await page.click('a[href="/"]'); | ||
| await page.waitForNavigation(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve navigation test reliability
The navigation test uses a conditional approach that silently skips the test if navigation elements don't exist, which could hide potential issues. Additionally, using waitForNavigation() without a timeout can lead to flaky tests.
test('should navigate between pages', async () => {
- // 如果有导航元素，测试导航功能
const hasNavigation = await page.$('nav, .navigation');
if (hasNavigation) {
// 点击导航项
await page.click('nav a:first-child');
// 检查URL或页面内容变化
- await page.waitForNavigation();
+ await page.waitForNavigation({ timeout: 5000 }).catch(e => {
+ throw new Error(`Navigation failed: ${e.message}`);
+ });
// 导航回主页
await page.click('a[href="/"]');
- await page.waitForNavigation();
+ await page.waitForNavigation({ timeout: 5000 }).catch(e => {
+ throw new Error(`Navigation back to home failed: ${e.message}`);
+ });
+ } else {
+ console.warn('No navigation elements found, skipping navigation test');
}
});📝 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.
| test('should navigate between pages', async () => { | |
| // 如果有导航元素，测试导航功能 | |
| const hasNavigation = await page.$('nav, .navigation'); | |
| if (hasNavigation) { | |
| // 点击导航项 | |
| await page.click('nav a:first-child'); | |
| // 检查URL或页面内容变化 | |
| await page.waitForNavigation(); | |
| // 导航回主页 | |
| await page.click('a[href="/"]'); | |
| await page.waitForNavigation(); | |
| } | |
| }); | |
| test('should navigate between pages', async () => { | |
| const hasNavigation = await page.$('nav, .navigation'); | |
| if (hasNavigation) { | |
| // 点击导航项 | |
| await page.click('nav a:first-child'); | |
| // 检查URL或页面内容变化 | |
| await page.waitForNavigation({ timeout: 5000 }).catch(e => { | |
| throw new Error(`Navigation failed: ${e.message}`); | |
| }); | |
| // 导航回主页 | |
| await page.click('a[href="/"]'); | |
| await page.waitForNavigation({ timeout: 5000 }).catch(e => { | |
| throw new Error(`Navigation back to home failed: ${e.message}`); | |
| }); | |
| } else { | |
| console.warn('No navigation elements found, skipping navigation test'); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In extension/__tests__/e2e/popup.spec.ts around lines 51 to 66, the navigation
test conditionally skips if navigation elements are missing, which can hide
issues, and uses waitForNavigation() without a timeout, risking flaky tests.
Modify the test to assert the presence of navigation elements explicitly to fail
if missing, and add a reasonable timeout to waitForNavigation() calls to improve
test reliability.
| const dashboardUrl = "http://localhost:3000/dashboard" | ||
|
|
||
| console.log("Attempting to navigate to dashboard:", dashboardUrl) | ||
|
|
||
| // Try multiple navigation methods | ||
| try { | ||
| // Method 1: Direct location change | ||
| window.location.href = dashboardUrl | ||
| } catch (err) { | ||
| console.log("Direct navigation failed, trying window.open", err) | ||
|
|
||
| try { | ||
| // Method 2: Open in new tab | ||
| const newTab = window.open(dashboardUrl, "_self") | ||
|
|
||
| if (!newTab) { | ||
| throw new Error("Could not open new tab") | ||
| } | ||
| } catch (err2) { | ||
| console.error("All automatic navigation methods failed", err2) | ||
|
|
||
| // Method 3: Show clickable link | ||
| document.body.innerHTML = ` | ||
| <div style="font-family: Arial, sans-serif; text-align: center; padding: 50px;"> | ||
| <h1>Navigation Failed</h1> | ||
| <p>We couldn't automatically redirect you to the dashboard.</p> | ||
| <p>Please click the button below:</p> | ||
| <a href="${dashboardUrl}" style="display: inline-block; margin-top: 20px; padding: 10px 20px; background-color: #4285f4; color: white; text-decoration: none; border-radius: 4px; font-weight: bold;"> | ||
| Go to Dashboard | ||
| </a> | ||
| </div> | ||
| ` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-coded localhost dashboard URL limits production usability
navigateToDashboard always redirects to http://localhost:3000/dashboard, which will break for end-users or staging/production deployments.
Make the URL configurable, e.g. via:
process.env.NEXUS_DASHBOARD_URLat build time.- Reading
chrome.runtime.getManifest().homepage_url. - A relative path (
/dashboard) if same-origin.
-const dashboardUrl = "http://localhost:3000/dashboard"
+const dashboardUrl =
+ process.env.DASHBOARD_URL ??
+ chrome.runtime.getURL("/dashboard") ??
+ "/dashboard"📝 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 dashboardUrl = "http://localhost:3000/dashboard" | |
| console.log("Attempting to navigate to dashboard:", dashboardUrl) | |
| // Try multiple navigation methods | |
| try { | |
| // Method 1: Direct location change | |
| window.location.href = dashboardUrl | |
| } catch (err) { | |
| console.log("Direct navigation failed, trying window.open", err) | |
| try { | |
| // Method 2: Open in new tab | |
| const newTab = window.open(dashboardUrl, "_self") | |
| if (!newTab) { | |
| throw new Error("Could not open new tab") | |
| } | |
| } catch (err2) { | |
| console.error("All automatic navigation methods failed", err2) | |
| // Method 3: Show clickable link | |
| document.body.innerHTML = ` | |
| <div style="font-family: Arial, sans-serif; text-align: center; padding: 50px;"> | |
| <h1>Navigation Failed</h1> | |
| <p>We couldn't automatically redirect you to the dashboard.</p> | |
| <p>Please click the button below:</p> | |
| <a href="${dashboardUrl}" style="display: inline-block; margin-top: 20px; padding: 10px 20px; background-color: #4285f4; color: white; text-decoration: none; border-radius: 4px; font-weight: bold;"> | |
| Go to Dashboard | |
| </a> | |
| </div> | |
| ` | |
| const dashboardUrl = | |
| process.env.DASHBOARD_URL ?? | |
| chrome.runtime.getURL("/dashboard") ?? | |
| "/dashboard" | |
| console.log("Attempting to navigate to dashboard:", dashboardUrl) | |
| // Try multiple navigation methods | |
| try { | |
| // Method 1: Direct location change | |
| window.location.href = dashboardUrl | |
| } catch (err) { | |
| // … rest of snippet unchanged … |
🤖 Prompt for AI Agents
In extension/content-scripts/oauth-receiver.ts around lines 136 to 167, the
dashboard URL is hard-coded to "http://localhost:3000/dashboard", which is not
suitable for production or other environments. To fix this, make the dashboard
URL configurable by first checking for a build-time environment variable like
process.env.NEXUS_DASHBOARD_URL, then fallback to reading
chrome.runtime.getManifest().homepage_url if available, and finally default to a
relative path "/dashboard" if the previous options are not set. Replace the
hard-coded URL with this configurable approach to ensure the navigation works
correctly across different deployment environments.
| // 添加调试信息 | ||
| setDebugInfo(JSON.stringify(data, null, 2)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider limiting debug information in production
Exposing raw API response JSON could potentially leak sensitive information. Consider removing this or implementing environment-based conditional rendering.
- // 添加调试信息
- setDebugInfo(JSON.stringify(data, null, 2));
+ // Only show debug info in development environment
+ if (process.env.NODE_ENV === "development") {
+ setDebugInfo(JSON.stringify(data, null, 2));
+ }📝 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.
| // 添加调试信息 | |
| setDebugInfo(JSON.stringify(data, null, 2)); | |
| // Only show debug info in development environment | |
| if (process.env.NODE_ENV === "development") { | |
| setDebugInfo(JSON.stringify(data, null, 2)); | |
| } |
🤖 Prompt for AI Agents
In frontend/app/login/page.tsx around lines 64 to 65, the code sets debug
information by stringifying the raw API response data, which may expose
sensitive information in production. Modify this to conditionally set debug info
only in development environments by checking the environment variable (e.g.,
process.env.NODE_ENV) before calling setDebugInfo. Remove or disable this debug
output in production builds to prevent leaking sensitive data.
frontend/app/login/page.tsx
Outdated
| {debugInfo && ( | ||
| <div className="mt-4 p-2 border border-gray-200 dark:border-gray-700 rounded bg-gray-50 dark:bg-gray-800"> | ||
| <p className="text-xs text-gray-500 dark:text-gray-400 mb-1">调试信息：</p> | ||
| <pre className="text-xs overflow-auto max-h-32">{debugInfo}</pre> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Conditionally render debug information
Similar to the test user information, debug data should only be visible in development environments.
- {/* 调试信息 */}
- {debugInfo && (
+ {process.env.NODE_ENV === "development" && debugInfo && (
<div className="mt-4 p-2 border border-gray-200 dark:border-gray-700 rounded bg-gray-50 dark:bg-gray-800">
<p className="text-xs text-gray-500 dark:text-gray-400 mb-1">调试信息：</p>
<pre className="text-xs overflow-auto max-h-32">{debugInfo}</pre>
</div>
)}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/app/login/page.tsx around lines 249 to 254, the debug information is
currently rendered unconditionally when debugInfo exists. Update the conditional
rendering to also check if the environment is development, so that the debug
data is only visible in development environments. Use a suitable environment
variable or method to detect the development environment and wrap the debug info
rendering accordingly.
| // 修正登录端点 | ||
| const response = await fetch(`${apiUrl}/api/v1/login/access-token`, { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve login endpoint security using HTTPS
When constructing API URLs, ensure secure HTTPS connections are used for authentication endpoints, especially in production.
- // 修正登录端点
- const response = await fetch(`${apiUrl}/api/v1/login/access-token`, {
+ // 确保生产环境使用HTTPS连接
+ const secureApiUrl = process.env.NODE_ENV === "production"
+ ? apiUrl.replace(/^http:/, "https:")
+ : apiUrl;
+ const response = await fetch(`${secureApiUrl}/api/v1/login/access-token`, {📝 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 response = await fetch(`${apiUrl}/api/v1/login/access-token`, { | |
| // 确保生产环境使用HTTPS连接 | |
| const secureApiUrl = process.env.NODE_ENV === "production" | |
| ? apiUrl.replace(/^http:/, "https:") | |
| : apiUrl; | |
| const response = await fetch(`${secureApiUrl}/api/v1/login/access-token`, { |
🤖 Prompt for AI Agents
In frontend/app/login/page.tsx around lines 50 to 51, the login API URL is
constructed without explicitly enforcing HTTPS, which is critical for secure
authentication. Modify the code to ensure the URL uses HTTPS protocol,
especially in production environments, by either hardcoding 'https://' or
programmatically enforcing it based on environment variables. This change will
enhance the security of the login endpoint.
frontend/app/login/page.tsx
Outdated
| <div className="mt-4 p-2 border border-gray-200 dark:border-gray-700 rounded bg-gray-50 dark:bg-gray-800"> | ||
| <p className="text-xs text-gray-500 dark:text-gray-400 mb-1">测试用户信息：</p> | ||
| <p className="text-xs text-gray-700 dark:text-gray-300">邮箱: [email protected]</p> | ||
| <p className="text-xs text-gray-700 dark:text-gray-300">密码: password</p> | ||
| <p className="text-xs text-gray-700 dark:text-gray-300">用户ID: d17ab34d-b82f-4756-a315-82fca4264c4e</p> | ||
| </div> |
There was a problem hiding this comment.
Remove test user information from production UI
Displaying test credentials and user IDs in the UI creates security risks by exposing valid authentication information.
- {/* 测试用户信息 */}
- <div className="mt-4 p-2 border border-gray-200 dark:border-gray-700 rounded bg-gray-50 dark:bg-gray-800">
- <p className="text-xs text-gray-500 dark:text-gray-400 mb-1">测试用户信息：</p>
- <p className="text-xs text-gray-700 dark:text-gray-300">邮箱: [email protected]</p>
- <p className="text-xs text-gray-700 dark:text-gray-300">密码: password</p>
- <p className="text-xs text-gray-700 dark:text-gray-300">用户ID: d17ab34d-b82f-4756-a315-82fca4264c4e</p>
- </div>
+ {process.env.NODE_ENV === "development" && (
+ <div className="mt-4 p-2 border border-gray-200 dark:border-gray-700 rounded bg-gray-50 dark:bg-gray-800">
+ <p className="text-xs text-gray-500 dark:text-gray-400 mb-1">测试用户信息：</p>
+ <p className="text-xs text-gray-700 dark:text-gray-300">邮箱: [email protected]</p>
+ <p className="text-xs text-gray-700 dark:text-gray-300">密码: password</p>
+ <p className="text-xs text-gray-700 dark:text-gray-300">用户ID: d17ab34d-b82f-4756-a315-82fca4264c4e</p>
+ </div>
+ )}📝 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.
| <div className="mt-4 p-2 border border-gray-200 dark:border-gray-700 rounded bg-gray-50 dark:bg-gray-800"> | |
| <p className="text-xs text-gray-500 dark:text-gray-400 mb-1">测试用户信息：</p> | |
| <p className="text-xs text-gray-700 dark:text-gray-300">邮箱: [email protected]</p> | |
| <p className="text-xs text-gray-700 dark:text-gray-300">密码: password</p> | |
| <p className="text-xs text-gray-700 dark:text-gray-300">用户ID: d17ab34d-b82f-4756-a315-82fca4264c4e</p> | |
| </div> | |
| {process.env.NODE_ENV === "development" && ( | |
| <div className="mt-4 p-2 border border-gray-200 dark:border-gray-700 rounded bg-gray-50 dark:bg-gray-800"> | |
| <p className="text-xs text-gray-500 dark:text-gray-400 mb-1">测试用户信息：</p> | |
| <p className="text-xs text-gray-700 dark:text-gray-300">邮箱: [email protected]</p> | |
| <p className="text-xs text-gray-700 dark:text-gray-300">密码: password</p> | |
| <p className="text-xs text-gray-700 dark:text-gray-300">用户ID: d17ab34d-b82f-4756-a315-82fca4264c4e</p> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In frontend/app/login/page.tsx around lines 241 to 246, remove the entire div
block that displays test user information including email, password, and user ID
to prevent exposing sensitive test credentials in the production UI.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/openapi.json (1)
1-1: AddmaxItemsconstraints to array schemas for security.The minified OpenAPI specification lacks
maxItemsconstraints on several array schemas, which is a security concern that could lead to DoS vulnerabilities.As noted in previous reviews, please add appropriate
maxItemsconstraints to the following array paths:
components.schemas.HTTPValidationError.properties.detailcomponents.schemas.UsersPublic.properties.datacomponents.schemas.ValidationError.properties.locThis helps prevent potential DoS attacks where attackers could send or force the API to return excessively large arrays.
🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 1: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🧹 Nitpick comments (6)
.github/workflows/generate-client.yml (2)
59-59: Remove trailing spaces
Line 59 contains trailing spaces which cause a YAML lint error. Please delete the extra whitespace at end-of-line.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 59-59: trailing spaces
(trailing-spaces)
60-63: Install Admin dependencies before generation
Adding theInstall Admin Dependenciesstep ensures theadminworkspace has its dependencies, matching the frontend setup. Consider also cachingadmin/pnpm-lock.yamlundercache-dependency-pathfor faster CI runs.scripts/generate-admin-client.sh (2)
34-35: Remove unnecessary blank lines
Lines 34–35 are empty and can be collapsed to improve script readability.
36-44: Ensure admin dependencies are present
The check fornode_modulesand conditionalpnpm installis a robust guard. For reproducible builds, consider usingpnpm install --frozen-lockfileto honor the lockfile exactly.frontend/start.sh (1)
19-21: Use exec for the watcher process to handle signals.Consider replacing:
node watcher.jswith:
exec node watcher.jsso that the watcher process becomes PID 1 in this script and correctly receives termination signals for graceful shutdown.
frontend/Dockerfile (1)
18-22: Consolidate chmod commands to reduce image layers.Combine the two chmod operations into a single
RUNto minimize Docker image layers:- RUN chmod +x start.sh - RUN chmod +x watcher.js + RUN chmod +x start.sh watcher.js
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
extension/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.github/workflows/generate-client.yml(3 hunks)Makefile(3 hunks)admin/src/client/sdk.gen.ts(3 hunks)admin/src/client/types.gen.ts(0 hunks)extension/jest.config.js(1 hunks)extension/package.json(3 hunks)frontend/Dockerfile(1 hunks)frontend/app/openapi-client/sdk.gen.ts(3 hunks)frontend/app/openapi-client/types.gen.ts(0 hunks)frontend/openapi.json(1 hunks)frontend/start.sh(1 hunks)scripts/generate-admin-client.sh(1 hunks)
💤 Files with no reviewable changes (2)
- frontend/app/openapi-client/types.gen.ts
- admin/src/client/types.gen.ts
✅ Files skipped from review due to trivial changes (1)
- admin/src/client/sdk.gen.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- extension/package.json
- extension/jest.config.js
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/generate-client.yml
[error] 59-59: trailing spaces
(trailing-spaces)
🪛 Checkov (3.2.334)
frontend/openapi.json
[MEDIUM] 1: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-docker-compose
- GitHub Check: test-backend
🔇 Additional comments (17)
.github/workflows/generate-client.yml (2)
35-35: Bump pnpm version for client generation
Updatingpnpmfrom version 8 to 9.9.0 is fine and ensures compatibility with the latest features.
101-103: Stage admin client files alongside frontend artifacts
Includingadmin/src/clientin staging aligns with the move to generate separate admin client code. This will commit both client bundles in one pass.frontend/start.sh (3)
3-7: Ensure executable permissions robustly.This block correctly checks and sets executable permissions on the script itself, ensuring it can run regardless of host or CI environment.
9-13: Validate watcher.js execution rights.Checking for the file’s existence and adding executable permissions prevents runtime failures due to missing execute flags.
15-17: Add informative startup logging.Echoing when the Next.js app starts improves visibility during container startup and debugging.
frontend/Dockerfile (1)
24-25: Drop to non-root user for security.Switching to the
nodeuser after setting permissions follows best practices to reduce the container’s attack surface.Makefile (5)
84-85: Include extension unit tests in the primary test target
Thetesttarget now depends onextension-test-unit, aligning extension unit tests with the overarching test suite.
401-402: Run extension tests in theextension-allworkflow
Addingextension-testtoextension-allensures that tests are executed alongside build and packaging steps for the browser extension.
422-426: Aggregate extension tests under a single target
The newextension-testtarget cleanly groups unit and E2E testing, improving maintainability and clarity.
427-432: Defineextension-test-unitfor Jest unit tests
Consistent with other component test targets, this runs Jest unit tests in the extension directory after ensuring pnpm is installed.
433-438: Defineextension-test-e2efor Playwright end-to-end tests
This target correctly invokes Playwright tests under the extension directory, completing the extension’s test coverage.frontend/openapi.json (2)
1-1: Confirm the OpenAPI specification minification is intentional.The entire OpenAPI specification has been minified to a single line, which is good for reducing file size and improving parse performance in production. This practice aligns with best practices for delivering smaller payloads.
🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 1: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
1-1:✅ Verification successful
Verify the new API response types are correctly defined.
The OpenAPI specification now includes new response types (
ApiResponse_ItemPublic_,ApiResponse_ItemsPublic_,ApiResponse_NoneType_) which align with the PR objectives to enhance type safety in API interactions.
🏁 Script executed:
#!/bin/bash # Extract and validate the new response types from the OpenAPI specification cat frontend/openapi.json | jq '.components.schemas | with_entries(select(.key | contains("ApiResponse")))' | jq -cLength of output: 969
API Response Types Defined and Referenced Correctly
Verified that the following response wrapper schemas are present under
components.schemasand are correctly used in the endpoint responses:
ApiResponse_ItemPublic_ApiResponse_ItemsPublic_ApiResponse_NoneType_All three definitions include the expected
data,meta, anderrorproperties, and the item-related paths (/api/v1/items/,/api/v1/items/{id}) correctly reference these schemas. No further changes are required.🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 1: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
frontend/app/openapi-client/sdk.gen.ts (4)
513-513: Improved documentation for backward compatibility.The added clarification about backward compatibility helps developers understand that this endpoint is maintained for legacy reasons but not used in the new flow.
533-533: Enhanced Google Login documentation.The added clarification about the redirection behavior of this endpoint improves developer understanding of the OAuth flow.
551-551: Better documentation for Google Callback endpoint.The added explanation clearly indicates that this endpoint is called by Google after user authentication, which helps developers understand the OAuth flow sequence.
510-564:✅ Verification successful
Verify consistency between client SDK and backend implementation.
The enhanced documentation for Google OAuth endpoints should be aligned with the actual implementation in the backend.
🏁 Script executed:
#!/bin/bash # Check for consistency between SDK documentation and backend implementation echo "Checking for Google OAuth route implementations in the backend..." rg -A 5 "google.*callback|login/google" backend/app/api/routes/Length of output: 2107
Google OAuth endpoints aligned between client SDK and backend
Confirmed that the SDK methods in frontend/app/openapi-client/sdk.gen.ts match the backend routes in backend/app/api/routes/google_oauth.py:
- googleOauthGoogleCallbackApi → POST /auth/google-callback
- googleOauthGoogleLogin → GET /login/google
- googleOauthGoogleCallback → GET /login/google/callback
No inconsistencies found.
- Deleted unused environment example, content scripts, and stylesheets to streamline the extension. - Updated .gitignore to exclude local environment files. - Refactored the popup component to utilize a new structure for better maintainability. - Removed legacy test files and configurations to clean up the codebase. - Enhanced package.json with updated dependencies and permissions for improved functionality.
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (29)
extension/components/ui/button.tsx (1)
40-48: Remove unusedasChildprop.The
asChildprop is destructured but never used in the component since the conditional rendering withSlotwas removed.- ({ className, variant, size, asChild = false, ...props }, ref) => { + ({ className, variant, size, ...props }, ref) => {extension/README.md (1)
1-34: Improve README formatting and grammar.The simplified README provides good basic guidance for using the Plasmo framework, but has several minor grammar and formatting issues.
- This is a [Plasmo extension](https://docs.plasmo.com/) project bootstrapped with [`plasmo init`](https://www.npmjs.com/package/plasmo). + This is a [Plasmo extension](https://docs.plasmo.com/) project bootstrapped with [`plasmo init`](https://www.npmjs.com/package/plasmo). ## Getting Started First, run the development server: ```bash pnpm dev # or npm run dev
- Open your browser and load the appropriate development build. For example, if you are developing for the chrome browser, using manifest v3, use:
build/chrome-mv3-dev.
- Open your browser and load the appropriate development build. For example, if you are developing for the Chrome browser, using manifest v3, use:
build/chrome-mv3-dev.
- You can start editing the popup by modifying
popup.tsx. It should auto-update as you make changes. To add an options page, simply add aoptions.tsxfile to the root of the project, with a react component default exported. Likewise to add a content page, add acontent.tsfile to the root of the project, importing some module and do some logic, then reload the extension on your browser.
- You can start editing the popup by modifying
popup.tsx. It should auto-update as you make changes. To add an options page, simply add aoptions.tsxfile to the root of the project, with a React component default exported. Likewise, to add a content page, add acontent.tsfile to the root of the project, importing some module and doing some logic, then reload the extension on your browser.For further guidance, visit our Documentation
Making production build
Run the following:
pnpm build # or npm run buildThis should create a production bundle for your extension, ready to be zipped and published to the stores.
Submit to the webstores
- The easiest way to deploy your Plasmo extension is to use the built-in bpp GitHub action. Prior to using this action however, make sure to build your extension and upload the first version to the store to establish the basic credentials. Then, simply follow this setup instruction and you should be on your way for automated submission!
- The easiest way to deploy your Plasmo extension is to use the built-in bpp GitHub action. Before using this action, however, make sure to build your extension and upload the first version to the store to establish the basic credentials. Then, simply follow this setup instruction and you should be on your way for automated submission!
<details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [grammar] ~13-~13: The proper noun “Chrome” (= software from Google) needs to be capitalized. Context: ... example, if you are developing for the chrome browser, using manifest v3, use: `build... (GOOGLE_PRODUCTS) --- [grammar] ~15-~15: “React” is a proper noun and needs to be capitalized. Context: ...file to the root of the project, with a react component default exported. Likewise to... (A_GOOGLE) --- [uncategorized] ~15-~15: A comma may be missing after the conjunctive/linking adverb ‘Likewise’. Context: ...ith a react component default exported. Likewise to add a content page, add a `content.t... (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA) --- [uncategorized] ~15-~15: This verb may not be in the correct form. Consider using a different form for this context. Context: ...nt.ts` file to the root of the project, importing some module and do some logic, then rel... (AI_EN_LECTOR_REPLACEMENT_VERB_FORM) --- [uncategorized] ~15-~15: The grammatical number of this noun doesn’t look right. Consider replacing it. Context: ...the root of the project, importing some module and do some logic, then reload the exte... (AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER) --- [style] ~33-~33: ‘Prior to’ might be wordy. Consider a shorter alternative. Context: ...ps://bpp.browser.market) GitHub action. Prior to using this action however, make sure to... (EN_WORDINESS_PREMIUM_PRIOR_TO) --- [formatting] ~33-~33: Consider inserting a comma before ‘however’. Context: ...ket) GitHub action. Prior to using this action however, make sure to build your extension and u... (HOWEVER_MISSING_COMMA) </details> </details> </blockquote></details> <details> <summary>extension/components/Popup/ClipButton.tsx (2)</summary><blockquote> `26-32`: **Consider enhancing accessibility.** While the button implementation works well functionally, consider adding ARIA attributes for better accessibility. ```diff <Button className="w-full h-12 text-base mb-1" disabled={isSaving} onClick={onClip} + aria-busy={isSaving} + aria-label={isSaving ? "保存中..." : saveSuccess ? "已保存" : "一键剪藏到 Nexus"} > {isSaving ? "保存中..." : saveSuccess ? "✓ 已保存" : "一键剪藏到 Nexus"} </Button>
34-44: Make the action text more accessible.The text in the "打开侧边栏总结" button should be more accessible with proper button semantics.
{saveSuccess && ( <div className="text-xs text-center text-muted-foreground"> 已存入待看列表。需要 AI 处理？ - <button + <button + type="button" className="ml-1 text-primary hover:underline" + aria-label="打开侧边栏总结" onClick={onSummarize} > 打开侧边栏总结 </button> </div> )}extension/components/Popup/RecentItems.tsx (2)
21-26: Avoid duplicated functionality with different labels.The component has two buttons that call
onViewAllbut with different text labels. Consider consolidating this functionality or clarifying the purpose of each button if they're meant to be distinct.You could:
- Rename one of the handlers if they're intended for different purposes
- Extract the common button to a shared component
- Keep only one of the buttons if they truly perform the same action
If they're meant to have different behaviors, consider using more descriptive prop names like
onViewAllInAppandonOpenNexus.Also applies to: 49-54
21-26: Consider adding a rel attribute to external links.If the "view all" button opens external content, consider adding appropriate
relattributes.<button className="text-xs text-primary hover:underline" onClick={onViewAll} + rel="noopener" > 在应用中查看全部 </button>extension/styles/tailwind.css (1)
84-91: Consider making loading dots theme-aware.The loading dots currently have a fixed color (
#333), which might not be visible in dark mode. Consider using a theme variable instead..loading-dots div { - background-color: #333; + background-color: hsl(var(--foreground)); width: 8px; height: 8px; border-radius: 50%; margin: 0 4px; animation: dot-pulse 1.5s infinite ease-in-out; }extension/contents/oauth-receiver.ts (2)
51-52: Remove unnecessary empty export.The empty export at line 52 is unnecessary as there's another export (
config) at line 61.// 页面加载后执行处理 -export {} if (document.readyState === "complete") {
12-19: Validate token expiration calculation.The code calculates token expiration time, defaulting to 30 days if not provided. Consider validating that the
expiresInparameter is a valid number before parsing it.if (token && userId) { const storage = new Storage({ area: "local" }) // 计算过期时间 - const tokenExpiry = expiresIn - ? Date.now() + parseInt(expiresIn) * 1000 - : Date.now() + 30 * 24 * 60 * 60 * 1000 // 默认30天 + let tokenExpiry: number + if (expiresIn && !isNaN(parseInt(expiresIn))) { + tokenExpiry = Date.now() + parseInt(expiresIn) * 1000 + } else { + tokenExpiry = Date.now() + 30 * 24 * 60 * 60 * 1000 // 默认30天 + }extension/background/messages/notifications.ts (2)
22-31: DuplicatedonMessagelisteners can be merged to avoid double dispatchRegistering two independent listeners means every message traverses both callbacks, marginally hurting performance and making control-flow harder to read.
A single listener with aswitchkeeps the registry lean and prevents accidental fall-through when more actions are added.
24-30:return truewithout an asynchronoussendResponseis unnecessary
chrome.runtime.onMessageexpects a truthy return only when the listener will callsendResponseasynchronously.
Here nosendResponseis used, so returningundefinedis the correct, more explicit choice.extension/pages/options/index.tsx (2)
47-55: Surface save errors to the user via notificationsThe current implementation only logs to the console, which is invisible to most users.
Dispatching ashow_notificationmessage keeps UX consistent with the rest of the extension:} catch (error) { - console.error("保存设置失败:", error) + console.error("保存设置失败:", error) + chrome.runtime.sendMessage({ + action: "show_notification", + message: "保存设置失败，请重试。", + type: "error" + })
83-90: Update badge after logout to reflect cleared stateLogging out removes
userProfile, but pending clip-counts might also be purged server-side.
Trigger a badge refresh so the UI doesn’t keep stale numbers:await logout() setUserProfile(null) +chrome.runtime.sendMessage({ action: "updateBadgeCount" })extension/background/index.ts (3)
162-170: Collecting fullouterHTMLon everyonUpdatedfire is heavy and may violate privacy
document.documentElement.outerHTMLcan exceed several MB and includes user-sensitive data (e.g., inbox contents).
Consider:
- Pulling only
<title>and URL for history queues.- Sampling at most N pages per tab to bound storage.
- Requesting host permissions explicitly if full HTML is indispensable.
170-182: Simplify queue update with optional chaining & reduce duplicationStatic analysis flagged this block. You can push into the queue in one line and remove the preceding
ifguard:-const tabHistory = urlQueueListObj.urlQueueList.find( - (data) => data.tabsessionId === tabId -) - -if (tabHistory) { - tabHistory.urlQueue.push(pageData.url) - await storage.set("urlQueueList", urlQueueListObj) -} +urlQueueListObj?.urlQueueList + ?.find((d) => d.tabsessionId === tabId) + ?.urlQueue.push(pageData.url) + +await storage.set("urlQueueList", urlQueueListObj)Apply the same pattern to the time-queue block to keep both sections symmetric.
🧰 Tools
🪛 Biome (1.9.4)
[error] 171-171: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
124-139: AwaitingupdateBadgeCountin interval avoids overlapping executions
setInterval(updateBadgeCount, 60000)schedules the next run regardless of whether the previous promise has resolved, potentially causing race conditions with slow I/O.setInterval(async () => { - updateBadgeCount() + await updateBadgeCount() }, 60000)extension/content-scripts/content.tsx (3)
84-92:sidebarVisibleis written but never readThe flag is updated yet no subsequent logic depends on it.
Either remove the variable or use it (e.g. early-exit intoggleSidebar) to avoid dead code.
110-120: Hard-coded loading markup lacks styles and re-uses unsafeinnerHTML
showLoadingStateinjects a.loading-dotsskeleton but the CSS for the animation is missing, and
again uses an unsafe HTML string.Recommend abstracting a reusable
<LoadingDots/>component or building the nodes via
createElementto avoid repeated unsafeinnerHTML.
198-206: Alert-based validation degrades UX
alert()blocks the main thread and creates a poor experience.
Prefer a non-blocking toast viachrome.runtime.sendMessage({action:"show_notification", …})to
reuse your existing notification pipeline.extension/utils/commons.ts (2)
118-160:doc.bodycould benull; adopt optional chaining (static-analysis hint)When parsing arbitrary markup,
doc.bodymight be absent → runtime error.- return doc.body.textContent?.trim() || 'No content extracted' + return doc.body?.textContent?.trim() || 'No content extracted'🧰 Tools
🪛 Biome (1.9.4)
[error] 150-150: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
109-116:formatDateuses dd/mm/yyyy which is locale-ambiguousIf this timestamp is persisted or sent to APIs, prefer ISO-8601 (
yyyy-mm-dd) or include locale
awareness.-return `${day}/${month}/${year} ${hours}:${minutes}` +return `${year}-${month}-${day}T${hours}:${minutes}`extension/pages/popup/index.tsx (1)
77-88: Missing user feedback on clip errorsWhen
saveClippingthrows, the user sees no notification. Consider re-using the notification pattern
from the content script to surface the failure.catch (error) { console.error("剪藏错误:", error) + chrome.runtime.sendMessage({ + action: "show_notification", + message: "剪藏失败，请稍后重试", + type: "error" + }) } finally {extension/utils/api.ts (4)
20-46: Offline fallback for saveClipping is robust, but consider UUID for temp IDsThe offline fallback mechanism for saving clippings is well-implemented. However, using timestamp-based IDs could potentially lead to collisions if multiple clippings are created in the same millisecond.
Consider using a UUID library for generating temporary IDs:
- const tempId = `temp_${Date.now()}` + const tempId = `temp_${Date.now()}_${Math.random().toString(36).substring(2, 9)}`Additionally, you might want to implement a mechanism to sync these pending clippings when connectivity is restored.
48-64: Recommended: Add pagination support to getRecentClippingsThe function handles the basic case well, but as the application scales, you'll likely need pagination support beyond just a limit parameter.
Consider extending the function to support pagination:
-export async function getRecentClippings(limit: number = 5): Promise<ClippedItem[]> { +export async function getRecentClippings( + limit: number = 5, + page: number = 1 +): Promise<{ items: ClippedItem[], total: number }> { try { const headers = await getAuthHeaders() - const response = await fetch(`${API_BASE_URL}/api/clippings?limit=${limit}`, { + const response = await fetch(`${API_BASE_URL}/api/clippings?limit=${limit}&page=${page}`, { headers }) if (!response.ok) { throw new Error(`Failed to fetch clippings: ${response.statusText}`) } return await response.json() } catch (error) { console.error("Error fetching clippings:", error) - return [] + return { items: [], total: 0 } } }
95-116: Login function is well-implemented but missing return typeThe login function correctly handles authentication and storage of the user profile, but it's missing an explicit return type.
-export async function login(email: string, password: string): Promise<UserProfile> { +export async function login(email: string, password: string): Promise<UserProfile> {Additionally, consider validating inputs before making the API call.
118-130: Logout function handles errors gracefullyGood implementation that ensures the user profile is removed from storage regardless of API call success. However, add an explicit return type for consistency.
-export async function logout(): Promise<void> { +export async function logout(): Promise<void> {extension/utils/interfaces.ts (3)
7-18: ClippedItem interface is well-structured but consider enum for statusThe interface is comprehensive and covers all necessary fields for clipped content. However, the status field could benefit from using an enum for better code organization.
Consider extracting the status to an enum:
export enum ClippedItemStatus { Unread = 'unread', Reading = 'reading', Completed = 'completed' } export interface ClippedItem { // ...other fields status: ClippedItemStatus; // ...remaining fields }
20-27: UserSettings interface could use more specific language typeThe interface is well-defined, but
defaultLanguageas a generic string could be improved for better type safety.Consider using a more specific type for languages:
- defaultLanguage: string; + defaultLanguage: 'en' | 'fr' | 'es' | 'de' | 'zh' | 'ja' | string;This approach hints at common languages while still allowing any string value.
39-45: Consider stronger typing for AIResponse metadataThe interface is well-defined, but using
Record<string, any>for metadata is quite loose.If the structure of metadata is known or predictable for different AI response types, consider using more specific types:
export interface SummaryMetadata { wordCount: number; confidence: number; } export interface TranslationMetadata { sourceLanguage: string; targetLanguage: string; } export interface AIResponse { type: 'summary' | 'explanation' | 'translation' | 'highlights'; content: string; sourceText?: string; timestamp: number; metadata?: SummaryMetadata | TranslationMetadata | Record<string, unknown>; }Or use a discriminated union based on the type:
export type AIResponse = | { type: 'summary'; content: string; sourceText?: string; timestamp: number; metadata?: SummaryMetadata } | { type: 'translation'; content: string; sourceText?: string; timestamp: number; metadata?: TranslationMetadata } | { type: 'explanation' | 'highlights'; content: string; sourceText?: string; timestamp: number; metadata?: Record<string, unknown> };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (8)
extension/assets/brain.pngis excluded by!**/*.pngextension/assets/icon.pngis excluded by!**/*.pngextension/assets/new_icons/icon_128.pngis excluded by!**/*.pngextension/assets/new_icons/icon_16.pngis excluded by!**/*.pngextension/assets/new_icons/icon_32.pngis excluded by!**/*.pngextension/assets/new_icons/icon_48.pngis excluded by!**/*.pngextension/assets/new_icons/nexus_icon_128.pngis excluded by!**/*.pngextension/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (50)
extension/.env.example(0 hunks)extension/.github/workflows/submit.yml(1 hunks)extension/.gitignore(1 hunks)extension/IMPLEMENTATION_NOTES.md(0 hunks)extension/IMPROVEMENTS.md(0 hunks)extension/PUBLISHING.md(0 hunks)extension/README.md(1 hunks)extension/ROADMAP.md(1 hunks)extension/SPECIFICATION.md(0 hunks)extension/background/index.ts(1 hunks)extension/background/messages/notifications.ts(1 hunks)extension/background/messages/savedata.ts(1 hunks)extension/background/messages/savesnapshot.ts(1 hunks)extension/components/Popup/ActionButtons.tsx(1 hunks)extension/components/Popup/ClipButton.tsx(1 hunks)extension/components/Popup/RecentItems.tsx(1 hunks)extension/components/ui/button.tsx(3 hunks)extension/content-scripts/content.tsx(1 hunks)extension/content-scripts/oauth-receiver.ts(1 hunks)extension/content.ts(0 hunks)extension/contents/oauth-receiver.ts(1 hunks)extension/font.css(0 hunks)extension/options.tsx(1 hunks)extension/package.json(4 hunks)extension/pages/options/index.tsx(1 hunks)extension/pages/popup/index.tsx(1 hunks)extension/popup.tsx(1 hunks)extension/postcss.config.js(1 hunks)extension/routes/index.tsx(0 hunks)extension/routes/pages/ApiKeyForm.tsx(0 hunks)extension/routes/pages/HomePage.tsx(0 hunks)extension/routes/pages/Loading.tsx(0 hunks)extension/routes/pages/LoginForm.tsx(0 hunks)extension/routes/pages/RegisterForm.tsx(0 hunks)extension/routes/ui/command.tsx(0 hunks)extension/routes/ui/dialog.tsx(0 hunks)extension/routes/ui/popover.tsx(0 hunks)extension/routes/ui/toast.tsx(0 hunks)extension/routes/ui/toaster.tsx(0 hunks)extension/routes/ui/use-toast.tsx(0 hunks)extension/sidebar.tsx(1 hunks)extension/src/utils/api.ts(0 hunks)extension/styles/tailwind.css(1 hunks)extension/tailwind.config.js(3 hunks)extension/tailwind.css(0 hunks)extension/tsconfig.json(1 hunks)extension/utils/api.ts(1 hunks)extension/utils/cn.ts(1 hunks)extension/utils/commons.ts(1 hunks)extension/utils/interfaces.ts(1 hunks)
💤 Files with no reviewable changes (21)
- extension/.env.example
- extension/font.css
- extension/content.ts
- extension/routes/pages/Loading.tsx
- extension/routes/ui/toaster.tsx
- extension/routes/index.tsx
- extension/tailwind.css
- extension/routes/pages/LoginForm.tsx
- extension/routes/pages/RegisterForm.tsx
- extension/routes/ui/use-toast.tsx
- extension/routes/pages/ApiKeyForm.tsx
- extension/routes/ui/popover.tsx
- extension/PUBLISHING.md
- extension/IMPROVEMENTS.md
- extension/SPECIFICATION.md
- extension/IMPLEMENTATION_NOTES.md
- extension/routes/ui/dialog.tsx
- extension/routes/pages/HomePage.tsx
- extension/routes/ui/command.tsx
- extension/routes/ui/toast.tsx
- extension/src/utils/api.ts
✅ Files skipped from review due to trivial changes (11)
- extension/background/messages/savedata.ts
- extension/postcss.config.js
- extension/background/messages/savesnapshot.ts
- extension/.gitignore
- extension/ROADMAP.md
- extension/tsconfig.json
- extension/options.tsx
- extension/utils/cn.ts
- extension/sidebar.tsx
- extension/.github/workflows/submit.yml
- extension/components/Popup/ActionButtons.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- extension/package.json
- extension/content-scripts/oauth-receiver.ts
🧰 Additional context used
🧬 Code Graph Analysis (7)
extension/contents/oauth-receiver.ts (1)
extension/utils/interfaces.ts (1)
UserProfile(29-37)
extension/background/messages/notifications.ts (1)
extension/background/index.ts (1)
updateBadgeCount(123-140)
extension/background/index.ts (2)
extension/utils/interfaces.ts (2)
UserSettings(20-27)WebHistory(1-5)extension/utils/commons.ts (3)
initWebHistory(44-82)initQueues(84-107)getRenderedHtml(6-18)
extension/components/Popup/RecentItems.tsx (1)
extension/utils/interfaces.ts (1)
ClippedItem(7-18)
extension/pages/options/index.tsx (3)
extension/utils/interfaces.ts (2)
UserSettings(20-27)UserProfile(29-37)extension/utils/api.ts (1)
logout(118-130)extension/components/ui/button.tsx (1)
Button(53-53)
extension/utils/commons.ts (1)
extension/utils/interfaces.ts (1)
WebHistory(1-5)
extension/utils/api.ts (1)
extension/utils/interfaces.ts (3)
UserProfile(29-37)ClippedItem(7-18)AIResponse(39-45)
🪛 LanguageTool
extension/README.md
[grammar] ~13-~13: The proper noun “Chrome” (= software from Google) needs to be capitalized.
Context: ... example, if you are developing for the chrome browser, using manifest v3, use: `build...
(GOOGLE_PRODUCTS)
[grammar] ~15-~15: “React” is a proper noun and needs to be capitalized.
Context: ...file to the root of the project, with a react component default exported. Likewise to...
(A_GOOGLE)
[uncategorized] ~15-~15: A comma may be missing after the conjunctive/linking adverb ‘Likewise’.
Context: ...ith a react component default exported. Likewise to add a content page, add a `content.t...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~15-~15: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...nt.ts` file to the root of the project, importing some module and do some logic, then rel...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
[uncategorized] ~15-~15: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...the root of the project, importing some module and do some logic, then reload the exte...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[style] ~33-~33: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ...ps://bpp.browser.market) GitHub action. Prior to using this action however, make sure to...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[formatting] ~33-~33: Consider inserting a comma before ‘however’.
Context: ...ket) GitHub action. Prior to using this action however, make sure to build your extension and u...
(HOWEVER_MISSING_COMMA)
🪛 Biome (1.9.4)
extension/contents/oauth-receiver.ts
[error] 60-61: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
extension/background/index.ts
[error] 171-171: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
extension/utils/commons.ts
[error] 150-150: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-docker-compose
- GitHub Check: test-backend
🔇 Additional comments (19)
extension/popup.tsx (1)
1-14: Clean, simplified popup structure.The refactoring improves code organization by replacing previous components with a single
<PopupApp />component, which streamlines the extension's entry point.extension/tailwind.config.js (3)
4-13: Good expansion of content paths for comprehensive style processing.The expanded content paths ensure Tailwind correctly processes styles across all component directories, supporting the new structure of the extension.
59-61: Clean border radius definition.The border radius values have been simplified from template literals to plain strings, making the configuration more consistent.
65-70: Proper keyframe value types.Changing height from string
"0"to numeric0is better practice for CSS animations and improves type consistency.extension/components/ui/button.tsx (2)
3-3: Updated utility import path.The import path change for the
cnfunction reflects the reorganization of utility functions.
6-6: Simplified button styles.The button variant styles were streamlined by removing unnecessary classes like
gap-2and SVG-related styles.extension/components/Popup/ClipButton.tsx (1)
1-49: Well-structured React component with good state management.The
ClipButtoncomponent is well-designed with appropriate props, clean separation of concerns, and good conditional rendering. It handles different states (saving, success) effectively.extension/components/Popup/RecentItems.tsx (1)
1-66: Well-structured component with appropriate types and rendering logic.The component effectively uses the
ClippedIteminterface and implements conditional rendering based on the presence of items. The truncation of long titles and URLs is a good UX consideration.extension/styles/tailwind.css (2)
1-76: Well-structured Tailwind CSS configuration with good light/dark theme support.The CSS variables are properly defined with appropriate HSL values for colors. The light and dark theme configurations provide good contrast and consistent styling.
78-110: Nice loading animation implementation.The loading dots animation is well-designed with appropriate timing and visual feedback. The implementation is clean and reusable.
extension/contents/oauth-receiver.ts (1)
1-63: OAuth callback handler implementation is functional but could use improvements.The handler correctly processes OAuth parameters, updates user profile, and notifies the background script. The script also properly handles document ready states.
🧰 Tools
🪛 Biome (1.9.4)
[error] 60-61: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
extension/pages/popup/index.tsx (2)
108-114: Message actionprocessPageis unsupported by the content script
content.tsxonly handlesprocessSelection. Either update the content script or rename the action
to match to avoid silent no-ops.
118-123:openAIChataction not implementedSame mismatch as above. Confirm the intended behaviour or implement the handler in the content
script.extension/utils/api.ts (4)
1-6: API configuration looks good, but consider environment-specific URLsThe API base URL setup with environment variable fallback is a good practice. However, you might want to consider adding additional endpoints for development/staging environments.
7-18: Authentication header setup is clean and well-typedThe function correctly retrieves the user profile and sets appropriate headers. Good practice to check if the token exists before appending it.
66-93: Error handling in getAIInsight is well-implementedGood approach to return a fallback response object instead of throwing an error. This makes it easier for UI components to handle failed AI requests without additional try/catch blocks.
132-155: Registration function looks goodThe register function correctly handles the registration process with proper error handling.
extension/utils/interfaces.ts (2)
1-5: WebHistory interface improvement is more type-safeGood refactoring by replacing the generic
tabHistory: any[]with specificurlQueueand optionaltimeQueueproperties. This improves type safety and code readability.
29-37: UserProfile interface looks goodThe interface provides a comprehensive structure for user profile data with appropriate optional fields.
| <div | ||
| key={item.id} | ||
| className="p-2 border rounded hover:bg-secondary cursor-pointer" | ||
| onClick={() => onViewItem(item)} | ||
| > | ||
| <div className="truncate text-sm font-medium">{item.title}</div> | ||
| <div className="truncate text-xs text-muted-foreground">{item.url}</div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add keyboard accessibility for clickable items.
The items are clickable but don't have keyboard accessibility support. Consider making them focusable and handling keyboard interactions.
<div
key={item.id}
className="p-2 border rounded hover:bg-secondary cursor-pointer"
onClick={() => onViewItem(item)}
+ tabIndex={0}
+ role="button"
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ onViewItem(item);
+ }
+ }}
>
<div className="truncate text-sm font-medium">{item.title}</div>
<div className="truncate text-xs text-muted-foreground">{item.url}</div>
</div>📝 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.
| <div | |
| key={item.id} | |
| className="p-2 border rounded hover:bg-secondary cursor-pointer" | |
| onClick={() => onViewItem(item)} | |
| > | |
| <div className="truncate text-sm font-medium">{item.title}</div> | |
| <div className="truncate text-xs text-muted-foreground">{item.url}</div> | |
| </div> | |
| <div | |
| key={item.id} | |
| className="p-2 border rounded hover:bg-secondary cursor-pointer" | |
| onClick={() => onViewItem(item)} | |
| tabIndex={0} | |
| role="button" | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| onViewItem(item); | |
| } | |
| }} | |
| > | |
| <div className="truncate text-sm font-medium">{item.title}</div> | |
| <div className="truncate text-xs text-muted-foreground">{item.url}</div> | |
| </div> |
🤖 Prompt for AI Agents
In extension/components/Popup/RecentItems.tsx around lines 32 to 39, the
clickable div elements lack keyboard accessibility. To fix this, make each div
focusable by adding a tabIndex attribute set to 0, and add keyboard event
handlers to handle key presses like Enter and Space to trigger the onViewItem
function, ensuring keyboard users can interact with the items.
| } catch (error) { | ||
| console.error("OAuth处理错误:", error) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling and user feedback.
The current error handling only logs to the console. Consider providing user feedback and more detailed error handling.
} catch (error) {
console.error("OAuth处理错误:", error)
+ // 通知后台脚本授权失败
+ chrome.runtime.sendMessage({
+ action: "auth_failure",
+ error: error instanceof Error ? error.message : String(error)
+ })
+
+ // 显示错误给用户
+ const errorElement = document.createElement('div')
+ errorElement.style.cssText = 'position:fixed;top:0;left:0;right:0;padding:16px;background:#f44336;color:white;text-align:center;z-index:9999;'
+ errorElement.textContent = '登录失败，请重试'
+ document.body.appendChild(errorElement)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| console.error("OAuth处理错误:", error) | |
| } | |
| } catch (error) { | |
| console.error("OAuth处理错误:", error) | |
| // 通知后台脚本授权失败 | |
| chrome.runtime.sendMessage({ | |
| action: "auth_failure", | |
| error: error instanceof Error ? error.message : String(error) | |
| }) | |
| // 显示错误给用户 | |
| const errorElement = document.createElement('div') | |
| errorElement.style.cssText = 'position:fixed;top:0;left:0;right:0;padding:16px;background:#f44336;color:white;text-align:center;z-index:9999;' | |
| errorElement.textContent = '登录失败，请重试' | |
| document.body.appendChild(errorElement) | |
| } |
🤖 Prompt for AI Agents
In extension/contents/oauth-receiver.ts around lines 46 to 48, the catch block
only logs the error to the console without informing the user. Enhance error
handling by adding user feedback, such as displaying an error message or
notification in the UI, and consider handling specific error cases if applicable
to improve user experience.
| import("../index").then(({ updateBadgeCount }) => { | ||
| updateBadgeCount() | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling around dynamic import
import("../index") may fail if the background script is relocated by the bundler or blocked by CSP.
Consider wrapping the import in a try/catch and logging (or notifying) the user to avoid silent badge failures.
🤖 Prompt for AI Agents
In extension/background/messages/notifications.ts around lines 37 to 39, the
dynamic import of "../index" lacks error handling, which can cause silent
failures if the import is blocked or fails. Wrap the import call in a try/catch
block, and in the catch block log the error or notify the user to ensure any
import failure is detected and handled gracefully.
| const iconMap = { | ||
| success: "/assets/icon-check.png", | ||
| error: "/assets/icon-error.png", | ||
| info: "/assets/icon-info.png" | ||
| } |
There was a problem hiding this comment.
Use chrome.runtime.getURL to resolve icons inside packaged extensions
Hard-coding an absolute path (e.g. /assets/icon-check.png) frequently yields a “Could not load image” error once the extension is packed, because the browser will look for the image on disk, not inside the extension bundle.
-const iconMap = {
- success: "/assets/icon-check.png",
- error: "/assets/icon-error.png",
- info: "/assets/icon-info.png"
-}
+const iconMap = {
+ success: chrome.runtime.getURL("assets/icon-check.png"),
+ error: chrome.runtime.getURL("assets/icon-error.png"),
+ info: chrome.runtime.getURL("assets/icon-info.png")
+}🤖 Prompt for AI Agents
In extension/background/messages/notifications.ts around lines 6 to 10, the icon
paths are hard-coded as absolute strings, which causes loading errors in
packaged extensions. Replace the string paths with calls to
chrome.runtime.getURL for each icon path to correctly resolve the URLs inside
the extension bundle. This ensures the icons load properly regardless of the
extension packaging.
…ssibility and error handling - Deleted the SECURITY.md file to streamline documentation. - Enhanced ActionButtons and ClipButton components with a disabled state for better user experience. - Improved RecentItems component to handle empty states and display appropriate messages. - Updated PopupApp to check page access and handle errors gracefully, ensuring users receive feedback when actions cannot be performed.
- Modified jest.config.js to include Babel presets for improved TypeScript and React support. - Updated testPathIgnorePatterns to exclude specific test files from being run. - Enhanced test setup by mocking the @plasmohq/storage module for better isolation in tests. - Refactored Popup component tests for improved clarity and error handling. - Added utility functions for better handling of mock storage in tests. - Updated commons utility functions for better content extraction and URL validation.
There was a problem hiding this comment.
Actionable comments posted: 19
♻️ Duplicate comments (4)
extension/components/Popup/RecentItems.tsx (1)
46-50: 🛠️ Refactor suggestionAdd keyboard accessibility for clickable items.
The div elements that respond to click events should also be accessible via keyboard for users who cannot use a mouse. This ensures compliance with accessibility standards.
<div key={item.id} onClick={() => onViewItem(item)} className="p-2 rounded-md bg-gray-50 hover:bg-gray-100 cursor-pointer" + tabIndex={0} + role="button" + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + onViewItem(item); + } + }} >extension/pages/popup/index.tsx (2)
87-106: Content script message handling needs implementation.The "getPageContent" message is sent to the content script but there's no evidence that it's implemented there. This is a potential issue flagged in past reviews.
If the content script doesn't implement the "getPageContent" handler, the message will never receive a proper response, and the promise will resolve to an empty string. The proper approach would be to either:
- Implement the "getPageContent" handler in content.tsx, or
- Use chrome.scripting.executeScript to run extractMainContent directly in the page context
#!/bin/bash # Check if the content script implements the "getPageContent" handler rg "getPageContent.*case" -A 5 -B 5 extension/content-scripts/
109-109: 🛠️ Refactor suggestionIneffective fallback for content extraction.
When page content retrieval fails, the code fallbacks to a generic error message rather than attempting an alternative extraction method.
-const content = pageContent || "无法提取页面内容。" +// Use extractMainContent if available, otherwise fallback to error message +let content = pageContent; +if (!content) { + try { + // Get the HTML directly via chrome.scripting API to run in page context + const [{result}] = await chrome.scripting.executeScript({ + target: {tabId: tabs[0].id}, + func: () => document.documentElement.outerHTML + }); + content = extractMainContent(result); + } catch (error) { + console.error("备用内容提取失败:", error); + content = "无法提取页面内容。"; + } +}extension/utils/commons.ts (1)
38-38:⚠️ Potential issueTypo in referrer field name.
The metadata key is misspelled as "VisitedWebPageReffererURL" instead of "VisitedWebPageReferrerURL".
- VisitedWebPageReffererURL: item.reffererUrl || "", + VisitedWebPageReferrerURL: item.referrerUrl || "",
🧹 Nitpick comments (36)
.github/workflows/generate-client.yml (1)
59-59: Remove trailing whitespace
YAMLlint flagged trailing spaces on this blank line. Please remove the extra spaces to satisfy lint checks.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 59-59: trailing spaces
(trailing-spaces)
frontend/start.sh (3)
3-7: Redundant self-chmod check
Whenstart.shis launched viaCMD ["./start.sh"], it must already have executable permissions—this block will never be triggered in practice.Consider removing lines 3–7 to simplify the script.
1-2: Add strict error handling
To catch failures in any command (e.g.,chmod,pnpm start), add at the top:+set -euo pipefailThis ensures the script exits on errors and treats unset variables as failures.
Also applies to: 22-26
27-35: Gracefully forward signals to child processes
When youwait, SIGINT/SIGTERM might not be propagated to background jobs. Add a trap to kill all children on exit:+trap 'kill 0' EXIT INT TERMPlace this after
set -euo pipefailto ensure background processes (e.g., dev server, watcher) are terminated appropriately.frontend/Dockerfile (3)
1-14: Explicitly pin lockfile for dependency stage
Usingpnpm-lock.yaml*may inadvertently match backup lockfiles.Replace:
-COPY package.json pnpm-lock.yaml* ./ +COPY package.json pnpm-lock.yaml ./to ensure only the intended lockfile is used.
58-60: Avoid redundant chmod in runner
After removingwatcher.js, onlystart.shneeds exec bit.Simplify:
-RUN chmod +x start.sh watcher.js +RUN chmod +x start.sh
61-64: Prune development dependencies in production
You copy the fullnode_modulesfrom the builder (including dev dependencies). To minimize image size:Add a prune step before switching user:
RUN pnpm prune --prodThis reduces the footprint by removing dev-only packages.
extension/components/Popup/Popup.tsx (2)
45-47: Improve accessibility and internationalizationThe component currently has hardcoded Chinese text and minimal accessibility attributes. Consider extracting text to constants/i18n files and enhancing form accessibility.
- Add
aria-describedbyattributes to connect error messages with form fields- Implement form labels with better styling
- Consider extracting text to constants or i18n utilities for future localization
Also applies to: 49-75
77-94: Consider enhancing the clippings list UI and interactionThe current implementation provides basic rendering of clippings, but could benefit from additional interaction capabilities and improved display.
<ul> {clippings.map(clip => ( - <li key={clip.id} data-testid="clipping-item"> - <div className="title">{clip.title}</div> - <div>{clip.content.substring(0, 100)}...</div> + <li key={clip.id} data-testid="clipping-item" className="p-2 border-b border-gray-200 hover:bg-gray-50"> + <div className="font-medium text-blue-600">{clip.title}</div> + <div className="text-sm text-gray-600">{clip.content.length > 100 ? `${clip.content.substring(0, 100)}...` : clip.content}</div> + {clip.url && <div className="text-xs text-gray-400 truncate">{clip.url}</div>} </li> ))} </ul>extension/components/Popup/ClipButton.tsx (3)
20-20: Consider making the title truncation length configurableThe title truncation length is hardcoded to 30 characters. Consider making this a configurable prop with a default value.
interface ClipButtonProps { isSaving: boolean saveSuccess: boolean title: string onClip: () => void onSummarize: () => void disabled?: boolean + titleMaxLength?: number } const ClipButton: React.FC<ClipButtonProps> = ({ isSaving, saveSuccess, title, onClip, onSummarize, disabled = false, + titleMaxLength = 30 }) => { - const truncatedTitle = title.length > 30 ? `${title.substring(0, 30)}...` : title + const truncatedTitle = title.length > titleMaxLength ? `${title.substring(0, titleMaxLength)}...` : title
27-54: Use a utility like clsx/classnames for conditional class namesThe current implementation uses template literals with complex conditional logic for class names. Consider using a utility like
clsxorclassnamesto make the code more readable.+ import clsx from 'clsx'; // ... <button onClick={onClip} disabled={isSaving || saveSuccess || disabled} - className={`flex-1 py-2 px-3 rounded-md text-sm font-medium ${ - disabled - ? "bg-gray-300 text-gray-500 cursor-not-allowed" - : saveSuccess - ? "bg-green-500 text-white" - : "bg-blue-600 text-white hover:bg-blue-700" - }`} + className={clsx( + "flex-1 py-2 px-3 rounded-md text-sm font-medium", + { + "bg-gray-300 text-gray-500 cursor-not-allowed": disabled, + "bg-green-500 text-white": !disabled && saveSuccess, + "bg-blue-600 text-white hover:bg-blue-700": !disabled && !saveSuccess + } + )}Apply similar changes to the summarize button as well.
42-53: Enhance accessibility for the summarize buttonThe summarize button has a title attribute for tooltip but could benefit from additional accessibility attributes.
<button onClick={onSummarize} disabled={disabled} className={`py-2 px-3 rounded-md text-sm font-medium ${ disabled ? "bg-gray-300 text-gray-500 cursor-not-allowed" : "bg-gray-100 text-gray-900 hover:bg-gray-200" }`} title="总结此页面" + aria-label="总结此页面" + type="button" > 总结 </button>extension/playwright.config.ts (1)
4-6: Consider adding comments to explain configuration choicesAdding explanatory comments would help developers understand the configuration decisions.
const config: PlaywrightTestConfig = { + // Path to e2e test files testDir: './e2e', + // Maximum timeout for test execution (30 seconds) timeout: 30000, + // Prevent tests marked with 'only' from running in CI forbidOnly: !!process.env.CI,extension/tests/test-utils.tsx (1)
4-9: Add type safety to the custom render functionThe custom render function should have stronger type safety and include documentation for future developers.
+/** + * Custom render function that wraps RTL's render function. + * This allows for future enhancements like adding providers. + */ const customRender = ( ui: ReactElement, options?: Omit<RenderOptions, 'wrapper'>, -) => rtlRender(ui, { ...options }); +): ReturnType<typeof rtlRender> => rtlRender(ui, { ...options });extension/components/Popup/RecentItems.tsx (1)
56-64: Refactor status text and color logic for better maintainability.The current implementation duplicates the status check logic for both color and text. Consider extracting this logic into variables or a helper function for better maintainability.
-<span className={` - ${item.status === 'unread' ? 'text-blue-600' : - item.status === 'reading' ? 'text-amber-600' : - 'text-green-600'} -`}> - {item.status === 'unread' ? '未读' : - item.status === 'reading' ? '阅读中' : - '已读完'} -</span> +<span className={getStatusColorClass(item.status)}> + {getStatusText(item.status)} +</span>You would then need to add these helper functions to your component:
const getStatusColorClass = (status: ClippedItem['status']) => { switch (status) { case 'unread': return 'text-blue-600'; case 'reading': return 'text-amber-600'; default: return 'text-green-600'; } }; const getStatusText = (status: ClippedItem['status']) => { switch (status) { case 'unread': return '未读'; case 'reading': return '阅读中'; default: return '已读完'; } };extension/components/Popup/ActionButtons.tsx (1)
23-27: Extract common button styles to reduce code duplication.The conditional styling logic is duplicated across all three buttons. Consider extracting this into a helper function or variable to maintain DRY principles.
+ const getButtonClass = (baseColor) => `p-2 rounded-md text-xs font-medium flex flex-col items-center ${ + disabled + ? "bg-gray-300 text-gray-500 cursor-not-allowed" + : `bg-${baseColor}-50 text-${baseColor}-700 hover:bg-${baseColor}-100` + }`; <button onClick={onSummarize} disabled={disabled} - className={`p-2 rounded-md text-xs font-medium flex flex-col items-center ${ - disabled - ? "bg-gray-300 text-gray-500 cursor-not-allowed" - : "bg-blue-50 text-blue-700 hover:bg-blue-100" - }`} + className={getButtonClass('blue')} >Apply the same pattern to the other buttons, replacing with
getButtonClass('purple')andgetButtonClass('green')respectively.Also applies to: 36-40, 49-53
extension/e2e/popup.spec.ts (1)
4-23: Refactor extension ID retrieval for more robust testing.The
getExtensionIdfunction is defined but commented out in use. Consider extracting this as a utility function that tries to get the ID dynamically but falls back to environment variables, making tests more robust.-// 获取扩展ID的辅助函数 -async function getExtensionId(page) { +// Utility to get extension ID - tries dynamic approach first, then falls back to env var +export async function getExtensionId(page) { await page.goto('chrome://extensions'); // 通过检查开发模式下加载的扩展来获取ID const devModeToggle = await page.$('extensions-manager').then( em => em.$('#devMode') ); await devModeToggle.click(); const extensionList = await page.$('extensions-manager').then( em => em.$('extensions-item-list') ); const extensionId = await extensionList.evaluate(list => { const item = list.shadowRoot.querySelector('extensions-item'); return item.dataset.extensionId; }); return extensionId; }Then in the test:
test.beforeEach(async ({ page, context }) => { - // 根据需要获取扩展ID并设置 - // const extensionId = await getExtensionId(page); - // 直接使用环境变量或设置的ID - const extensionId = process.env.EXTENSION_ID || '[extension-id]'; + // Try to get extension ID dynamically, fall back to env var + let extensionId; + try { + extensionId = await getExtensionId(page); + } catch (error) { + console.warn('Failed to get extension ID dynamically, using env var'); + extensionId = process.env.EXTENSION_ID || '[extension-id]'; + } // 打开扩展的popup页面 await page.goto(`chrome-extension://${extensionId}/popup.html`); });extension/__tests__/components/Popup/Popup.test.tsx (2)
92-94: Simplify element selection logic in tests.Using multiple selectors with OR operators (
||) makes the test logic harder to understand and potentially brittle. Consider using a more targeted approach withgetByRoleor creating custom queries that better represent your component's structure.-const emailInput = screen.getByLabelText(/邮箱/i) || screen.getByLabelText(/Email/i); -const passwordInput = screen.getByLabelText(/密码/i) || screen.getByLabelText(/Password/i); -const loginButton = screen.getByRole('button', { name: /登录/i }) || screen.getByRole('button', { name: /Login/i }); +// Create a custom query that tries multiple patterns +const getElementByMultiplePatterns = (screen, queryFn, patterns) => { + for (const pattern of patterns) { + try { + return queryFn(pattern); + } catch (e) { + // Continue to next pattern + } + } + throw new Error(`Failed to find element with patterns: ${patterns.join(', ')}`); +}; + +const emailInput = getElementByMultiplePatterns( + screen, + (pattern) => screen.getByLabelText(pattern), + [/邮箱/i, /Email/i] +); +const passwordInput = getElementByMultiplePatterns( + screen, + (pattern) => screen.getByLabelText(pattern), + [/密码/i, /Password/i] +); +const loginButton = getElementByMultiplePatterns( + screen, + (pattern) => screen.getByRole('button', { name: pattern }), + [/登录/i, /Login/i] +);Or a simpler approach:
-const emailInput = screen.getByLabelText(/邮箱/i) || screen.getByLabelText(/Email/i); -const passwordInput = screen.getByLabelText(/密码/i) || screen.getByLabelText(/Password/i); -const loginButton = screen.getByRole('button', { name: /登录/i }) || screen.getByRole('button', { name: /Login/i }); +// Use a function to get element by multiple labels +const byLabelTextMultiple = (patterns) => { + return (content, element) => { + return patterns.some(pattern => + content.match(pattern) && element.tagName === 'LABEL' + ); + }; +}; + +const emailInput = screen.getByText(byLabelTextMultiple([/邮箱/i, /Email/i])).nextElementSibling; +const passwordInput = screen.getByText(byLabelTextMultiple([/密码/i, /Password/i])).nextElementSibling; +const loginButton = screen.getByRole('button', { name: /登录|Login/i });This approach makes your test more reliable since it won't throw errors if one selector doesn't match.
127-129: Maintain consistent interaction style throughout tests.In some tests, you use
userEvent.type()for input interactions, while in others you usefireEvent.change(). For consistency and more realistic user simulation, preferuserEventthroughout.-fireEvent.change(emailInput, { target: { value: '[email protected]' } }); -fireEvent.change(passwordInput, { target: { value: 'wrongpassword' } }); -fireEvent.click(loginButton); +await userEvent.type(emailInput, '[email protected]'); +await userEvent.type(passwordInput, 'wrongpassword'); +await userEvent.click(loginButton);extension/__tests__/components/ui/Button.test.tsx (4)
11-11: Consider removing assumed class name commentsThe comment "假设默认类名" (assuming default class name) suggests uncertainty about the actual implementation. Consider replacing these assumptions with actual component class names from the implementation.
- expect(button).toHaveClass('btn'); // 假设默认类名 + expect(button).toHaveClass('btn');
18-18: Remove assumption comment for variant classSimilar to the above, remove the assumption comment about the primary variant class.
- expect(button).toHaveClass('btn-primary'); // 假设primary变体的类名 + expect(button).toHaveClass('btn-primary');
25-25: Remove assumption comments for size classesRemove the assumption comments about size-related classes.
- expect(button).toHaveClass('btn-sm'); // 假设small尺寸的类名 + expect(button).toHaveClass('btn-sm'); // And later: - expect(button).toHaveClass('btn-lg'); // 假设large尺寸的类名 + expect(button).toHaveClass('btn-lg');Also applies to: 29-29
76-76: Remove assumption comment for asChild classRemove the assumption comment about classes being applied to asChild elements.
- expect(link).toHaveClass('btn'); // 假设btn类也应用于asChild元素 + expect(link).toHaveClass('btn');extension/__tests__/utils/commons.test.ts (1)
90-90: Consider using exact date string matchingThe test uses a loose regex pattern (/2022.*06.*15/) which might not catch format issues. Consider using an exact match with the expected formatted date string.
- expect(formatRelativeTime(date.getTime())).toMatch(/2022.*06.*15/); // 格式可能因实现而异 + expect(formatRelativeTime(date.getTime())).toBe('2022-06-15'); // 使用精确匹配extension/e2e/content-script.spec.ts (2)
36-36: Replace fixed timeout with wait for conditionUsing a fixed timeout (1000ms) is brittle and may lead to flaky tests. Consider using a more reliable condition to verify content script readiness.
- // 确保内容脚本加载完成 - await page.waitForTimeout(1000); + // 等待内容脚本注入的标记元素或函数 + await page.waitForFunction(() => { + return window.hasOwnProperty('nexusExtensionLoaded') || + document.querySelector('.nexus-extension-loaded'); + }, { timeout: 5000 });
110-110: Use more specific selector for AI contentUsing a generic CSS class
.ai-contentcould be brittle if class names change. Consider using a more specific selector or data attribute.- await expect(page.locator('.ai-content')).toBeVisible(); + await expect(page.locator('[data-testid="ai-summary-content"]')).toBeVisible();extension/TESTING.md (1)
15-32: Add a language identifier to satisfy markdown-lint and improve readabilityThe directory-tree block (lines 15-32) has no language tag, so
markdownlint --rule MD040complains.
Declaring a dummy language such astext(orbash,plaintext) makes the linter happy and enables proper syntax highlighting.-``` +```text … -``` +```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
15-15: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
extension/__tests__/utils/api.test.ts (1)
70-71: Utility note – make sure mocks are reset between test suites
jest.clearAllMocks()resets spies on functions, but it does not clear the constructor override you create withjest.spyOn(..., 'Storage').mockImplementation.
Addjest.restoreAllMocks()afterclearAllMocks()(or inafterEach) to avoid cross-test leakage.Also applies to: 248-256, 513-517
extension/utils/api.ts (3)
24-27: Minor optimisation – check network status before the expensive storage look-up
getAuthHeaders()hits IndexedDB every time.
When the user is offline we abort anyway, so swapping the order avoids unnecessary I/O:- const headers = await getAuthHeaders() - - if (!navigator.onLine) { - throw new Error("离线状态") - } + if (!navigator.onLine) { + throw new Error("离线状态") + } + + const headers = await getAuthHeaders()
48-50: Guard against missingchrome.runtimein non-extension contexts
chrome.runtimeis undefined in Node/Jest and will raise a ReferenceError.
Wrap the message dispatch in a try/catch:- chrome.runtime.sendMessage({ action: "updateBadgeCount" }) + try { + chrome.runtime?.sendMessage?.({ action: "updateBadgeCount" }) + } catch (_) { + /* ignore – background not available */ + }This keeps the method usable in shared environments such as unit tests.
101-107: String construction is hard to maintain – extract to a lookup mapThe nested ternary produces hard-to-read and linter-warning code.
A small helper reduces cyclomatic complexity:- content: `处理${type === "summary" ? "总结" : - type === "explanation" ? "解释" : - type === "translation" ? "翻译" : "要点提取"}失败。${ - navigator.onLine ? "请稍后重试。" : "您当前处于离线状态。" - }`, + content: `处理${{ + summary: "总结", + explanation: "解释", + translation: "翻译", + highlights: "要点提取" + }[type]}失败。${navigator.onLine ? "请稍后重试。" : "您当前处于离线状态。"}`,extension/pages/popup/index.tsx (3)
131-133: Missing cleanup for setTimeout.The setTimeout is created but never cleared, which could lead to memory leaks if the component unmounts before timeout completes.
Consider storing the timeout ID in a ref and clearing it in a useEffect cleanup function:
+const saveSuccessTimerRef = useRef<number | null>(null); +// Clean up any pending timeouts when component unmounts +useEffect(() => { + return () => { + if (saveSuccessTimerRef.current) { + clearTimeout(saveSuccessTimerRef.current); + } + }; +}, []); // Later in the code: -setTimeout(() => { - setSaveSuccess(false) -}, 2000) +saveSuccessTimerRef.current = window.setTimeout(() => { + setSaveSuccess(false); + saveSuccessTimerRef.current = null; +}, 2000);
39-39: Localization opportunity for error messages.All error messages are hardcoded in Chinese, limiting accessibility for non-Chinese speakers.
Consider implementing a localization system that supports multiple languages:
+// Import or implement a localization utility +import { t } from "~/utils/i18n"; // Then replace hardcoded strings with localized versions -setError("无法在此页面使用扩展。请访问正常网页使用扩展功能。"); +setError(t("error.cannot_access_page"));Also applies to: 74-74, 150-150, 173-173, 199-199
123-128: Handling potential errors during communication with content script.The code sends a message but doesn't properly handle potential errors when the content script might not be initialized.
Consider implementing a more robust approach for handling content script communication:
if (settings?.openSidebarOnClip) { try { - chrome.tabs.sendMessage(tabs[0].id, { action: "summarizePage" }) + // Check if content script is loaded first or inject it + chrome.tabs.sendMessage( + tabs[0].id, + { action: "ping" }, + (response) => { + if (chrome.runtime.lastError) { + // Content script not loaded, inject it first + chrome.scripting.executeScript({ + target: { tabId: tabs[0].id }, + files: ["content-script.js"] + }, () => { + // Now try to send the message + chrome.tabs.sendMessage(tabs[0].id, { action: "summarizePage" }); + }); + } else { + // Content script is loaded, send message directly + chrome.tabs.sendMessage(tabs[0].id, { action: "summarizePage" }); + } + } + ); } catch (e) { console.warn("发送summarizePage消息失败:", e) } }extension/utils/commons.ts (2)
168-176: Enhance URL validation.The current URL validation simply checks for HTTP/HTTPS protocols but doesn't validate the structure further.
Consider strengthening the URL validation to check for common issues:
export function isValidUrl(url: string | null | undefined): boolean { if (!url) return false; try { const parsedUrl = new URL(url); - return ['http:', 'https:'].includes(parsedUrl.protocol); + // First, check the protocol + if (!['http:', 'https:'].includes(parsedUrl.protocol)) { + return false; + } + + // Check that there's a non-empty hostname + if (!parsedUrl.hostname || parsedUrl.hostname === '') { + return false; + } + + // Check for a valid domain pattern (at least one dot) + // This helps filter out invalid domains like "localhost" or "example" + if (!parsedUrl.hostname.includes('.') && parsedUrl.hostname !== 'localhost') { + return false; + } + + return true; } catch { return false; } }
178-180: Improve ID generation security.The current implementation of
generateIdusesMath.random()which isn't cryptographically secure and could potentially lead to ID collisions.Consider using a more secure random generation method:
-export function generateId(prefix: string = ''): string { - return `${prefix}${Date.now()}_${Math.random().toString(36).substring(2, 9)}`; -} +export function generateId(prefix: string = ''): string { + const timestamp = Date.now(); + const randomValues = new Uint8Array(4); + + // Use crypto API for secure random values if available + if (typeof crypto !== 'undefined') { + crypto.getRandomValues(randomValues); + } else { + // Fallback to Math.random if crypto API is not available + for (let i = 0; i < 4; i++) { + randomValues[i] = Math.floor(Math.random() * 256); + } + } + + // Convert random bytes to hex string + const randomHex = Array.from(randomValues) + .map(b => b.toString(16).padStart(2, '0')) + .join(''); + + return `${prefix}${timestamp}_${randomHex}`; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
extension/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
.github/workflows/generate-client.yml(4 hunks)SECURITY.md(0 hunks)extension/.swcrc(1 hunks)extension/TESTING.md(1 hunks)extension/__tests__/components/Popup/Popup.test.tsx(1 hunks)extension/__tests__/components/ui/Button.test.tsx(1 hunks)extension/__tests__/utils/api.test.ts(1 hunks)extension/__tests__/utils/commons.test.ts(1 hunks)extension/babel.config.js(1 hunks)extension/components/Popup/ActionButtons.tsx(1 hunks)extension/components/Popup/ClipButton.tsx(1 hunks)extension/components/Popup/Popup.tsx(1 hunks)extension/components/Popup/RecentItems.tsx(1 hunks)extension/components/ui/button.tsx(1 hunks)extension/content.ts(1 hunks)extension/e2e/content-script.spec.ts(1 hunks)extension/e2e/popup.spec.ts(1 hunks)extension/jest.config.js(1 hunks)extension/package.json(4 hunks)extension/pages/popup/index.tsx(1 hunks)extension/playwright.config.ts(1 hunks)extension/tests/setup.ts(1 hunks)extension/tests/test-utils.tsx(1 hunks)extension/tsconfig.json(1 hunks)extension/utils/api.ts(1 hunks)extension/utils/commons.ts(1 hunks)frontend/.dockerignore(1 hunks)frontend/Dockerfile(1 hunks)frontend/next.config.js(1 hunks)frontend/start.sh(1 hunks)
💤 Files with no reviewable changes (1)
- SECURITY.md
✅ Files skipped from review due to trivial changes (6)
- extension/.swcrc
- extension/babel.config.js
- frontend/next.config.js
- extension/tests/setup.ts
- extension/jest.config.js
- frontend/.dockerignore
🚧 Files skipped from review as they are similar to previous changes (3)
- extension/tsconfig.json
- extension/content.ts
- extension/components/ui/button.tsx
🧰 Additional context used
🧬 Code Graph Analysis (7)
extension/components/Popup/Popup.tsx (3)
extension/utils/interfaces.ts (1)
ClippedItem(7-18)extension/utils/api.ts (1)
getRecentClippings(54-75)website/src/components/ui/button.tsx (1)
Button(65-65)
extension/playwright.config.ts (1)
extension/content.ts (1)
config(4-9)
extension/__tests__/components/Popup/Popup.test.tsx (2)
extension/tests/test-utils.tsx (2)
generateMockClipping(11-19)generateMockUserProfile(21-29)extension/utils/api.ts (2)
getRecentClippings(54-75)login(114-139)
extension/components/Popup/RecentItems.tsx (2)
extension/utils/interfaces.ts (1)
ClippedItem(7-18)extension/utils/commons.ts (1)
formatDate(110-117)
extension/__tests__/utils/commons.test.ts (1)
extension/utils/commons.ts (5)
extractMainContent(119-137)formatRelativeTime(139-160)truncateText(162-166)isValidUrl(168-176)generateId(178-180)
extension/utils/commons.ts (1)
extension/utils/interfaces.ts (1)
WebHistory(1-5)
extension/utils/api.ts (1)
extension/utils/interfaces.ts (3)
UserProfile(29-37)ClippedItem(7-18)AIResponse(39-45)
🪛 YAMLlint (1.37.1)
.github/workflows/generate-client.yml
[error] 59-59: trailing spaces
(trailing-spaces)
🪛 markdownlint-cli2 (0.17.2)
extension/TESTING.md
15-15: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-docker-compose
- GitHub Check: test-backend
🔇 Additional comments (31)
.github/workflows/generate-client.yml (3)
35-35: PNPM version upgrade to 9.9.0
Bumping to pnpm 9.9.0 aligns with other workflows (e.g., Playwright) and ensures consistency in package management.
60-62: Add Admin dependencies installation step
Good addition of theInstall Admin Dependenciesstep to ensure the admin client’s dependencies are installed.
101-103: Include admin client in staged files
Stagingadmin/src/clientalongsidefrontend/app/openapi-clientensures both generated clients get committed.extension/components/Popup/Popup.tsx (1)
14-32: LGTM: Good error handling and loading state managementThe implementation correctly handles loading states, error conditions, and unauthorized scenarios. The finally block ensures loading state is properly reset regardless of success or failure.
extension/components/Popup/ClipButton.tsx (1)
3-10: LGTM: Well-defined interface with appropriate propsThe component interface is clearly defined with appropriate props for managing the button states and callbacks.
extension/e2e/popup.spec.ts (1)
55-58:✅ Verification successful
Add fallback handling for test selectors.
The test is using data-testid attributes that might not be present in the actual components. Make sure these test selectors exist in the component or add fallback selectors.
🏁 Script executed:
#!/bin/bash # Verify that the data-testid attributes used in the tests are present in the components echo "Searching for 'clipping-item' data-testid in component files:" rg -A 2 'data-testid=.clipping-item' --glob '*.tsx' ./extension/components/ echo "Searching for items that might be the clipping items but don't have the data-testid:" rg -A 3 'key=\{item\.id\}' --glob '*.tsx' ./extension/components/Length of output: 1111
No missing test selectors in Popup component
The
data-testid="clipping-item"used in popup.spec.ts is already present in extension/components/Popup/Popup.tsx, so the test will correctly locate the element. No additional fallback selectors are required for this test.extension/__tests__/components/ui/Button.test.tsx (8)
1-12: Good job on creating a comprehensive test suite for the Button componentThe test setup is well-structured, using React Testing Library to verify the Button's default rendering behavior.
14-19: Test for primary variant is properly structuredThe test correctly verifies that the Button component applies the expected class when the primary variant is specified.
21-30: Well-structured test for button sizesThe test appropriately verifies multiple size variants using the rerender method, which is an efficient approach.
32-37: Good test for disabled stateThe test correctly verifies the disabled attribute is applied when the disabled prop is set.
39-47: Proper testing of click handlerThe test correctly verifies that onClick handlers are invoked when the button is clicked.
49-57: Good test for disabled click behaviorThe test properly verifies that click handlers are not invoked when the button is disabled.
59-64: Appropriate testing of custom class namesThe test correctly verifies that custom class names are applied alongside the default classes.
66-77: Good test for asChild polymorphic behaviorThe test properly verifies that the Button component can render as a different element when the asChild prop is used.
extension/__tests__/utils/commons.test.ts (6)
1-2: Good imports for utility function testsThe imports correctly reference the utility functions being tested.
3-54: Well-structured tests for extractMainContent functionThe tests for extractMainContent are comprehensive, covering main tag extraction, body fallback, and malformed HTML scenarios. The assertions properly verify content inclusion and exclusion.
56-92: Comprehensive tests for formatRelativeTime functionThe tests for formatRelativeTime properly cover different time ranges and formats. The use of mocking Date.now() ensures consistent test results.
94-116: Good tests for truncateText functionThe tests comprehensively verify truncation behavior, including edge cases for null and undefined inputs.
118-137: Thorough tests for isValidUrl functionThe tests properly verify URL validation for various inputs, including invalid URLs and edge cases.
139-152: Good tests for generateId functionThe tests correctly verify that generateId produces unique IDs and properly handles prefixes.
extension/package.json (5)
2-2: Package name updated to standard formatGood change from
nexus_browser_extensiontonexus-browser-extensionusing hyphens, which follows npm naming conventions.
10-13: Well-defined test scripts addedThe addition of test scripts (test, test:watch, test:coverage) provides good support for running Jest tests in various modes.
29-29: Plasmo dependency updatedThe Plasmo dependency has been updated from 0.89.4 to 0.90.3, which is a good practice to keep dependencies current.
40-64: Comprehensive testing dependencies addedGood addition of testing dependencies including Jest, Testing Library, Playwright, and related tools. These provide a robust framework for component, unit, and e2e testing.
73-80: Permissions moved to manifest object and new permission addedThe permissions array has been properly moved inside the manifest object, following Chrome extension best practices. The addition of the "notifications" permission indicates new functionality for showing notifications to users.
extension/e2e/content-script.spec.ts (5)
1-37: Well-structured setup for E2E content script testsThe test setup creates a mock page with test content and properly initializes the testing environment. Good practice to wait for content scripts to load.
39-48: Good test for context menu visibilityThe test correctly verifies that the context menu appears when text is selected and right-clicked.
50-75: Well-structured test for save functionalityThe test properly verifies the end-to-end flow of selecting text, saving it, and confirming the saved content appears in the extension UI.
77-91: Good test for AI options menuThe test correctly verifies that the AI options submenu appears with the expected options.
93-111: Comprehensive test for AI summary functionalityThe test properly verifies the end-to-end flow of selecting text, triggering an AI summary, and confirming the summary appears.
extension/__tests__/utils/api.test.ts (1)
135-138:chromeglobal may be undefined in Jest environment
chrome.runtime.sendMessage = jest.fn();will throw ifglobal.chromeis not pre-defined.
Consider importingjest-chrome(orsinon-chrome) once intests/setup.ts:// tests/setup.ts import 'jest-webextension-mock'; // or 'jest-chrome'and remove the manual assignment.
| # Pull before pushing to avoid conflicts if possible (optional) | ||
| # git pull --ff-only origin ${{ github.head_ref || github.ref_name }} | ||
| # Pull before pushing to avoid conflicts | ||
| git pull --rebase origin ${{ github.head_ref || github.ref_name }} |
There was a problem hiding this comment.
Enable full history for git pull --rebase
The workflow’s comments note that a full history (fetch-depth: 0) is required for accurate diffs and successful rebases, but the checkout step isn’t configured accordingly. Without it, git pull --rebase may fail due to missing history.
Apply this diff at the checkout step (lines 21–23) to set fetch-depth: 0:
- - name: Checkout Code
- uses: actions/checkout@v4
+ - name: Checkout Code
+ uses: actions/checkout@v4
+ with:
+ fetch-depth: 0🤖 Prompt for AI Agents
In .github/workflows/generate-client.yml at the checkout step around lines 21 to
23, the git checkout action is missing the fetch-depth: 0 setting, which is
necessary to enable full git history for accurate diffs and successful rebases.
Modify the checkout step to include fetch-depth: 0 so that the full repository
history is fetched, preventing git pull --rebase failures due to incomplete
history.
| # 确保watcher脚本有可执行权限 | ||
| if [ -f "./watcher.js" ] && [ ! -x "./watcher.js" ]; then | ||
| echo "Setting executable permission on watcher.js" | ||
| chmod +x "./watcher.js" | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unnecessary exec-permission for watcher.js
You invoke watcher.js with node watcher.js, so the executable bit is not required.
You can remove lines 9–13 to avoid confusion and reduce script complexity.
🤖 Prompt for AI Agents
In frontend/start.sh around lines 9 to 13, the script sets executable permission
on watcher.js, but since watcher.js is run using 'node watcher.js', the
executable bit is unnecessary. Remove lines 9 to 13 entirely to simplify the
script and avoid confusion about permissions.
| # 检查.next目录是否存在，它应该在构建时由 builder 阶段生成 | ||
| if [ ! -d "./.next" ] && [ "$NODE_ENV" = "production" ]; then | ||
| echo "Warning: .next directory not found. The application might not start correctly." | ||
| echo "This could happen if the build process was not completed successfully." | ||
| ls -la | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle missing build artifacts more strictly
Logging a warning and listing files helps debugging, but if ./.next is missing in production, the app will likely fail silently.
Add an exit 1 after the warning or change the check to abort on failure:
if [ ! -d "./.next" ] && [ "$NODE_ENV" = "production" ]; then
echo "Warning: .next directory not found. The application might not start correctly."
echo "This could happen if the build process was not completed successfully."
ls -la
+ exit 1
fi📝 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.
| # 检查.next目录是否存在，它应该在构建时由 builder 阶段生成 | |
| if [ ! -d "./.next" ] && [ "$NODE_ENV" = "production" ]; then | |
| echo "Warning: .next directory not found. The application might not start correctly." | |
| echo "This could happen if the build process was not completed successfully." | |
| ls -la | |
| fi | |
| # 检查.next目录是否存在，它应该在构建时由 builder 阶段生成 | |
| if [ ! -d "./.next" ] && [ "$NODE_ENV" = "production" ]; then | |
| echo "Warning: .next directory not found. The application might not start correctly." | |
| echo "This could happen if the build process was not completed successfully." | |
| ls -la | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In frontend/start.sh around lines 15 to 20, the script only logs a warning if
the .next directory is missing in production, which can lead to silent failures.
Modify the script to exit with status 1 immediately after logging the warning
and listing files to prevent the app from starting without the necessary build
artifacts.
| # Copy necessary files for production | ||
| COPY --from=builder /app/public ./public | ||
| COPY --from=builder /app/.next ./.next | ||
| COPY --from=builder /app/node_modules ./node_modules | ||
| COPY --from=builder /app/package.json ./package.json | ||
| COPY --from=builder /app/start.sh ./start.sh | ||
| COPY --from=builder /app/watcher.js ./watcher.js | ||
| COPY --from=builder /app/next.config.js ./next.config.js | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Drop unneeded runtime artifacts
Copying watcher.js and next.config.js into the production image increases size and surface area without runtime benefit.
Remove:
-COPY --from=builder /app/watcher.js ./watcher.js
-COPY --from=builder /app/next.config.js ./next.config.jsand drop related chmod in the next step.
🤖 Prompt for AI Agents
In frontend/Dockerfile around lines 49 to 57, remove the COPY commands that copy
watcher.js and next.config.js from the builder stage to the production image, as
these files are not needed at runtime and increase image size. Also, remove any
subsequent chmod commands related to watcher.js or next.config.js to clean up
unnecessary steps.
| # Ensure that tsconfig paths are set correctly | ||
| RUN cat tsconfig.json | ||
|
|
||
| # Make sure the components directory is present | ||
| RUN ls -la components | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove build-time debug steps
RUN cat tsconfig.json and RUN ls -la components are useful locally but bloat CI logs.
If you need presence checks, use:
-RUN cat tsconfig.json
-RUN ls -la components
+RUN test -f tsconfig.json && test -d componentsor remove these lines entirely once verified.
🤖 Prompt for AI Agents
In frontend/Dockerfile lines 29 to 34, remove the debug commands `RUN cat
tsconfig.json` and `RUN ls -la components` to avoid bloating CI logs. Replace
them with a single line that checks for the presence of the tsconfig.json file
and components directory using `RUN test -f tsconfig.json && test -d
components`, or remove these lines entirely if the checks are no longer needed.
| // 辅助函数：创建Storage模拟 | ||
| // 模拟api.ts中的实现 | ||
| jest.mock('../../utils/api', () => { | ||
| const originalModule = jest.requireActual('../../utils/api'); | ||
|
|
||
| // 模拟Storage.get方法，确保正确处理数组 | ||
| const mockGet = jest.fn().mockImplementation((key) => { | ||
| if (key === 'pendingClippings') { | ||
| return Promise.resolve([]); | ||
| } | ||
| if (key === 'userProfile') { | ||
| return Promise.resolve(null); | ||
| } | ||
| return Promise.resolve(null); | ||
| }); | ||
|
|
||
| // 模拟其他方法 | ||
| const mockSet = jest.fn().mockResolvedValue(undefined); | ||
| const mockRemove = jest.fn().mockResolvedValue(undefined); | ||
| const mockClear = jest.fn().mockResolvedValue(undefined); | ||
|
|
||
| // 模拟Storage类 | ||
| const MockStorage = function() { | ||
| return { | ||
| get: mockGet, | ||
| set: mockSet, | ||
| remove: mockRemove, | ||
| clear: mockClear | ||
| }; | ||
| }; | ||
|
|
||
| // 替换原始Storage类 | ||
| const originalStorage = require('@plasmohq/storage').Storage; | ||
| require('@plasmohq/storage').Storage = MockStorage; | ||
|
|
||
| return { ...originalModule }; | ||
| }); |
There was a problem hiding this comment.
Mock @plasmohq/storage directly instead of the module under test
jest.mock('../../utils/api', …) is executed before jest.requireActual loads the real module, so extension/utils/api.ts is initialised with the original Storage constructor.
Replacing require('@plasmohq/storage').Storage afterwards therefore does not affect the pre-created storage instance inside utils/api, and your tests will silently hit the real implementation (or fail when it’s missing).
Move the mock one level down and let Jest provide the stub before utils/api is evaluated:
-// 模拟api.ts中的实现
-jest.mock('../../utils/api', () => {
- const originalModule = jest.requireActual('../../utils/api');
-
- // … create MockStorage …
-
- // 替换原始Storage类
- const originalStorage = require('@plasmohq/storage').Storage;
- require('@plasmohq/storage').Storage = MockStorage;
-
- return { ...originalModule };
-});
+/**
+ * Provide a stub for the Storage class **before** utils/api is imported.
+ * utils/api will therefore instantiate `storage` with this fake Storage.
+ */
+jest.mock('@plasmohq/storage', () => {
+ const mockGet = jest.fn().mockResolvedValue(undefined);
+ const mockSet = jest.fn().mockResolvedValue(undefined);
+ const mockRemove = jest.fn().mockResolvedValue(undefined);
+ const mockClear = jest.fn().mockResolvedValue(undefined);
+
+ return {
+ Storage: function () {
+ return { get: mockGet, set: mockSet, remove: mockRemove, clear: mockClear };
+ }
+ };
+});This eliminates the double-mocking complexity and guarantees that every call inside utils/api hits your stubbed storage.
🤖 Prompt for AI Agents
In extension/__tests__/utils/api.test.ts around lines 8 to 44, the current mock
replaces the Storage class after the utils/api module is loaded, so the original
Storage instance inside utils/api remains unaffected. To fix this, move the
jest.mock call to directly mock '@plasmohq/storage' before utils/api is
imported, ensuring the Storage class is stubbed before utils/api initializes.
This guarantees all calls inside utils/api use the mocked Storage, eliminating
the double-mocking issue and making tests reliable.
extension/utils/api.ts
Outdated
| export async function logout(): Promise<void> { | ||
| try { | ||
| if (navigator.onLine) { | ||
| const headers = await getAuthHeaders() | ||
| await fetch(`${API_BASE_URL}/api/auth/logout`, { | ||
| method: "POST", | ||
| headers | ||
| }) | ||
| } | ||
| } catch (error) { | ||
| console.error("Logout error:", error) | ||
| } finally { | ||
| await storage.remove("userProfile") | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
logout swallows fetch errors silently
If /auth/logout fails (network error, 500, etc.) the user is logged out locally but the server still thinks the session is alive.
Consider escalating the error (or retrying) so the caller can react:
- } catch (error) {
- console.error("Logout error:", error)
- } finally {
- await storage.remove("userProfile")
- }
+ } catch (error) {
+ console.error("Logout error:", error)
+ // Re-throw so the UI can display feedback / trigger retry logic
+ throw error
+ } finally {
+ await storage.remove("userProfile")
+ }📝 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 async function logout(): Promise<void> { | |
| try { | |
| if (navigator.onLine) { | |
| const headers = await getAuthHeaders() | |
| await fetch(`${API_BASE_URL}/api/auth/logout`, { | |
| method: "POST", | |
| headers | |
| }) | |
| } | |
| } catch (error) { | |
| console.error("Logout error:", error) | |
| } finally { | |
| await storage.remove("userProfile") | |
| } | |
| } | |
| export async function logout(): Promise<void> { | |
| try { | |
| if (navigator.onLine) { | |
| const headers = await getAuthHeaders() | |
| await fetch(`${API_BASE_URL}/api/auth/logout`, { | |
| method: "POST", | |
| headers | |
| }) | |
| } | |
| } catch (error) { | |
| console.error("Logout error:", error) | |
| // Re-throw so the UI can display feedback / trigger retry logic | |
| throw error | |
| } finally { | |
| await storage.remove("userProfile") | |
| } | |
| } |
🤖 Prompt for AI Agents
In extension/utils/api.ts around lines 141 to 155, the logout function catches
fetch errors silently, which causes the client to remove the user profile
locally even if the server logout fails. Modify the function to propagate the
error after logging it or implement a retry mechanism so the caller can handle
the failure appropriately, ensuring the server session state stays consistent
with the client.
- Added a new target to the Makefile for generating client code as part of the build process. - Refactored various components to enhance code readability by standardizing formatting and spacing. - Improved error handling and logging in the Google OAuth flow and authentication components. - Updated the OpenAPI configuration to ensure comprehensive API documentation. - Enhanced user experience in the login and customer pages with better debug information and error handling.
- Integrated a React-based sidebar for better user interaction and content management. - Established a messaging system between content scripts and the sidebar for seamless communication. - Improved error handling and logging in the API interactions, particularly during user authentication and data fetching. - Added new utility functions for managing OAuth login and user profile retrieval. - Updated package.json with new dependencies for enhanced functionality and performance. - Refactored Popup and content scripts for better structure and maintainability, ensuring a smoother user experience.
- Introduced a new diagnose feature in the PopupApp to troubleshoot sidebar issues, including script injection and temporary sidebar creation. - Updated ActionButtons component to include a diagnose button for user interaction. - Enhanced package.json to include new resources for sidebar functionality. - Improved content extraction logic in utility functions for better handling of web page structures.
- Included additional resources for independent developers, such as user guides and project lists, to enhance the documentation and support for the community.
- Changed the favicon link type in theme.config.tsx from 'image/svg+xml' to 'image/x-icon' for better compatibility. - Updated icon assets in the extension and website directories to ensure consistency and improved visual representation.
- Modified the Makefile to include a new target for cleaning extension build artifacts and cache. - Updated the extension configuration to support side panel features and permissions. - Improved error handling in the backend API for better debugging of JWT token issues. - Enhanced the extension's sidebar components to support new messaging features and user interactions. - Added prompt shortcuts and keyboard shortcut settings in the options page for improved user experience. - Refactored various components for better maintainability and performance.
- Updated Makefile to build the browser extension with Tailwind CSS and added post-build fixes. - Expanded README.md with a comprehensive restructuring plan for the Nexus browser extension, detailing goals, current issues, and future steps. - Improved error logging in the backend API for better debugging of JWT token issues. - Refactored various components in the extension for improved maintainability and user experience. - Enhanced sidebar functionality with new UI elements and improved state management. - Updated package.json to include new build scripts and dependencies for better performance.
- Added a minimum password length requirement in the .env.example file for better security practices. - Improved test environment setup by directly creating database tables using SQLModel instead of relying on Alembic migrations. - Updated test database utilities to ensure proper isolation and cleanup after tests, enhancing reliability. - Adjusted test cases to accommodate changes in API response formats and improved error handling. - Enhanced documentation in README.md to clarify the test database isolation mechanism and setup process.
- Introduced a new SocialMediaBar component to the options page, allowing users to easily access WeChat, Facebook, and Discord. - Enhanced the layout of the options page with improved card designs and descriptions for better user experience. - Updated styles for buttons and sections to ensure a cohesive design and improved interactivity. - Added new dependencies for react-icons to support social media icons. - Refactored existing components for better maintainability and responsiveness.
User description
🫰 Thanks for your Pull Request! Here are some helpful tips:
👀 Purpose and Importance of this PR:
🅰 Fixes for Issues:
Fixes #
📝 Notes for the Reviewer:
🎯 How to Verify this PR:
📑 Additional Documentation, e.g., KEPs (Telepace Enhancement Proposals), Usage Docs, etc.:
Description
ItemsPublic,ItemResponse, andItemsResponseto enhance type safety.ItemsTablecomponent to improve data handling and reduce type assertions.Changes walkthrough 📝
index.ts
Enhance API Type Exports for Improved Clarityadmin/src/client/index.ts
ItemsPublic,ItemResponse, andItemsResponse.types.ts
Introduce New Types for API Responsesadmin/src/client/types.ts
ItemsPublic,ItemResponse, andItemsResponse.items.tsx
Refactor ItemsTable for New API Response Structureadmin/src/routes/_layout/items.tsx
ItemsTableto utilize the new response structure.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Chores