chore: update .gitignore to exclude playwright reports, generated #47
Conversation
…t files, and source maps
…ness - Changed NODE_ENV to production in docker-compose.override.yml for optimized builds. - Updated Dockerfile to ensure stable production builds and modified the start command. - Enhanced CORS middleware in main.py to restrict origins based on environment. - Improved logging in get_current_user function for better debugging. - Updated Playwright configuration for improved testing setup. - Refactored frontend components and styles for better performance and user experience. - Added WebSocket connection handling in manifest.json to resolve CSP issues. - Removed onboarding page and adjusted login flow to redirect to web login. - Updated package dependencies for better compatibility and performance.
- Updated onboarding process to redirect to the web setup page instead of the login page, including a unique plugin ID for session management. - Implemented token saving functionality in the setup handler, ensuring secure communication between the extension and web application. - Added user session synchronization from the web to the extension, improving user experience and state management. - Enhanced options page to reflect changes in user authentication status and real-time updates. - Introduced utility functions for retrieving and saving the plugin ID and token, streamlining the integration process.
…omponents - Removed unnecessary blank lines and adjusted indentation for better code clarity in ExtensionLauncher and SetupContent components. - Enhanced the formatting of utility function imports in extension-utils for consistency. - Streamlined the handling of plugin ID and token saving logic to improve maintainability.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 17297692 | Triggered | JSON Web Token | 6d5c97e | frontend/app/dashboard/TokenDebugTool.tsx | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces a comprehensive onboarding and setup flow for a browser extension and its companion web application. Key changes include a multi-step setup wizard, extension detection and sidebar integration, robust extension-to-web authentication, and improved development tooling and test coverage. Additional enhancements cover CORS handling, environment configuration, font management, and Playwright-based end-to-end testing for both extension and web components. Changes
Sequence Diagram(s)Extension Onboarding and Setup FlowsequenceDiagram
participant User
participant Extension (BG)
participant Web Setup Page
participant Web App (API)
participant OAuth Callback Page
User->>Extension (BG): Install Extension
Extension (BG)->>User: Open remote setup page (WEB_URL?pluginId&callback)
User->>Web Setup Page: Loads with pluginId & callback
Web Setup Page->>User: Shows setup wizard
User->>Web Setup Page: Completes setup steps
Web Setup Page->>Web App (API): Auth via login (with extension_callback)
Web App (API)->>OAuth Callback Page: Redirect with token
OAuth Callback Page->>Extension (BG): Sends token via messaging
Extension (BG)->>OAuth Callback Page: Confirms receipt
OAuth Callback Page->>User: Shows success, closes window
Extension Sidebar Launch from WebsequenceDiagram
participant User
participant Web App
participant Extension (BG)
participant Browser
User->>Web App: Clicks "Open Sidebar"
Web App->>Extension (BG): Send "openSidebar" message
Extension (BG)->>Browser: Opens sidePanel for active tab
Extension (BG)->>Web App: Responds with success/failure
Web App->>User: Shows tooltip or error
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure ✨ 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 (
|
penify-dev
bot
added
enhancement
PR Review 🔍
|
PR Code Suggestions ✨
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🔭 Outside diff range comments (1)
frontend/__tests__/components/setup/SetupContentTest.tsx (1)
1-84: 🛠️ Refactor suggestionConsider testing token handling functionality.
Based on the relevant code snippet from
SetupContent.tsx, the component handles sending tokens to the extension after setup completion. This important functionality isn't covered in the current tests.+// 模拟token保存功能 +jest.mock("@/lib/extension-utils", () => ({ + getExtensionPluginId: jest.fn().mockResolvedValue("test-plugin-id"), + saveTokenToExtension: jest.fn().mockResolvedValue(true) +})); + +// 模拟useAuth hook +jest.mock("@/lib/auth", () => ({ + useAuth: () => ({ + user: { token: "test-token" } + }) +})); + +// 模拟useToast hook +jest.mock("@/components/ui/use-toast", () => ({ + useToast: () => ({ + toast: jest.fn() + }) +})); describe("SetupContent", () => { // Existing tests... + it("应该在完成时尝试保存token到扩展", async () => { + const { saveTokenToExtension } = require("@/lib/extension-utils"); + + render(<SetupContent />); + + // 导航到最后一步 + fireEvent.click(screen.getByRole("button", { name: /下一步/i })); + fireEvent.click(screen.getByRole("button", { name: /下一步/i })); + + // 点击完成按钮 + fireEvent.click(screen.getByRole("button", { name: /完成/i })); + + // 验证token保存函数被调用 + expect(saveTokenToExtension).toHaveBeenCalledWith("test-token", "test-plugin-id"); + }); });
🧹 Nitpick comments (29)
frontend/app/password-recovery/confirm/page.tsx (1)
37-37: Minor formatting inconsistency with summaryThe code shows an empty div with opening and closing tags, but according to the AI summary, this change was meant to convert a multi-line empty element into a self-closing tag. While this doesn't affect functionality, consider using a proper self-closing tag for empty elements.
- <div className="flex min-h-screen w-full items-center justify-center"></div> + <div className="flex min-h-screen w-full items-center justify-center" />frontend/app/password-recovery/page.tsx (1)
50-50: Consider using self-closing tag for empty divSimilar to the change in the confirm page, this empty div could be simplified using a self-closing tag for better readability and consistency with modern React practices.
- <div className="flex min-h-screen w-full items-center justify-center"></div> + <div className="flex min-h-screen w-full items-center justify-center" />backend/app/api/deps.py (2)
81-86: Consider standardizing logger usageWhile adding error logging is good, there's inconsistency in how the logger is obtained. In some places you're importing logging and getting the logger inline, while in others you're using the logger variable defined at the top of the function.
- import logging - - logging.getLogger("app").error( + logger.error( "JWT Token Validation Error: Invalid payload format" )
93-96: Same inconsistency with logger usageSimilar to the previous comment, consider standardizing how you obtain and use the logger throughout the function.
- import logging - - logging.getLogger("app").error("Token found in blacklist") + logger.error("Token found in blacklist")extension/background/index.ts (1)
329-332: Consider using optional chainingStatic analysis suggests using optional chaining for this operation to improve code safety.
- tabHistory.history.push({ + tabHistory.history?.push({ url: pageData.url, title: pageData.title, content: pageData.renderedHtml, timestamp: pageData.entryTime })Although you've added the null check before, using optional chaining provides additional safety if the code structure changes in the future.
🧰 Tools
🪛 Biome (1.9.4)
[error] 329-330: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/__tests__/components/setup/ExtensionLauncherTest.tsx (1)
1-60: Consider using English for test descriptions for consistency.The test descriptions are written in Chinese while function and variable names are in English. Consider using English for test descriptions as well if the rest of the codebase follows English naming conventions, to maintain consistency.
extension/scripts/setup-tests.sh (1)
1-28: Consider checking prerequisites and adding a cleanup option.The script would benefit from checking if pnpm is installed before running and offering an option to clean up test artifacts after testing.
#!/bin/bash # 设置错误时退出 set -e +# 检查pnpm是否安装 +if ! command -v pnpm &> /dev/null; then + echo "错误: pnpm未安装，请先安装pnpm" + exit 1 +fi + echo "开始设置测试环境..." # 安装依赖 echo "安装依赖..." pnpm install # 安装Playwright浏览器 echo "安装Playwright浏览器..." pnpm exec playwright install chromium # 构建扩展 echo "构建扩展..." pnpm build # 验证安装 echo "验证安装..." pnpm exec playwright --version echo "测试环境设置完成！" echo "你现在可以运行以下命令来执行测试：" echo " pnpm test - 运行所有测试" echo " pnpm test:debug - 使用调试模式运行测试" echo " pnpm test:ui - 使用UI模式运行测试" +echo " pnpm test:clean - 清理测试生成的临时文件"docker-compose.override.yml (1)
120-121: Consider translating comment to English for consistency.The Chinese comment explains removing the custom command to use the default Dockerfile startup command, which aligns with the PR objectives to standardize deployment. However, since most comments in the codebase appear to be in English, consider translating this comment for consistency.
- # 移除自定义command，使用Dockerfile中的启动命令 + # Removed custom command to use the default startup command from Dockerfile # command: sh -c "pnpm run start"frontend/next.config.js (1)
18-45: Added CSS loader configuration to fix Tailwind directive processing.This webpack configuration enhances CSS processing capabilities with proper handling of CSS modules, URL imports, and hash-based class naming. This should resolve issues with Tailwind directives.
Consider translating the Chinese comment to English for consistency with the rest of the codebase.
- // 修复CSS加载问题 - 处理@tailwind指令 + // Fix CSS loading issues - handle @tailwind directivesextension/utils/config.ts (1)
12-13: Consistent purpose for API_CONFIG.WEB_URLThe newly added
WEB_URLproperty appears to duplicate the existingFRONTEND_URLwith identical values. Consider if this is necessary or ifFRONTEND_URLcould be used instead to avoid redundancy.If there's a specific reason to have both, consider adding a more detailed comment explaining the distinction between these two properties.
extension/utils/dev-helper.ts (1)
10-32: Robust WebSocket connection handlingThe
createSafeWebSocketfunction has good error handling and environment awareness. It gracefully falls back when connections fail and provides appropriate logging.A few suggestions to further enhance this function:
Consider implementing:
- A timeout parameter to prevent hanging connections
- A retry mechanism for important connections
-export function createSafeWebSocket(url: string | number, protocols?: string | string[]): WebSocket | null { +export function createSafeWebSocket( + url: string | number, + protocols?: string | string[], + timeoutMs: number = 5000 +): WebSocket | null { // 生产环境或者url已经是完整URL时直接连接 if (!ENV_CONFIG.IS_DEV || (typeof url === 'string' && url.startsWith('ws'))) { try { - return new WebSocket(url, protocols); + const ws = new WebSocket(url, protocols); + + // Add timeout handling + const timeoutId = setTimeout(() => { + if (ws.readyState !== WebSocket.OPEN) { + console.error(`[Nexus] WebSocket连接超时`); + ws.close(); + } + }, timeoutMs); + + // Clear timeout when connected + ws.addEventListener('open', () => clearTimeout(timeoutId)); + + return ws; } catch (error) { console.error(`[Nexus] WebSocket连接失败: ${error}`); return null; } } // 开发环境中尝试连接不同端口 const portToTry = typeof url === 'number' ? url : ENV_CONFIG.DEV_WS_PORTS[0]; try { const wsUrl = `ws://localhost:${portToTry}`; console.log(`[Nexus] 尝试连接开发WebSocket: ${wsUrl}`); - return new WebSocket(wsUrl, protocols); + const ws = new WebSocket(wsUrl, protocols); + + // Add timeout handling + const timeoutId = setTimeout(() => { + if (ws.readyState !== WebSocket.OPEN) { + console.error(`[Nexus] 开发WebSocket连接超时`); + ws.close(); + } + }, timeoutMs); + + // Clear timeout when connected + ws.addEventListener('open', () => clearTimeout(timeoutId)); + + return ws; } catch (error) { console.error(`[Nexus] 开发WebSocket连接失败: ${error}`); return null; } }extension/background/messages/setup.ts (1)
6-12: Consider using English for documentation comments.The documentation is written in Chinese, which might be inconsistent with the rest of the codebase that appears to use English for variable names, functions, and comments. For better maintainability and collaboration, consider translating these comments to English.
extension/README.md (1)
152-154: Add language specifier to code block.For better syntax highlighting and adherence to Markdown best practices, add a language specifier to the fenced code block.
-``` +```text Refused to connect to 'ws://localhost:1815/' because it violates the following Content Security Policy directive: "connect-src 'self' http://localhost:8000 https://* http://*".🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
152-152: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
frontend/components/ui/stepper.tsx (1)
1-74: Well-implemented stepper component with good visual states.The component is cleanly implemented with appropriate visual states for completed, current, and pending steps. The responsive design with different widths at various breakpoints is a nice touch.
Consider enhancing accessibility with ARIA attributes:
<div data-testid="setup-stepper" + role="progressbar" + aria-valuemin="1" + aria-valuemax={steps.length} + aria-valuenow={currentStep + 1} className={cn("flex justify-between w-full", className)} >Also, it would be good to add validation for the
currentStepprop to handle edge cases:export function Stepper({ steps, currentStep, className }: StepperProps) { + // Ensure currentStep is within bounds + const safeCurrentStep = Math.max(0, Math.min(currentStep, steps.length - 1)); + return (extension/utils/auth-handler.ts (1)
40-86: Enhanced user profile data retrieval with proper fallbacks.The implementation now fetches detailed user information from the API and handles various failure scenarios gracefully. The code stores either complete user profile data or falls back to basic token information when needed.
Consider adding a retry mechanism for temporary network failures:
try { const userInfoResponse = await fetch(`${API_CONFIG.API_URL}/api/v1/extension/auth/me`, { method: "GET", headers: { "Authorization": `${AUTH_CONFIG.TOKEN_PREFIX} ${data.access_token}` } + // Add timeout to prevent long-hanging requests + timeout: API_CONFIG.TIMEOUT }); // Rest of the code... } catch (error) { + // Add retry logic for network errors + if (error instanceof TypeError && error.message.includes('network')) { + console.warn(`${LOG_PREFIX} Network error when fetching user info, retrying once...`); + try { + // Retry the fetch after a short delay + await new Promise(resolve => setTimeout(resolve, 1000)); + // Retry fetch code... + } catch (retryError) { + console.error(`${LOG_PREFIX} Retry failed:`, retryError); + } + } // Fallback code... }frontend/components/setup/ExtensionLauncher.tsx (3)
21-61: Well-structured extension detection and sidebar launch implementation.The component effectively detects extension installation status and manages opening the sidebar. The use of state management for tracking installation status and sidebar state is appropriate.
Consider adding periodic checking for extension installation to improve user experience:
useEffect(() => { const checkExtension = async () => { const installed = await isExtensionInstalled(); setExtensionInstalled(installed); }; checkExtension(); + + // Periodically check for extension installation if not detected + let intervalId: NodeJS.Timeout | null = null; + if (extensionInstalled === false) { + intervalId = setInterval(checkExtension, 3000); + } + + return () => { + if (intervalId) clearInterval(intervalId); + }; }, [extensionInstalled]);
114-155: Consider making the tooltip dismissible.The tooltip implementation is visually appealing with nice animations, but users can't dismiss it manually before the 15-second timeout. Consider adding a dismiss button for better user control.
<motion.div initial={{ opacity: 0, y: 10, scale: 0.9 }} animate={{ opacity: 1, y: 0, scale: 1 }} exit={{ opacity: 0, y: 10, scale: 0.9 }} style={{ backgroundColor: "var(--primary)", color: "var(--primary-foreground)", padding: "1rem", borderRadius: "0.375rem", boxShadow: "0 10px 15px -3px rgba(0, 0, 0, 0.1), 0 4px 6px -2px rgba(0, 0, 0, 0.05)", width: "16rem", + position: "relative" }} > + <button + onClick={() => setShowTooltip(false)} + aria-label="Close tooltip" + className="absolute top-1 right-1 p-1 rounded-full hover:bg-black/10" + > + <svg width="14" height="14" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> + <path d="M18 6L6 18M6 6L18 18" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round" /> + </svg> + </button> <h4 className="font-bold text-sm mb-1">提示</h4>
62-157: Extract hardcoded UI strings for easier localization.The component has several hardcoded Chinese strings. Consider extracting these strings to a separate constants file or using a localization library to make future translations easier.
+// In a separate file like lib/constants.ts +export const EXTENSION_TEXTS = { + title: "浏览器扩展", + description: "启动 Nexus 侧边栏，体验更便捷的浏览体验", + notDetected: "未检测到 Nexus 扩展", + installPrompt: "请先安装浏览器扩展以使用此功能", + installed: "已安装 Nexus 扩展", + usageHint: "您可以启动侧边栏使用更多功能", + openSidebar: "打开 Nexus 侧边栏", + sidebarFeature: "使用侧边栏可随时访问 Nexus 功能", + tooltipTitle: "提示", + tooltipContent: "点击侧边栏右上角的图钉图标可以固定侧边栏，方便随时使用" +}; // In component -<CardTitle className="text-xl">浏览器扩展</CardTitle> +<CardTitle className="text-xl">{EXTENSION_TEXTS.title}</CardTitle> // ... and so on for other stringsextension/tests/e2e/onboarding.test.ts (2)
38-38: Avoid hardcoded long timeout values.Setting a 2-minute timeout directly in the test might mask underlying performance issues and could lead to unnecessarily long test runs.
Consider setting timeouts in the Playwright config and keeping individual test timeouts shorter:
- test.setTimeout(120000); // 为这个测试设置2分钟超时 + test.setTimeout(60000); // 设置更合理的超时时间
49-75: Simplify test structure and improve error handling.The try/catch structure around the entire test makes error diagnostics more difficult as it catches all errors. Playwright already handles test failures appropriately.
- try { - // 访问onboarding页面 - const url = `chrome-extension://${extensionId}/pages/onboarding.html`; - console.log('Navigating to:', url); - await page.goto(url); - await waitForPageLoad(page); - - // 验证页面标题 - const title = await page.title(); - console.log('Page title:', title); - expect(title).toBe('欢迎使用 Nexus AI'); - - // 验证根元素存在 - const root = await page.$('#root'); - expect(root).toBeTruthy(); - - // 验证主要内容已加载 - const heading = await page.getByRole('heading', { name: '欢迎使用 Nexus AI!' }); - expect(heading).toBeTruthy(); - - // 确保没有页面错误 - expect(errors).toHaveLength(0); - } catch (error) { - console.error('Test failed:', error); - throw error; - } + // 访问onboarding页面 + const url = `chrome-extension://${extensionId}/pages/onboarding.html`; + console.log('Navigating to:', url); + await page.goto(url); + await waitForPageLoad(page); + + // 验证页面标题 + const title = await page.title(); + expect(title).toBe('欢迎使用 Nexus AI'); + + // 验证根元素存在 + const root = await page.$('#root'); + expect(root).toBeTruthy(); + + // 验证主要内容已加载 + const heading = await page.getByRole('heading', { name: '欢迎使用 Nexus AI!' }); + expect(heading).toBeTruthy(); + + // 确保没有页面错误 + expect(await errors).toHaveLength(0);extension/utils/theme-helper.js (2)
5-61: Implement WebSocket CSP workaround for development.This code provides a workaround for Content Security Policy issues with WebSockets during development. The approach is clever, but there are some improvements to consider:
- Add debug logging options to make troubleshooting easier
- Improve the mock WebSocket interface to be more robust
function fixWebSocketCSP() { // 如果运行时存在WebSocket构造函数，我们尝试包装它 if (typeof WebSocket !== 'undefined') { const OriginalWebSocket = WebSocket; + // 存储原始WebSocket以便检测是否被包装 + window.__originalWebSocket = OriginalWebSocket; // 覆盖原始的WebSocket构造函数 window.WebSocket = function(url, protocols) { try { console.log(`[Nexus] 尝试WebSocket连接: ${url}`); return new OriginalWebSocket(url, protocols); } catch (e) { console.error(`[Nexus] WebSocket连接错误: ${e}`); // 如果是CSP错误并且是localhost连接，提示用户 if (e.toString().includes('Content Security Policy') && url.toString().includes('localhost')) { console.warn('[Nexus] WebSocket连接被CSP阻止，这在开发模式下是正常的'); + // 存储日志用于测试和调试 + if (!window.wsLogs) window.wsLogs = []; + window.wsLogs.push(`CSP阻止连接到: ${url}`); // 创建一个不会触发CSP错误的空对象作为WebSocket的替代 const mockWS = { addEventListener: () => {}, removeEventListener: () => {}, send: () => {}, close: () => {}, readyState: 3, // CLOSED onopen: null, onclose: null, onmessage: null, onerror: null };
63-100: Implement theme management for extension.The theme management code is well-structured and robust. It correctly handles user preferences and system theme changes. One minor improvement to consider:
Add an event listener for storage changes to update the theme in real-time:
// 主题设置逻辑 (() => { // 页面加载时检查和设置主题 document.addEventListener('DOMContentLoaded', async () => { try { // 从存储中获取用户主题首选项 const result = await chrome.storage.local.get('userSettings'); const settings = result.userSettings || {}; const theme = settings.theme || 'system'; // 应用主题 applyTheme(theme); // 监听系统主题变化 if (theme === 'system') { const darkModeMediaQuery = window.matchMedia('(prefers-color-scheme: dark)'); darkModeMediaQuery.addEventListener('change', (e) => { document.documentElement.classList.toggle('dark', e.matches); }); } + + // 监听存储变化，实时更新主题 + chrome.storage.onChanged.addListener((changes, area) => { + if (area === 'local' && changes.userSettings) { + const newSettings = changes.userSettings.newValue || {}; + const newTheme = newSettings.theme; + if (newTheme) { + applyTheme(newTheme); + } + } + });frontend/components/setup/SetupContent.tsx (1)
216-253: Implement proper error handling for extension communication.The token sending logic has good error handling with toast notifications for success/failure, but it could be improved:
Add a retry mechanism for extension communication failures:
const handleFinish = async () => { if (user?.token && extensionPluginId && !tokenSent) { + let attempts = 0; + const maxAttempts = 3; + try { console.log("Setup页面尝试向扩展发送Token"); - const success = await saveTokenToExtension( - user.token, - extensionPluginId, - ); + let success = false; + + // 实现重试逻辑 + while (!success && attempts < maxAttempts) { + attempts++; + console.log(`尝试发送Token到扩展 (尝试 ${attempts}/${maxAttempts})`); + + success = await saveTokenToExtension( + user.token, + extensionPluginId, + ); + + if (!success && attempts < maxAttempts) { + // 等待一段时间后重试 + await new Promise(resolve => setTimeout(resolve, 1000)); + } + } if (success) {frontend/lib/extension-utils.ts (3)
31-64: Unresolved promise path whensendMessageAPI is absent
new Promise<boolean>((resolve) => { … })always installs a timeout, but
if theif (…)guard at lines 36-43 fails (i.e.chrome.runtimeexists
but has nosendMessage), the promise neither callsresolve()nor
clears the timeout.
setTimeoutwill eventually resolve, but only if the current
event-loop is still alive after 500 ms. In SSR or fast-terminating
scripts this may hang.Safer pattern:
if (hasSendMessage) { chrome.runtime.sendMessage(…); -} + return; // handled by callback +} +// Fallback when API is unavailable +clearTimeout(timeoutId); +resolve(false);
70-123: Duplicate low-level plumbing – extract a helper
isExtensionInstalled,openSidebar,getExtensionPluginId,
saveTokenToExtensionall repeat:
- env var resolution
- defensive
chrome.runtime.sendMessagechecks- timeout boilerplate
Factor this into a private
sendMessageToExtensionutil to reduce ~200
lines, lower maintenance cost, and centralise edge-case handling.
DRYing this out will also remove four nearly identical cast blocks.
255-258: Type either too lax or too strict
type ExtensionResponse = { success?: boolean } | undefined;Most responses carry additional data (
pluginId, errors, etc.). Making
ExtensionResponsegeneric or at least{ success?: boolean; [k: string]: any }
prevents repetitive re-casts at every call-site.extension/playwright.config.ts (2)
31-36:path.join(__dirname)is a no-op
path.join(__dirname)will always equal__dirname. Passing the string
directly avoids an unnecessary syscall and intent confusion:-`--disable-extensions-except=${path.join(__dirname)}`, -`--load-extension=${path.join(__dirname)}`, +`--disable-extensions-except=${__dirname}`, +`--load-extension=${__dirname}`,
41-46: Potential port clash & hard-coded command
webServer.port = 1815collides easily with local dev tools.
Exposing it asprocess.env.PORT(with default) lets contributors avoid
conflicts and opens room for concurrent test runs.Also,
command: 'pnpm dev'assumes the caller has installedpnpm
globally. Prefernpx pnpmornpm run devto stay package-manager
agnostic.extension/tests/utils/test-helpers.ts (1)
82-93: Memory leak – error arrays never cleared
checkPageErrorspushes intoerrorson every navigation but never
resets between test cases. Return a helper that stops listeners:export async function watchPageErrors(page: Page) { const errors: unknown[] = []; const handler = (err: unknown) => errors.push(err); page.on('pageerror', handler); page.on('console', (msg) => msg.type() === 'error' && errors.push(msg)); return () => { page.off('pageerror', handler); page.off('console', handler); return errors; }; }
📜 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 (4)
extension/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlfrontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlfrontend/public/fonts/GeistMonoVF.woffis excluded by!**/*.wofffrontend/public/fonts/GeistVF.woffis excluded by!**/*.woff
📒 Files selected for processing (47)
Makefile(1 hunks)backend/app/api/deps.py(2 hunks)backend/app/main.py(1 hunks)docker-compose.override.yml(1 hunks)extension/.gitignore(1 hunks)extension/README.md(1 hunks)extension/background/index.ts(2 hunks)extension/background/messages/onboarding.ts(1 hunks)extension/background/messages/setup.ts(1 hunks)extension/manifest.json(1 hunks)extension/package.json(2 hunks)extension/pages/oauth-callback.html(1 hunks)extension/pages/onboarding.html(0 hunks)extension/pages/onboarding/index.tsx(0 hunks)extension/pages/options/index.tsx(3 hunks)extension/playwright.config.ts(1 hunks)extension/scripts/reload-extension.js(1 hunks)extension/scripts/setup-tests.sh(1 hunks)extension/tests/e2e/onboarding.test.ts(1 hunks)extension/tests/utils/test-helpers.ts(1 hunks)extension/utils/api.ts(1 hunks)extension/utils/auth-handler.ts(3 hunks)extension/utils/config.ts(3 hunks)extension/utils/dev-helper.ts(1 hunks)extension/utils/theme-helper.js(1 hunks)frontend/Dockerfile(3 hunks)frontend/__tests__/app/setup/SetupPageTest.tsx(1 hunks)frontend/__tests__/components/setup/ExtensionLauncherTest.tsx(1 hunks)frontend/__tests__/components/setup/SetupContentTest.tsx(1 hunks)frontend/app/dashboard/TokenDebugTool.tsx(1 hunks)frontend/app/dashboard/page.tsx(2 hunks)frontend/app/globals.css(1 hunks)frontend/app/layout.tsx(1 hunks)frontend/app/login/page.tsx(3 hunks)frontend/app/page.tsx(1 hunks)frontend/app/password-recovery/confirm/page.tsx(1 hunks)frontend/app/password-recovery/page.tsx(1 hunks)frontend/app/register/page.tsx(1 hunks)frontend/app/setup/page.tsx(1 hunks)frontend/components/setup/ExtensionLauncher.tsx(1 hunks)frontend/components/setup/SetupContent.tsx(1 hunks)frontend/components/ui/stepper.tsx(1 hunks)frontend/lib/client-auth.ts(5 hunks)frontend/lib/extension-utils.ts(1 hunks)frontend/next.config.js(2 hunks)frontend/package.json(0 hunks)frontend/tailwind.config.js(1 hunks)
💤 Files with no reviewable changes (3)
- frontend/package.json
- extension/pages/onboarding.html
- extension/pages/onboarding/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (12)
extension/background/messages/onboarding.ts (1)
extension/utils/config.ts (2)
LOG_PREFIX(56-56)API_CONFIG(5-20)
extension/utils/api.ts (1)
extension/utils/config.ts (1)
AUTH_CONFIG(23-35)
frontend/app/dashboard/page.tsx (1)
frontend/app/dashboard/TokenDebugTool.tsx (1)
TokenDebugTool(12-50)
extension/playwright.config.ts (1)
frontend/next.config.js (1)
path(2-2)
extension/tests/e2e/onboarding.test.ts (1)
extension/tests/utils/test-helpers.ts (6)
createExtensionContext(21-32)loadExtension(10-16)checkPageErrors(81-94)mockChromeAPI(100-125)waitForPageLoad(38-41)monitorWebSocketConnections(48-61)
backend/app/api/deps.py (2)
backend/app/models.py (2)
TokenPayload(147-149)User(59-72)backend/app/crud.py (1)
is_token_blacklisted(75-81)
extension/utils/dev-helper.ts (1)
extension/utils/config.ts (1)
ENV_CONFIG(59-65)
extension/utils/auth-handler.ts (2)
extension/utils/config.ts (3)
AUTH_CONFIG(23-35)API_CONFIG(5-20)LOG_PREFIX(56-56)extension/utils/interfaces.ts (1)
UserProfile(40-53)
frontend/app/dashboard/TokenDebugTool.tsx (1)
frontend/lib/client-auth.ts (1)
useAuth(30-238)
frontend/app/login/page.tsx (1)
frontend/lib/client-auth.ts (1)
useAuth(30-238)
extension/pages/options/index.tsx (1)
extension/utils/interfaces.ts (1)
UserProfile(40-53)
frontend/app/setup/page.tsx (1)
frontend/components/setup/SetupContent.tsx (1)
SetupContent(176-322)
🪛 Biome (1.9.4)
extension/background/index.ts
[error] 329-330: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
extension/tests/e2e/onboarding.test.ts
[error] 164-164: Expected a statement but instead found '})'.
Expected a statement here.
(parse)
frontend/app/setup/page.tsx
[error] 21-21: Unexpected constant condition.
(lint/correctness/noConstantCondition)
🪛 markdownlint-cli2 (0.17.2)
extension/README.md
152-152: 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-playwright (4, 4)
- GitHub Check: test-playwright (2, 4)
🔇 Additional comments (79)
backend/app/main.py (1)
74-84: Good security practice for environment-specific CORSThe environment-specific CORS configuration provides a good balance between development flexibility and production security. This approach allows unrestricted access during local development while enforcing proper origin restrictions in production.
backend/app/api/deps.py (3)
61-71: Improved logging for JWT token decodingAdding detailed logging for JWT token operations will help with debugging authentication issues. This is a good addition.
101-113: Improved debugging for user lookupThe added logging for user lookup operations, especially the detailed dump of all users when a specific user is not found, will be very helpful for troubleshooting authentication issues in production. This is a valuable addition for operational support.
However, be cautious about logging all users in production as this could potentially expose sensitive information in logs. Consider adding a conditional to only perform this detailed logging in non-production environments.
116-116: Good additional loggingThe added log statement for successful user lookup provides important context for authentication flows.
Makefile (1)
280-280: Command visibility change improves debugging transparency.Removing the
@prefix before the bash command makes the command itself visible during execution, which is helpful for transparency and debugging. This aligns with modern CI/CD practices where command visibility aids in troubleshooting pipeline issues.frontend/app/register/page.tsx (1)
56-56: Code formatting improvement for empty element.Converting the multi-line empty div to a self-closing tag improves code readability and follows JSX best practices for empty elements.
frontend/app/layout.tsx (1)
24-24:❓ Verification inconclusive
Font management approach changed.
The custom font classes have been removed from the body element as part of shifting font management from Next.js's
localFontutility to CSS@font-facedeclarations. This architectural change provides more flexibility in font loading but ensure that font loading performance hasn't been negatively impacted.
🌐 Web query:
Are there performance differences between Next.js localFont and CSS @font-face declarations?💡 Result:
When comparing Next.js's
localFontfeature with traditional CSS@font-facedeclarations, several performance considerations emerge:1. Self-Hosting and Network Requests:
localFont: Fonts are self-hosted within your application, eliminating external network requests. This approach reduces latency and enhances privacy by serving fonts directly from your server. (nextjs.org)@font-face: Typically involves linking to external font files, which can introduce additional HTTP requests and potential delays, especially if the fonts are hosted on third-party servers.2. Font Optimization:
localFont: Automatically optimizes fonts by subsetting them to include only the necessary characters, thereby reducing file sizes and improving load times. (nextjs.org)@font-face: Requires manual optimization, such as subsetting fonts and compressing them to achieve similar performance benefits.3. Layout Stability:
localFont: Utilizes thesize-adjustproperty to ensure that fallback fonts closely match the metrics of the custom font, minimizing layout shifts and enhancing visual stability. (vercel.com)@font-face: Without careful configuration, can lead to layout shifts if fallback fonts differ significantly from the custom font in terms of size and spacing.4. Browser Behavior and Rendering:
localFont: By default, Next.js setsfont-display: swap;for fonts, ensuring that text remains visible during font loading and reducing the likelihood of invisible text flashes. (nextjs.org)@font-face: Requires explicit setting of thefont-displayproperty to achieve similar behavior, which can be overlooked, leading to potential rendering issues.5. Performance Overheads:
localFont: Integrates seamlessly with Next.js's build process, offering performance benefits without significant overhead.@font-face: May introduce additional complexity and potential performance bottlenecks, especially if not optimized correctly.In summary, Next.js's
localFontfeature provides a more streamlined and performance-optimized approach to handling custom fonts compared to traditional CSS@font-facedeclarations. It automates many optimization tasks, reduces external dependencies, and enhances layout stability, leading to a more efficient and user-friendly web application.Citations:
- 1: https://nextjs.org/docs/pages/building-your-application/optimizing/fonts?utm_source=openai
- 2: https://nextjs.org/docs/pages/building-your-application/optimizing/fonts?utm_source=openai
- 3: https://vercel.com/blog/optimizing-web-fonts?utm_source=openai
- 4: https://nextjs.org/docs/pages/building-your-application/optimizing/fonts?utm_source=openai
Double-check CSS @font-face optimizations after removing
localFontFile: frontend/app/layout.tsx
Lines: 24-24<body>{children}</body>We’ve shifted from Next.js’s
localFontto manual CSS@font-facefor font loading. To ensure we retain the performance benefits oflocalFont, please verify that your CSS declarations include:
- Self-hosting and subsetting font files (to reduce payload, as Next.js does automatically).
font-display: swap;(to prevent invisible text/FOUT).size-adjustor matching fallback metrics (to minimize layout shifts).- Preloading of critical fonts if needed (e.g., using
<link rel="preload">).- A quick performance audit (Lighthouse/WebPageTest) to confirm no regressions in load time or Cumulative Layout Shift.
extension/.gitignore (1)
34-42: Good practice: Excluding test artifacts and generated files.Adding Playwright reports, generated test files, and source maps to .gitignore follows best practices for repository cleanliness. This ensures that:
- Test reports aren't tracked in version control
- Generated test files don't clutter the repository
- Source maps (which can be large) don't bloat the repository
These exclusions align well with the migration to Playwright for testing mentioned in the PR objectives.
frontend/app/dashboard/page.tsx (2)
25-26: Token debugging tool import added.The import for the debug component follows React best practices as a named import.
194-200:✅ Verification successful
Development-only debugging component added with proper environment check.
The token debugging tool is correctly implemented with:
- Conditional rendering based on environment (
NODE_ENV === "development")- Clear visual distinction (yellow background) to identify it as a debugging element
- Proper isolation from production code
This is a good approach for adding developer tooling that shouldn't appear in production environments.
🏁 Script executed:
#!/bin/bash # Description: Verify the TokenDebugTool component is properly implemented and all imports are correctly handled # Check that the TokenDebugTool component exists echo "Checking for TokenDebugTool component..." if fd "TokenDebugTool.tsx" --type f; then echo "✓ TokenDebugTool component found" else echo "✗ TokenDebugTool component not found" exit 1 fi # Check that useAuth hook is properly imported in the TokenDebugTool component echo "Checking for useAuth hook import in TokenDebugTool..." rg "import.*useAuth" --type ts frontend/app/dashboard/TokenDebugTool.tsxLength of output: 470
Development-only debugging component properly implemented and verified
- Confirmed
frontend/app/dashboard/TokenDebugTool.tsxexists.- Verified
useAuthis correctly imported (import { useAuth } from "@/lib/auth";).- Conditional rendering with
process.env.NODE_ENV === "development"ensures it won’t appear in production.No further changes needed—approving these code changes.
extension/manifest.json (1)
60-60: WebSocket endpoints added to CSPThe Content Security Policy has been expanded to include WebSocket protocols (
ws://localhost:*andwss://*), which allows the extension to establish WebSocket connections locally and to secure endpoints. This change aligns with the new onboarding flow that requires WebSocket communication between the extension and web app.extension/package.json (3)
18-22: Test framework migrated from Jest to PlaywrightThe test scripts have been updated to use Playwright instead of Jest, providing better support for end-to-end and UI testing with browser automation capabilities. This is a positive change for testing extension functionality, especially for the new onboarding flow.
62-62: Playwright test version specifiedUsing Playwright version 1.42.1 for testing, which is appropriate for the extension testing needs.
66-66: Chrome types updatedChrome types package has been updated from 0.0.258 to 0.0.260 to ensure compatibility with the latest Chrome APIs.
frontend/app/page.tsx (2)
28-33: Main CTA button redirects to setup flowThe main button now directs users to the new setup page instead of directly to the dashboard, with the button text changed to Chinese "开始设置" (Start Setup). This change supports the new onboarding process implemented in this PR.
35-41: Added direct dashboard access linkA secondary link has been added to allow users to bypass the setup process and go directly to the dashboard. This provides flexibility for returning users who may not need to go through setup again.
extension/background/index.ts (4)
12-12: New setup message handler importedA new message handler for setup-related commands has been imported, supporting the web-based setup flow introduced in this PR.
19-20: Setup handler registered in messaging systemThe onboarding and setup handlers are properly registered in the messaging object, making them available to handle messages from the extension UI and web pages.
323-326: Added safeguard for history propertyA check has been added to ensure the history property exists before pushing a new entry, preventing potential null/undefined errors if the history array hasn't been initialized yet.
333-333: Fixed storage operation for URL queue listWhen saving the updated URL queue list, the code now correctly stores the entire object instead of just the array property, ensuring proper data structure in storage.
frontend/tailwind.config.js (1)
14-38: Well-structured font family configuration.The new font family configuration correctly implements a CSS variable-based approach with comprehensive fallback chains for both sans-serif and monospace fonts. This configuration aligns well with modern font management practices.
extension/utils/api.ts (1)
411-422: Good enhancement to logout synchronization.The addition of runtime messaging ensures that all extension components are notified when a user logs out. The try-catch block properly handles any errors that might occur during message dispatch, preventing them from interrupting the logout flow.
extension/scripts/reload-extension.js (1)
1-36: Well-implemented extension reload utility.This development utility script follows good practices for Chrome extension development. The async/await pattern, proper error handling, and appropriate delays between disabling and re-enabling the extension will ensure reliable reloading during development.
frontend/__tests__/components/setup/ExtensionLauncherTest.tsx (5)
5-10: Good use of mocking for isolated testing.The mock implementation of extension utility functions is well-structured, allowing each test to control the expected behavior independently.
17-24: Test validates basic component rendering correctly.This test properly validates that the essential text and button elements are present in the rendered component.
26-34: Good handling of asynchronous state updates.The test correctly uses
await screen.findByText()to handle the asynchronous nature of the extension installation check, ensuring the test doesn't falsely fail due to timing issues.
36-43: Thorough testing of negative condition.Testing both positive and negative cases (extension installed vs. not installed) provides thorough test coverage of the component's conditional rendering logic.
45-58: Effective interaction testing.The test correctly simulates user interaction and verifies that the expected function is called, ensuring the component's interactive behavior works as intended.
frontend/app/setup/page.tsx (2)
20-29: Good responsive design with appropriate content hierarchy.The layout makes effective use of Tailwind CSS for responsive design and establishes a clear visual hierarchy with the heading, subheading, and content arrangement. The component also correctly handles dark mode with appropriate class names.
🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: Unexpected constant condition.
(lint/correctness/noConstantCondition)
1-31:✅ Verification successful
Verify if SetupContent receives necessary authentication data.
The
SetupContentcomponent may need access to the authenticated user data based on the relevant code snippet, but this data isn't explicitly passed from theSetupPagecomponent.
🏁 Script executed:
#!/bin/bash # Check how SetupContent accesses user authentication data grep -A 10 "function SetupContent" frontend/components/setup/SetupContent.tsxLength of output: 451
No prop needed—SetupContent uses its own auth hook
TheSetupContentcomponent callsuseAuth()internally to access the authenticated user, so you don’t need to pass any auth data fromSetupPage. Just ensure that your root layout wraps the app with the correspondingAuthProviderso thatuseAuth()returns the correct user context.🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: Unexpected constant condition.
(lint/correctness/noConstantCondition)
extension/scripts/setup-tests.sh (3)
4-4: Good error handling practice.Using
set -eensures the script will exit immediately if any command fails, preventing partial setup that could lead to confusing test failures.
8-23: Comprehensive test environment setup.The script efficiently covers all necessary steps for test preparation: dependency installation, browser setup, build process, and verification of the installation.
24-28: Clear user instructions.The script provides clear instructions for different testing modes after setup, which is helpful for developers.
frontend/__tests__/components/setup/SetupContentTest.tsx (4)
5-17: Effective mocking of dependencies.The test correctly mocks both the Next.js router and the ExtensionLauncher component, ensuring proper isolation of the SetupContent component under test.
19-56: Comprehensive testing of navigation flow.These tests thoroughly validate the navigation between steps, including both forward and backward navigation, ensuring the multi-step flow works correctly.
58-69: Complete flow testing to final step.The test properly verifies navigation to the final step and checks for the presence of completion messages and action buttons.
71-82: Good integration testing with ExtensionLauncher.This test verifies that the ExtensionLauncher component is correctly integrated into the final step of the setup process.
docker-compose.override.yml (2)
113-113: Environment variable change to production mode.Changed build argument from development to production mode, which will optimize the frontend build for production performance rather than development experience.
119-119: Environment variable aligned with build argument.Setting NODE_ENV to production in runtime environment, consistent with the build argument change.
frontend/next.config.js (2)
2-3: Improved module imports for better code organization.The explicit path module import is a good practice that improves code readability.
15-15: Using the imported path module for alias resolution.This change leverages the previously imported path module for alias resolution, making the code more consistent.
frontend/__tests__/app/setup/SetupPageTest.tsx (2)
1-4: New test suite for the SetupPage component.Good test setup with appropriate imports for testing React components.
5-16: Mock implementations for Next.js navigation and authentication.Properly mocking external dependencies for isolated component testing.
frontend/app/globals.css (3)
5-18: Added font-face declarations for variable fonts.Good implementation of variable fonts with proper font-display setting for performance. The font weight range (100-900) provides flexibility for the design system.
20-48: Restructured CSS variables and added font family custom properties.Moved CSS variables outside of @layer base and added font-related custom properties. This improves organization and makes the variables available globally without Tailwind's layering constraints.
50-75: Consistent restructuring for dark mode variables.Dark mode variables are also moved outside of @layer base, maintaining consistency with the light mode variables structure.
extension/utils/config.ts (2)
43-46: Good addition of MAX_RETRIES parameterThe
MAX_RETRIESconfiguration parameter is a solid addition for controlling synchronization retry attempts, improving reliability and preventing excessive retries.
58-65: Well-structured environment configurationThe new
ENV_CONFIGobject provides a clean way to handle environment-specific behavior, particularly for WebSocket debugging. The explicit port list will help with connection handling in development environments.extension/utils/dev-helper.ts (1)
3-9: Well-documented function purposeThe documentation clearly explains the function's purpose and parameters, which is excellent for maintainability.
extension/background/messages/onboarding.ts (1)
9-17: Improved onboarding flow to web-based setupThe change from a local onboarding page to a web-based setup flow is a solid improvement. The code redirects users to a web setup page with proper plugin identification and callback URL.
extension/pages/options/index.tsx (2)
94-94: Changed default active section to "account"The default section is now "account" instead of "general", which makes sense if user authentication is a primary focus of the application.
153-181: Real-time user profile updates via event listenersAdding listeners for storage changes and runtime messages is excellent for keeping the UI in sync with backend state. This implementation ensures the component responds to login/logout events in real-time.
The cleanup in the useEffect is properly implemented, removing the listeners when the component unmounts to prevent memory leaks.
extension/background/messages/setup.ts (4)
13-28: LGTM: Ping functionality well implemented.The setup ping handler properly confirms extension installation and retrieves the plugin ID from storage. The response structure with success status and message is clear and consistent.
31-50: Strong security check for token verification.Good implementation of the CSRF protection by verifying the plugin ID matches before accepting tokens from external sources. This prevents cross-site request forgery attacks against the extension.
80-110: Well-implemented sidebar feature detection.The code correctly checks for sidebar API support before attempting to use it, and provides clear error messages when the feature isn't available. The feature detection approach is particularly good for browser compatibility.
113-124: Comprehensive error handling.The implementation includes good error handling with appropriate logging and consistent response structures. Every request receives a meaningful response even in error conditions.
frontend/lib/client-auth.ts (4)
15-15: Good addition of token to User interface.Adding the optional token field to the User interface allows for more convenient access to the authentication token throughout the application where the user object is available.
90-93: Token preservation in user object.Storing the token in the user object during fetch ensures the token is accessible alongside other user data, creating a more cohesive authentication state.
164-170: Token synchronization when user exists.Good enhancement to ensure the token is synchronized with the user object when a login occurs and the user already exists in state.
180-205: Well-structured custom token setter.The
setCustomTokenimplementation follows the established pattern of the existing authentication methods, with appropriate validation, error handling, and logging. This facilitates testing and debugging of authentication flows.extension/README.md (1)
148-188: Excellent documentation for WebSocket connection issues.This section provides comprehensive documentation about the WebSocket connection issues encountered during development and their solutions. The explanations of the problems, causes, and solutions are clear and detailed, which will be valuable for other developers.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
152-152: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
frontend/Dockerfile (4)
23-26: Good addition of build environment variables.These environment variables enhance build stability by ensuring consistent production builds and disabling telemetry. Setting
NEXT_SHARP_PATHis also a good practice for reliable image processing.
34-35: Good build verification steps.Creating the
.nextdirectory explicitly and verifying its contents after the build improves build reliability by catching potential failures early.Also applies to: 46-47
59-61: Appropriate runtime environment configuration.Setting these environment variables properly configures the container networking. Using
HOSTNAME=0.0.0.0ensures the server binds to all network interfaces, making it accessible from outside the container.
82-82: Improved container start command.Changing from a shell script to directly executing the Next.js server is more reliable and aligns with Next.js standalone output mode. This change simplifies the startup process and reduces potential script-related issues.
extension/utils/auth-handler.ts (2)
14-15: Improved authentication check with isAuthenticated flag.Adding a check for both token existence and authentication status provides a more robust validation, preventing unnecessary web session synchronization when a user is already authenticated.
197-215: Simplified OAuth callback return value and improved error handling.Changing the return type from
Promise<UserProfile>toPromise<boolean>makes the API more straightforward. The improved error handling with graceful failure (returningfalseinstead of throwing) is also a good practice.frontend/components/setup/ExtensionLauncher.tsx (1)
62-113: Clear and informative UI states for extension status.The component provides excellent visual feedback with different states for when the extension is detected or not. The button state management is also well implemented.
frontend/app/login/page.tsx (4)
31-54: Improved callback handling with extension support.The component now supports extension integration via the
extension_callbackparameter. The implementation is clean and follows React best practices.
58-62: Default callback URL changed to '/setup'.The default redirect path has been updated from '/dashboard' to '/setup', which aligns with the new onboarding flow. This is a good change to ensure users complete the setup process.
73-85: Properly handle extension authentication redirection.The login flow now correctly redirects to the extension with the authentication token when an extension callback is present. This implementation enables seamless authentication between the web app and browser extension.
159-164: Added user feedback for extension login requests.The notification banner provides clear feedback when the login is initiated from the browser extension, improving the user experience and clarifying the flow's context.
frontend/components/setup/SetupContent.tsx (5)
15-22: Good integration with extension utilities.The component properly imports and uses authentication and extension-related utilities. This provides a seamless integration between the web app and the browser extension.
176-188: Well-structured component state management.The state management in this component follows React best practices with clear separation of concerns. The component properly manages the setup flow state, extension integration, and navigation.
190-214: Robust extension integration with fallback pattern.The component smartly tries to get the plugin ID from the URL first, then falls back to fetching it from the extension directly. This provides flexibility in how the setup flow is initiated.
270-280: Clever dynamic component composition pattern.The approach of dynamically creating the step components array and including the CompleteStep with extension-awareness is elegant and maintainable.
296-307: Smooth animation transitions between steps.The use of framer-motion for step transitions creates a polished user experience with smooth animations between steps.
frontend/lib/extension-utils.ts (1)
245-253: False negatives on sidebar support detection
"sidePanel" in window.chromefails in Manifest V2
(chrome.sidePanelwas introduced in MV3). For V2 builds the property
lives onchrome.*runtime*.sidePanel.Consider widening the check:
return typeof chrome?.sidePanel !== "undefined" || typeof (chrome as any)?.runtime?.sidePanel !== "undefined";extension/playwright.config.ts (1)
14-21: Headlessfalsemay break CI runnersFor Docker / GitHub Actions the default, non-GPU Linux image often
rejects a headed Chromium unless extra libraries are installed. Consider
switching to:- headless: false, + headless: !process.env.CI,or expose it via an env-flag (
HEADLESS=0 pnpm test).
| <title>登录中 - Nexus AI</title> | ||
| <link rel="icon" href="../assets/icon16.png"> | ||
| <script src="../utils/theme-helper.js" defer></script> | ||
| <meta http-equiv="Content-Security-Policy" content="default-src 'self'; connect-src 'self' http://localhost:* https://* http://* ws://localhost:* wss://*; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline';"> |
There was a problem hiding this comment.
Security concern with permissive Content Security Policy.
The CSP includes overly permissive directives like http://* in connect-src and 'unsafe-inline' for both script-src and style-src, which could expose the extension to security risks.
Consider restricting the CSP to only the specific domains and origins needed:
- <meta http-equiv="Content-Security-Policy" content="default-src 'self'; connect-src 'self' http://localhost:* https://* http://* ws://localhost:* wss://*; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline';">
+ <meta http-equiv="Content-Security-Policy" content="default-src 'self'; connect-src 'self' http://localhost:* https://* ws://localhost:* wss://*; script-src 'self'; style-src 'self';">If inline scripts or styles are necessary, consider using nonces or hashes instead of 'unsafe-inline'.
🤖 Prompt for AI Agents
In extension/pages/oauth-callback.html at line 9, the Content Security Policy is
too permissive, allowing broad connect-src domains and using 'unsafe-inline' for
scripts and styles. To fix this, restrict connect-src to only the specific
trusted domains your extension needs, removing wildcards like http://*. Replace
'unsafe-inline' in script-src and style-src by using nonces or hashes for any
inline scripts or styles to enhance security.
| export default async function SetupPage() { | ||
| // 检查用户登录状态 | ||
| const authState = await getAuthState(); | ||
|
|
||
| // 未登录直接跳转到登录页 | ||
| if (!authState.isAuthenticated) { | ||
| redirect("/login?redirect=/setup"); | ||
| } | ||
|
|
||
| // 如果已登录且已完成设置，重定向到仪表盘 | ||
| // 这里可以根据实际情况添加一个检查用户是否已完成设置的逻辑 | ||
| if (authState.isAuthenticated && false /* 已完成设置的条件 */) { | ||
| redirect("/dashboard"); | ||
| } |
There was a problem hiding this comment.
Fix the conditional logic for setup completion.
There's a hardcoded false value in the condition that checks if setup is completed. This creates a constant condition that will never be true, which could prevent users from being redirected properly once setup is actually completed.
- if (authState.isAuthenticated && false /* 已完成设置的条件 */) {
+ // TODO: Replace with actual setup completion check
+ if (authState.isAuthenticated && authState.user?.setupCompleted) {
redirect("/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.
| export default async function SetupPage() { | |
| // 检查用户登录状态 | |
| const authState = await getAuthState(); | |
| // 未登录直接跳转到登录页 | |
| if (!authState.isAuthenticated) { | |
| redirect("/login?redirect=/setup"); | |
| } | |
| // 如果已登录且已完成设置，重定向到仪表盘 | |
| // 这里可以根据实际情况添加一个检查用户是否已完成设置的逻辑 | |
| if (authState.isAuthenticated && false /* 已完成设置的条件 */) { | |
| redirect("/dashboard"); | |
| } | |
| export default async function SetupPage() { | |
| // 检查用户登录状态 | |
| const authState = await getAuthState(); | |
| // 未登录直接跳转到登录页 | |
| if (!authState.isAuthenticated) { | |
| redirect("/login?redirect=/setup"); | |
| } | |
| // 如果已登录且已完成设置，重定向到仪表盘 | |
| // 这里可以根据实际情况添加一个检查用户是否已完成设置的逻辑 | |
| // TODO: Replace with actual setup completion check | |
| if (authState.isAuthenticated && authState.user?.setupCompleted) { | |
| redirect("/dashboard"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In frontend/app/setup/page.tsx around lines 5 to 18, replace the hardcoded false
in the condition checking if the user has completed setup with the actual logic
or state that determines setup completion. This will ensure the redirect to the
dashboard occurs correctly when the user has finished setup.
| describe("SetupPage", () => { | ||
| it("应该渲染设置向导标题", async () => { | ||
| render(await SetupPage()); | ||
|
|
||
| expect( | ||
| screen.getByRole("heading", { name: /设置向导/i }), | ||
| ).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("应该显示多步骤进度指示器", async () => { | ||
| render(await SetupPage()); | ||
|
|
||
| expect(screen.getByTestId("setup-stepper")).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("应该在第一页显示欢迎信息", async () => { | ||
| render(await SetupPage()); | ||
|
|
||
| expect(screen.getByText(/欢迎使用/i)).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("应该能够导航到下一步", async () => { | ||
| render(await SetupPage()); | ||
|
|
||
| fireEvent.click(screen.getByRole("button", { name: /下一步/i })); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText(/个性化设置/i)).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
SetupPage test suite with basic test cases.
The test suite covers basic rendering and navigation functionality of the setup wizard. However, the tests appear to be using Jest while the PR objectives mention replacing Jest with Playwright for testing.
Also, consider translating the test descriptions to English for consistency with the codebase:
- it("应该渲染设置向导标题", async () => {
+ it("should render the setup wizard title", async () => {And similar translations for the other test cases.
🌐 Web query:
What's the difference between Jest and Playwright for React component testing?
💡 Result:
Difference Between Jest and Playwright for React Component Testing
Jest and Playwright are both popular choices in the JavaScript testing ecosystem, but they serve different purposes, especially when it comes to React component testing. Below is a comprehensive comparison:
Jest: Unit and Integration Testing
- Primary Use Case: Jest is primarily used for unit and integration testing. It operates in a Node.js environment and is designed for testing individual JavaScript functions, modules, and React components in isolation[6][7].
- Testing Approach: With Jest (typically in combination with utilities like React Testing Library), you render components in a simulated DOM (using jsdom), enabling you to quickly verify component logic, state changes, props handling, and interactions without running a real browser[6][9].
- Features:
- Built-in mocking for isolating dependencies
- Snapshot testing to monitor any changes to component output
- Fast test execution and parallelism for quick feedback cycles[1][4]
- Deep integration with React (officially supported by the React team)[4][6]
- Best For: Fast feedback when testing component logic, UI structure, and stateful behavior; ideal for test-driven development and CI pipelines[4][6].
Playwright: End-to-End (E2E) and Functional Testing
- Primary Use Case: Playwright is an end-to-end (E2E) testing framework designed to test how entire applications work in real browsers (Chromium, Firefox, WebKit)[5][7].
- Testing Approach: Playwright starts a real browser and interacts with your app just like a user would—clicking, typing, and navigating through the UI. For React apps, this means you can test components in the most realistic environment, including verifying actual rendering, network requests, and multi-browser compatibility[5][7].
- Features:
- Cross-browser and cross-device testing capabilities
- Support for parallel test execution and retries to handle flaky tests[1][3]
- Advanced features like network request mocking and fixture management[3][5]
- Suitable for both UI and API testing in realistic conditions[2][5]
- Best For: Comprehensive validation that your React components and overall application work as expected in real browsers, including user interactions, navigation, and integration with backend services[5][7].
Comparison Table
| Aspect | Jest | Playwright |
|---|---|---|
| Typical Scope | Unit & Integration tests | End-to-End & Functional tests |
| Testing Environment | Node.js + jsdom (simulated browser) | Real browser (Chromium, Firefox, WebKit) |
| Best Suited For | Testing component logic, state, and rendering | Testing user behavior, cross-browser compatibility |
| Mocking APIs | Extensive, easy mocking of modules and functions | Network and API request mocking at browser level |
| Speed | Very fast due to no actual browser being run | Slower (real browser), but optimized for E2E scale |
| Setup Complexity | Minimal, often pre-configured in React projects | Requires app server running and browser setup |
| Snapshot Testing | Yes | No (not primary use case) |
| React Integration | Deep, officially supported | Framework-agnostic |
Which One Should You Use?
- Use Jest when you want to verify your React components’ logic, props, and state handling in isolation or with simulated user events. It’s perfect for unit and small-scale integration testing and is the default choice in most React projects[4][6][9].
- Use Playwright when you need to ensure your whole application—including React components—works as expected across browsers and devices, covering real-world flows and full user journeys[5][7].
Most modern React applications benefit from using both:
- Start with Jest for fast, isolated unit/component tests.
- Add Playwright for crucial end-to-end scenarios to catch issues only visible in a real browser environment.
“Jest is designed for unit and integration testing, focusing on JavaScript code and application logic. Playwright, on the other hand, is a tool for end-to-end testing, enabling developers to test how applications perform across different browsers and devices.”[7]
Citations:
- 1: https://www.headspin.io/blog/playwright-vs-jest-which-framework-to-consider
- 2: https://javascript.plainenglish.io/api-testing-comparison-cypress-vs-playwright-vs-jest-2ff1f80c5a7b
- 3: https://knapsackpro.com/testing_frameworks/difference_between/jest/vs/playwright
- 4: https://raygun.com/blog/javascript-unit-testing-frameworks/
- 5: https://sapegin.me/blog/react-testing-5-playwright/
- 6: https://www.browserstack.com/guide/unit-testing-of-react-apps-using-jest
- 7: https://lemon.io/answers/jest/what-is-the-difference-between-jest-and-playwright/
- 8: https://blog.logrocket.com/comparing-next-js-testing-tools-strategies/
- 9: https://daily.dev/blog/react-functional-testing-best-practices
- 10: https://www.youtube.com/watch?v=Djl_Kv7iYKY
Align tests with Playwright and translate descriptions
The current SetupPageTest.tsx file is still using Jest + React Testing Library, but this PR’s goal is to migrate tests to Playwright’s E2E runner. Also, the test titles are in Chinese—let’s convert them to English for consistency.
Please address the following:
- Migrate this suite from Jest (
render,fireEvent,waitFor) to Playwright’s E2E style (test,page.goto,page.click,expect(page).toHaveText, etc.). - Confirm that Playwright is configured (e.g.
playwright.config.ts) and update import/runner calls accordingly. - Translate all
it("…", ...)descriptions to English.
Example diff for one test case:
- it("应该渲染设置向导标题", async () => {
- render(await SetupPage());
- expect(screen.getByRole("heading", { name: /设置向导/i })).toBeInTheDocument();
- });
+ test("should render the setup wizard title", async ({ page }) => {
+ await page.goto("/setup");
+ await expect(page.getByRole("heading", { name: /Setup Wizard/i })).toBeVisible();
+ });Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/__tests__/app/setup/SetupPageTest.tsx lines 18 to 48, the tests are
written using Jest and React Testing Library but need to be migrated to
Playwright's end-to-end testing style. Replace Jest functions like render,
fireEvent, and waitFor with Playwright's test runner syntax, using page.goto to
navigate, page.click for interactions, and Playwright's expect assertions for
validations. Also, translate all test descriptions from Chinese to English for
consistency. Ensure Playwright is properly configured and update imports and
test runner calls accordingly.
| // 存储pluginId用于后续验证 | ||
| chrome.storage.local.set({ "plugin_id": pluginId }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for plugin ID storage
While storing the plugin ID is essential for verification, there's no error handling for the storage operation.
If the storage operation fails, the verification process might break later. Add error handling:
-// 存储pluginId用于后续验证
-chrome.storage.local.set({ "plugin_id": pluginId });
+// 存储pluginId用于后续验证
+chrome.storage.local.set({ "plugin_id": pluginId }, () => {
+ if (chrome.runtime.lastError) {
+ console.error(`${LOG_PREFIX} 存储plugin_id失败: ${chrome.runtime.lastError.message}`);
+ } else {
+ console.log(`${LOG_PREFIX} 成功存储plugin_id用于后续验证`);
+ }
+});🤖 Prompt for AI Agents
In extension/background/messages/onboarding.ts around lines 19 to 20, the code
stores the plugin ID using chrome.storage.local.set but lacks error handling.
Modify the storage call to include a callback function that checks for
chrome.runtime.lastError and logs or handles the error appropriately to ensure
any storage failure is detected and managed.
| // 新增：尝试与Web端同步登录状态 | ||
| if (!profile?.isAuthenticated) { | ||
| try { | ||
| // 使用sendMessage向background脚本请求同步Web会话 | ||
| const response = await chrome.runtime.sendMessage({ | ||
| name: "auth", | ||
| body: { | ||
| action: "syncWebSession" | ||
| } | ||
| }) | ||
|
|
||
| if (response?.success) { | ||
| // 同步成功，重新获取用户配置 | ||
| const updatedProfile = await storage.get("userProfile") as UserProfile | ||
| if (updatedProfile) { | ||
| setUserProfile(updatedProfile) | ||
| console.log("[Nexus] 成功从Web端同步登录状态") | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.error("[Nexus] 同步Web登录状态失败:", error) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improved web session synchronization
The added code attempts to sync the user's login status with the web application when the user is not authenticated, which enhances the user experience by maintaining consistent login state across platforms.
Consider adding a loading state to prevent UI interactions during synchronization:
+const [isSyncing, setIsSyncing] = useState<boolean>(false);
// In the loadData function:
if (!profile?.isAuthenticated) {
+ setIsSyncing(true);
try {
// Existing code...
} catch (error) {
console.error("[Nexus] 同步Web登录状态失败:", error);
+ } finally {
+ setIsSyncing(false);
}
}Also, instead of just logging the error to the console, consider showing a user-friendly message:
} catch (error) {
console.error("[Nexus] 同步Web登录状态失败:", error);
+ // Use a toast notification or a status message component
+ // showNotification("无法同步登录状态，请重试", "error");
}🤖 Prompt for AI Agents
In extension/pages/options/index.tsx around lines 122 to 144, the code
synchronizes the login state with the web session but lacks a loading state and
user feedback on errors. Add a loading state variable to disable UI interactions
during synchronization and update it appropriately before and after the async
call. Also, replace or supplement the console.error with a user-friendly error
message display, such as setting an error state or showing a notification, to
inform the user when synchronization fails.
| await page.waitForTimeout(2000); | ||
|
|
||
| // 验证WebSocket连接 | ||
| const wsLogs = await page.evaluate(() => (window as any).wsLogs || []); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Use more specific assertion for WebSocket logs.
The current implementation just logs WebSocket connections without making specific assertions about them.
You should verify that WebSockets behave as expected by making more specific assertions:
- console.log('WebSocket连接日志:', wsLogs);
- console.log('已建立的WebSocket连接:', connections);
+ console.log('WebSocket连接日志:', wsLogs);
+ console.log('已建立的WebSocket连接:', connections);
+
+ // 验证WebSocket连接或连接尝试
+ if (connections.length > 0) {
+ // 如果成功建立了连接
+ expect(connections[0]).toMatch(/^wss?:\/\//);
+ } else if (wsLogs.length > 0) {
+ // 如果仅有连接尝试
+ expect(wsLogs).toContain(expect.stringMatching(/连接被CSP阻止|WebSocket连接/));
+ }Can you verify if the WebSockets are expected to connect successfully or be blocked by CSP rules?
🏁 Script executed:
#!/bin/bash
# Check if the manifest.json includes WebSocket permissions in CSP
cat extension/manifest.json | grep -E "connect-src|web_accessible_resources" | grep -E "ws:|wss:"Length of output: 248
Add explicit WebSocket connection assertions (CSP allows ws:// & wss://)
The manifest’s connect-src already permits both ws:// and wss://, so your e2e test should assert that at least one WebSocket connection is successfully established and that its URL is valid.
• File: extension/tests/e2e/onboarding.test.ts (at ~line 100)
• Change the logging-only block into explicit assertions:
const wsLogs = await page.evaluate(() => (window as any).wsLogs || []);
const connections = await page.evaluate(() => (window as any).wsConnections || []);
- console.log('WebSocket连接日志:', wsLogs);
- console.log('已建立的WebSocket连接:', connections);
+ console.log('WebSocket连接日志:', wsLogs);
+ console.log('已建立的WebSocket连接:', connections);
+
+ // 验证 WebSocket 连接已成功建立（CSP 已允许 ws:// 和 wss://）
+ expect(connections.length).toBeGreaterThan(0);
+ expect(connections[0]).toMatch(/^wss?:\/\//);📝 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 wsLogs = await page.evaluate(() => (window as any).wsLogs || []); | |
| const wsLogs = await page.evaluate(() => (window as any).wsLogs || []); | |
| const connections = await page.evaluate(() => (window as any).wsConnections || []); | |
| console.log('WebSocket连接日志:', wsLogs); | |
| console.log('已建立的WebSocket连接:', connections); | |
| // 验证 WebSocket 连接已成功建立（CSP 已允许 ws:// 和 wss://） | |
| expect(connections.length).toBeGreaterThan(0); | |
| expect(connections[0]).toMatch(/^wss?:\/\//); |
🤖 Prompt for AI Agents
In extension/tests/e2e/onboarding.test.ts at line 100, replace the current code
that only logs WebSocket connections with explicit assertions. Verify that at
least one WebSocket connection was successfully established by checking that
wsLogs contains entries, and assert that each WebSocket URL is valid and uses
either ws:// or wss:// as allowed by the manifest's CSP connect-src. This
ensures the test confirms expected WebSocket behavior rather than just logging
it.
| test.beforeEach(async ({ context }) => { | ||
| // 创建新的浏览器上下文并加载扩展 | ||
| context = await createExtensionContext(); | ||
| await loadExtension(context); | ||
|
|
||
| // 获取扩展ID | ||
| const backgroundPage = context.backgroundPages()[0]; | ||
| extensionId = await backgroundPage.evaluate(() => { | ||
| return chrome.runtime.id; | ||
| }); | ||
| console.log('Extension ID:', extensionId); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor context creation to avoid duplication.
You're creating a new context in beforeEach but also in createExtensionContext(), which is redundant.
- test.beforeEach(async ({ context }) => {
- // 创建新的浏览器上下文并加载扩展
- context = await createExtensionContext();
+ test.beforeEach(async ({ browser }) => {
+ // 创建新的浏览器上下文并加载扩展
+ const context = await createExtensionContext();
await loadExtension(context);
// 获取扩展ID
const backgroundPage = context.backgroundPages()[0];
extensionId = await backgroundPage.evaluate(() => {
return chrome.runtime.id;
});
console.log('Extension ID:', extensionId);
+ test.info().attachments.push({
+ name: 'extension-id.txt',
+ contentType: 'text/plain',
+ body: Buffer.from(extensionId)
+ });
});📝 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.beforeEach(async ({ context }) => { | |
| // 创建新的浏览器上下文并加载扩展 | |
| context = await createExtensionContext(); | |
| await loadExtension(context); | |
| // 获取扩展ID | |
| const backgroundPage = context.backgroundPages()[0]; | |
| extensionId = await backgroundPage.evaluate(() => { | |
| return chrome.runtime.id; | |
| }); | |
| console.log('Extension ID:', extensionId); | |
| }); | |
| test.beforeEach(async ({ browser }) => { | |
| // 创建新的浏览器上下文并加载扩展 | |
| const context = await createExtensionContext(); | |
| await loadExtension(context); | |
| // 获取扩展ID | |
| const backgroundPage = context.backgroundPages()[0]; | |
| extensionId = await backgroundPage.evaluate(() => { | |
| return chrome.runtime.id; | |
| }); | |
| console.log('Extension ID:', extensionId); | |
| test.info().attachments.push({ | |
| name: 'extension-id.txt', | |
| contentType: 'text/plain', | |
| body: Buffer.from(extensionId) | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In extension/tests/e2e/onboarding.test.ts around lines 19 to 30, the
test.beforeEach hook redundantly creates a new browser context both directly and
inside createExtensionContext(). Refactor by removing the direct context
creation in beforeEach and rely solely on createExtensionContext() to create and
return the new context. Update the beforeEach to assign the returned context
from createExtensionContext() without duplicating context creation.
| // 尝试通过扩展 ID 发送消息来检测扩展是否安装 | ||
| try { | ||
| // 注意：在生产环境需替换为实际的扩展 ID | ||
| const extensionId = | ||
| process.env.NEXT_PUBLIC_EXTENSION_ID || | ||
| "jhjpjjpkmepceonjdjgpfhhllilmdnae"; | ||
|
|
||
| return new Promise<boolean>((resolve) => { | ||
| // 设置超时，如果扩展没有响应 | ||
| const timeoutId = setTimeout(() => resolve(false), 500); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-coded default extension ID could ship to prod by accident
"jhjpjjpkmepceonjdjgpfhhllilmdnae" is kept as a fallback when
NEXT_PUBLIC_EXTENSION_ID is missing.
In a staging/prod build, forgetting to set the env-var would silently
switch the client to that hard-coded ID, making real users talk to the
wrong extension.
Consider failing fast instead of falling back:
-const extensionId =
- process.env.NEXT_PUBLIC_EXTENSION_ID ||
- "jhjpjjpkmepceonjdjgpfhhllilmdnae";
+const extensionId = process.env.NEXT_PUBLIC_EXTENSION_ID;
+if (!extensionId) {
+ throw new Error(
+ "NEXT_PUBLIC_EXTENSION_ID is not set – refusing to use hard-coded ID.",
+ );
+}📝 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.
| // 尝试通过扩展 ID 发送消息来检测扩展是否安装 | |
| try { | |
| // 注意：在生产环境需替换为实际的扩展 ID | |
| const extensionId = | |
| process.env.NEXT_PUBLIC_EXTENSION_ID || | |
| "jhjpjjpkmepceonjdjgpfhhllilmdnae"; | |
| return new Promise<boolean>((resolve) => { | |
| // 设置超时，如果扩展没有响应 | |
| const timeoutId = setTimeout(() => resolve(false), 500); | |
| // 尝试通过扩展 ID 发送消息来检测扩展是否安装 | |
| try { | |
| // 注意：在生产环境需替换为实际的扩展 ID | |
| const extensionId = process.env.NEXT_PUBLIC_EXTENSION_ID; | |
| if (!extensionId) { | |
| throw new Error( | |
| "NEXT_PUBLIC_EXTENSION_ID is not set – refusing to use hard-coded ID.", | |
| ); | |
| } | |
| return new Promise<boolean>((resolve) => { | |
| // 设置超时，如果扩展没有响应 | |
| const timeoutId = setTimeout(() => resolve(false), 500); |
🤖 Prompt for AI Agents
In frontend/lib/extension-utils.ts around lines 24 to 33, the code uses a
hard-coded default extension ID as a fallback if the environment variable
NEXT_PUBLIC_EXTENSION_ID is missing, which risks shipping this fallback to
production. To fix this, remove the hard-coded default and instead throw an
error or return a failure immediately when NEXT_PUBLIC_EXTENSION_ID is not set,
ensuring the app fails fast and does not silently use an incorrect extension ID.
| export async function createExtensionContext() { | ||
| const context = await chromium.launchPersistentContext('', { | ||
| headless: false, | ||
| args: [ | ||
| `--disable-extensions-except=${EXTENSION_PATH}`, | ||
| `--load-extension=${EXTENSION_PATH}`, | ||
| '--no-sandbox' | ||
| ] | ||
| }); |
There was a problem hiding this comment.
'' as user-data-dir may break on Windows & parallel runs
chromium.launchPersistentContext('', …) stores the profile in the
current working directory. In CI or parallel jobs that leads to cross-run
state bleed and file-locking issues.
Use tmp.dirSync().name:
-import { tmpdir } from 'os';
-…
-const context = await chromium.launchPersistentContext('', {
+import { tmpdir } from 'os';
+import fs from 'fs';
+const userDataDir = fs.mkdtempSync(path.join(tmpdir(), 'pw-ext-'));
+const context = await chromium.launchPersistentContext(userDataDir, {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In extension/tests/utils/test-helpers.ts around lines 21 to 29, the call to
chromium.launchPersistentContext uses an empty string as the user-data-dir,
which can cause issues on Windows and during parallel test runs due to shared
state and file locks. Replace the empty string with a unique temporary directory
path generated by tmp.dirSync().name to isolate the profile for each run and
avoid conflicts.
| export async function mockChromeAPI(page: Page) { | ||
| await page.addInitScript(() => { | ||
| window.chrome = { | ||
| runtime: { | ||
| sendMessage: (message, callback) => { | ||
| if (callback) { | ||
| callback({ success: true }); | ||
| } | ||
| return true; | ||
| }, | ||
| onMessage: { | ||
| addListener: () => {} | ||
| } | ||
| }, | ||
| management: { | ||
| getAll: (callback) => { | ||
| callback([{ | ||
| id: 'test-extension-id', | ||
| name: 'Nexus AI', | ||
| enabled: true | ||
| }]); | ||
| } | ||
| } | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
mockChromeAPI diverges from real API signatures
The stubbed sendMessage ignores the extensionId argument and
overloads parameter order (message, callback). When frontend code calls
sendMessage(extensionId, msg, cb) the mock returns undefined,
potentially hiding bugs.
Recommend aligning signature:
-sendMessage: (message, callback) => {
+sendMessage: (_id, message, _opts, callback) => {Alternatively use vi.fn() / Playwright’s page.exposeFunction to gain
call-tracking.
📝 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 mockChromeAPI(page: Page) { | |
| await page.addInitScript(() => { | |
| window.chrome = { | |
| runtime: { | |
| sendMessage: (message, callback) => { | |
| if (callback) { | |
| callback({ success: true }); | |
| } | |
| return true; | |
| }, | |
| onMessage: { | |
| addListener: () => {} | |
| } | |
| }, | |
| management: { | |
| getAll: (callback) => { | |
| callback([{ | |
| id: 'test-extension-id', | |
| name: 'Nexus AI', | |
| enabled: true | |
| }]); | |
| } | |
| } | |
| }; | |
| }); | |
| export async function mockChromeAPI(page: Page) { | |
| await page.addInitScript(() => { | |
| window.chrome = { | |
| runtime: { | |
| sendMessage: (_id, message, _opts, callback) => { | |
| if (callback) { | |
| callback({ success: true }); | |
| } | |
| return true; | |
| }, | |
| onMessage: { | |
| addListener: () => {} | |
| } | |
| }, | |
| management: { | |
| getAll: (callback) => { | |
| callback([{ | |
| id: 'test-extension-id', | |
| name: 'Nexus AI', | |
| enabled: true | |
| }]); | |
| } | |
| } | |
| }; | |
| }); |
🤖 Prompt for AI Agents
In extension/tests/utils/test-helpers.ts around lines 100 to 124, the
mockChromeAPI function's sendMessage method does not match the real Chrome API
signature, as it ignores the extensionId argument and uses an incorrect
parameter order. To fix this, update sendMessage to accept (extensionId,
message, callback) parameters in that order and handle them accordingly to mimic
the real API behavior. Optionally, replace the stub with vi.fn() or Playwright's
page.exposeFunction to enable call tracking and better test verification.
- Introduced new endpoints for managing prompts and tags, including creation, reading, updating, and deletion functionalities. - Updated the OpenAPI client and types to support the new prompt and tag operations. - Enhanced the environment configuration to include necessary API keys for OpenAI, GitHub, and Volcengine. - Refactored the database models and CRUD operations to accommodate the new features, ensuring proper integration with the existing architecture. - Improved the response handling in the API middleware for consistent error and success responses.
- Added new API functions for managing prompts and tags, including create, read, update, and delete operations. - Updated the OpenAPI client to support the new prompt functionalities. - Introduced a new button in the homepage for navigating to the prompts section. - Enhanced the dashboard layout with tooltips for better user guidance on navigation links. - Configured ESLint and TypeScript settings in the Next.js configuration to ignore build-time checks for faster builds. - Updated package dependencies to include new libraries for improved functionality.
- Updated password reset confirmation page to include better user feedback and loading states. - Enhanced form validation and error handling for password reset actions. - Improved test cases for password reset and registration processes, including localization of labels and messages. - Mocked additional Next.js navigation hooks and React state management for more comprehensive testing. - Refactored password reset action to include basic email validation and improved error messages.
There was a problem hiding this comment.
Actionable comments posted: 28
♻️ Duplicate comments (3)
frontend/__tests__/app/setup/SetupPageTest.tsx (1)
19-47: Test suite is skipped and needs migration to Playwright.This test suite is marked with
describe.skip, making the tests inactive. Additionally, these tests use Jest and React Testing Library rather than Playwright as mentioned in the PR objectives.The test descriptions are currently in Chinese and should be translated to English for consistency with the codebase. For example:
- it("应该渲染设置向导标题", () => { + it("should render setup wizard title", () => {Since the PR mentions transitioning from Jest to Playwright for testing, these tests should be migrated to the Playwright format.
frontend/app/openapi-client/types.gen.ts (1)
498-502: Same linter warning as admin build – suppress in generated codeSee earlier note in
admin/src/client/types.gen.ts. Add a blanket ignore directive or exclude theopenapi-clientfolder from Biome/ES-lint so CI does not fail on generatedvoidaliases.Also applies to: 556-559
🧰 Tools
🪛 Biome (1.9.4)
[error] 500-500: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
backend/app/crud.py (1)
106-120:get_items()repeats the same in-Python slicingSame performance concern as above. Re-use
get_multi()or apply SQL-leveloffset/limitto avoid full-table loads.
🧹 Nitpick comments (56)
admin/playwright.config.ts (2)
14-14: Global timeout still short compared to new 5-minute limits elsewhereAdmin tests now have 60 s per test, while the extension suite was raised to 5 min.
If the same E2E flows are exercised here (login + dashboard bootstrap, etc.) the 1-minute ceiling may still be hit on CI under load.‣ Verify average run-time for slowest admin tests.
‣ Consider bumping to 2-3 min or re-using the 5 min value for consistency.
32-35: Always-on screenshots may bloat artefacts & includes non-English commentCapturing a screenshot for every single step (
screenshot: 'on') quickly produces hundreds of large PNGs and inflates the HTML report. In CI this can exceed artefact size limits.Additionally, the in-line comment is in Chinese, whereas the rest of the config is English – please keep comments consistent.
- // 添加截图设置 - screenshot: 'on', + // Capture screenshots only when a test fails (keeps artefacts small) + screenshot: 'only-on-failure',If full-run screenshots are truly required, consider uploading them to external storage and cleaning the local report afterwards.
admin/tests/auth.setup.ts (1)
44-46: Screenshot filename will be overwritten on parallel/redo runsEvery run writes to
test-results/after-login-click.png, so subsequent workers or retries clobber the same file.- await page.screenshot({ path: 'test-results/after-login-click.png' }) + await page.screenshot({ path: `test-results/after-login-click-${Date.now()}.png` })Appends a timestamp to guarantee uniqueness.
backend/app/alembic/versions/5ee6589f44e3_merge_prompt_tables_with_main.py (1)
8-10: Consider removing unused imports.The imported modules
op,sqlalchemy, andsqlmodel.sql.sqltypesaren't used in this migration script since bothupgrade()anddowngrade()functions are empty.-from alembic import op -import sqlalchemy as sa -import sqlmodel.sql.sqltypes +from alembic import op # Kept for consistency with other migration files🧰 Tools
🪛 Ruff (0.11.9)
8-8:
alembic.opimported but unusedRemove unused import:
alembic.op(F401)
9-9:
sqlalchemyimported but unusedRemove unused import:
sqlalchemy(F401)
10-10:
sqlmodel.sql.sqltypesimported but unusedRemove unused import:
sqlmodel.sql.sqltypes(F401)
frontend/app/dashboard/layout.tsx (1)
82-99: Missing styling classes on the Prompt Hub linkThe new Prompt Hub link is missing the text styling classes that are present on the other navigation links, which could lead to inconsistent hover behavior.
Add the missing styling classes to maintain consistent styling with other navigation links:
<Link href="/prompts" aria-label="Prompt Hub" + className="flex items-center gap-2 text-muted-foreground hover:text-foreground" >frontend/__tests__/passwordReset.test.tsx (1)
49-119: Test suite properly covers UI states for password reset flow.The test suite provides good coverage of different UI states (default, success, error, validation errors) and correctly asserts the presence of expected UI elements based on these states.
Consider translating the Chinese strings in both the tests and assertions to English for consistency with other code. For example:
- expect(screen.getByRole("textbox", { name: /邮箱/i })).toBeInTheDocument(); + expect(screen.getByRole("textbox", { name: /Email/i })).toBeInTheDocument();- const messageElement = screen.queryByText((content) => { - return content.includes("密码重置链接已发送到您的邮箱"); + const messageElement = screen.queryByText((content) => { + return content.includes("Password reset link has been sent to your email");frontend/app/prompts/_components/DuplicateButton.tsx (1)
13-44: Good error handling and state management.The component properly manages loading state and provides user feedback through toast notifications. Error handling is comprehensive with specific messages for different error scenarios.
Consider adding a more specific error type check beyond just
instanceof Errorto handle different API error types that might occur.- description: error instanceof Error ? error.message : "未知错误", + description: error instanceof Error + ? error.message + : (typeof error === 'object' && error !== null && 'message' in error) + ? String(error.message) + : "未知错误",frontend/app/prompts/edit/[id]/page.tsx (1)
92-158: Strong implementation of content component with concurrent data fetching.Using Promise.all for fetching prompt and tag data concurrently improves performance. The comprehensive error handling provides clear feedback to users when things go wrong.
Consider extracting the error alert UI into a reusable component since it's used multiple times with similar structure.
frontend/app/password-recovery/confirm/page.tsx (1)
55-154: Consider implementing i18n for user-facing text.The UI has been significantly improved with a modern design including background gradients, animated elements, and improved form feedback. However, all user-facing text is in Chinese, which suggests missing internationalization.
Consider implementing a proper i18n solution to support multiple languages. This would make the application more accessible to users worldwide.
frontend/app/prompts/tags/_components/DeleteTagButton.tsx (1)
29-43: Enhance modal accessibility and reusability.The current modal implementation uses fixed positioning and z-index, which might conflict with other overlays. Consider using a reusable Dialog component that handles keyboard interactions (such as Escape to close) and focuses management for better accessibility.
- {confirming && ( - <div className="fixed inset-0 flex items-center justify-center bg-black bg-opacity-30 z-50"> - <div className="bg-white rounded shadow-lg p-6 w-80"> - <div className="mb-4 text-center text-sm text-gray-700">确定要删除该标签吗？此操作不可撤销。</div> - <div className="flex justify-end gap-2"> - <Button variant="outline" size="sm" onClick={() => setConfirming(false)}> - 取消 - </Button> - <Button variant="destructive" size="sm" onClick={handleDelete}> - 确认删除 - </Button> - </div> - </div> - </div> - )} + <Dialog open={confirming} onOpenChange={setConfirming}> + <DialogContent className="w-80"> + <DialogTitle>{t('deleteTag')}</DialogTitle> + <DialogDescription>{t('deleteTagConfirmation')}</DialogDescription> + <DialogFooter> + <Button variant="outline" size="sm" onClick={() => setConfirming(false)}> + {t('cancel')} + </Button> + <Button variant="destructive" size="sm" onClick={handleDelete}> + {t('confirmDelete')} + </Button> + </DialogFooter> + </DialogContent> + </Dialog>frontend/__tests__/passwordResetPage.test.tsx (1)
54-55: Consider using test IDs instead of text content for selections.The tests are selecting elements based on their text content in Chinese, which would break if the text changes or if internationalization is implemented. Consider using test IDs for more robust element selection.
- expect(screen.getByRole("textbox", { name: /邮箱/i })).toBeInTheDocument(); + expect(screen.getByTestId("email-input")).toBeInTheDocument(); - expect(screen.getByRole("button", { name: /发送重置链接/i })).toBeInTheDocument(); + expect(screen.getByTestId("submit-button")).toBeInTheDocument();And in your component:
<input data-testid="email-input" ... /> <button data-testid="submit-button" ... />Also applies to: 69-69, 83-83, 99-99
frontend/__tests__/passwordResetConfirm.test.tsx (1)
11-14: Dead helper –customRenderis defined but never usedRemove unused code to reduce noise and avoid ESLint warnings.
-// 创建自定义 render 函数处理 App Router 上下文 -const customRender = (ui: ReactNode) => { - return render(ui); -};frontend/app/prompts/tags/_components/TagForm.tsx (3)
143-147: Use the native colour picker for better UX and validationAn
<input type="color">gives users a convenient picker and guarantees valid hex output, eliminating the need for manual blur-fix logic.-<Input - type="text" - placeholder="#RRGGBB" +<Input + type="color" {...field}
147-164: Blur-fix logic still allows invalid short hex codesIf a user types “#1” the fixer keeps “#1”, which fails schema validation and may be confusing.
Consider normalising 3-digit shorthands (#abc→#aabbcc) or simply rely on the native colour input (see previous comment).
176-183: Disable inputs while submitting to prevent double submitsOnly the button is disabled; users could still edit fields mid-request.
Wrap the form with<fieldset disabled={submitting}>or passdisabled={submitting}to each<Input>for full protection.frontend/components/ui/submitButton.tsx (1)
50-50: Consider using a translation system instead of hardcoded textThe translation of "Processing..." to "处理中..." is a positive internationalization step, but consider using a translation system for better maintainability.
- 处理中... + {t('processing')}Where
twould be imported from your translation library.frontend/README-TEST-FIX.md (1)
48-48: Minor grammatical correction neededThere's a small grammatical issue in the Chinese text.
- 检查注册页面源码以了解确切的提交机制 + 检查注册页面源码以了解确切地提交机制The modifier for verbs in Chinese typically follows the pattern "adjective + 地 + verb".
🧰 Tools
🪛 LanguageTool
[uncategorized] ~48-~48: 动词的修饰一般为‘形容词（副词）+地+动词’。您的意思是否是：确切"地"提交
Context: ... registerPage 表单提交问题： - 检查注册页面源码以了解确切的提交机制 - 根据实际调用方式更新测试 3. 更新 login.test...(wb4)
frontend/__tests__/passwordResetConfirmPage.test.tsx (2)
11-14: Remove unused custom render functionThe
customRenderfunction doesn't add any functionality beyond the standard render function.- // 创建自定义 render 函数处理 App Router 上下文 - const customRender = (ui: ReactNode) => { - return render(ui); - };
146-147: Use more specific selector instead of array indexUsing
getAllByText(...)[0]suggests multiple elements with the same text, which could lead to flaky tests.- // 使用getAllByText来处理可能有多个匹配的情况 - expect(screen.getAllByText("密码已成功重置！正在跳转到登录页面...")[0]).toBeInTheDocument(); + // Use a more specific selector or container to target the exact element + expect(screen.getByRole("alert")).toHaveTextContent("密码已成功重置！正在跳转到登录页面...");Assuming the success message is in an element with role="alert". If not, use a more appropriate selector based on your component structure.
litellm/config.yaml (1)
17-17: Remove trailing whitespaceThere are trailing spaces on these lines that should be removed.
- base_url: "os.environ/OPENAI_BASE_URL" + base_url: "os.environ/OPENAI_BASE_URL"- api_key: "os.environ/OR_API_KEY" + api_key: "os.environ/OR_API_KEY"Also applies to: 47-47
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 17-17: trailing spaces
(trailing-spaces)
frontend/app/prompts/searchForm.tsx (3)
26-26: Remove unnecessary non-null assertion operator.The non-null assertion operator (
!) here is unnecessary because you've already checked ifsearchParams.get('tags')exists in the conditional.- const [selectedTags, setSelectedTags] = useState<string[]>( - searchParams.get('tags') ? searchParams.get('tags')!.split(',') : [] - ); + const [selectedTags, setSelectedTags] = useState<string[]>( + searchParams.get('tags') ? searchParams.get('tags').split(',') : [] + );
23-27: Consider translating comments to English for consistency.There are Chinese comments throughout the code. If your codebase standard is English comments, consider translating them for consistency.
- // 初始化状态 + // Initialize state
148-148: Improve color opacity handling.The current approach of appending "20" to color hex codes works but is not very maintainable or clear. Consider using a utility function to convert hex colors to RGBA format for better readability and maintainability.
- style={{ backgroundColor: `${tag.color}20` || '#f3f3f3' }} + style={{ backgroundColor: tag.color ? hexToRgba(tag.color, 0.12) : '#f3f3f3' }}You would need to add a utility function:
// Add this function to a utils file or at the top of your component function hexToRgba(hex: string | undefined, alpha: number): string { if (!hex) return '#f3f3f3'; // Remove # if present hex = hex.replace(/^#/, ''); // Parse hex to RGB const bigint = parseInt(hex, 16); const r = (bigint >> 16) & 255; const g = (bigint >> 8) & 255; const b = bigint & 255; return `rgba(${r}, ${g}, ${b}, ${alpha})`; }frontend/app/prompts/deleteButton.tsx (1)
61-95: Consider internationalization for UI text.The component contains hardcoded Chinese text. If your application needs to support multiple languages, consider implementing internationalization.
You could use a translation library or context to manage these strings:
- <AlertDialogTitle>确认删除</AlertDialogTitle> + <AlertDialogTitle>{t('confirm_delete')}</AlertDialogTitle> - <AlertDialogDescription> - 您确定要删除这个提示词吗？此操作无法撤销，一旦删除将无法恢复。 - </AlertDialogDescription> + <AlertDialogDescription> + {t('delete_confirmation_message')} + </AlertDialogDescription> - <AlertDialogCancel disabled={isDeleting}>取消</AlertDialogCancel> + <AlertDialogCancel disabled={isDeleting}>{t('cancel')}</AlertDialogCancel> - {isDeleting ? "删除中..." : "确认删除"} + {isDeleting ? t('deleting') : t('confirm_delete')}frontend/components/actions/password-reset-action.ts (4)
14-21: Enhance email validation robustness.The current email validation only checks for the presence of "@", which is minimal. Consider using a more comprehensive validation pattern for emails.
- // 基本的电子邮件验证 - if (!email || !email.includes("@")) { + // Basic email validation + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + if (!email || !emailRegex.test(email)) { return { errors: { - email: ["请输入有效的电子邮件地址"] + email: ["请输入有效的电子邮件地址"] } }; }
34-34: Ensure consistent use of internationalization.Hard-coded Chinese strings are used throughout the file. If you're implementing internationalization, consider using a translation library instead of hard-coded strings.
You might want to use a translation function instead of hardcoded strings:
- return { message: "密码重置链接已发送到您的邮箱，请查收。" }; + return { message: t("password_reset_link_sent") }; - server_error: "发生了意外错误，请稍后再试。", + server_error: t("unexpected_error_try_again"), - return { message: "密码已成功重置！正在跳转到登录页面..." }; + return { message: t("password_reset_success_redirecting") }; - server_error: "发生了意外错误，请稍后再试。", + server_error: t("unexpected_error_try_again"),Also applies to: 38-38, 73-73, 77-77
14-14: Translate comments to English.The comment on line 14 is in Chinese. For consistency with code, consider translating all comments to English.
- // 基本的电子邮件验证 + // Basic email validation
72-72: Translate comments to English.The comment on line 72 is in Chinese. For consistency with code, consider translating all comments to English.
- // 成功后先返回成功消息，延迟重定向让用户看到成功提示 + // Return success message first, then redirect after delay to let users see the confirmationfrontend/app/prompts/create/page.tsx (2)
22-36: Consider server-side redirect instead of rendering an alert for unauthenticated users.Because this component is a Server Component, you can directly call
redirect("/login")to short-circuit rendering and save an extra network round-trip of sending markup that just re-navigates.-if (!authState.isAuthenticated) { - return ( - <div className="container py-10"> - … - </div> - ); -} +if (!authState.isAuthenticated) { + redirect("/login"); +}(Optional: keep current UX if the alert is intentional.)
78-85: Graceful typing forfetchTags()error branch.
fetchTags()appears to return eitherTag[]or{ error: string }.
Consider giving it a discriminated-union return type so TypeScript can enforce theArray.isArrayguard and surface autocompletion for both branches.type FetchTagsOk = Tag[]; type FetchTagsErr = { error: string }; function fetchTags(): Promise<FetchTagsOk | FetchTagsErr> { … }backend/app/alembic/versions/add_prompt_management_tables.py (2)
74-75: Composite uniqueness for(prompt_id, version)recommended.Two rows with the same version number for a prompt would break deterministic version history.
op.create_index(op.f('ix_prompt_versions_version'), 'prompt_versions', ['version'], unique=False) +op.create_unique_constraint('uq_prompt_versions_prompt_id_version', + 'prompt_versions', ['prompt_id', 'version'])
1-99: Minor style issues flagged by Ruff.Trailing/blank-line whitespace and missing EOF newline will fail
pre-commithooks configured with Ruff/black.Fix by running
ruff --fixorblack ..🧰 Tools
🪛 Ruff (0.11.9)
4-4: Trailing whitespace
Remove trailing whitespace
(W291)
10-10:
sqlmodelimported but unusedRemove unused import:
sqlmodel(F401)
52-52: Blank line contains whitespace
Remove whitespace from blank line
(W293)
72-72: Blank line contains whitespace
Remove whitespace from blank line
(W293)
99-99: Trailing whitespace
Remove trailing whitespace
(W291)
99-99: No newline at end of file
Add trailing newline
(W292)
frontend/app/password-recovery/page.tsx (3)
3-11: Merge duplicated React imports.
useEffect,useState, andSuspenseare imported separately in line 11 whileuseActionStateis imported in line 3. Consolidating reduces noise and avoids accidental version mismatches.-import { useActionState } from "react"; +import { useActionState, useEffect, useState, Suspense } from "react"; … -import { useEffect, useState, Suspense } from "react";
43-49: Early return for authenticated users improves UX.Instead of waiting for React to mount, you can short-circuit during the first render when
useris already known (SSR). This avoids flashing the form.if (user && !isLoading) { router.replace("/dashboard"); return null; }
78-90: Success alert lacks ARIA role.Add
role="alert"to the success container so screen-reader users are notified automatically.-<div className="bg-green-50 …" > +<div role="alert" className="bg-green-50 …" >frontend/app/prompts/[id]/page.tsx (1)
20-35: Consider memoising / re-using data to cut one API round-trip
fetchPromptis invoked ingenerateMetadataand again insidePromptDetailContent, doubling latency and server-load for every page view.
If the helper already caches internally this is harmless, otherwise you can:• return the prompt in
generateMetadataviaheadersorcookiesand read it later, or
• callfetchPromptonce in the outer component and pass the result down.Not critical, but improves TTFB and quota usage.
Also applies to: 106-110
frontend/app/prompts/_components/PromptForm.tsx (2)
92-100: Post-create UX could be improvedAfter successful creation (
addPrompt) the user remains on the form with no automatic navigation.
Consider reading the returned prompt ID androuter.push(/prompts/${id})so the user sees the new resource immediately (mirrors the update flow).Optional but enhances flow consistency.
Also applies to: 120-137
1-10: Unused importuuidv4can be removed
uuidv4is imported yet never referenced, adding to bundle size.frontend/components/setup/SetupContent.tsx (2)
194-205: Use the standardURLSearchParams.get()instead of optional-chained invocation
useSearchParams()in Next returns a non-nullReadonlyURLSearchParamswhose.get()method is always present.
UsingsearchParamsObj.get?.("plugin_id")is therefore unnecessary and slightly confusing. Switching to a direct call makes the intent clearer and avoids the subtle risk of calling anundefinedvalue if the API changes.-const pluginId = searchParamsObj.get?.("plugin_id") || null; -const callback = searchParamsObj.get?.("extension_callback") || null; +const pluginId = searchParamsObj.get("plugin_id") ?? null; +const callback = searchParamsObj.get("extension_callback") ?? null;
254-256: Preferrouter.replaceafter a server-side redirectWhen the extension flow is not involved, the user is forwarded to
/dashboard:router.push("/dashboard");Using
router.replaceavoids leaving an unnecessary “/setup” entry in the browser history, preventing the Back button from returning the user to a page that immediately redirects again.-router.push("/dashboard"); +router.replace("/dashboard");frontend/openapi.json (1)
1-1: Duplicate tag entries in several operationsIn multiple path definitions the
tagsarray reads["prompts", "prompts"].
OpenAPI generators and some tooling treat this as two distinct tags and may render duplicate sections in the generated docs.Search & replace the redundant second entry:
-"tags": ["prompts", "prompts"] +"tags": ["prompts"]🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 1: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
frontend/app/prompts/page.tsx (2)
72-83: Leverage optional chaining for a shorter null-safe checkThe current pattern is perfectly valid but can be condensed:
-{prompt.tags && prompt.tags.map((tag) => ( +{prompt.tags?.map((tag) => (This removes one level of boolean logic and quiets the Biome linter warning.
🧰 Tools
🪛 Biome (1.9.4)
[error] 74-82: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
186-193: Remove debugconsole.logcalls from server components
console.log('[prompts-${renderID}] …')executes on every request and will pollute production logs.
If the information is still useful, switch to a structured logger or wrap it behind a feature flag.backend/app/api/routes/prompts.py (1)
242-251: Redundantif Prompt.description is not Noneexpression
Prompt.descriptionis a mapped column attribute;Prompt.description is not Noneis alwaysTrue, so the ternary operator addsFalseliteral to theor_, generatingOR false, which the SQL compiler will warn about.- col(Prompt.description).contains(search) - if Prompt.description is not None - else False, + col(Prompt.description).contains(search),If you want to ignore NULL descriptions, add
.is_not(None)or wrap withfunc.coalesce.frontend/jest.setup.js (4)
14-40:framer-motionproxy swallows props & breaks component identityReturning
jest.fn(({ children, ...rest }) => …)inside aProxymeans everymotion.*call yields a new mock component each render.
- React treats components with different identities as unmount/mount → state loss.
- All props except
childrenare discarded in the fallback div path.Consider memoising the generated component or forwarding props unchanged:
return { …actualFramerMotion, AnimatePresence: jest.fn(({ children }) => <>{children}</>), motion: new Proxy({}, { get: (target, prop) => { - return jest.fn(({ children, ...rest }) => { + if (!target[prop]) { + // create once per tag to preserve identity + target[prop] = ({ children, ...rest }) => { const Component = prop; try { // @ts-expect-error dynamic element - return <Component {...rest}>{children}</Component>; + return <Component {...rest}>{children}</Component>; } catch { return ( <div data-testid={`mock-motion-${String(prop)}`} {...rest}> {children} </div> ); } - }); + }; + } + return target[prop]; } }),
72-85: Permanent override ofconsole.errorhides unrelated failuresSuppressing errors globally can mask genuine regressions. Prefer a scoped spy inside the few tests that expect
LOGIN_BAD_CREDENTIALS.-const originalConsoleError = console.error; -console.error = (...args) => { - if (args[0] && typeof args[0] === 'string' && - args[0].includes('LOGIN_BAD_CREDENTIALS')) { - return; - } - originalConsoleError(...args); -};Inside the specific test file:
beforeEach(() => { jest.spyOn(console, "error").mockImplementation((...args) => { if (String(args[0]).includes("LOGIN_BAD_CREDENTIALS")) return; // fall back to original behaviour (console.error as jest.Mock).mockRestore(); }); });
162-179: Customconsole.logproxy bypasses Jest’s diff & timing outputRedirecting logs to
process.stdout.write:
- Loses colourised diff output that Jest provides.
- Can break reporters that monkey-patch
console.log.Unless absolutely necessary, remove the spy or scope it per-test with
jest.spyOn(console, 'log').
182-186: Avoiddeletefor global cleanup – use reassignment
delete global.mockSearchParamsincurs hidden de-opts in V8. Re-assigning toundefinedachieves the same without performance penalties.-afterEach(() => { - jest.clearAllMocks(); - delete global.mockSearchParams; - delete global.mockPathname; -}); +afterEach(() => { + jest.clearAllMocks(); + global.mockSearchParams = undefined; + global.mockPathname = undefined; +});🧰 Tools
🪛 Biome (1.9.4)
[error] 184-185: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
backend/app/models/prompt.py (1)
62-66: Serialising Pydantic models inside a JSON column needs a custom encoder/decoder
input_vars: list[InputVariable] | None = Field(..., sa_column=Column(JSON))stores Python dicts in Postgres. On retrieval SQLModel will return raw dicts, notInputVariableinstances, breaking type expectations.Add a validator or custom
TypeDecoratorto (de)serialise:from sqlalchemy.types import TypeDecorator, JSON as SA_JSON class InputVariableList(TypeDecorator): impl = SA_JSON cache_ok = True def process_bind_param(self, value, dialect): return [v.model_dump() for v in value] if value else None def process_result_value(self, value, dialect): return [InputVariable(**v) for v in value] if value else None # then input_vars: list[InputVariable] | None = Field( default=None, sa_column=Column(InputVariableList) )frontend/components/actions/prompts-action.ts (2)
124-139: Avoid printing even partial auth tokens to server logs
console.logingtoken.substring(0, 5)still exposes sufficient entropy for some token formats and violates the principle of minimising secret exposure. Remove or mask completely.- console.log( - `[${requestId}] 获取prompts数据，token: ${token.substring(0, 5)}...`, - ); + console.log(`[${requestId}] 获取prompts数据`);
381-392:JSON.parseon untrustedFormDatafields can throwIf the client sends malformed JSON (
tag_ids,input_vars) the action will raise and return a 500.
Add defensive parsing:-const tagIdsValue = formData.get('tag_ids') as string; -const tag_ids = tagIdsValue ? JSON.parse(tagIdsValue) : []; +let tag_ids: string[] = []; +try { + const raw = formData.get('tag_ids'); + if (raw) tag_ids = JSON.parse(String(raw)); +} catch { + return { error: "tag_ids 格式无效", status: 400 }; +}Repeat for
input_vars.admin/src/client/types.gen.ts (1)
348-353: Suppress linter for generatedvoidresponse aliasesBiome/ES-lint flags the standalone
voidaliases (e.g.PromptsDeleteTagResponse = (void)) as “confusing outside a return type”.
Because this file is 100 % auto-generated, changing the output manually is brittle.
Instead, add a project-level ignore rule or a/* biome-ignore lint/suspicious/noConfusingVoidType */header for generated artifacts and keep the spec-correctvoidtype untouched.Also applies to: 500-501
🧰 Tools
🪛 Biome (1.9.4)
[error] 352-352: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
backend/app/crud.py (4)
73-80: Unnecessarysession.add(obj)on an already-loaded entity
update()loads the entity withsession.get()(so it’s already attached).
Callingsession.add(obj)is redundant and can confuse readers.
After mutating attributes you can safelycommit()andrefresh().- session.add(obj) session.commit()
251-269:add_token_to_blacklist()duplicates timestamp fields
TokenBlacklistnow stores bothcreated_atandblacklisted_at. Creating two virtually identical datetime columns complicates analytics and filtering. Consider keeping justblacklisted_at(time of insertion) and derive “created” from it, or document the semantic difference.
122-144: Function-local imports duplicate top-level imports
create_user()(and several others) dofrom app.models import UserdespiteUseralready being imported at module scope. Unless you are deliberately breaking import cycles, prefer the existing import to improve clarity and micro-performance.
322-359: Tag CRUD helpers lack DB-level pagination and input validation
get_tags()– same “load all then slice” issue; reuse genericget_multi().create_tag()/update_tag()acceptAny; we risk runtime surprises. Either type them (TagCreate,TagUpdate) or validate withmodel_validate.
📜 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)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (84)
.cursor/rules/extension.mdc(1 hunks).cursor/rules/frontend.mdc(1 hunks).cursor/rules/python.mdc(1 hunks).cursor/rules/tdd-sequential-api-flow.mdc(1 hunks).cursor/rules/ui-colors.mdc(1 hunks).env.example(1 hunks)Makefile(2 hunks)admin/playwright.config.ts(2 hunks)admin/src/client/sdk.gen.ts(2 hunks)admin/src/client/types.gen.ts(3 hunks)admin/tests/auth.setup.ts(1 hunks)backend/app/__init__.py(1 hunks)backend/app/alembic/env.py(1 hunks)backend/app/alembic/versions/5ee6589f44e3_merge_prompt_tables_with_main.py(1 hunks)backend/app/alembic/versions/add_prompt_management_tables.py(1 hunks)backend/app/api/main.py(2 hunks)backend/app/api/middlewares/response.py(2 hunks)backend/app/api/routes/__init__.py(2 hunks)backend/app/api/routes/extension_auth.py(2 hunks)backend/app/api/routes/google_oauth.py(2 hunks)backend/app/api/routes/items.py(1 hunks)backend/app/api/routes/prompts.py(1 hunks)backend/app/base.py(0 hunks)backend/app/crud.py(2 hunks)backend/app/models/__init__.py(1 hunks)backend/app/models/prompt.py(1 hunks)backend/app/tests/api/test_prompts.py(1 hunks)backend/app/tests/scripts/test_backend_pre_start.py(1 hunks)backend/app/tests/scripts/test_test_pre_start.py(1 hunks)backend/app/tests/utils/item.py(2 hunks)backend/app/tests/utils/test_db.py(2 hunks)backend/app/tests/utils/user.py(2 hunks)backend/app/utils/error.py(1 hunks)backend/app/utils/response.py(1 hunks)docker-compose.yml(1 hunks)extension/tests/e2e/onboarding.test.ts(1 hunks)frontend/README-TEST-FIX.md(1 hunks)frontend/__tests__/app/dashboard/PromptNavigation.test.tsx(1 hunks)frontend/__tests__/app/setup/SetupPageTest.tsx(1 hunks)frontend/__tests__/components/setup/SetupContentTest.tsx(1 hunks)frontend/__tests__/customers.test.tsx(7 hunks)frontend/__tests__/login.test.tsx(3 hunks)frontend/__tests__/loginPage.test.tsx(2 hunks)frontend/__tests__/passwordReset.test.tsx(1 hunks)frontend/__tests__/passwordResetConfirm.test.tsx(1 hunks)frontend/__tests__/passwordResetConfirmPage.test.tsx(2 hunks)frontend/__tests__/passwordResetPage.test.tsx(2 hunks)frontend/__tests__/register.test.ts(3 hunks)frontend/__tests__/registerPage.test.tsx(1 hunks)frontend/app/clientService.ts(2 hunks)frontend/app/dashboard/layout.tsx(3 hunks)frontend/app/openapi-client/sdk.gen.ts(2 hunks)frontend/app/openapi-client/types.gen.ts(3 hunks)frontend/app/page.tsx(1 hunks)frontend/app/password-recovery/confirm/page.tsx(2 hunks)frontend/app/password-recovery/page.tsx(1 hunks)frontend/app/prompts/[id]/page.tsx(1 hunks)frontend/app/prompts/_components/DuplicateButton.tsx(1 hunks)frontend/app/prompts/_components/PromptForm.tsx(1 hunks)frontend/app/prompts/create/page.tsx(1 hunks)frontend/app/prompts/deleteButton.tsx(1 hunks)frontend/app/prompts/edit/[id]/page.tsx(1 hunks)frontend/app/prompts/page.tsx(1 hunks)frontend/app/prompts/searchForm.tsx(1 hunks)frontend/app/prompts/tags/_components/DeleteTagButton.tsx(1 hunks)frontend/app/prompts/tags/_components/TagForm.tsx(1 hunks)frontend/app/prompts/tags/page.tsx(1 hunks)frontend/app/register/page.tsx(2 hunks)frontend/components/actions/login-action.ts(3 hunks)frontend/components/actions/password-reset-action.ts(3 hunks)frontend/components/actions/prompts-action.ts(1 hunks)frontend/components/setup/SetupContent.tsx(1 hunks)frontend/components/ui/FormError.tsx(2 hunks)frontend/components/ui/select.tsx(1 hunks)frontend/components/ui/submitButton.tsx(2 hunks)frontend/components/ui/textarea.tsx(1 hunks)frontend/components/ui/tooltip.tsx(1 hunks)frontend/components/ui/use-toast.tsx(1 hunks)frontend/jest.setup.js(1 hunks)frontend/lib/definitions.ts(1 hunks)frontend/next.config.js(1 hunks)frontend/openapi.json(1 hunks)frontend/package.json(3 hunks)litellm/config.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- backend/app/base.py
✅ Files skipped from review due to trivial changes (19)
- .cursor/rules/tdd-sequential-api-flow.mdc
- backend/app/init.py
- backend/app/utils/error.py
- backend/app/api/main.py
- backend/app/api/routes/init.py
- backend/app/tests/utils/user.py
- backend/app/api/routes/extension_auth.py
- backend/app/api/routes/items.py
- frontend/app/register/page.tsx
- .cursor/rules/extension.mdc
- .cursor/rules/frontend.mdc
- backend/app/alembic/env.py
- .env.example
- backend/app/utils/response.py
- backend/app/models/init.py
- frontend/components/ui/textarea.tsx
- .cursor/rules/ui-colors.mdc
- frontend/components/ui/select.tsx
- frontend/components/ui/tooltip.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/package.json
- Makefile
- frontend/tests/components/setup/SetupContentTest.tsx
- extension/tests/e2e/onboarding.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (14)
backend/app/tests/scripts/test_test_pre_start.py (3)
backend/app/core/db.py (1)
init_db(57-87)backend/app/tests_pre_start.py (1)
init(33-40)backend/app/tests/scripts/test_backend_pre_start.py (1)
test_init_successful_connection(6-26)
backend/app/api/middlewares/response.py (2)
backend/app/utils/response.py (1)
ApiResponse(9-12)backend/app/crud.py (1)
model_dump(14-14)
frontend/__tests__/login.test.tsx (1)
frontend/app/clientService.ts (1)
authJwtLogin(38-38)
frontend/components/actions/login-action.ts (2)
frontend/app/clientService.ts (1)
authJwtLogin(38-38)frontend/lib/utils.ts (1)
getErrorMessage(13-53)
frontend/app/dashboard/layout.tsx (1)
frontend/components/ui/tooltip.tsx (4)
TooltipProvider(30-30)Tooltip(30-30)TooltipTrigger(30-30)TooltipContent(30-30)
backend/app/tests/utils/test_db.py (1)
backend/app/models/prompt.py (4)
Prompt(117-148)PromptTagLink(19-23)PromptVersion(97-113)Tag(36-53)
backend/app/tests/scripts/test_backend_pre_start.py (2)
backend/app/core/db.py (1)
init_db(57-87)backend/app/tests_pre_start.py (1)
init(33-40)
frontend/app/prompts/tags/_components/TagForm.tsx (2)
frontend/components/actions/prompts-action.ts (1)
addTag(624-669)frontend/components/ui/use-toast.tsx (1)
toast(40-43)
frontend/app/prompts/searchForm.tsx (2)
frontend/components/actions/prompts-action.ts (1)
TagData(767-767)website/src/components/ui/button.tsx (1)
Button(65-65)
admin/src/client/sdk.gen.ts (4)
admin/src/client/types.gen.ts (26)
PromptsReadTagsData(328-331)PromptsReadTagsResponse(333-333)PromptsCreateTagData(335-337)PromptsCreateTagResponse(339-339)PromptsUpdateTagData(341-344)PromptsUpdateTagResponse(346-346)PromptsDeleteTagData(348-350)PromptsDeleteTagResponse(352-352)PromptsCreatePromptData(354-356)PromptsCreatePromptResponse(358-358)PromptsReadPromptsData(360-367)PromptsReadPromptsResponse(369-369)PromptsReadPromptData(371-373)PromptsReadPromptResponse(375-375)PromptsUpdatePromptData(377-381)PromptsUpdatePromptResponse(383-383)PromptsDeletePromptData(385-387)PromptsDeletePromptResponse(389-389)PromptsReadPromptVersionsData(391-393)PromptsReadPromptVersionsResponse(395-395)PromptsCreatePromptVersionData(397-400)PromptsCreatePromptVersionResponse(402-402)PromptsReadPromptVersionData(404-407)PromptsReadPromptVersionResponse(409-409)PromptsDuplicatePromptData(411-413)PromptsDuplicatePromptResponse(415-415)admin/src/client/index.ts (2)
CancelablePromise(3-3)OpenAPI(4-4)admin/src/client/core/CancelablePromise.ts (1)
CancelablePromise(20-126)admin/src/client/core/OpenAPI.ts (1)
OpenAPI(43-57)
frontend/app/openapi-client/sdk.gen.ts (3)
admin/src/client/types.gen.ts (26)
PromptsReadTagsData(328-331)PromptsReadTagsResponse(333-333)PromptsCreateTagData(335-337)PromptsCreateTagResponse(339-339)PromptsUpdateTagData(341-344)PromptsUpdateTagResponse(346-346)PromptsDeleteTagData(348-350)PromptsDeleteTagResponse(352-352)PromptsCreatePromptData(354-356)PromptsCreatePromptResponse(358-358)PromptsReadPromptsData(360-367)PromptsReadPromptsResponse(369-369)PromptsReadPromptData(371-373)PromptsReadPromptResponse(375-375)PromptsUpdatePromptData(377-381)PromptsUpdatePromptResponse(383-383)PromptsDeletePromptData(385-387)PromptsDeletePromptResponse(389-389)PromptsReadPromptVersionsData(391-393)PromptsReadPromptVersionsResponse(395-395)PromptsCreatePromptVersionData(397-400)PromptsCreatePromptVersionResponse(402-402)PromptsReadPromptVersionData(404-407)PromptsReadPromptVersionResponse(409-409)PromptsDuplicatePromptData(411-413)PromptsDuplicatePromptResponse(415-415)frontend/app/openapi-client/types.gen.ts (39)
PromptsReadTagsData(464-469)PromptsReadTagsResponse(471-471)PromptsReadTagsError(473-473)PromptsCreateTagData(475-477)PromptsCreateTagResponse(479-479)PromptsCreateTagError(481-481)PromptsUpdateTagData(483-488)PromptsUpdateTagResponse(490-490)PromptsUpdateTagError(492-492)PromptsDeleteTagData(494-498)PromptsDeleteTagResponse(500-500)PromptsDeleteTagError(502-502)PromptsCreatePromptData(504-506)PromptsCreatePromptResponse(508-508)PromptsCreatePromptError(510-510)PromptsReadPromptsData(512-521)PromptsReadPromptsResponse(523-523)PromptsReadPromptsError(525-525)PromptsReadPromptData(527-531)PromptsReadPromptResponse(533-533)PromptsReadPromptError(535-535)PromptsUpdatePromptData(537-545)PromptsUpdatePromptResponse(547-547)PromptsUpdatePromptError(549-549)PromptsDeletePromptData(551-555)PromptsDeletePromptResponse(557-557)PromptsDeletePromptError(559-559)PromptsReadPromptVersionsData(561-565)PromptsReadPromptVersionsResponse(567-567)PromptsReadPromptVersionsError(569-569)PromptsCreatePromptVersionData(571-576)PromptsCreatePromptVersionResponse(578-578)PromptsCreatePromptVersionError(580-580)PromptsReadPromptVersionData(582-587)PromptsReadPromptVersionResponse(589-589)PromptsReadPromptVersionError(591-591)PromptsDuplicatePromptData(593-597)PromptsDuplicatePromptResponse(599-599)PromptsDuplicatePromptError(601-601)backend/app/tests/conftest.py (1)
client(76-78)
admin/src/client/types.gen.ts (2)
backend/app/models/prompt.py (10)
InputVariable(12-15)Prompt(117-148)PromptCreate(72-73)PromptReadWithTags(152-161)Tag(36-53)PromptUpdate(77-86)PromptVersion(97-113)PromptVersionCreate(90-93)TagCreate(165-168)TagUpdate(172-175)frontend/app/openapi-client/types.gen.ts (36)
InputVariable(47-51)Prompt(79-96)PromptCreate(98-111)PromptReadWithTags(113-131)Tag(164-170)PromptUpdate(133-145)PromptVersion(147-156)PromptVersionCreate(158-162)TagCreate(172-176)TagUpdate(178-182)PromptsReadTagsData(464-469)PromptsReadTagsResponse(471-471)PromptsCreateTagData(475-477)PromptsCreateTagResponse(479-479)PromptsUpdateTagData(483-488)PromptsUpdateTagResponse(490-490)PromptsDeleteTagData(494-498)PromptsDeleteTagResponse(500-500)PromptsCreatePromptData(504-506)PromptsCreatePromptResponse(508-508)PromptsReadPromptsData(512-521)PromptsReadPromptsResponse(523-523)PromptsReadPromptData(527-531)PromptsReadPromptResponse(533-533)PromptsUpdatePromptData(537-545)PromptsUpdatePromptResponse(547-547)PromptsDeletePromptData(551-555)PromptsDeletePromptResponse(557-557)PromptsReadPromptVersionsData(561-565)PromptsReadPromptVersionsResponse(567-567)PromptsCreatePromptVersionData(571-576)PromptsCreatePromptVersionResponse(578-578)PromptsReadPromptVersionData(582-587)PromptsReadPromptVersionResponse(589-589)PromptsDuplicatePromptData(593-597)PromptsDuplicatePromptResponse(599-599)
frontend/app/openapi-client/types.gen.ts (2)
admin/src/client/types.gen.ts (36)
InputVariable(47-51)Prompt(79-96)PromptCreate(98-111)PromptReadWithTags(113-131)Tag(164-170)PromptUpdate(133-145)PromptVersion(147-156)PromptVersionCreate(158-162)TagCreate(172-176)TagUpdate(178-182)PromptsReadTagsData(328-331)PromptsReadTagsResponse(333-333)PromptsCreateTagData(335-337)PromptsCreateTagResponse(339-339)PromptsUpdateTagData(341-344)PromptsUpdateTagResponse(346-346)PromptsDeleteTagData(348-350)PromptsDeleteTagResponse(352-352)PromptsCreatePromptData(354-356)PromptsCreatePromptResponse(358-358)PromptsReadPromptsData(360-367)PromptsReadPromptsResponse(369-369)PromptsReadPromptData(371-373)PromptsReadPromptResponse(375-375)PromptsUpdatePromptData(377-381)PromptsUpdatePromptResponse(383-383)PromptsDeletePromptData(385-387)PromptsDeletePromptResponse(389-389)PromptsReadPromptVersionsData(391-393)PromptsReadPromptVersionsResponse(395-395)PromptsCreatePromptVersionData(397-400)PromptsCreatePromptVersionResponse(402-402)PromptsReadPromptVersionData(404-407)PromptsReadPromptVersionResponse(409-409)PromptsDuplicatePromptData(411-413)PromptsDuplicatePromptResponse(415-415)backend/app/models/prompt.py (10)
InputVariable(12-15)Prompt(117-148)PromptCreate(72-73)PromptReadWithTags(152-161)Tag(36-53)PromptUpdate(77-86)PromptVersion(97-113)PromptVersionCreate(90-93)TagCreate(165-168)TagUpdate(172-175)
backend/app/alembic/versions/5ee6589f44e3_merge_prompt_tables_with_main.py (1)
backend/app/alembic/versions/add_prompt_management_tables.py (2)
upgrade(20-85)downgrade(88-99)
🪛 Ruff (0.11.9)
backend/app/alembic/versions/add_prompt_management_tables.py
4-4: Trailing whitespace
Remove trailing whitespace
(W291)
10-10: sqlmodel imported but unused
Remove unused import: sqlmodel
(F401)
52-52: Blank line contains whitespace
Remove whitespace from blank line
(W293)
72-72: Blank line contains whitespace
Remove whitespace from blank line
(W293)
99-99: Trailing whitespace
Remove trailing whitespace
(W291)
99-99: No newline at end of file
Add trailing newline
(W292)
backend/app/alembic/versions/5ee6589f44e3_merge_prompt_tables_with_main.py
8-8: alembic.op imported but unused
Remove unused import: alembic.op
(F401)
9-9: sqlalchemy imported but unused
Remove unused import: sqlalchemy
(F401)
10-10: sqlmodel.sql.sqltypes imported but unused
Remove unused import: sqlmodel.sql.sqltypes
(F401)
🪛 GitHub Actions: Test Backend
backend/app/tests/utils/test_db.py
[warning] 1-1: PytestReturnNotNoneWarning: test_database_connection returned True instead of None, which will be an error in future pytest versions.
🪛 YAMLlint (1.37.1)
litellm/config.yaml
[error] 17-17: trailing spaces
(trailing-spaces)
[error] 47-47: trailing spaces
(trailing-spaces)
🪛 Biome (1.9.4)
frontend/app/prompts/page.tsx
[error] 74-82: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/jest.setup.js
[error] 184-185: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
admin/src/client/sdk.gen.ts
[error] 344-652: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
admin/src/client/types.gen.ts
[error] 352-352: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 389-389: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
frontend/app/openapi-client/types.gen.ts
[error] 500-500: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 557-557: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🪛 Checkov (3.2.334)
frontend/openapi.json
[MEDIUM] 1: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🪛 GitHub Check: Spell Check
backend/app/models/prompt.py
[warning] 49-49:
"selectin" should be "selection" or "selecting".
🪛 GitHub Actions: Spell Check with Typos
backend/app/models/prompt.py
[error] 49-49: Typo detected: selectin should be selection or selecting as per typos check.
🪛 LanguageTool
frontend/README-TEST-FIX.md
[uncategorized] ~48-~48: 动词的修饰一般为‘形容词（副词）+地+动词’。您的意思是否是：确切"地"提交
Context: ... registerPage 表单提交问题： - 检查注册页面源码以了解确切的提交机制 - 根据实际调用方式更新测试 3. 更新 login.test...
(wb4)
🔇 Additional comments (89)
backend/app/api/middlewares/response.py (3)
8-9: Good update to use the new ApiResponse modelThis import change correctly references the centralized ApiResponse model from utils/response.py, aligning with the architecture refactoring mentioned in the summary.
67-70: Improved response standardization using ApiResponseGreat improvement replacing manual dictionary construction with the strongly-typed ApiResponse model for success responses. This enhances type safety and ensures consistent API responses.
77-80: Consistent error response handling with ApiResponseGood job using the ApiResponse model for error responses, maintaining consistency with the success response pattern and ensuring standardized API output.
backend/app/tests/scripts/test_test_pre_start.py (1)
1-26: Improved test with direct mocking approachThe refactoring from using
@patchdecorator to directly creating aMagicMockinstance makes the test more self-contained and explicit. This approach matches the pattern in other test files (like test_backend_pre_start.py) and clearly shows the test's intent and expectations.The mocking of
execandfirstmethods, along with assertions foraddandcommitcalls, effectively verifies the database initialization logic.backend/app/tests/utils/item.py (1)
1-2: Good type safety improvement with explicit castingAdding the explicit type cast to
Itemenhances type safety without changing runtime behavior. This is particularly valuable for test utilities that interact with the database through ORM functions, as it ensures proper type inference for static analysis tools and IDE autocompletion.Also applies to: 18-18
backend/app/api/routes/google_oauth.py (2)
77-79: Good defensive validation.This validation adds an extra safety check to ensure that tokens are never created for non-existent users, even though the logic above this point should already handle user creation. This follows defensive programming principles.
234-235: Consistent validation implementation.This validation check is identical to the one added in the API endpoint, ensuring consistent error handling across both OAuth flows. The extra validation enhances the robustness of the authentication process.
frontend/__tests__/register.test.ts (4)
28-28: Fixed test expectations to include full_name field.The test now correctly includes the
full_name: nullparameter in the expected call toregisterUser, making it more accurate.
52-52: Consistent test expectations.Similar to the previous change, the test now correctly includes
full_name: nullin the expected parameters for consistency.
69-69: Updated expected error message.The test now expects the correct error message "Invalid email" that matches the current implementation.
76-76: Fixed logical assertion.The assertion was corrected to
.not.toHaveBeenCalled(), which accurately verifies that the registration service is not called when validation fails.backend/app/alembic/versions/5ee6589f44e3_merge_prompt_tables_with_main.py (2)
1-17: Appropriate migration merge revision structure.This migration script correctly establishes a merge point between two revision branches ('0001' and '1e3b5c7f9d2a'), which is essential for maintaining the Alembic migration history when merging changes from different development branches.
🧰 Tools
🪛 Ruff (0.11.9)
8-8:
alembic.opimported but unusedRemove unused import:
alembic.op(F401)
9-9:
sqlalchemyimported but unusedRemove unused import:
sqlalchemy(F401)
10-10:
sqlmodel.sql.sqltypesimported but unusedRemove unused import:
sqlmodel.sql.sqltypes(F401)
20-25: Empty upgrade/downgrade functions are appropriate.The empty functions are correct for a merge migration that doesn't need to make any schema changes itself, as its purpose is solely to connect different branches of the migration history.
docker-compose.yml (1)
249-253: Additional LLM provider support.The added environment variables provide integration options for OpenAI, GitHub, and Volcengine models, expanding the application's LLM provider support. This aligns with the broader pattern of integration flexibility in the application.
frontend/app/page.tsx (2)
28-40: Clear navigation structure with appropriate button styling.The implementation of navigation links to Setup, Prompts, and Dashboard provides a clean user flow that aligns well with the onboarding experience described in the PR objectives. The buttons use consistent styling with appropriate visual hierarchy.
42-48: Good addition of direct dashboard access link.Providing a direct path to the dashboard enhances user flexibility, allowing both guided setup and direct access depending on user preference. This is a user-friendly approach.
frontend/__tests__/customers.test.tsx (3)
3-3: Good addition of React's act for test stability.Adding the React
actimport improves test stability by ensuring state updates are processed before assertions.
85-87: Proper wrapping of user events in act().Wrapping all user interaction events (click, clear, type) in
act()ensures that all updates from these actions are processed and applied before making assertions, preventing potential test flakiness.Also applies to: 97-102, 122-124, 142-144, 157-159, 171-173
179-182: Improved selector specificity for better test reliability.Using a role-based selector with a name matcher is more reliable than searching for text directly. This approach is less brittle when facing UI text changes or translations.
frontend/__tests__/login.test.tsx (3)
33-42: Enhanced test flexibility with expect.objectContaining.Using
expect.objectContaining()for API call verification makes tests more resilient to additions of new parameters. Including OAuth2 parameters (grant_type, client_id, client_secret, scope) aligns with industry standards and improves authentication flexibility.Also applies to: 62-71
45-45: Added flexibility for cookie parameters.Using
expect.anything()for the third parameter of the cookie set method accommodates potential configuration changes without breaking tests.
92-92: Updated validation error message to reflect input type.Changing from "Username is required" to "Email is required" better communicates to users that an email address is expected, improving form usability.
frontend/__tests__/app/dashboard/PromptNavigation.test.tsx (2)
5-20: Well-structured mocks for external dependencies.The mocking approach for Next.js components and the logout action properly isolates the component under test, creating a controlled test environment.
22-30: Effective test for navigation structure.The test accurately verifies both the presence and the destination of the "Prompt Hub" navigation link, ensuring the dashboard navigation includes access to the new prompt features.
frontend/lib/definitions.ts (2)
26-26: Improve password validation for registration schemaUsing the shared
passwordSchemafor registration enhances security by enforcing stronger password requirements (minimum length, uppercase, special character).
3-44: Inconsistent password validation between registration and password updateWhile registration now uses the enhanced
passwordSchema, theupdatePasswordSchemastill uses different validation rules with a maximum length of 40 characters and no requirements for uppercase or special characters.Consider updating
updatePasswordSchemato usepasswordSchemaas well to ensure consistent password requirements across the application:export const updatePasswordSchema = z.object({ - current_password: z - .string() - .min(8, "Current password must be at least 8 characters") - .max(40, "Current password must be less than 40 characters"), - new_password: z - .string() - .min(8, "New password must be at least 8 characters") - .max(40, "New password must be less than 40 characters"), + current_password: z.string().min(1, "Current password is required"), + new_password: passwordSchema, });frontend/components/ui/FormError.tsx (3)
22-22: Enhanced form error styling and testabilityChanging to semantic color class
text-destructiveand adding a data-testid attribute improves both design system consistency and testability.
39-43: Simplified array error rendering with improved test hooksUsing individual paragraph elements with indexed test IDs improves testability while maintaining the visual error presentation.
47-47: Enhanced field error styling and testabilityAdding a data-testid attribute and using the semantic color class improves both testability and design system alignment.
frontend/app/clientService.ts (1)
16-28: Good addition of prompt and tag management API functionsThe new imports and exports for prompt and tag management functions follow the existing pattern in the file and are well-structured.
However, I notice there are no new types being imported for these prompt-related functions, even though they likely have unique response and error types.
Verify if you need to import additional types from
./openapi-client/types.gen.tsfor these new prompt-related functions to ensure proper type checking throughout the application.Also applies to: 61-73
frontend/app/dashboard/layout.tsx (2)
2-2: Good addition of required importsAdding the MessageSquare icon and tooltip components is necessary for the new sidebar navigation features.
Also applies to: 20-25
48-81: Enhanced accessibility with tooltips for existing navigationWrapping the existing dashboard and customers links with tooltips improves usability and accessibility.
backend/app/tests/scripts/test_backend_pre_start.py (1)
6-26: Great improvement to test readability and maintainability.The test has been effectively refactored to remove dependencies on pytest patching and use direct mocking instead. This makes the test more straightforward and easier to understand.
One small improvement I would suggest is to translate the test documentation string from Chinese to English for consistency with other code comments:
- """测试数据库初始化连接成功""" + """Test successful database initialization connection"""frontend/__tests__/passwordReset.test.tsx (1)
9-47: Good implementation of mocks for isolated testing.The mocks for React's
useActionState, Next.js navigation hooks, authentication, and Image component are well structured to isolate the component under test.frontend/components/actions/login-action.ts (3)
5-5: Update to use authJwtLogin alias.The change from
loginAccessTokentoauthJwtLoginis appropriate since they refer to the same function (based on the provided context). This change likely improves code readability or aligns with naming conventions elsewhere in the codebase.
28-29: Error handling improvements and comment translations.The refined error handling improves the UX by returning more specific error details. Comments have been translated to Chinese, which appears to be the preferred language for comments in this codebase.
Also applies to: 39-55
70-71: Documentation improvements for cookie settings and placeholder function.The comments for cookie maxAge and the Google login placeholder function have been translated to Chinese to maintain consistency with other comments in the file.
Also applies to: 80-87
backend/app/tests/utils/test_db.py (3)
173-176: Strong enhancement for test table creation reliability.Explicitly importing all prompt-related models ensures SQLModel can properly discover and create all tables during testing.
181-188: Good verification after table creation.Checking the actual tables created in the database helps identify potential issues with the automated table creation process.
261-262: Helpful comment about model imports.This comment provides important context for developers about ensuring all models are imported before table creation.
frontend/app/prompts/_components/DuplicateButton.tsx (2)
1-12: Well-structured component with clean imports and proper interface definition.The component follows React best practices with the 'use client' directive and a clearly defined props interface.
46-56: Clean and accessible button implementation.The button provides good visual feedback during the duplication process and includes an icon for better usability.
frontend/app/prompts/edit/[id]/page.tsx (2)
12-33: Well-implemented metadata generation.The function handles errors gracefully and provides appropriate fallback metadata when the prompt data can't be loaded.
35-90: Excellent page structure with proper authentication and error handling.The main page component checks authentication status and provides appropriate UI for unauthenticated users. The use of ErrorBoundary and Suspense provides robust error handling and loading states.
frontend/__tests__/loginPage.test.tsx (6)
3-3: Good addition of React's act for async testing.Using act helps ensure that all state updates are properly processed during testing.
13-19: Improved test setup with global fetch mock.Mocking fetch at the global level provides a more realistic test environment that better reflects how the component interacts with external services.
29-33: More reliable element selection in tests.Using placeholder text for input selection is more reliable than using labels that might change.
36-64: Better test implementation for form submission.Mocking fetch directly and wrapping interactions in act() provides a more accurate test of the component's behavior. The test now verifies the actual API call rather than an internal function call.
66-92: Comprehensive testing of error scenarios.The test properly handles login failure scenarios by mocking appropriate error responses.
94-117: Good coverage of network error scenarios.Testing network errors ensures the component can handle unexpected failures gracefully.
frontend/app/password-recovery/confirm/page.tsx (4)
37-45: Good implementation of post-success redirection logic.The redirect logic for after password reset is well-implemented with proper cleanup of the timeout on component unmount, preventing potential memory leaks.
47-49: Better error handling with proper return.The change from calling
notFound()to returning it is correct, asnotFound()is a function that returns a value rather than throwing an exception.
89-91: Good use of hover effects for input fields.The implementation of animated gradient borders on hover provides excellent visual feedback to users, enhancing the overall user experience.
Also applies to: 107-108
160-162: Improved fallback text consistency.The suspense fallback text is now in English instead of Chinese, creating better consistency with standard development practices.
frontend/next.config.js (3)
11-18: Consider enabling TypeScript and ESLint checks for production builds.Disabling TypeScript type checking and ESLint during builds might lead to issues in production that would otherwise be caught during the build process.
These settings are typically used to speed up development builds, but for production deployments, it's generally recommended to keep these checks enabled to ensure code quality and prevent runtime errors.
26-52: Good solution for CSS loading and Tailwind directive issues.The code effectively resolves CSS loading issues with Tailwind by configuring the CSS loader options properly. This approach ensures that CSS modules are handled correctly with proper hashing for local identifiers.
2-3: Improved path resolution with explicit imports.The change from inline require to explicit import at the top of the file improves readability and follows better coding practices.
Also applies to: 23-23
frontend/__tests__/passwordResetPage.test.tsx (2)
12-37: Good implementation of comprehensive mocks.The tests now include detailed mocks for Next.js navigation, authentication, and image components, which creates a more controlled testing environment and improves test reliability.
44-56: Good refactoring to use React.useActionState.Refactoring the tests to use the mocked
useActionStatehook improves test focus on component rendering based on different states rather than implementation details of how the state is updated.frontend/__tests__/registerPage.test.tsx (2)
33-34: Redundant mock-clearing call
jest.clearAllMocks()already clears every mock, so(register as jest.Mock).mockClear()is superfluous.
51-75: Skipped tests hide regressions – consider fixing instead of.skipTwo important behavioural tests are skipped. Skipping weakens confidence in the registration flow and silently hides failures in CI.
At minimum, add a TODO comment with an issue link; ideally fix the underlying fetch / validation handling so the tests can run.frontend/components/ui/submitButton.tsx (4)
4-10: Interface extension improves reusabilityThe new
SubmitButtonPropsinterface extendingButtonHTMLAttributes<HTMLButtonElement>enables passing native button attributes, improving component flexibility and maintainability.
19-19: Smart button text detection logicGood approach using a fallback mechanism to determine button text from either the
textprop or string children.
26-26: Props spreading enhances component adaptabilityAdding
{...props}allows passing through native button attributes, making this component more versatile.
61-63: Improved conditional rendering for flexible contentThe addition of a fallback to render non-string children directly enhances the component's versatility.
frontend/README-TEST-FIX.md (1)
1-60: Well-structured test documentation with clear next stepsThe document provides a comprehensive overview of the current testing status, completed fixes, remaining issues, and actionable next steps. This is valuable for team collaboration and maintaining momentum on test improvements.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~48-~48: 动词的修饰一般为‘形容词（副词）+地+动词’。您的意思是否是：确切"地"提交
Context: ... registerPage 表单提交问题： - 检查注册页面源码以了解确切的提交机制 - 根据实际调用方式更新测试 3. 更新 login.test...(wb4)
frontend/__tests__/passwordResetConfirmPage.test.tsx (5)
27-40: Good mock setup for authentication and Next.js componentsThe addition of proper mocks for authentication hooks and Next.js components improves test reliability.
54-57: Proper useActionState mock implementationGood usage of React.useActionState mocking to simulate form submission states.
61-64: Localized text verification improves test robustnessUsing the actual Chinese placeholders and button text in the test ensures the UI properly displays localized content.
100-121: Comprehensive validation error handling testGood test coverage for field-specific validation errors, ensuring proper error display for both password fields.
123-158: Well-implemented success flow test with timer managementExcellent test for the success path using fake timers and proper act() wrapping to verify the redirect after successful password reset.
litellm/config.yaml (5)
7-9: Environment variable configuration improvedGood use of the "os.environ/" format for environment variables, providing a consistent approach to configuration.
11-41: Expanded OpenAI model support with proper configurationThe configuration now properly sets up multiple OpenAI models with consistent parameters and environment variable references.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 17-17: trailing spaces
(trailing-spaces)
42-91: Comprehensive OpenRouter model integrationGood addition of diverse AI models through OpenRouter with consistent configuration patterns.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 47-47: trailing spaces
(trailing-spaces)
111-112: Added Langfuse integration for loggingGood addition of Langfuse to success callbacks and comprehensive default tags for enhanced observability.
110-110:❓ Verification inconclusive
Verify impact of drop_params change
The
drop_paramssetting has been changed fromfalsetoTrue. This changes case sensitivity and could potentially affect how parameters are handled.Verify that this change was intentional and doesn't break existing functionality:
🏁 Script executed:
#!/bin/bash # Check for any references to drop_params in the codebase rg -i "drop_params" --type=pythonLength of output: 64
🏁 Script executed:
#!/bin/bash # Search for any occurrences of drop_params across the repository rg -n "drop_params"Length of output: 106
Review
drop_paramsduplication in config.yamlI found two definitions of
drop_paramsin litellm/config.yaml:
- Line 5:
drop_params: false- Line 110:
drop_params: TruePlease verify that:
- The override at line 110 is intentional.
- Your YAML loader will apply the last definition as expected.
- You remove or document the redundant default to avoid confusion.
frontend/app/prompts/searchForm.tsx (3)
33-46: LGTM! Good URL parameter handling.The form submission logic correctly preserves existing sort parameters while updating search and tag filters. This is a good practice for maintaining user preferences across search actions.
58-70: LGTM! Clean filter reset implementation.The
clearAllFiltersfunction properly clears search and tag filters while preserving sorting parameters, which creates a consistent user experience.
121-181: LGTM! Well-implemented filter display section.The filter display section with badges and removal options is well-designed and provides good user feedback and control.
frontend/app/prompts/deleteButton.tsx (2)
24-59: LGTM! Well-implemented delete functionality with proper error handling.The delete handler includes appropriate state management, error handling, and user feedback via toast notifications. The component correctly manages the loading state during deletion to prevent duplicate actions.
83-86: LGTM! Good practice preventing default form behavior.Preventing the default form behavior on the AlertDialogAction click ensures that the delete handler is executed as intended without unexpected page reloads.
.cursor/rules/python.mdc (2)
3-3: LGTM! Appropriate glob pattern for Python backend files.The glob pattern correctly targets Python files in the backend directory.
6-13: LGTM! Concise and relevant Python backend guidelines.The rules have been simplified to a focused checklist covering essential Python backend best practices, including organization, authentication, testing, and documentation standards.
frontend/components/actions/password-reset-action.ts (2)
71-74: LGTM! Good UX improvement with delayed redirect.Returning a success message instead of immediately redirecting is a good UX improvement, allowing users to see the success feedback before being redirected.
82-85: LGTM! Good separation of concerns.Creating a separate function for redirection is a good practice that separates the concerns of displaying messages and handling navigation.
frontend/app/prompts/tags/page.tsx (1)
125-198: Solid implementation – no blocking issues spottedAuthentication gating, error boundaries, suspense loading states and a11y texts are covered. API data are validated before render, and destructive actions are delegated to client components.
Nice work!backend/app/tests/api/test_prompts.py (1)
8-38: Great coverage of the create-prompt happy pathThe test clearly asserts every critical response field and status code, giving confidence in the endpoint.
Nice work!admin/src/client/sdk.gen.ts (1)
344-652: Static-only class—rule violation is safe to ignore (auto-generated code)
Biome flagsPromptsServicebecause it only contains static members, recommending plain functions instead.
Since this file is produced by an OpenAPI generator, hand-editing will be overwritten on the next generation. I’d leave it as-is and configure the linter to ignore the generator output path instead.🧰 Tools
🪛 Biome (1.9.4)
[error] 344-652: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
frontend/app/openapi-client/sdk.gen.ts (1)
606-828: Generated SDK additions look consistent with types—no blocking issuesPath placeholders (
{tag_id},{prompt_id}…) rely on the caller passingoptions.path; that matches the pattern used elsewhere in the file, so the new prompt/tag helpers are coherent with existing endpoints.No action required.
- Enhanced the password reset confirmation page with improved user feedback and loading indicators. - Strengthened form validation and error handling for password reset actions. - Expanded test coverage for password reset and registration processes, including localization improvements. - Mocked additional Next.js navigation hooks and React state management for thorough testing. - Refactored password reset action to incorporate basic email validation and clearer error messages.
- Updated the default `OPENAI_BASE_URL` to `https://api.openai.com/v1` in `.env.example` and `docker-compose.yml`. - Added detailed instructions for upgrading the OpenAI API version in the README files. - Refactored prompt-related types to use enums for visibility and type fields, improving type safety. - Enhanced CRUD operations for prompts, including better error handling and access control. - Improved test coverage for prompt management functionalities and adjusted test setups for better reliability.
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (1)
backend/app/crud.py (1)
330-366: 🛠️ Refactor suggestionAdd error handling with rollback to tag CRUD functions.
Unlike other CRUD functions (e.g., lines 50-58), the tag functions don't have error handling with rollback. Add it to ensure database consistency in case of failures.
def create_tag(db: Session, tag_in: Any): """创建新标签""" from app.models import Tag - tag = Tag(**tag_in.model_dump()) - db.add(tag) - db.commit() - db.refresh(tag) - return tag + try: + tag = Tag(**tag_in.model_dump()) + db.add(tag) + db.commit() + db.refresh(tag) + return tag + except IntegrityError as e: + db.rollback() + raise eSimilarly for
update_tag:def update_tag(db: Session, tag: Any, tag_in: Any): """更新标签""" - update_data = tag_in.model_dump(exclude_unset=True) - for field, value in update_data.items(): - setattr(tag, field, value) - db.add(tag) - db.commit() - db.refresh(tag) - return tag + try: + update_data = tag_in.model_dump(exclude_unset=True) + for field, value in update_data.items(): + setattr(tag, field, value) + db.add(tag) + db.commit() + db.refresh(tag) + return tag + except IntegrityError as e: + db.rollback() + raise e
♻️ Duplicate comments (3)
backend/app/api/routes/prompts.py (2)
214-270: Private prompts are still leaked in list endpoint
Previous feedback hasn’t been addressed:read_prompts()returns every prompt without applying_check_prompt_access, so users can see others’ private data.At minimum:
- prompts = db.exec(query).all() + prompts = [ + p for p in db.exec(query).all() + if _check_prompt_access(p, _current_user) + ]For performance, translate the visibility logic into SQL WHERE clauses.
319-353:⚠️ Potential issueVersioning captures the updated content, not the prior state
update_prompt()mutatesprompt.contentbefore you copy it intoold_content, so the version record reflects the new text, defeating the purpose of history tracking.- # 更新其他字段 - update_data = prompt_in.model_dump(exclude_unset=True) - for key, value in update_data.items(): - setattr(prompt, key, value) - - # 如果内容有更改且需要创建新版本 - if create_version and "content" in update_data: - # 1. 先复制旧内容 - old_content = prompt.content - old_input_vars = prompt.input_vars + # 1. 备份旧内容 + pre_content = prompt.content + pre_input_vars = prompt.input_vars + + # 2. 应用更新 + update_data = prompt_in.model_dump(exclude_unset=True) + for key, value in update_data.items(): + setattr(prompt, key, value) + + # 3. 创建版本 + if create_version and "content" in update_data: ... - content=old_content, - input_vars=old_input_vars, + content=pre_content, + input_vars=pre_input_vars,This preserves historical accuracy and avoids race-condition gaps.
backend/app/crud.py (1)
106-120: 🛠️ Refactor suggestionImplement efficient SQL-based pagination in get_items function.
Unlike the generic
get_multifunction,get_itemsstill fetches all results before paginating in Python. This is inefficient for large datasets and should be changed to use SQL pagination.def get_items( session: Session, *, owner_id: uuid.UUID | None = None, skip: int = 0, limit: int = 100, ) -> list[Item]: """获取物品列表""" query = select(Item) if owner_id: query = query.where(Item.owner_id == owner_id) - # 先执行查询获取所有结果 - results = session.exec(query).all() - # 然后在 Python 中进行分页 - return list(results[skip : skip + limit]) + # Apply pagination at the database level + query = query.offset(skip).limit(limit) + return list(session.exec(query).all())
🧹 Nitpick comments (27)
frontend/app/prompts/[id]/CopyToClipboardButton.tsx (1)
21-22: Add accessibility attributes for better international support.The button currently lacks accessibility attributes which would help screen readers and international users understand its purpose.
onClick={() => { navigator.clipboard.writeText(content); // Optionally, show a toast notification here }} + aria-label="Copy content to clipboard" > <Copy className="h-4 w-4 mr-2" /> 复制内容frontend/app/password-recovery/confirm/page.tsx (3)
37-45: Consider increasing the redirect timeout for better user experience.A 2-second timeout might be too short for users to read the success message before being redirected. Consider increasing this value to give users more time to process the information.
useEffect(() => { if (state?.message) { const timer = setTimeout(() => { router.push("/login"); - }, 2000); + }, 3500); return () => clearTimeout(timer); } }, [state?.message, router]);
86-101: Improve accessibility by adding aria-describedby for error messages.The password input fields could benefit from being linked to their respective error messages for better screen reader support.
<div className="grid gap-2"> <Label htmlFor="password">密码</Label> <div className="relative group"> <div className="absolute -inset-0.5 bg-gradient-to-r from-blue-50 to-indigo-50 dark:from-blue-900/20 dark:to-indigo-900/20 rounded-lg blur opacity-0 group-hover:opacity-100 transition duration-500" /> <Input id="password" name="password" type="password" placeholder="至少8个字符，包含大写字母和特殊字符" required + aria-describedby={errors?.password ? "password-error" : undefined} /> </div> {errors?.password && ( - <FieldError state={state} field="password" /> + <FieldError state={state} field="password" id="password-error" /> )} </div>You'll need to modify the FieldError component to accept and apply the id:
// In FieldError component export function FieldError({ state, field, id }: { state: any; field: string; id?: string }) { return ( <p id={id} className="text-sm text-red-500"> {state?.errors?.[field]} </p> ); }
129-134: Add loading state to the submit button for better user feedback.The submit button doesn't indicate when form submission is in progress, which might lead to users clicking multiple times.
{!success && ( <div className="relative group mt-2"> <div className="absolute -inset-0.5 bg-gradient-to-r from-blue-600 to-indigo-600 dark:from-blue-500 dark:to-indigo-500 rounded-lg blur opacity-75 group-hover:opacity-100 transition duration-300" /> - <SubmitButton>重置密码</SubmitButton> + <SubmitButton> + {state?.submitting ? "处理中..." : "重置密码"} + </SubmitButton> </div> )}You may need to ensure the SubmitButton component properly handles the submitting state from the useActionState hook.
frontend/__tests__/passwordResetPage.test.tsx (1)
45-48: Add type safety to the useActionState mock.The current implementation lacks type safety, which could make the tests brittle if the hook's interface changes.
// Mock useActionState 返回默认状态 jest .spyOn(React, "useActionState") - .mockImplementation(() => [undefined, jest.fn()]); + .mockImplementation(() => [undefined, jest.fn()] as [any, React.Dispatch<any>]);frontend/__tests__/loginPage.test.tsx (1)
13-19: Use consistent fetch mocking approach.The test uses both
window.fetchandglobal.fetchfor mocking, which could lead to confusion. Standardize on one approach.- // 模拟 window.fetch - window.fetch = jest.fn().mockImplementation(() => - Promise.resolve({ - ok: true, - json: () => Promise.resolve({ access_token: "fake-token" }), - }), - ); + // 设置全局 fetch mock + beforeEach(() => { + global.fetch = jest.fn().mockImplementation(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve({ access_token: "fake-token" }), + }), + ); + });frontend/__tests__/passwordResetConfirm.test.tsx (3)
1-9: Remove unused & potentially misleading imports
fireEvent,waitFor, andpasswordResetConfirmare imported but never referenced, and the wildcardPlusimport is not used directly.
Keeping unused imports makes type-checking stricter (no-unused-vars) projects fail and increases the risk of accidentally executing real implementations instead of mocks (in the case ofpasswordResetConfirm).-import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import { render, screen } from "@testing-library/react"; ... -import { passwordResetConfirm } from "@/components/actions/password-reset-action";
11-15: Delete deadcustomRenderhelperThe wrapper adds no additional behaviour over
renderand is never used after the refactor.-// 创建自定义 render 函数处理 App Router 上下文 -const customRender = (ui: ReactNode) => { - return render(ui); -};
49-51: Clear mock call counts between tests
jest.restoreAllMocks()restores originals but does not reset call history, which may lead to flaky expectations if any mock is shared across tests. Addjest.clearAllMocks()(orjest.resetAllMocks()if preferred) inside the sameafterEach.afterEach(() => { jest.restoreAllMocks(); + jest.clearAllMocks(); });backend/app/alembic/versions/add_prompt_management_tables.py (1)
97-98: Nit: tidy trailing whitespace & add final newlineCI linters (Ruff W291/W292) will fail; remove the trailing space and append a newline.
🧰 Tools
🪛 Ruff (0.11.9)
98-98: Trailing whitespace
Remove trailing whitespace
(W291)
98-98: No newline at end of file
Add trailing newline
(W292)
frontend/app/prompts/_components/PromptForm.tsx (2)
8-10: Resolve duplicate & unused imports
uuidv4is imported but not used, andPlusis imported twice (once asPlus, once aliased asPlusIcon).
Either remove them or useuuidv4to generate stable keys for the dynamic variable list (see next comment).-import { v4 as uuidv4 } from "uuid"; -import { X, Plus, Trash, Plus as PlusIcon } from "lucide-react"; +import { X, Trash, Plus as PlusIcon } from "lucide-react";
384-386: Use stable keys instead of array indices
indexkeys break React’s diffing when items are removed/re-ordered and are flagged by the React docs.
Sinceuuidv4is already available, assign anidwhen the variable is created and use that askey.- const addInputVar = () => { - setInputVars([ - ...inputVars, - { name: "", description: "", required: false }, - ]); - }; + const addInputVar = () => { + setInputVars([ + ...inputVars, + { id: uuidv4(), name: "", description: "", required: false }, + ]); + }; ... - inputVars.map((variable, index) => ( - <Card key={index} className="p-4"> + inputVars.map((variable, index) => ( + <Card key={variable.id ?? index} className="p-4">You will also need to extend the
InputVariabletype with an optionalidfield.frontend/app/prompts/[id]/page.tsx (2)
10-17: Prune unusedCopyicon import
Copyis not used in this file (the client component re-imports it internally). Keeping dead imports increases bundle size and may trigger linter complaints.- Copy,
117-140: Gracefully surfaceversionsDatafetch errors
versionsDatais assumed to be an array; when the API returns an error object, the tab silently disappears, providing no feedback. Consider showing an inline error alert similar to the prompt-fetch error branch so the user understands why version history is missing.frontend/app/prompts/page.tsx (1)
25-31:Searchicon is unusedSame rationale as above—delete the import to keep the file clean and linter-friendly.
- Search,backend/app/models/prompt.py (2)
50-50: Fix typo in lazy loading parameter name.The
"selectin"value insa_relationship_kwargsis flagged as a typo by the static analysis tool. While this is a valid SQLAlchemy value for eager loading strategies, it's triggering the spell checker.- sa_relationship_kwargs={"lazy": "selectin"}, # noqa: typos + sa_relationship_kwargs={"lazy": "selectin"}, # noqa🧰 Tools
🪛 GitHub Check: Spell Check
[warning] 50-50:
"selectin" should be "selection" or "selecting".🪛 GitHub Actions: Spell Check with Typos
[warning] 50-50: "selectin" should be "selection" or "selecting". Typo detected by typos tool.
[error] 50-50: Typo error:
selectinshould beselectionorselecting. Command './typos . --write-changes --config _typos.toml' failed with exit code 2.
12-17: Consider translating Chinese comments to English for consistency.The codebase appears to use English for most code and documentation. For better maintainability and collaboration, translate Chinese comments to English throughout the file.
-# 输入变量模型 +# Input variable model class InputVariable(BaseModel): name: str description: str | None = None required: bool = Falsefrontend/jest.setup.js (3)
24-24: Use optional chaining for cleaner URL handling.The URL extraction logic can be simplified using optional chaining.
- url: typeof input === "string" ? input : (input && input.url) || "", + url: typeof input === "string" ? input : input?.url || "",🧰 Tools
🪛 Biome (1.9.4)
[error] 24-24: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
93-97: Translate Chinese comments for better code readability.Maintain consistency by translating Chinese comments to English throughout the codebase.
- // 根据参数名返回适当的值 + // Return appropriate values based on parameter name if (param === "step") return "1"; if (param === "plugin_id") return null; if (param === "extension_callback") return null;
223-224: Use assignment to undefined instead of delete for better performance.The
deleteoperator can impact performance. Use undefined assignment instead.- delete global.mockSearchParams; - delete global.mockPathname; + global.mockSearchParams = undefined; + global.mockPathname = undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 223-224: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
frontend/components/actions/prompts-action.ts (3)
21-48: Consider translating comments and variable names to English.For consistency and to improve collaboration across teams, translate Chinese comments and consider using English for all documentation.
-// 定义Prompt和Tag数据类型 +// Define Prompt and Tag data types
80-86: Consider a longer cache TTL for improved performance.A 3-second cache TTL is quite short and might lead to frequent refetching. Consider increasing this value (e.g., to 30 seconds or more) unless there's a specific requirement for near real-time data.
-const CACHE_TTL = 3000; // 3秒缓存 +const CACHE_TTL = 30000; // 30 seconds cache
155-169: Extract repeated error handling logic to a helper function.Error handling code is duplicated across multiple functions. Consider extracting it to a reusable helper function to improve maintainability.
// Helper function for API error handling function handleApiError( requestId: string, error: any, cacheRef: any, lastFetchTimeRef: number, now: number ): ApiErrorResponse { console.error(`[${requestId}] API error:`, error || "Empty error object"); const errorResult = { error: typeof error === "string" ? error : error ? JSON.stringify(error) : "Unknown API error", status: 500, }; // Update cache if needed if (cacheRef !== null) { cacheRef = errorResult; lastFetchTimeRef = now; } return errorResult; } // Usage in your functions: if (error) { return handleApiError(requestId, error, promptsCache, lastPromptsFetchTime, now); }Also applies to: 313-327
frontend/app/openapi-client/types.gen.ts (2)
504-504: Consider using undefined instead of void for response type.The static analyzer flagged this line because
voidis meant for function return types, not as a value type in TypeScript.-export type PromptsDeleteTagResponse = void; +export type PromptsDeleteTagResponse = undefined;Note: Since this is an auto-generated file, this change should be made in the generator configuration rather than directly in this file.
🧰 Tools
🪛 Biome (1.9.4)
[error] 504-504: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
561-561: Consider using undefined instead of void for response type.The static analyzer flagged this line because
voidis meant for function return types, not as a value type in TypeScript.-export type PromptsDeletePromptResponse = void; +export type PromptsDeletePromptResponse = undefined;Note: Since this is an auto-generated file, this change should be made in the generator configuration rather than directly in this file.
🧰 Tools
🪛 Biome (1.9.4)
[error] 561-561: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
admin/src/client/types.gen.ts (2)
356-356: Consider using undefined instead of void for response type.The static analyzer flagged this line because
voidis meant for function return types, not as a value type in TypeScript.-export type PromptsDeleteTagResponse = (void); +export type PromptsDeleteTagResponse = (undefined);Note: Since this is an auto-generated file, this change should be made in the generator configuration rather than directly in this file.
🧰 Tools
🪛 Biome (1.9.4)
[error] 356-356: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
393-393: Consider using undefined instead of void for response type.The static analyzer flagged this line because
voidis meant for function return types, not as a value type in TypeScript.-export type PromptsDeletePromptResponse = (void); +export type PromptsDeletePromptResponse = (undefined);Note: Since this is an auto-generated file, this change should be made in the generator configuration rather than directly in this file.
🧰 Tools
🪛 Biome (1.9.4)
[error] 393-393: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (52)
.cursor/website.mdc(1 hunks).env.example(1 hunks)README.md(2 hunks)README_zh-CN.md(1 hunks)admin/src/client/types.gen.ts(4 hunks)admin/tests/auth.setup.ts(1 hunks)backend/app/alembic/versions/add_prompt_management_tables.py(1 hunks)backend/app/api/routes/prompts.py(1 hunks)backend/app/crud.py(2 hunks)backend/app/models/prompt.py(1 hunks)backend/app/tests/api/test_prompts.py(1 hunks)backend/app/tests/utils/test_db.py(3 hunks)docker-compose.yml(1 hunks)frontend/__tests__/app/dashboard/PromptNavigation.test.tsx(1 hunks)frontend/__tests__/app/setup/SetupPageTest.tsx(1 hunks)frontend/__tests__/components/setup/SetupContentTest.tsx(1 hunks)frontend/__tests__/customers.test.tsx(7 hunks)frontend/__tests__/login.test.tsx(3 hunks)frontend/__tests__/loginPage.test.tsx(2 hunks)frontend/__tests__/passwordReset.test.tsx(1 hunks)frontend/__tests__/passwordResetConfirm.test.tsx(1 hunks)frontend/__tests__/passwordResetConfirmPage.test.tsx(2 hunks)frontend/__tests__/passwordResetPage.test.tsx(2 hunks)frontend/__tests__/registerPage.test.tsx(1 hunks)frontend/app/dashboard/layout.tsx(3 hunks)frontend/app/openapi-client/types.gen.ts(4 hunks)frontend/app/password-recovery/confirm/page.tsx(2 hunks)frontend/app/password-recovery/page.tsx(1 hunks)frontend/app/prompts/[id]/CopyToClipboardButton.tsx(1 hunks)frontend/app/prompts/[id]/page.tsx(1 hunks)frontend/app/prompts/_components/DuplicateButton.tsx(1 hunks)frontend/app/prompts/_components/PromptForm.tsx(1 hunks)frontend/app/prompts/create/page.tsx(1 hunks)frontend/app/prompts/deleteButton.tsx(1 hunks)frontend/app/prompts/edit/[id]/page.tsx(1 hunks)frontend/app/prompts/page.tsx(1 hunks)frontend/app/prompts/searchForm.tsx(1 hunks)frontend/app/prompts/tags/_components/DeleteTagButton.tsx(1 hunks)frontend/app/prompts/tags/_components/TagForm.tsx(1 hunks)frontend/app/prompts/tags/page.tsx(1 hunks)frontend/components/actions/password-reset-action.ts(3 hunks)frontend/components/actions/prompts-action.ts(1 hunks)frontend/components/setup/SetupContent.tsx(1 hunks)frontend/components/ui/FormError.tsx(2 hunks)frontend/components/ui/select.tsx(1 hunks)frontend/components/ui/submitButton.tsx(2 hunks)frontend/components/ui/textarea.tsx(1 hunks)frontend/components/ui/tooltip.tsx(1 hunks)frontend/components/ui/use-toast.tsx(1 hunks)frontend/jest.setup.js(1 hunks)frontend/openapi.json(1 hunks)frontend/package.json(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- .cursor/website.mdc
- README_zh-CN.md
- README.md
🚧 Files skipped from review as they are similar to previous changes (32)
- frontend/tests/customers.test.tsx
- backend/app/tests/utils/test_db.py
- frontend/components/ui/use-toast.tsx
- frontend/tests/app/dashboard/PromptNavigation.test.tsx
- frontend/tests/login.test.tsx
- frontend/app/dashboard/layout.tsx
- frontend/components/ui/FormError.tsx
- .env.example
- frontend/components/ui/textarea.tsx
- frontend/tests/app/setup/SetupPageTest.tsx
- frontend/package.json
- frontend/tests/components/setup/SetupContentTest.tsx
- frontend/app/prompts/tags/_components/DeleteTagButton.tsx
- frontend/app/prompts/_components/DuplicateButton.tsx
- frontend/app/prompts/tags/_components/TagForm.tsx
- frontend/components/ui/submitButton.tsx
- frontend/tests/passwordResetConfirmPage.test.tsx
- frontend/app/prompts/searchForm.tsx
- frontend/tests/passwordReset.test.tsx
- frontend/app/prompts/deleteButton.tsx
- frontend/components/ui/tooltip.tsx
- frontend/components/ui/select.tsx
- docker-compose.yml
- frontend/components/actions/password-reset-action.ts
- frontend/tests/registerPage.test.tsx
- frontend/app/prompts/edit/[id]/page.tsx
- frontend/app/password-recovery/page.tsx
- frontend/app/prompts/create/page.tsx
- admin/tests/auth.setup.ts
- frontend/components/setup/SetupContent.tsx
- frontend/app/prompts/tags/page.tsx
- backend/app/tests/api/test_prompts.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
frontend/components/actions/prompts-action.ts (4)
backend/app/models/prompt.py (1)
InputVariable(13-16)frontend/app/openapi-client/types.gen.ts (1)
InputVariable(47-51)backend/app/utils/response.py (1)
ApiResponse(9-12)frontend/app/clientService.ts (11)
readPrompts(61-61)readPrompt(63-63)readTags(66-66)createPrompt(62-62)updatePrompt(64-64)deletePrompt(65-65)readPromptVersions(70-70)duplicatePrompt(73-73)createTag(67-67)updateTag(68-68)deleteTag(69-69)
backend/app/crud.py (5)
backend/app/core/security.py (2)
get_password_hash(26-27)verify_password(22-23)backend/app/api/routes/users.py (2)
create_user(54-75)update_user(183-207)backend/app/api/routes/items.py (3)
create_item(69-79)update_item(83-105)delete_item(109-122)backend/app/models/prompt.py (1)
Tag(37-54)backend/app/api/routes/prompts.py (2)
create_tag(56-85)update_tag(89-128)
🪛 Biome (1.9.4)
frontend/app/prompts/page.tsx
[error] 86-98: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/jest.setup.js
[error] 24-24: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 223-224: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
frontend/app/openapi-client/types.gen.ts
[error] 504-504: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 561-561: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
admin/src/client/types.gen.ts
[error] 356-356: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 393-393: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🪛 Checkov (3.2.334)
frontend/openapi.json
[MEDIUM] 1: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🪛 Ruff (0.11.9)
backend/app/alembic/versions/add_prompt_management_tables.py
4-4: Trailing whitespace
Remove trailing whitespace
(W291)
51-51: Blank line contains whitespace
Remove whitespace from blank line
(W293)
71-71: Blank line contains whitespace
Remove whitespace from blank line
(W293)
98-98: Trailing whitespace
Remove trailing whitespace
(W291)
98-98: No newline at end of file
Add trailing newline
(W292)
🪛 GitHub Check: Spell Check
backend/app/models/prompt.py
[warning] 50-50:
"selectin" should be "selection" or "selecting".
🪛 GitHub Actions: Spell Check with Typos
backend/app/models/prompt.py
[warning] 50-50: "selectin" should be "selection" or "selecting". Typo detected by typos tool.
[error] 50-50: Typo error: selectin should be selection or selecting. Command './typos . --write-changes --config _typos.toml' failed with exit code 2.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test-playwright (1, 4)
- GitHub Check: test-playwright (3, 4)
- GitHub Check: test-playwright (4, 4)
- GitHub Check: test-playwright (2, 4)
🔇 Additional comments (17)
frontend/openapi.json (1)
1-1: Spec file looks goodNo structural problems detected in this diff chunk.
🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 1: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
backend/app/models/prompt.py (4)
19-25: Well-structured many-to-many relationship implementation.The
PromptTagLinkjunction table is correctly set up with composite primary keys using foreign keys to both related tables.
58-63: Great use of string Enums for type constraints.Using
Enumtypes forPromptTypeprovides compile-time hints, runtime validation, and ensures only valid values are stored in the database.
78-82: Ensure consistent SQL column definitions for enum fields.Both enum fields correctly use explicit SQL column definitions with proper nullability constraints.
148-152: Good use of cascade delete for version history.The relationship configuration with
"cascade": "all, delete-orphan"ensures that when a prompt is deleted, all its versions are automatically deleted as well, maintaining database integrity.frontend/jest.setup.js (2)
5-43: Comprehensive fetch mock implementation - much improved!This comprehensive fetch mock implementation now includes all standard Response properties and methods, including headers, URL, redirect status, response body, and appropriate methods with state tracking.
🧰 Tools
🪛 Biome (1.9.4)
[error] 24-24: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
45-79: Well-implemented framer-motion mock with proxy pattern.The mock implementation for framer-motion uses a Proxy to dynamically handle any motion component, which is an elegant approach that avoids having to explicitly mock each component type.
backend/app/crud.py (2)
13-19: Well-designed use of Protocol and TypeVar for generic CRUD operations.Using the
Protocoltype with amodel_dumpmethod requirement andTypeVarfor generic typing is a clean approach for creating reusable CRUD functions that work with different models.
31-41: Efficient implementation of pagination in get_multi function.The implementation correctly uses SQL's
offsetandlimitclauses for efficient pagination, which is much better than fetching all records and slicing in Python.frontend/app/openapi-client/types.gen.ts (4)
47-51: Well-structured InputVariable type definition.The InputVariable type provides a clean structure for prompt input variables with appropriate optional fields.
79-96: Comprehensive Prompt type with all necessary fields.This type definition properly captures all aspects of a prompt entity including content, type, visibility, metadata, and user/timestamp information.
133-134: Clear enum definition for PromptType.The string literal union type provides a well-defined set of allowed prompt types.
242-242: Concise Visibility enum definition.The visibility options are clearly defined as a string literal union type.
admin/src/client/types.gen.ts (4)
47-51: Well-structured InputVariable type definition.The InputVariable type provides a clean structure for prompt input variables with appropriate optional fields.
79-96: Comprehensive Prompt type with all necessary fields.The type definition properly captures all aspects of a prompt entity including content, type, visibility, metadata, and user/timestamp information.
133-134: Clear enum definition for PromptType.The string literal union type provides a well-defined set of allowed prompt types.
242-242: Concise Visibility enum definition.The visibility options are clearly defined as a string literal union type.
| export function CopyToClipboardButton({ content }: CopyToClipboardButtonProps) { | ||
| return ( | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={() => { | ||
| navigator.clipboard.writeText(content); | ||
| // Optionally, show a toast notification here | ||
| }} | ||
| > | ||
| <Copy className="h-4 w-4 mr-2" /> | ||
| 复制内容 | ||
| </Button> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and user feedback for clipboard operations.
The component handles the basic copy functionality, but lacks error handling for clipboard operations (which can fail in some environments) and doesn't provide user feedback on successful copying.
Here's an improved implementation with error handling and feedback:
export function CopyToClipboardButton({ content }: CopyToClipboardButtonProps) {
+ const [copied, setCopied] = React.useState(false);
+
+ const handleCopy = async () => {
+ try {
+ await navigator.clipboard.writeText(content);
+ setCopied(true);
+ setTimeout(() => setCopied(false), 2000);
+ // Optionally, show a toast notification here
+ } catch (error) {
+ console.error("Failed to copy content:", error);
+ // Optionally, show an error toast notification here
+ }
+ };
return (
<Button
variant="outline"
size="sm"
- onClick={() => {
- navigator.clipboard.writeText(content);
- // Optionally, show a toast notification here
- }}
+ onClick={handleCopy}
>
<Copy className="h-4 w-4 mr-2" />
- 复制内容
+ {copied ? "已复制" : "复制内容"}
</Button>
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function CopyToClipboardButton({ content }: CopyToClipboardButtonProps) { | |
| return ( | |
| <Button | |
| variant="outline" | |
| size="sm" | |
| onClick={() => { | |
| navigator.clipboard.writeText(content); | |
| // Optionally, show a toast notification here | |
| }} | |
| > | |
| <Copy className="h-4 w-4 mr-2" /> | |
| 复制内容 | |
| </Button> | |
| ); | |
| } | |
| export function CopyToClipboardButton({ content }: CopyToClipboardButtonProps) { | |
| const [copied, setCopied] = React.useState(false); | |
| const handleCopy = async () => { | |
| try { | |
| await navigator.clipboard.writeText(content); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 2000); | |
| // Optionally, show a toast notification here | |
| } catch (error) { | |
| console.error("Failed to copy content:", error); | |
| // Optionally, show an error toast notification here | |
| } | |
| }; | |
| return ( | |
| <Button | |
| variant="outline" | |
| size="sm" | |
| onClick={handleCopy} | |
| > | |
| <Copy className="h-4 w-4 mr-2" /> | |
| {copied ? "已复制" : "复制内容"} | |
| </Button> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In frontend/app/prompts/[id]/CopyToClipboardButton.tsx around lines 11 to 25,
the clipboard copy operation lacks error handling and user feedback. Update the
onClick handler to use async/await with try-catch to handle potential errors
from navigator.clipboard.writeText. Add user feedback such as a toast
notification or alert to inform the user whether the copy succeeded or failed.
| it("displays success message on successful form submission", async () => { | ||
| // Mock a successful password reset | ||
| (passwordReset as jest.Mock).mockResolvedValue({ | ||
| message: "Password reset instructions sent to your email.", | ||
| }); | ||
| // Mock useActionState 返回成功状态 | ||
| jest.spyOn(React, "useActionState").mockImplementation(() => [ | ||
| { | ||
| message: "密码重置链接已发送到您的邮箱，请查收。", | ||
| }, | ||
| jest.fn(), | ||
| ]); | ||
|
|
||
| render(<Page />); | ||
|
|
||
| const emailInput = screen.getByLabelText(/email/i); | ||
| const submitButton = screen.getByRole("button", { name: /send/i }); | ||
|
|
||
| fireEvent.change(emailInput, { target: { value: "[email protected]" } }); | ||
| fireEvent.click(submitButton); | ||
|
|
||
| await waitFor(() => { | ||
| expect( | ||
| screen.getByText("Password reset instructions sent to your email."), | ||
| ).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| const formData = new FormData(); | ||
| formData.set("email", "[email protected]"); | ||
| expect(passwordReset).toHaveBeenCalledWith(undefined, formData); | ||
| expect( | ||
| screen.getByText("密码重置链接已发送到您的邮箱，请查收。"), | ||
| ).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify form interaction in addition to rendering.
The test only verifies that the success message is rendered but doesn't verify the user interaction flow that leads to it.
it("displays success message on successful form submission", async () => {
+ const mockDispatch = jest.fn();
// Mock useActionState 返回成功状态
jest.spyOn(React, "useActionState").mockImplementation(() => [
{
message: "密码重置链接已发送到您的邮箱，请查收。",
},
- jest.fn(),
+ mockDispatch,
]);
render(<Page />);
+ // 模拟用户输入和表单提交
+ const emailInput = screen.getByRole("textbox", { name: /邮箱/i });
+ const submitButton = screen.getByRole("button", { name: /发送重置链接/i });
+
+ fireEvent.change(emailInput, { target: { value: "[email protected]" } });
+ fireEvent.click(submitButton);
+
+ // 验证 dispatch 被调用
+ expect(mockDispatch).toHaveBeenCalled();
+
expect(
screen.getByText("密码重置链接已发送到您的邮箱，请查收。"),
).toBeInTheDocument();
});📝 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.
| it("displays success message on successful form submission", async () => { | |
| // Mock a successful password reset | |
| (passwordReset as jest.Mock).mockResolvedValue({ | |
| message: "Password reset instructions sent to your email.", | |
| }); | |
| // Mock useActionState 返回成功状态 | |
| jest.spyOn(React, "useActionState").mockImplementation(() => [ | |
| { | |
| message: "密码重置链接已发送到您的邮箱，请查收。", | |
| }, | |
| jest.fn(), | |
| ]); | |
| render(<Page />); | |
| const emailInput = screen.getByLabelText(/email/i); | |
| const submitButton = screen.getByRole("button", { name: /send/i }); | |
| fireEvent.change(emailInput, { target: { value: "[email protected]" } }); | |
| fireEvent.click(submitButton); | |
| await waitFor(() => { | |
| expect( | |
| screen.getByText("Password reset instructions sent to your email."), | |
| ).toBeInTheDocument(); | |
| }); | |
| const formData = new FormData(); | |
| formData.set("email", "[email protected]"); | |
| expect(passwordReset).toHaveBeenCalledWith(undefined, formData); | |
| expect( | |
| screen.getByText("密码重置链接已发送到您的邮箱，请查收。"), | |
| ).toBeInTheDocument(); | |
| }); | |
| it("displays success message on successful form submission", async () => { | |
| const mockDispatch = jest.fn(); | |
| // Mock useActionState 返回成功状态 | |
| jest.spyOn(React, "useActionState").mockImplementation(() => [ | |
| { | |
| message: "密码重置链接已发送到您的邮箱，请查收。", | |
| }, | |
| mockDispatch, | |
| ]); | |
| render(<Page />); | |
| // 模拟用户输入和表单提交 | |
| const emailInput = screen.getByRole("textbox", { name: /邮箱/i }); | |
| const submitButton = screen.getByRole("button", { name: /发送重置链接/i }); | |
| fireEvent.change(emailInput, { target: { value: "[email protected]" } }); | |
| fireEvent.click(submitButton); | |
| // 验证 dispatch 被调用 | |
| expect(mockDispatch).toHaveBeenCalled(); | |
| expect( | |
| screen.getByText("密码重置链接已发送到您的邮箱，请查收。"), | |
| ).toBeInTheDocument(); | |
| }); |
🤖 Prompt for AI Agents
In frontend/__tests__/passwordResetPage.test.tsx around lines 59 to 73, the test
only checks if the success message is rendered but does not simulate the user
interaction that triggers the form submission. Update the test to simulate
filling out the form fields and submitting the form before asserting that the
success message appears. This ensures the test verifies the full user
interaction flow leading to the success state.
| it("handles login failure", async () => { | ||
| // Mock fetch to return an error response | ||
| global.fetch = jest.fn().mockImplementation(() => | ||
| Promise.resolve({ | ||
| ok: false, | ||
| json: () => Promise.resolve({ detail: "LOGIN_BAD_CREDENTIALS" }), | ||
| }), | ||
| ); | ||
|
|
||
| render(<Page />); | ||
|
|
||
| const usernameInput = screen.getByLabelText(/username/i); | ||
| const passwordInput = screen.getByLabelText(/password/i); | ||
| const submitButton = screen.getByRole("button", { name: /sign in/i }); | ||
|
|
||
| fireEvent.change(usernameInput, { target: { value: "[email protected]" } }); | ||
| fireEvent.change(passwordInput, { target: { value: "wrongpass" } }); | ||
| fireEvent.click(submitButton); | ||
| const usernameInput = screen.getByPlaceholderText(/email/i); | ||
| const passwordInput = screen.getByPlaceholderText(/password/i); | ||
| const submitButton = screen.getByRole("button", { name: /^登录$/i }); | ||
|
|
||
| // 填写表单并提交 | ||
| await act(async () => { | ||
| fireEvent.change(usernameInput, { | ||
| target: { value: "[email protected]" }, | ||
| }); | ||
| fireEvent.change(passwordInput, { target: { value: "wrongpass" } }); | ||
| fireEvent.click(submitButton); | ||
| }); | ||
|
|
||
| // 验证 fetch 调用 | ||
| await waitFor(() => { | ||
| expect(screen.getByText("LOGIN_BAD_CREDENTIALS")).toBeInTheDocument(); | ||
| expect(global.fetch).toHaveBeenCalled(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add assertion for error message display.
The test verifies that fetch is called but doesn't check if error messages are properly displayed to the user.
it("handles login failure", async () => {
// Mock fetch to return an error response
global.fetch = jest.fn().mockImplementation(() =>
Promise.resolve({
ok: false,
json: () => Promise.resolve({ detail: "LOGIN_BAD_CREDENTIALS" }),
}),
);
render(<Page />);
const usernameInput = screen.getByPlaceholderText(/email/i);
const passwordInput = screen.getByPlaceholderText(/password/i);
const submitButton = screen.getByRole("button", { name: /^登录$/i });
// 填写表单并提交
await act(async () => {
fireEvent.change(usernameInput, {
target: { value: "[email protected]" },
});
fireEvent.change(passwordInput, { target: { value: "wrongpass" } });
fireEvent.click(submitButton);
});
// 验证 fetch 调用
await waitFor(() => {
expect(global.fetch).toHaveBeenCalled();
});
+
+ // 验证错误消息显示
+ await waitFor(() => {
+ expect(screen.getByText(/用户名或密码错误/i)).toBeInTheDocument();
+ });
});📝 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.
| it("handles login failure", async () => { | |
| // Mock fetch to return an error response | |
| global.fetch = jest.fn().mockImplementation(() => | |
| Promise.resolve({ | |
| ok: false, | |
| json: () => Promise.resolve({ detail: "LOGIN_BAD_CREDENTIALS" }), | |
| }), | |
| ); | |
| render(<Page />); | |
| const usernameInput = screen.getByLabelText(/username/i); | |
| const passwordInput = screen.getByLabelText(/password/i); | |
| const submitButton = screen.getByRole("button", { name: /sign in/i }); | |
| fireEvent.change(usernameInput, { target: { value: "[email protected]" } }); | |
| fireEvent.change(passwordInput, { target: { value: "wrongpass" } }); | |
| fireEvent.click(submitButton); | |
| const usernameInput = screen.getByPlaceholderText(/email/i); | |
| const passwordInput = screen.getByPlaceholderText(/password/i); | |
| const submitButton = screen.getByRole("button", { name: /^登录$/i }); | |
| // 填写表单并提交 | |
| await act(async () => { | |
| fireEvent.change(usernameInput, { | |
| target: { value: "[email protected]" }, | |
| }); | |
| fireEvent.change(passwordInput, { target: { value: "wrongpass" } }); | |
| fireEvent.click(submitButton); | |
| }); | |
| // 验证 fetch 调用 | |
| await waitFor(() => { | |
| expect(screen.getByText("LOGIN_BAD_CREDENTIALS")).toBeInTheDocument(); | |
| expect(global.fetch).toHaveBeenCalled(); | |
| }); | |
| }); | |
| it("handles login failure", async () => { | |
| // Mock fetch to return an error response | |
| global.fetch = jest.fn().mockImplementation(() => | |
| Promise.resolve({ | |
| ok: false, | |
| json: () => Promise.resolve({ detail: "LOGIN_BAD_CREDENTIALS" }), | |
| }), | |
| ); | |
| render(<Page />); | |
| const usernameInput = screen.getByPlaceholderText(/email/i); | |
| const passwordInput = screen.getByPlaceholderText(/password/i); | |
| const submitButton = screen.getByRole("button", { name: /^登录$/i }); | |
| // 填写表单并提交 | |
| await act(async () => { | |
| fireEvent.change(usernameInput, { | |
| target: { value: "[email protected]" }, | |
| }); | |
| fireEvent.change(passwordInput, { target: { value: "wrongpass" } }); | |
| fireEvent.click(submitButton); | |
| }); | |
| // 验证 fetch 调用 | |
| await waitFor(() => { | |
| expect(global.fetch).toHaveBeenCalled(); | |
| }); | |
| // 验证错误消息显示 | |
| await waitFor(() => { | |
| expect(screen.getByText(/用户名或密码错误/i)).toBeInTheDocument(); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In frontend/__tests__/loginPage.test.tsx around lines 64 to 92, the test mocks a
failed login fetch call but does not assert that the error message is displayed
to the user. Add an assertion after the fetch call to check that the error
message corresponding to "LOGIN_BAD_CREDENTIALS" is rendered on the screen,
ensuring the UI properly informs the user of the login failure.
| op.create_table( | ||
| 'prompt_versions', | ||
| sa.Column('id', postgresql.UUID(), nullable=False), | ||
| sa.Column('prompt_id', postgresql.UUID(), nullable=False), | ||
| sa.Column('version', sa.Integer(), nullable=False), | ||
| sa.Column('content', sa.Text(), nullable=False), | ||
| sa.Column('input_vars', sa.JSON(), nullable=True), | ||
| sa.Column('created_by', postgresql.UUID(), nullable=False), | ||
| sa.Column('created_at', sa.DateTime(), nullable=False, server_default=sa.text('CURRENT_TIMESTAMP')), | ||
| sa.Column('change_notes', sa.String(), nullable=True), | ||
| sa.ForeignKeyConstraint(['prompt_id'], ['prompts.id'], ondelete='CASCADE'), | ||
| sa.PrimaryKeyConstraint('id') | ||
| ) | ||
|
|
||
| # 为版本表添加索引 | ||
| op.create_index(op.f('ix_prompt_versions_prompt_id'), 'prompt_versions', ['prompt_id'], unique=False) | ||
| op.create_index(op.f('ix_prompt_versions_version'), 'prompt_versions', ['version'], unique=False) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guarantee single version per prompt
A prompt should not have two identical version numbers. Instead of a plain index on version, create a composite UNIQUE constraint on (prompt_id, version); it is both semantically correct and speeds up look-ups.
- op.create_index(op.f('ix_prompt_versions_version'), 'prompt_versions', ['version'], unique=False)
+ op.create_unique_constraint(
+ 'uq_prompt_versions_prompt_id_version',
+ 'prompt_versions',
+ ['prompt_id', 'version']
+ )📝 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.
| op.create_table( | |
| 'prompt_versions', | |
| sa.Column('id', postgresql.UUID(), nullable=False), | |
| sa.Column('prompt_id', postgresql.UUID(), nullable=False), | |
| sa.Column('version', sa.Integer(), nullable=False), | |
| sa.Column('content', sa.Text(), nullable=False), | |
| sa.Column('input_vars', sa.JSON(), nullable=True), | |
| sa.Column('created_by', postgresql.UUID(), nullable=False), | |
| sa.Column('created_at', sa.DateTime(), nullable=False, server_default=sa.text('CURRENT_TIMESTAMP')), | |
| sa.Column('change_notes', sa.String(), nullable=True), | |
| sa.ForeignKeyConstraint(['prompt_id'], ['prompts.id'], ondelete='CASCADE'), | |
| sa.PrimaryKeyConstraint('id') | |
| ) | |
| # 为版本表添加索引 | |
| op.create_index(op.f('ix_prompt_versions_prompt_id'), 'prompt_versions', ['prompt_id'], unique=False) | |
| op.create_index(op.f('ix_prompt_versions_version'), 'prompt_versions', ['version'], unique=False) | |
| # 为版本表添加索引 | |
| op.create_index(op.f('ix_prompt_versions_prompt_id'), 'prompt_versions', ['prompt_id'], unique=False) | |
| - op.create_index(op.f('ix_prompt_versions_version'), 'prompt_versions', ['version'], unique=False) | |
| + op.create_unique_constraint( | |
| + 'uq_prompt_versions_prompt_id_version', | |
| + 'prompt_versions', | |
| + ['prompt_id', 'version'] | |
| + ) |
🧰 Tools
🪛 Ruff (0.11.9)
71-71: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🤖 Prompt for AI Agents
In backend/app/alembic/versions/add_prompt_management_tables.py around lines 58
to 75, replace the plain index on the 'version' column with a composite UNIQUE
constraint on ('prompt_id', 'version') in the 'prompt_versions' table. This
ensures that each prompt cannot have duplicate version numbers and improves
query performance. Remove the existing index on 'version' and add a unique
constraint combining 'prompt_id' and 'version' instead.
| op.create_table( | ||
| 'tags', | ||
| sa.Column('id', postgresql.UUID(), nullable=False), | ||
| sa.Column('name', sa.String(), nullable=False), | ||
| sa.Column('description', sa.String(), nullable=True), | ||
| sa.Column('color', sa.String(), nullable=True), | ||
| sa.Column('created_at', sa.DateTime(), nullable=False), | ||
| sa.PrimaryKeyConstraint('id') | ||
| ) | ||
| op.create_index(op.f('ix_tags_name'), 'tags', ['name'], unique=True) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add server_default to tags.created_at
Unlike the other tables, tags.created_at lacks an automatic default. Without it, every insert must supply a value or the row will fail.
- sa.Column('created_at', sa.DateTime(), nullable=False),
+ sa.Column(
+ 'created_at',
+ sa.DateTime(timezone=True),
+ nullable=False,
+ server_default=sa.text('CURRENT_TIMESTAMP')
+ ),📝 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.
| op.create_table( | |
| 'tags', | |
| sa.Column('id', postgresql.UUID(), nullable=False), | |
| sa.Column('name', sa.String(), nullable=False), | |
| sa.Column('description', sa.String(), nullable=True), | |
| sa.Column('color', sa.String(), nullable=True), | |
| sa.Column('created_at', sa.DateTime(), nullable=False), | |
| sa.PrimaryKeyConstraint('id') | |
| ) | |
| op.create_index(op.f('ix_tags_name'), 'tags', ['name'], unique=True) | |
| op.create_table( | |
| 'tags', | |
| sa.Column('id', postgresql.UUID(), nullable=False), | |
| sa.Column('name', sa.String(), nullable=False), | |
| sa.Column('description', sa.String(), nullable=True), | |
| sa.Column('color', sa.String(), nullable=True), | |
| sa.Column( | |
| 'created_at', | |
| sa.DateTime(timezone=True), | |
| nullable=False, | |
| server_default=sa.text('CURRENT_TIMESTAMP') | |
| ), | |
| sa.PrimaryKeyConstraint('id') | |
| ) | |
| op.create_index(op.f('ix_tags_name'), 'tags', ['name'], unique=True) |
🤖 Prompt for AI Agents
In backend/app/alembic/versions/add_prompt_management_tables.py around lines 21
to 30, the created_at column in the tags table lacks a server_default, causing
inserts to fail if no value is provided. Add a server_default to the created_at
column using a suitable SQL function like sa.func.now() to ensure it
automatically sets the current timestamp on insert.
frontend/app/prompts/page.tsx
Outdated
| {/* 排序控制区域 */} | ||
| <div className="flex justify-between items-center mb-6"> | ||
| <div className="text-sm text-muted-foreground"> | ||
| 共 {promptsList.length} 个提示词 | ||
| </div> | ||
| <div className="flex items-center gap-2"> | ||
| <Link | ||
| href={`/prompts?sort=${sort || "updated_at"}&order=${order === "asc" ? "desc" : "asc"}`} | ||
| className="flex items-center gap-1 text-sm text-muted-foreground hover:text-primary" | ||
| > | ||
| {order === "asc" ? ( | ||
| <SortAsc className="h-4 w-4" /> | ||
| ) : ( | ||
| <SortDesc className="h-4 w-4" /> | ||
| )} | ||
| {sort === "created_at" ? "创建时间" : "更新时间"} | ||
| </Link> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Sorting link drops existing filters & search terms
Building the sort URL from scratch wipes query, tags, and other future params, causing the user to lose their current context.
Suggestion (high-level):
- Read current
searchParamsinto aURLSearchParamsinstance. - Toggle only the
sort/orderkeys. - Generate the
hrefviaparams.toString().
This preserves active filters while still switching ordering.
🤖 Prompt for AI Agents
In frontend/app/prompts/page.tsx around lines 327 to 345, the current sort link
rebuilds the URL from scratch, causing loss of existing query parameters like
filters and search terms. To fix this, read the current search parameters into a
URLSearchParams object, update only the sort and order keys accordingly, and
then generate the href using params.toString() to preserve all other active
query parameters while toggling sorting.
| console.log( | ||
| `[${requestId}] 获取prompts数据，token: ${token.substring(0, 5)}...`, | ||
| ); | ||
|
|
||
| const response = await readPrompts({ | ||
| headers: { | ||
| Authorization: `Bearer ${token}`, | ||
| }, | ||
| query: { | ||
| search: options?.search, | ||
| tag_ids: options?.tag_ids, | ||
| sort: options?.sort, | ||
| order: options?.order || "desc", | ||
| }, | ||
| }); | ||
|
|
||
| // 检查响应是否为空或无效 | ||
| if (!response) { | ||
| console.error(`[${requestId}] API 返回空响应`); | ||
| const errorResult = { error: "服务器返回空响应" }; | ||
| if (!options) { | ||
| promptsCache = errorResult; | ||
| lastPromptsFetchTime = now; | ||
| } | ||
| return errorResult; | ||
| } | ||
|
|
||
| const { data, error } = response; | ||
|
|
||
| // 增强的错误处理 | ||
| if (error) { | ||
| console.error(`[${requestId}] API 错误:`, error || "空错误对象"); | ||
| const errorResult = { | ||
| error: | ||
| typeof error === "string" | ||
| ? error | ||
| : error | ||
| ? JSON.stringify(error) | ||
| : "未知API错误", | ||
| status: 500, | ||
| }; | ||
| if (!options) { | ||
| promptsCache = errorResult; | ||
| lastPromptsFetchTime = now; | ||
| } | ||
| return errorResult; | ||
| } | ||
|
|
||
| // 如果API返回的数据为空或无效，返回适当的错误信息 | ||
| if (!data) { | ||
| console.warn(`[${requestId}] API 未返回数据`); | ||
| const errorResult = { error: "API未返回数据", status: 404 }; | ||
| if (!options) { | ||
| promptsCache = errorResult; | ||
| lastPromptsFetchTime = now; | ||
| } | ||
| return errorResult; | ||
| } | ||
|
|
||
| // 处理数据 | ||
| let result: FetchPromptsReturn; | ||
|
|
||
| if (Array.isArray(data)) { | ||
| result = data; | ||
| } else if ( | ||
| data && | ||
| typeof data === "object" && | ||
| "data" in data && | ||
| Array.isArray(data.data) | ||
| ) { | ||
| result = data.data; | ||
| } else { | ||
| console.warn(`[${requestId}] 意外的 API 响应格式:`, data); | ||
| result = { error: "API返回了意外的数据格式", status: 400 }; | ||
| } | ||
|
|
||
| // 只有在不带过滤条件的情况下才更新缓存 | ||
| if (!options) { | ||
| promptsCache = result; | ||
| lastPromptsFetchTime = now; | ||
| } | ||
|
|
||
| console.log( | ||
| `[${requestId}] fetchPrompts 执行完成，数据${Array.isArray(result) ? `长度: ${result.length}` : "是错误对象"}`, | ||
| ); | ||
| return result; | ||
| } catch (error) { | ||
| console.error(`[${requestId}] 获取prompts数据出错:`, error); | ||
| const errorMessage = error instanceof Error ? error.message : "未知错误"; | ||
| const errorResult = { error: `获取数据失败: ${errorMessage}`, status: 500 }; | ||
|
|
||
| // 只有在不带过滤条件的情况下才更新缓存 | ||
| if (!options) { | ||
| promptsCache = errorResult; | ||
| lastPromptsFetchTime = now; | ||
| } | ||
|
|
||
| return errorResult; | ||
| } | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Inconsistent caching strategy between fetch functions.
fetchPrompts uses manual TTL caching without React's cache() wrapper, while fetchPrompt, fetchTags, and fetchPromptVersions use both React's cache() and manual TTL caching. This inconsistency might lead to different caching behaviors.
Choose one consistent approach:
- Remove
cache()from all functions and rely solely on your manual TTL - Use only React's
cache()and remove manual TTL logic
The current mixed approach can lead to stale data as the outer cache may never reach the TTL-based invalidation.
Also applies to: 225-258, 261-374, 555-588
🌐 Web query:
React server components cache() function behavior with TTL caching
💡 Result:
React Server Components cache() Function and TTL (Time-to-Live) Caching
cache() is a utility function provided exclusively for React Server Components (RSC). It allows you to memoize (cache) the results of data-fetching or computational functions to prevent redundant work during a single server-side render. Here’s how it operates and how it relates to time-to-live (TTL) caching:
How cache() Works in React Server Components
- The
cache()function wraps another function (typically a data-fetching function), creating a memoized version. - When the wrapped function is called with the same arguments during the same server render, it returns the cached result instead of executing again. This is useful to avoid repeat fetches or computations if the same data is needed in multiple places in a component tree.
- Memoization scope is limited to a single server request—the cache lives only "the lifetime of a server request until the React component tree has finished rendering" and does not persist between different requests or render cycles[2][3][1].
Example Usage
import { cache } from 'react';
const getData = cache(async (id) => {
// fetch or compute heavy data
return fetchFromAPI(id);
});If two components in the same server render call getData(42), the computation or fetch occurs only once per request[1].
Request Memoization vs. Persistent/TTL Caching
- Request Memoization: The core behavior of
cache()is to deduplicate work during one server render (one HTTP request). After the render, the cache is cleared[2]. - No Built-in TTL: React's
cache()does not natively support time-based expiration (TTL), and its cache does not persist between different user requests. Each new request to the server will start with a fresh cache. - TTL Caching (Time-to-Live): If you need to cache data for longer (across requests) and add a TTL, you must integrate an external caching strategy (e.g., in-memory cache, Redis, file system cache) where you can control expiration and invalidation[4][5][9].
Implementing TTL/Expiring Caches
If your use case requires caching data for a specific period (e.g., 5 minutes, 1 day):
- Use an external cache layer (in-memory, Redis, file system, etc.) that supports expiration times (TTL).
- You can wrap your data-fetching logic in your own caching function that checks/sets cache entries with timestamps and evicts data after TTL expires[4][5].
- See the following file-system cache approach for TTL:
[4]
function isValidCacheEntry(filename) { const TTL = 1000 * 60 * 60 * 24; // 1 day in milliseconds if (!fs.existsSync(filename)) return false; const fileAge = Date.now() - fs.statSync(filename).mtimeMs; if (fileAge > TTL) return false; return true; }
Summary Table:
| Feature | React cache() |
External TTL Cache |
|---|---|---|
| Lifetime | One server request | Configurable (e.g. 5 min) |
| API for Expiry (TTL) | Not supported | Supported |
| Scope | Per server render | Shared across requests |
| Use case | Memoization in render | Application data caching |
Conclusion
cache()in React Server Components is for request-scoped memoization only—it is not a long-lived cache and does not offer TTL or persistence between requests[1][2][3].- For TTL or persistent caching, implement your own cache using external technologies, where you can control cache duration and invalidation[4][5][9].
If you need HTTP cache-like behavior (e.g., revalidate after 5 minutes), tools like Next.js offer fetch enhancements with the next: { revalidate: 300 } option, which is separate from React’s cache()[9].
For most apps, combine
cache()for deduplicating fetches during rendering and a TTL-aware external cache for lasting, shared caching needs.
Citations:
- 1: https://react.dev/reference/react/cache
- 2: https://nextjs.org/docs/app/deep-dive/caching
- 3: [Suggestion]: Clarification on react.cache Usage in Server Requests reactjs/react.dev#6671
- 4: https://latitude.so/blog/expressjs-react-server-side-cache/
- 5: https://blog.pixelfreestudio.com/how-to-handle-caching-in-server-side-rendering/
- 6: https://www.pronextjs.dev/workshops/pro-next-js-workshop-hl06z/data-caching-and-revalidation-with-react-server-components-pkgrz
- 7: https://www.toptal.com/react/stale-while-revalidate
- 8: https://dev.to/csjcode/ultimate-caching-guide-2-javascriptreact-2lie
- 9: https://infinum.com/handbook/frontend/react/recipes/caching-and-revalidation
- 10: https://redux-toolkit.js.org/rtk-query/usage/cache-behavior
Unify caching strategy: use only manual TTL-based caching and remove React’s cache() wrappers
React’s cache() is scoped to a single server render and doesn’t honor TTL or persist between requests, whereas your manual TTL logic does. Mixing both can be confusing and lead to unexpected staleness. Remove all cache() wrappers so every fetch util relies exclusively on your manual TTL cache.
Affected locations:
- frontend/components/actions/prompts-action.ts
• Removecache()fromfetchPrompt(lines 225–258)
• Removecache()fromfetchTags(lines 261–374)
• Removecache()fromfetchPromptVersions(lines 555–588)
Example diff:
- import { cache } from 'react';
-
- export const fetchPrompt = cache(async (options) => {
+ export const fetchPrompt = async (options) => {
// existing TTL-based caching logic…
};🤖 Prompt for AI Agents
In frontend/components/actions/prompts-action.ts from lines 88 to 222 and also
in the related functions at lines 225–258, 261–374, and 555–588, unify the
caching strategy by removing all React cache() wrappers from fetchPrompt,
fetchTags, and fetchPromptVersions. Keep only the manual TTL-based caching logic
you have implemented, as React's cache() only caches per server render and does
not respect TTL or persist between requests. This will ensure consistent caching
behavior and avoid stale data issues.
- Added `selectin` to `_typos.toml` for SQLAlchemy relationship loading strategy. - Updated the Makefile to include the `test` target in the `all` command and adjusted the frontend test command to clean the `.next` directory before running tests. - Refactored frontend components to improve layout structure and user experience, including the integration of `MainLayout` in `CustomersLayout` and `DashboardLayout`. - Improved error handling and logging in backend CRUD operations for prompts and tags. - Enhanced test coverage and reliability by mocking additional hooks and refining test configurations.
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (4)
frontend/components/ui/use-toast.tsx (1)
43-72: Consider adding proper type definitions instead of using 'any'.While exporting a direct toast function is a helpful addition, the use of
anytype for both parameters and return value reduces type safety. Consider using proper type definitions from the underlying toast implementation.The implementation of the non-hook toast function is well-designed, providing the same functionality as the hook version while avoiding React hook constraints. The structure with update and dismiss functions offers a good developer experience.
frontend/app/prompts/page.tsx (1)
327-333: Sorting link still drops existing filters – previous feedback unaddressed
Your newhrefreconstructs the query string from scratch and therefore erasesquery,tags, etc. when users toggle sort order.See earlier comment for preserving parameters with
URLSearchParams.backend/app/api/routes/prompts.py (1)
216-276:⚠️ Potential issueSecurity issue: Listing endpoint leaks private prompts
The
read_prompts()function retrieves all prompts from the database without filtering based on visibility or user permissions. This allows unauthorized users to see private prompts belonging to other users.Add permission filtering either through SQL or post-query filtering:
- # 执行查询 - prompts = db.exec(query).all() + # 执行查询 + prompts = db.exec(query).all() + + # 过滤掉无访问权限的记录 + prompts = [ + p for p in prompts if _check_prompt_access(p, current_user) + ]For better performance, consider filtering in the SQL query instead of Python.
backend/app/crud.py (1)
106-120: 🛠️ Refactor suggestionInefficient Python-based pagination in get_items function
While
get_multiandget_tagsfunctions now use SQL-based pagination, theget_itemsfunction still uses inefficient Python-based pagination.Apply SQL-based pagination:
- # 先执行查询获取所有结果 - results = session.exec(query).all() - # 然后在 Python 中进行分页 - return list(results[skip : skip + limit]) + query = query.offset(skip).limit(limit) + return session.exec(query).all()
🧹 Nitpick comments (13)
frontend/components/ui/toast.tsx (1)
296-296: Re-exporting internals leaks implementation details
genId,dispatch, andactionTypesexpose mutable global state; external callers can now bypass the reducer contract and cause hard-to-trace bugs (e.g., duplicate IDs, inconsistent state).
Consider exporting only a thin, documented API (toast(),dismiss(), etc.) and keep the internals file-private.If the extra exports are required for non-hook usage, wrap them in a dedicated helper instead of exposing the raw functions/constants.
frontend/jest.config.ts (1)
195-197: Consider uncommenting watchPathIgnorePatterns.These commented lines could be useful if you're using watch mode. Consider either uncommenting them or removing them to keep the configuration clean.
- // watchPathIgnorePatterns: [ - // "/.next/" - // ], + watchPathIgnorePatterns: [ + "/.next/" + ],frontend/components/ui/use-toast.tsx (1)
39-39: Translate comment to English for consistency.The comment is in Chinese while the rest of the codebase appears to be in English. Maintaining a consistent language improves maintainability.
-// 导入toast函数从toast.tsx文件中，避免直接在非组件函数中使用hooks +// Import toast function from toast.tsx to avoid using hooks directly in non-component functionsfrontend/components/layout/MainLayout.tsx (3)
33-41: Translate comment to English and use Tailwind's color system.For consistency, translate the Chinese comment to English. Also, instead of hard-coding color values, consider using Tailwind's color system directly to maintain consistency with the design system.
-// 定义主题颜色变量，符合 ui-colors 规则 +// Define theme color variables according to ui-colors rulesInstead of defining color hex values, you could rely on Tailwind's built-in colors:
-const themeColors = { - primary: "#3B82F6", // Primary Blue - success: "#2EC27E", // Green - info: "#0284C7", // Info Blue - warning: "#F59E0B", // Warning - error: "#DC2626", // Error - highlight: "#FEF9C3", // Highlight Yellow -}; // And then use Tailwind classes directly in your JSX: // className="text-blue-500" instead of `text-${themeColors.primary}`
64-137: Refactor repeated tooltip code into a reusable component.There's significant repetition in the tooltip implementation for each navigation link. This could be extracted into a reusable component to improve maintainability.
Consider creating a
NavLinkcomponent:type NavLinkProps = { href: string; icon: React.ReactNode; label: string; currentPath: string; }; function NavLink({ href, icon, label, currentPath }: NavLinkProps) { return ( <TooltipProvider> <Tooltip> <TooltipTrigger asChild> <Link href={href} className={`flex items-center gap-2 ${ currentPath === href ? "text-blue-500" : "text-gray-600 hover:text-gray-900 dark:text-gray-400 dark:hover:text-gray-50" }`} aria-label={label} > {icon} </Link> </TooltipTrigger> <TooltipContent side="right" className="bg-white dark:bg-gray-800 text-gray-900 dark:text-gray-100 border border-gray-200 dark:border-gray-700" > <p>{label}</p> </TooltipContent> </Tooltip> </TooltipProvider> ); } // Then use it like: // <NavLink href="/dashboard" icon={<List className="h-5 w-5" />} label="Dashboard" currentPath={currentPath} />
56-62: Consider making image dimensions responsive.The current implementation uses fixed dimensions for the logo image, which might not be optimal for all screen sizes.
<Image src="/images/vinta.png" alt="Vinta" - width={64} - height={64} + width={48} + height={48} + className="object-cover transition-transform duration-200 hover:scale-105 md:w-16 md:h-16" - className="object-cover transition-transform duration-200 hover:scale-105" />Makefile (2)
378-386: Frontend test pre-clean step removes.next/types/package.jsonrepeatedlyThe three
rm/mkdir/echolines execute on every test run, which slows down local loops and CI.
Because Playwright does not depend on.next/types, this block can be safely dropped or moved to a one-time bootstrap step (e.g.frontend-install).
462-470: Conflicting messaging: “Tests temporarily skipped …” but actually runs themThe banner still tells contributors that admin tests are skipped due to network issues, yet the next line executes
pnpm test.
Please remove the misleading echo or re-introduce the early return so expectations match reality.- @echo "===========> Tests temporarily skipped due to network issues" - @echo "===========> Consider running tests manually when network is stable" - @cd $(ADMIN_DIR) && $(PNPM) test + @cd $(ADMIN_DIR) && $(PNPM) testfrontend/app/prompts/_components/PromptForm.tsx (1)
384-446: Usingindexas React key can cause field mix-upsWhen users delete or reorder variables the same index gets re-assigned, leading React to preserve stale state inside child inputs.
Generate a stable identifier when the variable is created:
- { name: "", description: "", required: false }, + { id: crypto.randomUUID(), name: "", description: "", required: false },and then:
- {inputVars.map((variable, index) => ( - <Card key={index} … + {inputVars.map((variable, index) => ( + <Card key={variable.id} …This prevents subtle “field A shows value from field B” bugs.
frontend/app/prompts/page.tsx (1)
195-206: Remove verbose developmentconsole.logbefore mergingThe random
renderIDdebugging output ([prompts-${renderID}] …) will spam server logs in production.Either wrap it behind
process.env.NODE_ENV === "development"or drop it entirely.backend/app/api/routes/prompts.py (1)
371-371: Formatting issue: Blank line contains whitespaceLine 371 contains whitespace in an otherwise blank line.
- +🧰 Tools
🪛 Ruff (0.11.9)
371-371: Blank line contains whitespace
Remove whitespace from blank line
(W293)
frontend/jest.setup.js (2)
24-24: Use optional chaining for safer object property accessUsing optional chaining would make this code more robust against potential null/undefined values.
- url: typeof input === "string" ? input : (input && input.url) || "", + url: typeof input === "string" ? input : input?.url || "",🧰 Tools
🪛 Biome (1.9.4)
[error] 24-24: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
243-245: Avoid using delete operator for better performanceUsing
deleteon global objects can impact performance. Consider using undefined assignment instead.- delete global.mockSearchParams; - delete global.mockPathname; + global.mockSearchParams = undefined; + global.mockPathname = undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 243-244: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
Makefile(5 hunks)_typos.toml(1 hunks)admin/tests/auth.setup.ts(1 hunks)backend/app/api/routes/prompts.py(1 hunks)backend/app/crud.py(2 hunks)backend/app/models/prompt.py(1 hunks)frontend/.eslintrc.json(1 hunks)frontend/.gitignore(1 hunks)frontend/app/customers/layout.tsx(1 hunks)frontend/app/dashboard/layout.tsx(1 hunks)frontend/app/password-recovery/confirm/page.tsx(1 hunks)frontend/app/prompts/[id]/page.tsx(1 hunks)frontend/app/prompts/_components/PromptForm.tsx(1 hunks)frontend/app/prompts/edit/[id]/page.tsx(1 hunks)frontend/app/prompts/layout.tsx(1 hunks)frontend/app/prompts/page.tsx(1 hunks)frontend/app/prompts/searchForm.tsx(1 hunks)frontend/app/prompts/tags/page.tsx(1 hunks)frontend/components/actions/prompts-action.ts(1 hunks)frontend/components/layout/MainLayout.tsx(1 hunks)frontend/components/ui/submitButton.tsx(1 hunks)frontend/components/ui/toast.tsx(1 hunks)frontend/components/ui/use-toast.tsx(1 hunks)frontend/jest.config.ts(7 hunks)frontend/jest.env.js(1 hunks)frontend/jest.resolver.js(1 hunks)frontend/jest.resolver.stub.js(1 hunks)frontend/jest.setup.js(1 hunks)frontend/lib/extension-utils.ts(1 hunks)frontend/lib/server-auth.ts(2 hunks)
✅ Files skipped from review due to trivial changes (7)
- frontend/jest.env.js
- frontend/.gitignore
- _typos.toml
- frontend/jest.resolver.stub.js
- frontend/.eslintrc.json
- frontend/app/prompts/layout.tsx
- frontend/components/actions/prompts-action.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- frontend/app/password-recovery/confirm/page.tsx
- frontend/app/prompts/edit/[id]/page.tsx
- admin/tests/auth.setup.ts
- frontend/app/prompts/[id]/page.tsx
- frontend/app/prompts/tags/page.tsx
- frontend/components/ui/submitButton.tsx
- frontend/lib/extension-utils.ts
- frontend/app/prompts/searchForm.tsx
- backend/app/models/prompt.py
🧰 Additional context used
🧬 Code Graph Analysis (3)
frontend/app/customers/layout.tsx (1)
frontend/components/layout/MainLayout.tsx (1)
MainLayout(43-221)
frontend/components/ui/use-toast.tsx (1)
frontend/components/ui/toast.tsx (4)
ToastType(296-296)genId(296-296)dispatch(296-296)actionTypes(296-296)
frontend/components/layout/MainLayout.tsx (2)
frontend/components/ui/tooltip.tsx (4)
TooltipProvider(30-30)Tooltip(30-30)TooltipTrigger(30-30)TooltipContent(30-30)frontend/app/page.tsx (1)
Home(8-69)
🪛 Ruff (0.11.9)
backend/app/api/routes/prompts.py
371-371: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🪛 Biome (1.9.4)
frontend/jest.setup.js
[error] 24-24: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 243-244: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 244-247: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
frontend/app/prompts/page.tsx
[error] 77-89: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (14)
frontend/app/customers/layout.tsx (1)
9-13: Looks good – adopts sharedMainLayoutSwitching to the common layout improves consistency and removes duplicated markup. No issues spotted.
frontend/app/dashboard/layout.tsx (1)
9-11: Consistent layout refactor approvedDashboard now leverages
MainLayout, removing duplicated sidebar/header code. Implementation is straightforward.frontend/jest.resolver.js (1)
1-17:✅ Verification successful
Well-implemented custom Jest resolver for handling Next.js build artifacts.
This resolver elegantly handles the common issue of
.nextdirectory artifacts interfering with Jest tests by redirecting problematic paths to a stub file. This is a good solution for ensuring test reliability.However, I noticed this resolver requires a stub file at
./jest.resolver.stub.jswhich should export an empty object. Make sure this file exists in your project.
🏁 Script executed:
#!/bin/bash # Check if the stub file exists if [ -f "./frontend/jest.resolver.stub.js" ]; then echo "✅ Stub file exists" cat "./frontend/jest.resolver.stub.js" else echo "❌ Stub file missing: ./frontend/jest.resolver.stub.js is required" fiLength of output: 206
Approve: Custom Jest resolver and stub verified
The stub file at
frontend/jest.resolver.stub.jsexists and correctly exports an empty object, so the resolver will work as intended. No further changes needed.
- frontend/jest.resolver.js
- frontend/jest.resolver.stub.js
frontend/jest.config.ts (6)
37-37: Good addition to exclude.nextfrom coverage collection.This change ensures that Next.js build artifacts don't affect code coverage metrics.
102-102: Appropriate module path exclusion.Preventing Jest from considering modules in the
.nextdirectory helps avoid potential conflicts with build artifacts.
126-126: Good integration of custom resolver.This links to the custom resolver we just reviewed, creating a cohesive solution for handling Next.js artifacts during testing.
171-171: Comprehensive test path configuration.Adding both
.next/and.next/types/to test path ignore patterns ensures tests don't run against build artifacts.
186-186: Proper transformation exclusion.Skipping transformation for
.next/files prevents unnecessary processing of build artifacts.
143-143:✅ Verification successful
Environment setup properly configured.
Adding
jest.env.jsto setupFiles is a good practice to ensure consistent environment variables during testing.
🏁 Script executed:
#!/bin/bash # Check if the environment setup file exists and verify its contents if [ -f "./frontend/jest.env.js" ]; then echo "✅ Environment file exists" cat "./frontend/jest.env.js" else echo "❌ Environment file missing: ./frontend/jest.env.js is required" fiLength of output: 180
jest.env.js verified and environment setup approved
The
frontend/jest.env.jsfile exists and correctly setsprocess.env.NODE_ENV = "test". No further changes are needed—this configuration ensures a consistent test environment.frontend/components/ui/use-toast.tsx (1)
40-41: Good imports for implementing non-hook toast function.Importing the necessary types and utilities from the toast module provides a clean foundation for the non-hook implementation.
frontend/jest.setup.js (1)
4-43: Complete fetch mock implementation addresses previous issuesThe improved fetch mock now properly implements all the essential properties and methods of the Response interface, which will prevent test failures when code expects the full Response API.
🧰 Tools
🪛 Biome (1.9.4)
[error] 24-24: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
backend/app/crud.py (3)
39-41: Correct SQL-based pagination implementationThe
get_multifunction now properly uses SQL-based pagination with.offset(skip).limit(limit), which is more efficient than Python-based pagination.
229-242: Fixed token blacklist inconsistencyThe
create_token_blacklistfunction now correctly includes theblacklisted_atfield, which addresses the inconsistency withadd_token_to_blacklist.
335-336: Correct SQL-based pagination in get_tags functionThe
get_tagsfunction now properly uses SQL-based pagination, which is more efficient than the previous Python-based approach.
| : cache(async () => { | ||
| const cookieStore = await cookies(); | ||
| const token = cookieStore.get("accessToken")?.value; |
There was a problem hiding this comment.
Remove the unnecessary await when reading cookies
cookies() is synchronous and returns a RequestCookies instance, not a promise.
Using await here triggers a TypeScript error and the extra micro-task hop hurts performance for every request.
- const cookieStore = await cookies();
+ const cookieStore = cookies();📝 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.
| : cache(async () => { | |
| const cookieStore = await cookies(); | |
| const token = cookieStore.get("accessToken")?.value; | |
| : cache(async () => { | |
| const cookieStore = cookies(); | |
| const token = cookieStore.get("accessToken")?.value; |
🤖 Prompt for AI Agents
In frontend/lib/server-auth.ts around lines 64 to 66, remove the unnecessary
await before the call to cookies() since cookies() is synchronous and returns a
RequestCookies instance directly. This will fix the TypeScript error and improve
performance by avoiding an unnecessary micro-task.
| export const requireAuth = isTestEnv | ||
| ? requireAuthTest | ||
| : cache(async () => { | ||
| const authState = await getAuthState(); | ||
| return authState.isAuthenticated && authState.user; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
requireAuth return type is inconsistent & over-cached
• In prod it returns User | false, in tests it returns User.
Consumers now need runtime checks or as casts.
• Wrapping it in cache() (same problem as above) freezes the first result.
Suggest aligning the signature to always return User | null and drop cache():
-export const requireAuth = isTestEnv
- ? requireAuthTest
- : cache(async () => {
- const authState = await getAuthState();
- return authState.isAuthenticated && authState.user;
- });
+export const requireAuth = isTestEnv
+ ? requireAuthTest
+ : async (): Promise<User | null> => {
+ const { user, isAuthenticated } = await getAuthState();
+ return isAuthenticated ? user : null;
+ };📝 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 const requireAuth = isTestEnv | |
| ? requireAuthTest | |
| : cache(async () => { | |
| const authState = await getAuthState(); | |
| return authState.isAuthenticated && authState.user; | |
| }); | |
| export const requireAuth = isTestEnv | |
| ? requireAuthTest | |
| : async (): Promise<User | null> => { | |
| const { user, isAuthenticated } = await getAuthState(); | |
| return isAuthenticated ? user : null; | |
| }; |
🤖 Prompt for AI Agents
In frontend/lib/server-auth.ts around lines 170 to 175, the requireAuth function
returns inconsistent types between test and production environments and uses
cache(), which freezes the first result. To fix this, remove the cache() wrapper
and modify requireAuth to always return User or null, ensuring consistent return
types and avoiding stale cached results.
| export const getAuthState = isTestEnv | ||
| ? getAuthStateTest | ||
| : cache(async (): Promise<AuthState> => { | ||
| const now = Date.now(); | ||
|
|
||
| if (error) { | ||
| const newState = { | ||
| user: null, | ||
| isAuthenticated: false, | ||
| error: typeof error === "string" ? error : JSON.stringify(error), | ||
| timestamp: now, | ||
| }; | ||
| authStateCache = newState; | ||
| return newState; | ||
| } | ||
| // 使用缓存如果在有效期内 | ||
| if (authStateCache && now - authStateCache.timestamp < AUTH_CACHE_TTL) { | ||
| return authStateCache; | ||
| } | ||
|
|
||
| // 成功获取到用户信息，将UserPublic转换为User类型 | ||
| const newState = { | ||
| user: data ? convertToUser(data) : null, | ||
| isAuthenticated: true, | ||
| error: null, | ||
| timestamp: now, | ||
| }; | ||
| authStateCache = newState; | ||
| return newState; | ||
| } catch (error) { | ||
| // 异常处理 | ||
| const newState = { | ||
| user: null, | ||
| isAuthenticated: false, | ||
| error: | ||
| error instanceof Error ? error.message : "验证用户身份时发生未知错误", | ||
| timestamp: now, | ||
| }; | ||
| authStateCache = newState; | ||
| return newState; | ||
| } | ||
| }); | ||
| // 获取token | ||
| const token = await getAuthToken(); | ||
|
|
||
| // 未登录状态 | ||
| if (!token) { | ||
| const newState = { | ||
| user: null, | ||
| isAuthenticated: false, | ||
| error: "No authentication token found", | ||
| timestamp: now, | ||
| }; | ||
| authStateCache = newState; | ||
| return newState; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
cache() + manual TTL ⇒ stale auth forever
react’s cache() memoises the function for the lifetime of the server process, so the inner 5-second TTL will never be reached; user data (or lack thereof) becomes permanently cached until the process restarts.
Remove the outer cache() (or use unstable_cache with revalidate) and rely on the TTL or keep cache() and drop the manual cache—mixing both silently breaks auth refresh.
-export const getAuthState = isTestEnv
- ? getAuthStateTest
- : cache(async (): Promise<AuthState> => {
+export const getAuthState = isTestEnv
+ ? getAuthStateTest
+ : (async (): Promise<AuthState> => {Apply the same change to getAuthToken and requireAuth if short-lived caching is required.
📝 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 const getAuthState = isTestEnv | |
| ? getAuthStateTest | |
| : cache(async (): Promise<AuthState> => { | |
| const now = Date.now(); | |
| if (error) { | |
| const newState = { | |
| user: null, | |
| isAuthenticated: false, | |
| error: typeof error === "string" ? error : JSON.stringify(error), | |
| timestamp: now, | |
| }; | |
| authStateCache = newState; | |
| return newState; | |
| } | |
| // 使用缓存如果在有效期内 | |
| if (authStateCache && now - authStateCache.timestamp < AUTH_CACHE_TTL) { | |
| return authStateCache; | |
| } | |
| // 成功获取到用户信息，将UserPublic转换为User类型 | |
| const newState = { | |
| user: data ? convertToUser(data) : null, | |
| isAuthenticated: true, | |
| error: null, | |
| timestamp: now, | |
| }; | |
| authStateCache = newState; | |
| return newState; | |
| } catch (error) { | |
| // 异常处理 | |
| const newState = { | |
| user: null, | |
| isAuthenticated: false, | |
| error: | |
| error instanceof Error ? error.message : "验证用户身份时发生未知错误", | |
| timestamp: now, | |
| }; | |
| authStateCache = newState; | |
| return newState; | |
| } | |
| }); | |
| // 获取token | |
| const token = await getAuthToken(); | |
| // 未登录状态 | |
| if (!token) { | |
| const newState = { | |
| user: null, | |
| isAuthenticated: false, | |
| error: "No authentication token found", | |
| timestamp: now, | |
| }; | |
| authStateCache = newState; | |
| return newState; | |
| export const getAuthState = isTestEnv | |
| ? getAuthStateTest | |
| : (async (): Promise<AuthState> => { | |
| const now = Date.now(); | |
| // 使用缓存如果在有效期内 | |
| if (authStateCache && now - authStateCache.timestamp < AUTH_CACHE_TTL) { | |
| return authStateCache; | |
| } | |
| // 获取token | |
| const token = await getAuthToken(); | |
| // 未登录状态 | |
| if (!token) { | |
| const newState = { | |
| user: null, | |
| isAuthenticated: false, | |
| error: "No authentication token found", | |
| timestamp: now, | |
| }; | |
| authStateCache = newState; | |
| return newState; | |
| } | |
| // …rest of function unchanged… |
🤖 Prompt for AI Agents
In frontend/lib/server-auth.ts lines 88 to 110, the use of react's cache()
combined with a manual TTL causes the auth state to be cached indefinitely,
ignoring the intended 5-second TTL. To fix this, remove the outer cache()
wrapper and rely solely on the manual TTL logic for caching, or alternatively
keep cache() but eliminate the manual TTL cache to avoid stale data. Apply the
same approach to getAuthToken and requireAuth if they also use short-lived
caching.
| currentPath === "/dashboard" | ||
| ? `text-${themeColors.primary}` | ||
| : "text-gray-600 hover:text-gray-900 dark:text-gray-400 dark:hover:text-gray-50" |
There was a problem hiding this comment.
Fix dynamic class generation with Tailwind CSS.
Using string interpolation with text-${themeColors.primary} won't work properly with Tailwind, as it processes classes at build time. This will likely result in unstyled elements.
Replace dynamic class construction with explicit Tailwind classes:
-className={`flex items-center gap-2 ${
- currentPath === "/dashboard"
- ? `text-${themeColors.primary}`
- : "text-gray-600 hover:text-gray-900 dark:text-gray-400 dark:hover:text-gray-50"
-}`}
+className={`flex items-center gap-2 ${
+ currentPath === "/dashboard"
+ ? "text-blue-500"
+ : "text-gray-600 hover:text-gray-900 dark:text-gray-400 dark:hover:text-gray-50"
+}`}Apply similar changes to the other navigation links at lines 94-96 and 118-120.
Also applies to: 94-96, 118-120
🤖 Prompt for AI Agents
In frontend/components/layout/MainLayout.tsx at lines 70-72, 94-96, and 118-120,
the code uses dynamic string interpolation for Tailwind CSS classes like
text-${themeColors.primary}, which Tailwind cannot process at build time.
Replace these dynamic class names with explicit, static Tailwind class names
corresponding to the intended colors from themeColors.primary to ensure proper
styling. Update all affected navigation link class assignments similarly to use
fixed class names instead of dynamic strings.
| <button | ||
| onClick={logout} | ||
| className="block w-full px-4 py-2 text-sm text-gray-700 dark:text-gray-300 hover:bg-gray-100 dark:hover:bg-gray-700 rounded-md" | ||
| > | ||
| Logout | ||
| </button> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure logout function has proper error handling.
The logout function is called directly on click without any error handling. Consider adding error handling to provide feedback if logout fails.
🏁 Script executed:
#!/bin/bash
# Check if the logout action includes error handling
cat frontend/components/actions/logout-action.ts || cat frontend/components/actions/logout-action.tsx || cat frontend/components/actions/logout-action.jsLength of output: 1220
Add user-visible error handling to the logout flow
The logout action in frontend/components/actions/logout-action.ts does include a try/catch and logs errors to the console, but it always clears the cookie and redirects to /login even if the API call fails—meaning users won’t see any feedback on logout failures. Consider surfacing failures (e.g., via a toast or inline error) before redirecting.
• File: frontend/components/actions/logout-action.ts
– After await fetch(…), if !response.ok, throw or return an error instead of simply logging.
– In the catch block, propagate the error so the client can display it.
• File: frontend/components/layout/MainLayout.tsx
– Update the <button onClick={logout}> handler to catch errors from the server action and render a notification/toast if logout fails, delaying or preventing the redirect until the user has seen the message.
🤖 Prompt for AI Agents
In frontend/components/layout/MainLayout.tsx around lines 204 to 209, the logout
button calls the logout function without handling errors, so users get no
feedback if logout fails. Modify the logout action in
frontend/components/actions/logout-action.ts to throw or return an error when
the API response is not ok and propagate errors in the catch block. Then update
the button's onClick handler in MainLayout.tsx to catch these errors and display
a user-visible notification or toast, preventing or delaying the redirect until
the user sees the error message.
| test: backend-test frontend-test admin-test extension-test-unit #website-test | ||
| @echo "===========> All tests completed successfully" | ||
|
|
There was a problem hiding this comment.
test target may silently ship broken code – individual sub-targets ignore failures
frontend-test, website-test, and potentially others wrap their commands in || true, causing the overall make test (and therefore make all) to succeed even when unit / E2E tests fail.
This defeats the purpose of having make all gate a CI build.
- cd $(FRONTEND_DIR) && NODE_ENV=test $(PNPM) test -- --passWithNoTests || true;
+# Let the test command exit with its real status – Playwright/Jest already have
+# explicit flags when you really want to ignore “no test” situations.
+ cd $(FRONTEND_DIR) && NODE_ENV=test $(PNPM) test -- --passWithNoTestsConsider removing the || true guards (or hiding them behind an opt-in flag) so the build fails fast when any suite is red.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In the Makefile around lines 210 to 212, the test target runs multiple
sub-targets like frontend-test and website-test that use '|| true' to ignore
failures, causing 'make test' to succeed even if tests fail. To fix this, remove
the '|| true' from these sub-targets so that any test failure causes the overall
'make test' to fail, enabling fast failure in CI builds. Alternatively,
implement an opt-in flag to control this behavior if needed.
| const updateInputVar = ( | ||
| index: number, | ||
| field: keyof InputVariable, | ||
| value: string | boolean, | ||
| ) => { | ||
| const updatedVars = [...inputVars]; | ||
| updatedVars[index] = { ...updatedVars[index], [field]: value }; | ||
| setInputVars(updatedVars); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Input variable changes bypass Zod validation
updateInputVar mutates inputVars and then calls form.setValue("input_vars", inputVars) in a useEffect, but this does not trigger validation for the nested schema (inputVarSchema).
If a user leaves a variable name empty, the form will still submit successfully.
Trigger validation after every update:
updatedVars[index] = { ...updatedVars[index], [field]: value };
setInputVars(updatedVars);
+form.trigger("input_vars"); // ensure zod schema re-evaluatesAlternatively, manage the array directly with react-hook-form’s useFieldArray which will keep state and validation in sync.
📝 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 updateInputVar = ( | |
| index: number, | |
| field: keyof InputVariable, | |
| value: string | boolean, | |
| ) => { | |
| const updatedVars = [...inputVars]; | |
| updatedVars[index] = { ...updatedVars[index], [field]: value }; | |
| setInputVars(updatedVars); | |
| }; | |
| const updateInputVar = ( | |
| index: number, | |
| field: keyof InputVariable, | |
| value: string | boolean, | |
| ) => { | |
| const updatedVars = [...inputVars]; | |
| updatedVars[index] = { ...updatedVars[index], [field]: value }; | |
| setInputVars(updatedVars); | |
| form.trigger("input_vars"); // ensure zod schema re-evaluates | |
| }; |
🤖 Prompt for AI Agents
In frontend/app/prompts/_components/PromptForm.tsx around lines 166 to 174, the
updateInputVar function updates inputVars state directly but does not trigger
Zod validation for the nested inputVarSchema, allowing invalid input to pass. To
fix this, after updating inputVars, call form.setValue("input_vars",
updatedVars, { shouldValidate: true }) to trigger validation immediately.
Alternatively, refactor to use react-hook-form's useFieldArray to manage
inputVars state and validation together.
| prompts = db.exec(query).all() | ||
| # 手动加载标签 | ||
| for prompt in prompts: | ||
| db.refresh(prompt, ["tags"]) | ||
| # 在 Python 中进行分页 | ||
| return list(prompts[skip : skip + limit]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Performance issue: Inefficient pagination implemented in Python
Fetching all results and then slicing them in Python is inefficient for large datasets and will cause memory issues and slow responses.
Apply pagination directly in the SQL query:
- # 执行查询
- prompts = db.exec(query).all()
- # 手动加载标签
- for prompt in prompts:
- db.refresh(prompt, ["tags"])
- # 在 Python 中进行分页
- return list(prompts[skip : skip + limit])
+ # 在 SQL 中进行分页
+ query = query.offset(skip).limit(limit)
+ prompts = db.exec(query).all()
+ # 手动加载标签
+ for prompt in prompts:
+ db.refresh(prompt, ["tags"])
+ return prompts📝 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.
| prompts = db.exec(query).all() | |
| # 手动加载标签 | |
| for prompt in prompts: | |
| db.refresh(prompt, ["tags"]) | |
| # 在 Python 中进行分页 | |
| return list(prompts[skip : skip + limit]) | |
| # 在 SQL 中进行分页 | |
| query = query.offset(skip).limit(limit) | |
| prompts = db.exec(query).all() | |
| # 手动加载标签 | |
| for prompt in prompts: | |
| db.refresh(prompt, ["tags"]) | |
| return prompts |
🤖 Prompt for AI Agents
In backend/app/api/routes/prompts.py around lines 267 to 272, the current code
fetches all prompt records and then slices them in Python for pagination, which
is inefficient and can cause memory and performance issues. Modify the SQL query
to include pagination parameters using OFFSET and LIMIT clauses directly in the
query before execution. This will ensure only the required subset of records is
fetched from the database, improving performance and reducing memory usage.
| // Add mock for React act function | ||
| jest.mock("react", () => { | ||
| const originalReact = jest.requireActual("react"); | ||
| return { | ||
| ...originalReact, | ||
| act: (callback) => { | ||
| if (typeof callback === "function") { | ||
| return callback(); | ||
| } | ||
| return Promise.resolve(); | ||
| }, | ||
| cache: (fn) => fn, | ||
| }; | ||
| }); | ||
|
|
||
| // Add mock for react-dom act function | ||
| jest.mock("react-dom/test-utils", () => { | ||
| const originalTestUtils = jest.requireActual("react-dom/test-utils"); | ||
| return { | ||
| ...originalTestUtils, | ||
| act: (callback) => { | ||
| if (typeof callback === "function") { | ||
| return callback(); | ||
| } | ||
| return Promise.resolve(); | ||
| }, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate React act mock implementation
There are two separate implementations of the React act mock (one at lines 188-206 and another at lines 247-274). This duplication can lead to confusion and maintenance issues.
Consider removing one of the implementations or consolidating them into a single mock.
-// Add mock for React act function
-jest.mock("react", () => {
- const originalReact = jest.requireActual("react");
- return {
- ...originalReact,
- act: (callback) => {
- if (typeof callback === "function") {
- return callback();
- }
- return Promise.resolve();
- },
- cache: (fn) => fn,
- };
-});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Add mock for React act function | |
| jest.mock("react", () => { | |
| const originalReact = jest.requireActual("react"); | |
| return { | |
| ...originalReact, | |
| act: (callback) => { | |
| if (typeof callback === "function") { | |
| return callback(); | |
| } | |
| return Promise.resolve(); | |
| }, | |
| cache: (fn) => fn, | |
| }; | |
| }); | |
| // Add mock for react-dom act function | |
| jest.mock("react-dom/test-utils", () => { | |
| const originalTestUtils = jest.requireActual("react-dom/test-utils"); | |
| return { | |
| ...originalTestUtils, | |
| act: (callback) => { | |
| if (typeof callback === "function") { | |
| return callback(); | |
| } | |
| return Promise.resolve(); | |
| }, | |
| }; | |
| }); | |
| // Add mock for react-dom act function | |
| jest.mock("react-dom/test-utils", () => { | |
| const originalTestUtils = jest.requireActual("react-dom/test-utils"); | |
| return { | |
| ...originalTestUtils, | |
| act: (callback) => { | |
| if (typeof callback === "function") { | |
| return callback(); | |
| } | |
| return Promise.resolve(); | |
| }, | |
| }; | |
| }); |
🤖 Prompt for AI Agents
In frontend/jest.setup.js between lines 188-206 and 247-274, there are duplicate
mock implementations for the React act function, which can cause confusion and
maintenance problems. Identify both mocks and consolidate them into a single
implementation by removing one of the duplicates. Ensure the remaining mock
covers all necessary cases for both "react" and "react-dom/test-utils" if
needed, or keep only the relevant one based on usage.
- Added 'selectin' as a SQLAlchemy relationship loading strategy in _typos.toml for improved performance. - Updated Makefile to include 'test' in the 'all' target and adjusted frontend test commands for better environment setup. - Refactored frontend components to utilize MainLayout for consistent layout structure across pages. - Improved error handling and logging in prompt-related API routes, ensuring better debugging and user feedback. - Enhanced test coverage for prompts and related functionalities, including better handling of search parameters and form validation. - Cleaned up unused imports and optimized component structures for better maintainability.
- Refactored middleware to improve token validation handling. - Updated package.json to include type-checking flags during build. - Upgraded React and related type definitions to version 19.1.0 for better compatibility. - Enhanced test cases for password reset functionality, ensuring consistent state management. - Improved type safety in various components by using type assertions and defining state types. - Cleaned up unused imports and optimized component structures for maintainability.
- Updated frontend development guidelines to include new rules for UI library version control and component isolation. - Refactored UI color guidelines to establish a clearer design system with Tailwind CSS and shadcn/ui. - Improved type safety and consistency in various components, including layout and navigation. - Removed unused font files and optimized font imports for better performance. - Enhanced user experience in authentication flows with improved loading states and error handling. - Cleaned up code structure in several components for better maintainability and readability.
User description
…t files, and source maps…t files, and source maps
This pull request introduces several updates across the backend and frontend, focusing on improving logging, enhancing security, refining the onboarding process, and adding new testing configurations. Below is a summary of the most important changes grouped by theme:
Backend Improvements
get_current_userto improve debugging of JWT decoding, validation errors, and user lookup issues. [1] [2]backend/app/main.pyto allow all origins in the local environment and restrict origins based on configuration in production.Frontend Enhancements
manifest.jsonand added a new section in theREADME.mdto resolve WebSocket connection issues by updating the Content Security Policy. [1] [2]Extension Features
setupmessage handler to manage plugin-web communication, including saving tokens, verifying plugin IDs, and handling sidebar requests.Testing and Development Tools
.gitignoreUpdates: Added entries for Playwright reports, generated test files, and source maps to streamline development.Docker and Deployment
NODE_ENVindocker-compose.override.ymltoproductionand removed custom commands to align with the Dockerfile's default behavior.``Description
Changes walkthrough 📝
6 files
SetupContent.tsx
Multi-step Setup Wizard for User Onboardingfrontend/components/setup/SetupContent.tsx
integration.
setup.ts
Setup Message Handling Implementationextension/background/messages/setup.ts
dev-helper.ts
WebSocket Connection Helper for Developmentextension/utils/dev-helper.ts
api.ts
Improved Logout Functionalityextension/utils/api.ts
changes.
manifest.json
Content Security Policy Updateextension/manifest.json
page.tsx
User Setup Page Creationfrontend/app/setup/page.tsx
4 files
onboarding.test.ts
End-to-End Tests for Onboarding Processextension/tests/e2e/onboarding.test.ts
connections.
test-helpers.ts
Testing Utilities for Chrome Extensionsextension/tests/utils/test-helpers.ts
SetupContentTest.tsx
Tests for SetupContent Componentfrontend/tests/components/setup/SetupContentTest.tsx
rendering and navigation.
onboarding.test.ts
Onboarding Page E2E Testsextension/tests/e2e/onboarding.test.ts
connections.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Tests
Chores
Refactor
Revert