diff --git a/.cursor/rules/code-review.mdc b/.cursor/rules/code-review.mdc index a5a2516018..55bc1d4669 100644 --- a/.cursor/rules/code-review.mdc +++ b/.cursor/rules/code-review.mdc @@ -1,13 +1,14 @@ --- description: How to code review -globs: +globs: alwaysApply: false --- + # Role Description -- You are a senior full-stack engineer skilled in performance optimization, security, and design systems. -- You excel at reviewing code and providing constructive feedback. -- Your task is to review submitted Git diffs **in Chinese** and return a structured review report. +- You are a senior full-stack engineer skilled in performance optimization, security, and design systems. +- You excel at reviewing code and providing constructive feedback. +- Your task is to review submitted Git diffs **in Chinese** and return a structured review report. - Review style: concise, direct, focused on what matters most, with actionable suggestions. ## Before the Review @@ -16,54 +17,57 @@ Gather the modified code and context. Please strictly follow the process below: 1. Use `read_file` to read [package.json](mdc:package.json) 2. Use terminal to run command `git diff HEAD | cat` to obtain the diff and list the changed files. If you recieived empty result, run the same command once more. -3. Use `read_file` to open each changed file. -4. Use `read_file` to read [rules-attach.mdc](mdc:.cursor/rules/rules-attach.mdc). Even if you think it's unnecessary, you must read it. -5. combine changed files, step3 and `agent_requestable_workspace_rules`, list the rules which need to read +3. Use `read_file` to open each changed file. +4. Use `read_file` to read [rules-attach.mdc](mdc:.cursor/rules/rules-attach.mdc). Even if you think it's unnecessary, you must read it. +5. combine changed files, step3 and `agent_requestable_workspace_rules`, list the rules which need to read 6. Use `read_file` to read the rules list in step 5 ## Review ### Code Style -- Ensure JSDoc comments accurately reflect the implementation; update them when needed. -- Look for opportunities to simplify or modernize code with the latest JavaScript/TypeScript features. -- Prefer `async`/`await` over callbacks or chained `.then` promises. -- Use consistent, descriptive naming—avoid obscure abbreviations. -- Replace magic numbers or strings with well-named constants. +- Ensure JSDoc comments accurately reflect the implementation; update them when needed. +- Look for opportunities to simplify or modernize code with the latest JavaScript/TypeScript features. +- Prefer `async`/`await` over callbacks or chained `.then` promises. +- Use consistent, descriptive naming—avoid obscure abbreviations. +- Replace magic numbers or strings with well-named constants. - Use semantically meaningful variable, function, and class names. - Ignore purely formatting issues and other autofixable lint problems. ### Code Optimization -- Prefer `for…of` loops to index-based `for` loops when feasible. -- Decide whether callbacks should be **debounced** or **throttled**. -- Use components from `@lobehub/ui`, Ant Design, or the existing design system instead of raw HTML tags (e.g., `Button` vs. `button`). -- reuse npm packages already installed (e.g., `lodash/omit`) rather than reinventing the wheel. -- Design for dark mode and mobile responsiveness: - - Use the `antd-style` token system instead of hard-coded colors. - - Select the proper component variants. -- Performance considerations: - - Where safe, convert sequential async flows to concurrent ones with `Promise.all`, `Promise.race`, etc. +- Prefer `for…of` loops to index-based `for` loops when feasible. +- Decide whether callbacks should be **debounced** or **throttled**. +- Use components from `@lobehub/ui`, Ant Design, or the existing design system instead of raw HTML tags (e.g., `Button` vs. `button`). +- reuse npm packages already installed (e.g., `lodash/omit`) rather than reinventing the wheel. +- Design for dark mode and mobile responsiveness: + - Use the `antd-style` token system instead of hard-coded colors. + - Select the proper component variants. +- Performance considerations: + - Where safe, convert sequential async flows to concurrent ones with `Promise.all`, `Promise.race`, etc. - Query only the required columns from a database rather than selecting entire rows. ### Obvious Bugs -- Do not silently swallow errors in `catch` blocks; at minimum, log them. -- Revert temporary code used only for testing (e.g., debug logs, temporary configs). -- Remove empty handlers (e.g., an empty `onClick`). +- Do not silently swallow errors in `catch` blocks; at minimum, log them. +- Revert temporary code used only for testing (e.g., debug logs, temporary configs). +- Remove empty handlers (e.g., an empty `onClick`). - Confirm the UI degrades gracefully for unauthenticated users. +- Don't leave any debug logs in the code (except when using the `debug` module properly). + - When using the `debug` module, avoid `import { log } from 'debug'` as it logs directly to console. Use proper debug namespaces instead. +- Check logs for sensitive information like api key, etc ## After the Review: output 1. Summary - - Start with a brief explanation of what the change set does. - - Summarize the changes for each modified file (or logical group). + - Start with a brief explanation of what the change set does. + - Summarize the changes for each modified file (or logical group). 2. Comments Issues - - List the most critical issues first. - - Use an ordered list, which will be convenient for me to reference later. - - For each issue: - - Mark severity tag (`❌ Must fix`, `⚠️ Should fix`, `💅 Nitpick`) - - Provode file path to the relevant file. - - Provide recommended fix - - End with a **git commit** command, instruct the author to run it. - - We use gitmoji to label commit messages, format: [emoji] (): \ No newline at end of file + - List the most critical issues first. + - Use an ordered list, which will be convenient for me to reference later. + - For each issue: + - Mark severity tag (`❌ Must fix`, `⚠️ Should fix`, `💅 Nitpick`) + - Provode file path to the relevant file. + - Provide recommended fix + - End with a **git commit** command, instruct the author to run it. + - We use gitmoji to label commit messages, format: [emoji] (): diff --git a/.cursor/rules/system-role.mdc b/.cursor/rules/system-role.mdc index 61b270a637..90c46c6323 100644 --- a/.cursor/rules/system-role.mdc +++ b/.cursor/rules/system-role.mdc @@ -1,8 +1,9 @@ --- -description: -globs: +description: +globs: alwaysApply: true --- + ## System Role You are an expert in full-stack Web development, proficient in JavaScript, TypeScript, CSS, React, Node.js, Next.js, Postgresql, all kinds of network protocols. @@ -11,7 +12,6 @@ You are an expert in LLM and Ai art. In Ai image generation, you are proficient You are an expert in UI/UX design, proficient in web interaction patterns, responsive design, accessibility, and user behavior optimization. You excel at improving user retention and paid conversion rates through various interaction details. - ## Problem Solving - Before formulating any response, you must first gather context by using tools like codebase_search, grep_search, file_search, web_search, fetch_rules, context7, and read_file to avoid making assumptions. @@ -36,3 +36,8 @@ You are an expert in UI/UX design, proficient in web interaction patterns, respo - If you're unable to access or retrieve content from websites, please inform me immediately and request the specific information needed rather than making assumptions - You can use emojis, npm packages like `chalk`/`chalk-animation`/`terminal-link`/`gradient-string`/`log-symbols`/`boxen`/`consola`/`@clack/prompts` to create beautiful terminal output - Don't run `tsc --noEmit` to check ts syntax error, because our project is very large and the validate very slow + +## Some logging rules + +- Never log user private information like api key, etc +- Don't use `import { log } from 'debug'` to log messages, because it will directly log the message to the console. diff --git a/.cursor/rules/testing-guide/testing-guide.mdc b/.cursor/rules/testing-guide/testing-guide.mdc index 7140ffa9e3..bd63bef840 100644 --- a/.cursor/rules/testing-guide/testing-guide.mdc +++ b/.cursor/rules/testing-guide/testing-guide.mdc @@ -28,85 +28,39 @@ LobeChat 项目使用 Vitest 测试库,配置了两种不同的测试环境: ## 🚀 测试运行命令 -### package.json 脚本说明 +**🚨 性能警告**: 项目包含 3000+ 测试用例,完整运行需要约 10 分钟。务必使用文件过滤或测试名称过滤。 -查看 [package.json](mdc:package.json) 中的测试相关脚本: - -```json -{ - "test": "npm run test-app && npm run test-server", - "test-app": "vitest run --config vitest.config.ts", - "test-app:coverage": "vitest run --config vitest.config.ts --coverage", - "test-server": "vitest run --config vitest.config.server.ts", - "test-server:coverage": "vitest run --config vitest.config.server.ts --coverage" -} -``` - -### 推荐的测试运行方式 - -#### ⚠️ 重要提醒 - -**🚨 性能警告**: - -- **永远不要直接运行整个项目的所有测试用例** - 项目包含 3000+ 测试用例,完整运行需要约 10 分钟 -- **务必使用文件过滤或测试名称过滤** - 始终指定具体的测试文件或测试名称模式 -- **避免无意中触发全量测试** - 某些看似针对单个文件的命令实际上会运行所有测试 - -#### ✅ 正确的命令格式 +### ✅ 正确的命令格式 ```bash -# 运行所有客户端测试 -npx vitest run --config vitest.config.ts - -# 运行所有服务端测试 -npx vitest run --config vitest.config.server.ts +# 运行所有客户端/服务端测试 +npx vitest run --config vitest.config.ts # 客户端测试 +npx vitest run --config vitest.config.server.ts # 服务端测试 # 运行特定测试文件 (支持模糊匹配) -npx vitest run --config vitest.config.ts basic npx vitest run --config vitest.config.ts user.test.ts -# 运行特定文件的特定行号 -npx vitest run --config vitest.config.ts src/utils/helper.test.ts:25 -npx vitest run --config vitest.config.ts basic/foo.test.ts:10,basic/foo.test.ts:25 - -# 过滤特定测试用例名称 -npx vitest -t "test case name" --config vitest.config.ts +# 运行特定测试用例名称 (使用 -t 参数) +npx vitest run --config vitest.config.ts -t "test case name" # 组合使用文件和测试名称过滤 npx vitest run --config vitest.config.ts filename.test.ts -t "specific test" + +# 生成覆盖率报告 (使用 --coverage 参数) +npx vitest run --config vitest.config.ts --coverage ``` -#### ❌ 避免的命令格式 +### ❌ 避免的命令格式 ```bash # ❌ 这些命令会运行所有 3000+ 测试用例,耗时约 10 分钟! npm test -npm run test -pnpm test -pnpm run test - -# ❌ 这些命令看似针对单个文件,但实际会运行所有测试用例!, 需要直接运行 vitest 命令不要使用 test npm script -npm test src/libs/model-runtime/utils/openaiCompatibleFactory/index.test.ts -pnpm test src/components/Button/index.test.tsx - -# ❌ 不要使用 pnpm test xxx (这不是有效的 vitest 命令) -pnpm test some-file +npm test some-file.test.ts # ❌ 不要使用裸 vitest (会进入 watch 模式) vitest test-file.test.ts - -# ❌ 不要混淆测试环境 -npx vitest run --config vitest.config.server.ts client-component.test.ts ``` -### 关键运行参数说明 - -- **`vitest run`**: 运行一次测试然后退出 (避免 watch 模式) -- **`vitest`**: 默认进入 watch 模式,持续监听文件变化 -- **`--config`**: 指定配置文件,选择正确的测试环境 -- **`-t`**: 过滤测试用例名称,支持正则表达式 -- **`--coverage`**: 生成测试覆盖率报告 - ## 🔧 测试修复原则 ### 核心原则 ⚠️ @@ -116,45 +70,124 @@ npx vitest run --config vitest.config.server.ts client-component.test.ts 3. **专注单一问题**: 只修复指定的测试,不要添加额外测试或功能 4. **不自作主张**: 不要因为发现其他问题就直接修改,先提出再讨论 +### 测试协作最佳实践 🤝 + +基于实际开发经验总结的重要协作原则: + +#### 1. 失败处理策略 + +**核心原则**: 避免盲目重试,快速识别问题并寻求帮助。 + +- **失败阈值**: 当连续尝试修复测试 1-2 次都失败后,应立即停止继续尝试 +- **问题总结**: 分析失败原因,整理已尝试的解决方案及其失败原因 +- **寻求帮助**: 带着清晰的问题摘要和尝试记录向团队寻求帮助 +- **避免陷阱**: 不要陷入"不断尝试相同或类似方法"的循环 + +```typescript +// ❌ 错误做法:连续失败后继续盲目尝试 +// 第3次、第4次仍在用相似的方法修复同一个问题 + +// ✅ 正确做法:失败1-2次后总结问题 +/* +问题总结: +1. 尝试过的方法:修改 mock 数据结构 +2. 失败原因:仍然提示类型不匹配 +3. 具体错误:Expected 'UserData' but received 'UserProfile' +4. 需要帮助:不确定最新的 UserData 接口定义 +*/ +``` + +#### 2. 测试用例命名规范 + +**核心原则**: 测试应该关注"行为",而不是"实现细节"。 + +- **描述业务场景**: `describe` 和 `it` 的标题应该描述具体的业务场景和预期行为 +- **避免实现绑定**: 不要在测试名称中提及具体的代码行号、覆盖率目标或实现细节 +- **保持稳定性**: 测试名称应该在代码重构后仍然有意义 + +```typescript +// ❌ 错误的测试命名 +describe('User component coverage', () => { + it('covers line 45-50 in getUserData', () => { + // 为了覆盖第45-50行而写的测试 + }); + + it('tests the else branch', () => { + // 仅为了测试某个分支而存在 + }); +}); + +// ✅ 正确的测试命名 +describe('', () => { + it('should render fallback icon when image url is not provided', () => { + // 测试具体的业务场景,自然会覆盖相关代码分支 + }); + + it('should display user initials when avatar image fails to load', () => { + // 描述用户行为和预期结果 + }); +}); +``` + +**覆盖率提升的正确思路**: + +- ✅ 通过设计各种业务场景(正常流程、边缘情况、错误处理)来自然提升覆盖率 +- ❌ 不要为了达到覆盖率数字而写测试,更不要在测试中注释"为了覆盖 xxx 行" + +#### 3. 测试组织结构 + +**核心原则**: 维护清晰的测试层次结构,避免冗余的顶级测试块。 + +- **复用现有结构**: 添加新测试时,优先在现有的 `describe` 块中寻找合适的位置 +- **逻辑分组**: 相关的测试用例应该组织在同一个 `describe` 块内 +- **避免碎片化**: 不要为了单个测试用例就创建新的顶级 `describe` 块 + +```typescript +// ❌ 错误的组织方式:创建过多顶级块 +describe('', () => { + it('should render user name', () => {}); +}); + +describe('UserProfile new prop test', () => { + // 不必要的新块 + it('should handle email display', () => {}); +}); + +describe('UserProfile edge cases', () => { + // 不必要的新块 + it('should handle missing avatar', () => {}); +}); + +// ✅ 正确的组织方式:合并相关测试 +describe('', () => { + it('should render user name', () => {}); + + it('should handle email display', () => {}); + + it('should handle missing avatar', () => {}); + + describe('when user data is incomplete', () => { + // 只有在有多个相关子场景时才创建子组 + it('should show placeholder for missing name', () => {}); + it('should hide email section when email is undefined', () => {}); + }); +}); +``` + +**组织决策流程**: + +1. 是否存在逻辑相关的现有 `describe` 块? → 如果有,添加到其中 +2. 是否有多个(3个以上)相关的测试用例? → 如果有,可以考虑创建新的子 `describe` +3. 是否是独立的、无关联的功能模块? → 如果是,才考虑创建新的顶级 `describe` + ### 测试修复流程 -```mermaid -flowchart TD - subgraph "阶段一:分析与复现" - A[开始:收到测试失败报告] --> B[定位并运行失败的测试]; - B --> C{是否能在本地复现?}; - C -->|否| D[检查测试环境/配置/依赖]; - C -->|是| E[分析:阅读测试代码、错误日志、Git 历史]; - end - - subgraph "阶段二:诊断与调试" - E --> F[建立假设:问题出在测试、代码还是环境?]; - F --> G["调试:使用 console.log 或 debugger 深入检查"]; - G --> H{假设是否被证实?}; - H -->|否, 重新假设| F; - end - - subgraph "阶段三:修复与验证" - H -->|是| I{确定根本原因}; - I -->|测试逻辑错误| J[修复测试代码]; - I -->|实现代码 Bug| K[修复实现代码]; - I -->|环境/配置问题| L[修复配置或依赖]; - J --> M[验证修复:重新运行失败的测试]; - K --> M; - L --> M; - M --> N{测试是否通过?}; - N -->|否, 修复无效| F; - N -->|是| O[扩大验证:运行当前文件内所有测试]; - O --> P{是否全部通过?}; - P -->|否, 引入新问题| F; - end - - subgraph "阶段四:总结" - P -->|是| Q[完成:撰写修复总结]; - end - - D --> F; -``` +1. **复现问题**: 定位并运行失败的测试,确认能在本地复现 +2. **分析原因**: 阅读测试代码、错误日志和相关文件的 Git 修改历史 +3. **建立假设**: 判断问题出在测试逻辑、实现代码还是环境配置 +4. **修复验证**: 根据假设进行修复,重新运行测试确认通过 +5. **扩大验证**: 运行当前文件内所有测试,确保没有引入新问题 +6. **撰写总结**: 说明错误原因和修复方法 ### 修复完成后的总结 @@ -197,7 +230,7 @@ flowchart TD 例如: -``` +```plaintext src/components/Button/ ├── index.tsx # 源文件 └── index.test.tsx # 测试文件 @@ -205,13 +238,12 @@ src/components/Button/ ## 🛠️ 测试调试技巧 -### 运行失败测试的步骤 +### 测试调试步骤 -1. **确定测试类型**: 查看文件路径确定使用哪个配置 -2. **运行单个测试**: 使用 `-t` 参数隔离问题 -3. **检查错误日志**: 仔细阅读错误信息和堆栈跟踪 -4. **查看最近修改记录**: 检查相关文件的最近变更情况 -5. **添加调试日志**: 在测试中添加 `console.log` 了解执行流程 +1. **确定测试环境**: 根据文件路径选择正确的配置文件 +2. **隔离问题**: 使用 `-t` 参数只运行失败的测试用例 +3. **分析错误**: 仔细阅读错误信息、堆栈跟踪和最近的文件修改记录 +4. **添加调试**: 在测试中添加 `console.log` 了解执行流程 ### TypeScript 类型处理 📝 @@ -245,157 +277,47 @@ mockStream.toReadableStream = () => mockStream; ### 检查最近修改记录 🔍 -为了更好地判断测试失败的根本原因,需要**系统性地检查相关文件的修改历史**。这是问题定位的关键步骤。 +系统性地检查相关文件的修改历史是问题定位的关键步骤。 -#### 第一步:确定需要检查的文件范围 +#### 三步检查法 -1. **测试文件本身**: `path/to/component.test.ts` -2. **对应的实现文件**: `path/to/component.ts` 或 `path/to/component/index.ts` -3. **相关依赖文件**: 测试或实现中导入的其他模块 - -#### 第二步:检查当前工作目录状态 +**Step 1: 查看当前状态** ```bash -# 查看所有未提交的修改状态 -git status - -# 重点关注测试文件和实现文件是否有未提交的修改 -git status | grep -E "(test|spec)" +git status # 查看未提交的修改 +git diff path/to/component.test.ts | cat # 查看测试文件修改 +git diff path/to/component.ts | cat # 查看实现文件修改 ``` -#### 第三步:检查未提交的修改内容 +**Step 2: 查看提交历史** ```bash -# 查看测试文件的未提交修改 (工作区 vs 暂存区) -git diff path/to/component.test.ts | cat - -# 查看对应实现文件的未提交修改 -git diff path/to/component.ts | cat - -# 查看已暂存但未提交的修改 -git diff --cached path/to/component.test.ts | cat -git diff --cached path/to/component.ts | cat +git log --pretty=format:"%h %ad %s" --date=relative -3 path/to/component.ts | cat ``` -#### 第四步:检查提交历史和时间相关性 - -**首先查看提交时间,判断修改的时效性**: +**Step 3: 查看具体修改内容** ```bash -# 查看测试文件的最近提交历史,包含提交时间 -git log --pretty=format:"%h %ad %s" --date=relative -5 path/to/component.test.ts | cat - -# 查看实现文件的最近提交历史,包含提交时间 -git log --pretty=format:"%h %ad %s" --date=relative -5 path/to/component.ts | cat - -# 查看详细的提交时间(ISO格式,便于精确判断) -git log --pretty=format:"%h %ad %an %s" --date=iso -3 path/to/component.ts | cat -git log --pretty=format:"%h %ad %an %s" --date=iso -3 path/to/component.test.ts | cat +git show HEAD -- path/to/component.ts | cat # 查看最新提交的修改 ``` -**判断提交的参考价值**: +#### 时间相关性判断 -1. **最近提交(24小时内)**: 🔴 **高度相关** - 很可能是导致测试失败的直接原因 -2. **近期提交(1-7天内)**: 🟡 **中等相关** - 可能相关,需要仔细分析修改内容 -3. **较早提交(超过1周)**: ⚪ **低相关性** - 除非是重大重构,否则不太可能是直接原因 - -#### 第五步:基于时间相关性查看具体修改内容 - -**根据提交时间的远近,优先查看最近的修改**: - -```bash -# 如果有24小时内的提交,重点查看这些修改 -git show HEAD -- path/to/component.test.ts | cat -git show HEAD -- path/to/component.ts | cat - -# 查看次新的提交(如果最新提交时间较远) -git show HEAD~1 -- path/to/component.ts | cat -git show path/to/component.ts < recent-commit-hash > -- | cat - -# 对比最近两次提交的差异 -git diff HEAD~1 HEAD -- path/to/component.ts | cat -``` - -#### 第六步:分析修改与测试失败的关系 - -基于修改记录和时间相关性判断: - -1. **最近修改了实现代码**: - - ```bash - # 重点检查实现逻辑的变化 - git diff HEAD~1 path/to/component.ts | cat - ``` - - - 很可能是实现代码的变更导致测试失败 - - 检查实现逻辑是否正确 - - 确认测试是否需要相应更新 - -2. **最近修改了测试代码**: - - ```bash - # 重点检查测试逻辑的变化 - git diff HEAD~1 path/to/component.test.ts | cat - ``` - - - 可能是测试本身写错了 - - 检查测试逻辑和断言是否正确 - - 确认测试是否符合实现的预期行为 - -3. **两者都有最近修改**: - - ```bash - # 对比两个文件的修改时间 - git log --pretty=format:"%ad %f" --date=iso -1 path/to/component.ts | cat - git log --pretty=format:"%ad %f" --date=iso -1 path/to/component.test.ts | cat - ``` - - - 需要综合分析两者的修改 - - 确定哪个修改更可能导致问题 - - 优先检查时间更近的修改 - -4. **都没有最近修改**: - - 可能是依赖变更或环境问题 - - 检查 `package.json`、配置文件等的修改 - - 查看是否有全局性的代码重构 - -#### 修改记录检查示例 - -```bash -# 完整的检查流程示例 -echo "=== 检查文件修改状态 ===" -git status | grep component - -echo "=== 检查未提交修改 ===" -git diff src/components/Button/index.test.tsx | cat -git diff src/components/Button/index.tsx | cat - -echo "=== 检查提交历史和时间 ===" -git log --pretty=format:"%h %ad %s" --date=relative -3 src/components/Button/index.test.tsx | cat -git log --pretty=format:"%h %ad %s" --date=relative -3 src/components/Button/index.tsx | cat - -echo "=== 根据时间优先级查看修改内容 ===" -# 如果有24小时内的提交,重点查看 -git show HEAD -- src/components/Button/index.tsx | cat -``` +- **24小时内的提交**: 🔴 **高度相关** - 很可能是直接原因 +- **1-7天内的提交**: 🟡 **中等相关** - 需要仔细分析 +- **超过1周的提交**: ⚪ **低相关性** - 除非重大重构 ## 特殊场景的测试 -针对一些特殊场景的测试,需要阅读相关文件: +针对一些特殊场景的测试,需要阅读相关 rules: - [Electron IPC 接口测试策略](mdc:./electron-ipc-test.mdc) - [数据库 Model 测试指南](mdc:./db-model-test.mdc) -## 🎯 总结 +## 🎯 核心要点 -修复测试时,记住以下关键点: - -- **使用正确的命令**: `npx vitest run --config [config-file]` -- **理解测试意图**: 先读懂测试再修复 -- **查看最近修改**: 检查相关文件的 git 修改记录,判断问题根源 -- **选择正确环境**: 客户端测试用 `vitest.config.ts`,服务端用 `vitest.config.server.ts` -- **专注单一问题**: 只修复当前的测试失败 -- **验证修复结果**: 确保修复后测试通过且无副作用 -- **提供修复总结**: 说明错误原因和修复方法 -- **Model 测试安全第一**: 必须包含用户权限检查和对应的安全测试 -- **Model 双环境验证**: 必须在 PGLite 和 PostgreSQL 两个环境下都验证通过 +- **命令格式**: 使用 `npx vitest run --config [config-file]` 并指定文件过滤 +- **修复原则**: 失败1-2次后寻求帮助,测试命名关注行为而非实现细节 +- **调试流程**: 复现 → 分析 → 假设 → 修复 → 验证 → 总结 +- **文件组织**: 优先在现有 `describe` 块中添加测试,避免创建冗余顶级块 +- **安全要求**: Model 测试必须包含权限检查,并在双环境下验证通过 diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 2e95d1f01e..b154e80f11 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -20,7 +20,7 @@ "electron:run-unpack": "electron .", "format": "prettier --write ", "i18n": "bun run scripts/i18nWorkflow/index.ts && lobe-i18n", - "postinstall": "electron-builder install-app-deps && pnpm rebuild sharp", + "postinstall": "electron-builder install-app-deps", "install-isolated": "pnpm install", "lint": "eslint --cache ", "pg-server": "bun run scripts/pglite-server.ts", diff --git a/src/services/__tests__/chat.test.ts b/src/services/__tests__/chat.test.ts index 7bec8efe45..c17b1d3ab0 100644 --- a/src/services/__tests__/chat.test.ts +++ b/src/services/__tests__/chat.test.ts @@ -2,7 +2,7 @@ import { LobeChatPluginManifest } from '@lobehub/chat-plugin-sdk'; import { act } from '@testing-library/react'; import { merge } from 'lodash-es'; import OpenAI from 'openai'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { DEFAULT_AGENT_CONFIG } from '@/const/settings'; import { @@ -36,7 +36,8 @@ import { modelConfigSelectors } from '@/store/user/selectors'; import { UserSettingsState, initialSettingsState } from '@/store/user/slices/settings/initialState'; import { DalleManifest } from '@/tools/dalle'; import { WebBrowsingManifest } from '@/tools/web-browsing'; -import { ChatMessage } from '@/types/message'; +import { ChatErrorType } from '@/types/fetch'; +import { ChatImageItem, ChatMessage } from '@/types/message'; import { ChatStreamPayload, type OpenAIChatMessage } from '@/types/openai/chat'; import { LobeTool } from '@/types/tool'; @@ -58,15 +59,48 @@ vi.mock('@/utils/fetch', async (importOriginal) => { return { ...(module as any), getMessageError: vi.fn() }; }); -beforeEach(() => { +// Mock image processing utilities +vi.mock('@/utils/url', () => ({ + isLocalUrl: vi.fn(), +})); + +vi.mock('@/utils/imageToBase64', () => ({ + imageUrlToBase64: vi.fn(), +})); + +vi.mock('@/libs/model-runtime/utils/uriParser', () => ({ + parseDataUri: vi.fn(), +})); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +beforeEach(async () => { // 清除所有模块的缓存 vi.resetModules(); + // 默认设置 isServerMode 为 false vi.mock('@/const/version', () => ({ isServerMode: false, isDeprecatedEdition: true, isDesktop: false, })); + + // Reset all mocks + vi.clearAllMocks(); + + // Set default mock return values for image processing utilities + const { isLocalUrl } = await import('@/utils/url'); + const { imageUrlToBase64 } = await import('@/utils/imageToBase64'); + const { parseDataUri } = await import('@/libs/model-runtime/utils/uriParser'); + + vi.mocked(parseDataUri).mockReturnValue({ type: 'url', base64: null, mimeType: null }); + vi.mocked(isLocalUrl).mockReturnValue(false); + vi.mocked(imageUrlToBase64).mockResolvedValue({ + base64: 'mock-base64', + mimeType: 'image/jpeg', + }); }); // mock auth @@ -142,6 +176,164 @@ describe('ChatService', () => { ); }); + describe('extendParams functionality', () => { + it('should add reasoning parameters when model supports enableReasoning and user enables it', async () => { + const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); + const messages = [{ content: 'Test reasoning', role: 'user' }] as ChatMessage[]; + + // Mock aiModelSelectors for extend params support + vi.spyOn(aiModelSelectors, 'isModelHasExtendParams').mockReturnValue(() => true); + vi.spyOn(aiModelSelectors, 'modelExtendParams').mockReturnValue(() => ['enableReasoning']); + + // Mock agent chat config with reasoning enabled + vi.spyOn(agentChatConfigSelectors, 'currentChatConfig').mockReturnValue({ + enableReasoning: true, + reasoningBudgetToken: 2048, + searchMode: 'off', + } as any); + + await chatService.createAssistantMessage({ + messages, + model: 'deepseek-reasoner', + provider: 'deepseek', + plugins: [], + }); + + expect(getChatCompletionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + thinking: { + budget_tokens: 2048, + type: 'enabled', + }, + }), + undefined, + ); + }); + + it('should disable reasoning when model supports enableReasoning but user disables it', async () => { + const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); + const messages = [{ content: 'Test no reasoning', role: 'user' }] as ChatMessage[]; + + // Mock aiModelSelectors for extend params support + vi.spyOn(aiModelSelectors, 'isModelHasExtendParams').mockReturnValue(() => true); + vi.spyOn(aiModelSelectors, 'modelExtendParams').mockReturnValue(() => ['enableReasoning']); + + // Mock agent chat config with reasoning disabled + vi.spyOn(agentChatConfigSelectors, 'currentChatConfig').mockReturnValue({ + enableReasoning: false, + searchMode: 'off', + } as any); + + await chatService.createAssistantMessage({ + messages, + model: 'deepseek-reasoner', + provider: 'deepseek', + plugins: [], + }); + + expect(getChatCompletionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + thinking: { + budget_tokens: 0, + type: 'disabled', + }, + }), + undefined, + ); + }); + + it('should use default budget when reasoningBudgetToken is not set', async () => { + const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); + const messages = [{ content: 'Test default budget', role: 'user' }] as ChatMessage[]; + + // Mock aiModelSelectors for extend params support + vi.spyOn(aiModelSelectors, 'isModelHasExtendParams').mockReturnValue(() => true); + vi.spyOn(aiModelSelectors, 'modelExtendParams').mockReturnValue(() => ['enableReasoning']); + + // Mock agent chat config with reasoning enabled but no custom budget + vi.spyOn(agentChatConfigSelectors, 'currentChatConfig').mockReturnValue({ + enableReasoning: true, + // reasoningBudgetToken is undefined + searchMode: 'off', + } as any); + + await chatService.createAssistantMessage({ + messages, + model: 'deepseek-reasoner', + provider: 'deepseek', + plugins: [], + }); + + expect(getChatCompletionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + thinking: { + budget_tokens: 1024, // default value + type: 'enabled', + }, + }), + undefined, + ); + }); + + it('should set reasoning_effort when model supports reasoningEffort and user configures it', async () => { + const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); + const messages = [{ content: 'Test reasoning effort', role: 'user' }] as ChatMessage[]; + + // Mock aiModelSelectors for extend params support + vi.spyOn(aiModelSelectors, 'isModelHasExtendParams').mockReturnValue(() => true); + vi.spyOn(aiModelSelectors, 'modelExtendParams').mockReturnValue(() => ['reasoningEffort']); + + // Mock agent chat config with reasoning effort set + vi.spyOn(agentChatConfigSelectors, 'currentChatConfig').mockReturnValue({ + reasoningEffort: 'high', + searchMode: 'off', + } as any); + + await chatService.createAssistantMessage({ + messages, + model: 'test-model', + provider: 'test-provider', + plugins: [], + }); + + expect(getChatCompletionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + reasoning_effort: 'high', + }), + undefined, + ); + }); + + it('should set thinkingBudget when model supports thinkingBudget and user configures it', async () => { + const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); + const messages = [{ content: 'Test thinking budget', role: 'user' }] as ChatMessage[]; + + // Mock aiModelSelectors for extend params support + vi.spyOn(aiModelSelectors, 'isModelHasExtendParams').mockReturnValue(() => true); + vi.spyOn(aiModelSelectors, 'modelExtendParams').mockReturnValue(() => ['thinkingBudget']); + + // Mock agent chat config with thinking budget set + vi.spyOn(agentChatConfigSelectors, 'currentChatConfig').mockReturnValue({ + thinkingBudget: 5000, + searchMode: 'off', + } as any); + + await chatService.createAssistantMessage({ + messages, + model: 'test-model', + provider: 'test-provider', + plugins: [], + }); + + expect(getChatCompletionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + thinkingBudget: 5000, + }), + undefined, + ); + }); + }); + describe('should handle content correctly for vision models', () => { it('should include image content when with vision model', async () => { const messages = [ @@ -209,6 +401,263 @@ describe('ChatService', () => { }); }); + describe('local image URL conversion', () => { + it('should convert local image URLs to base64 and call processImageList', async () => { + const { isLocalUrl } = await import('@/utils/url'); + const { imageUrlToBase64 } = await import('@/utils/imageToBase64'); + const { parseDataUri } = await import('@/libs/model-runtime/utils/uriParser'); + + // Mock for local URL + vi.mocked(parseDataUri).mockReturnValue({ type: 'url', base64: null, mimeType: null }); + vi.mocked(isLocalUrl).mockReturnValue(true); // This is a local URL + vi.mocked(imageUrlToBase64).mockResolvedValue({ + base64: 'converted-base64-content', + mimeType: 'image/png', + }); + + const messages = [ + { + content: 'Hello', + role: 'user', + imageList: [ + { + id: 'file1', + url: 'http://127.0.0.1:3000/uploads/image.png', // Real local URL + alt: 'local-image.png', + }, + ], + createdAt: Date.now(), + id: 'test-id', + meta: {}, + updatedAt: Date.now(), + }, + ] as ChatMessage[]; + + // Spy on processImageList method + const processImageListSpy = vi.spyOn(chatService as any, 'processImageList'); + const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); + + await chatService.createAssistantMessage({ + messages, + plugins: [], + model: 'gpt-4-vision-preview', + }); + + // Verify processImageList was called with correct arguments + expect(processImageListSpy).toHaveBeenCalledWith({ + imageList: [ + { + id: 'file1', + url: 'http://127.0.0.1:3000/uploads/image.png', + alt: 'local-image.png', + }, + ], + model: 'gpt-4-vision-preview', + provider: undefined, + }); + + // Verify the utility functions were called + expect(parseDataUri).toHaveBeenCalledWith('http://127.0.0.1:3000/uploads/image.png'); + expect(isLocalUrl).toHaveBeenCalledWith('http://127.0.0.1:3000/uploads/image.png'); + expect(imageUrlToBase64).toHaveBeenCalledWith('http://127.0.0.1:3000/uploads/image.png'); + + // Verify the final result contains base64 converted URL + expect(getChatCompletionSpy).toHaveBeenCalledWith( + { + messages: [ + { + content: [ + { + text: 'Hello', + type: 'text', + }, + { + image_url: { + detail: 'auto', + url: 'data:image/png;base64,converted-base64-content', + }, + type: 'image_url', + }, + ], + role: 'user', + }, + ], + model: 'gpt-4-vision-preview', + }, + undefined, + ); + }); + + it('should not convert remote URLs to base64 and call processImageList', async () => { + const { isLocalUrl } = await import('@/utils/url'); + const { imageUrlToBase64 } = await import('@/utils/imageToBase64'); + const { parseDataUri } = await import('@/libs/model-runtime/utils/uriParser'); + + // Mock for remote URL + vi.mocked(parseDataUri).mockReturnValue({ type: 'url', base64: null, mimeType: null }); + vi.mocked(isLocalUrl).mockReturnValue(false); // This is NOT a local URL + vi.mocked(imageUrlToBase64).mockClear(); // Clear to ensure it's not called + + const messages = [ + { + content: 'Hello', + role: 'user', + imageList: [ + { + id: 'file1', + url: 'https://example.com/remote-image.jpg', // Remote URL + alt: 'remote-image.jpg', + }, + ], + createdAt: Date.now(), + id: 'test-id-2', + meta: {}, + updatedAt: Date.now(), + }, + ] as ChatMessage[]; + + // Spy on processImageList method + const processImageListSpy = vi.spyOn(chatService as any, 'processImageList'); + const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); + + await chatService.createAssistantMessage({ + messages, + plugins: [], + model: 'gpt-4-vision-preview', + }); + + // Verify processImageList was called + expect(processImageListSpy).toHaveBeenCalledWith({ + imageList: [ + { + id: 'file1', + url: 'https://example.com/remote-image.jpg', + alt: 'remote-image.jpg', + }, + ], + model: 'gpt-4-vision-preview', + provider: undefined, + }); + + // Verify the utility functions were called + expect(parseDataUri).toHaveBeenCalledWith('https://example.com/remote-image.jpg'); + expect(isLocalUrl).toHaveBeenCalledWith('https://example.com/remote-image.jpg'); + expect(imageUrlToBase64).not.toHaveBeenCalled(); // Should NOT be called for remote URLs + + // Verify the final result preserves original URL + expect(getChatCompletionSpy).toHaveBeenCalledWith( + { + messages: [ + { + content: [ + { + text: 'Hello', + type: 'text', + }, + { + image_url: { detail: 'auto', url: 'https://example.com/remote-image.jpg' }, + type: 'image_url', + }, + ], + role: 'user', + }, + ], + model: 'gpt-4-vision-preview', + }, + undefined, + ); + }); + + it('should handle mixed local and remote URLs correctly', async () => { + const { isLocalUrl } = await import('@/utils/url'); + const { imageUrlToBase64 } = await import('@/utils/imageToBase64'); + const { parseDataUri } = await import('@/libs/model-runtime/utils/uriParser'); + + // Mock parseDataUri to always return url type + vi.mocked(parseDataUri).mockReturnValue({ type: 'url', base64: null, mimeType: null }); + + // Mock isLocalUrl to return true only for 127.0.0.1 URLs + vi.mocked(isLocalUrl).mockImplementation((url: string) => { + return new URL(url).hostname === '127.0.0.1'; + }); + + // Mock imageUrlToBase64 for conversion + vi.mocked(imageUrlToBase64).mockResolvedValue({ + base64: 'local-file-base64', + mimeType: 'image/jpeg', + }); + + const messages = [ + { + content: 'Multiple images', + role: 'user', + imageList: [ + { + id: 'local1', + url: 'http://127.0.0.1:3000/local1.jpg', // Local URL + alt: 'local1.jpg', + }, + { + id: 'remote1', + url: 'https://example.com/remote1.png', // Remote URL + alt: 'remote1.png', + }, + { + id: 'local2', + url: 'http://127.0.0.1:8080/local2.gif', // Another local URL + alt: 'local2.gif', + }, + ], + createdAt: Date.now(), + id: 'test-id-3', + meta: {}, + updatedAt: Date.now(), + }, + ] as ChatMessage[]; + + const processImageListSpy = vi.spyOn(chatService as any, 'processImageList'); + const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); + + await chatService.createAssistantMessage({ + messages, + plugins: [], + model: 'gpt-4-vision-preview', + }); + + // Verify processImageList was called + expect(processImageListSpy).toHaveBeenCalledWith({ + imageList: [ + { id: 'local1', url: 'http://127.0.0.1:3000/local1.jpg', alt: 'local1.jpg' }, + { id: 'remote1', url: 'https://example.com/remote1.png', alt: 'remote1.png' }, + { id: 'local2', url: 'http://127.0.0.1:8080/local2.gif', alt: 'local2.gif' }, + ], + model: 'gpt-4-vision-preview', + provider: undefined, + }); + + // Verify isLocalUrl was called for each image + expect(isLocalUrl).toHaveBeenCalledWith('http://127.0.0.1:3000/local1.jpg'); + expect(isLocalUrl).toHaveBeenCalledWith('https://example.com/remote1.png'); + expect(isLocalUrl).toHaveBeenCalledWith('http://127.0.0.1:8080/local2.gif'); + + // Verify imageUrlToBase64 was called only for local URLs + expect(imageUrlToBase64).toHaveBeenCalledWith('http://127.0.0.1:3000/local1.jpg'); + expect(imageUrlToBase64).toHaveBeenCalledWith('http://127.0.0.1:8080/local2.gif'); + expect(imageUrlToBase64).toHaveBeenCalledTimes(2); // Only for local URLs + + // Verify the final result has correct URLs + const callArgs = getChatCompletionSpy.mock.calls[0][0]; + const imageContent = (callArgs.messages?.[0].content as any[])?.filter( + (c) => c.type === 'image_url', + ); + + expect(imageContent).toHaveLength(3); + expect(imageContent[0].image_url.url).toBe('data:image/jpeg;base64,local-file-base64'); // Local converted + expect(imageContent[1].image_url.url).toBe('https://example.com/remote1.png'); // Remote preserved + expect(imageContent[2].image_url.url).toBe('data:image/jpeg;base64,local-file-base64'); // Local converted + }); + }); + describe('with tools messages', () => { it('should inject a tool system role for models with tools', async () => { const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); @@ -607,6 +1056,15 @@ describe('ChatService', () => { }); describe('getChatCompletion', () => { + let mockFetchSSE: any; + + beforeEach(async () => { + // Setup common fetchSSE mock for getChatCompletion tests + const { fetchSSE } = await import('@/utils/fetch'); + mockFetchSSE = vi.fn().mockResolvedValue(new Response('mock response')); + vi.mocked(fetchSSE).mockImplementation(mockFetchSSE); + }); + it('should make a POST request with the correct payload', async () => { const params: Partial = { model: 'test-model', @@ -622,12 +1080,16 @@ describe('ChatService', () => { await chatService.getChatCompletion(params, options); - expect(global.fetch).toHaveBeenCalledWith(expect.any(String), { - body: JSON.stringify(expectedPayload), - headers: expect.any(Object), - method: 'POST', - }); + expect(mockFetchSSE).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + body: JSON.stringify(expectedPayload), + headers: expect.any(Object), + method: 'POST', + }), + ); }); + it('should make a POST request without response in non-openai provider payload', async () => { const params: Partial = { model: 'deepseek-reasoner', @@ -647,52 +1109,52 @@ describe('ChatService', () => { await chatService.getChatCompletion(params, options); - expect(global.fetch).toHaveBeenCalledWith(expect.any(String), { - body: JSON.stringify(expectedPayload), - headers: expect.any(Object), - method: 'POST', - }); + expect(mockFetchSSE).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + body: JSON.stringify(expectedPayload), + headers: expect.any(Object), + method: 'POST', + }), + ); }); - it('should throw InvalidAccessCode error when enableFetchOnClient is true and auth is enabled but user is not signed in', async () => { - // Mock userStore - const mockUserStore = { - enableAuth: () => true, - isSignedIn: false, - }; + it('should return InvalidAccessCode error when enableFetchOnClient is true and auth is enabled but user is not signed in', async () => { + // Mock fetchSSE to call onErrorHandle with the error + const { fetchSSE } = await import('@/utils/fetch'); - // Mock modelConfigSelectors - const mockModelConfigSelectors = { - isProviderFetchOnClient: () => () => true, - }; + const mockFetchSSEWithError = vi.fn().mockImplementation((url, options) => { + // Simulate the error being caught and passed to onErrorHandle + if (options.onErrorHandle) { + const error = { + errorType: ChatErrorType.InvalidAccessCode, + error: new Error('InvalidAccessCode'), + }; + options.onErrorHandle(error, { errorType: ChatErrorType.InvalidAccessCode }); + } + return Promise.resolve(new Response('')); + }); - vi.spyOn(useUserStore, 'getState').mockImplementationOnce(() => mockUserStore as any); - vi.spyOn(modelConfigSelectors, 'isProviderFetchOnClient').mockImplementationOnce( - mockModelConfigSelectors.isProviderFetchOnClient, - ); + vi.mocked(fetchSSE).mockImplementation(mockFetchSSEWithError); const params: Partial = { model: 'test-model', messages: [], - }; - const options = {}; - const expectedPayload = { - model: DEFAULT_AGENT_CONFIG.model, - stream: true, - ...DEFAULT_AGENT_CONFIG.params, - ...params, + provider: 'openai', }; - const result = await chatService.getChatCompletion(params, options); - - expect(global.fetch).toHaveBeenCalledWith(expect.any(String), { - body: JSON.stringify(expectedPayload), - headers: expect.objectContaining({ - 'Content-Type': 'application/json', - }), - method: 'POST', + let errorHandled = false; + const onErrorHandle = vi.fn((error: any) => { + errorHandled = true; + expect(error.errorType).toBe(ChatErrorType.InvalidAccessCode); }); - expect(result.status).toBe(401); + + // Call getChatCompletion with onErrorHandle to catch the error + await chatService.getChatCompletion(params, { onErrorHandle }); + + // Verify that the error was handled + expect(errorHandled).toBe(true); + expect(onErrorHandle).toHaveBeenCalled(); }); // Add more test cases to cover different scenarios and edge cases @@ -717,10 +1179,29 @@ describe('ChatService', () => { describe('fetchPresetTaskResult', () => { it('should handle successful chat completion response', async () => { - // 模拟 fetch 抛出错误的情况 - vi.mocked(fetch).mockResolvedValueOnce(new Response('AI response')); + // Mock getChatCompletion to simulate successful completion + const getChatCompletionSpy = vi + .spyOn(chatService, 'getChatCompletion') + .mockImplementation(async (params, options) => { + // Simulate successful response + if (options?.onFinish) { + options.onFinish('AI response', { + type: 'done', + observationId: null, + toolCalls: undefined, + traceId: null, + }); + } + if (options?.onMessageHandle) { + options.onMessageHandle({ type: 'text', text: 'AI response' }); + } + return Promise.resolve(new Response('')); + }); + const params = { - /* 填充参数 */ + messages: [{ content: 'Hello', role: 'user' as const }], + model: 'gpt-4', + provider: 'openai', }; const onMessageHandle = vi.fn(); @@ -748,25 +1229,31 @@ describe('ChatService', () => { }); expect(onError).not.toHaveBeenCalled(); expect(onMessageHandle).toHaveBeenCalled(); - expect(onLoadingChange).toHaveBeenCalledWith(false); // 确认加载状态已经被设置为 false + expect(onLoadingChange).toHaveBeenCalledWith(false); // Confirm loading state is set to false expect(onLoadingChange).toHaveBeenCalledTimes(2); }); it('should handle error in chat completion', async () => { - // 模拟 fetch 抛出错误的情况 - vi.mocked(fetch).mockResolvedValueOnce( - new Response(null, { status: 404, statusText: 'Not Found' }), - ); + // Mock getChatCompletion to simulate error + const getChatCompletionSpy = vi + .spyOn(chatService, 'getChatCompletion') + .mockImplementation(async (params, options) => { + // Simulate error response + if (options?.onErrorHandle) { + options.onErrorHandle({ message: 'translated_response.404', type: 404 }); + } + return Promise.resolve(new Response('')); + }); const params = { - /* 填充参数 */ + messages: [{ content: 'Hello', role: 'user' as const }], + model: 'gpt-4', + provider: 'openai', }; const onError = vi.fn(); const onLoadingChange = vi.fn(); const abortController = new AbortController(); - const trace = { - /* 填充跟踪信息 */ - }; + const trace = {}; await chatService.fetchPresetTaskResult({ params, @@ -780,7 +1267,7 @@ describe('ChatService', () => { message: 'translated_response.404', type: 404, }); - expect(onLoadingChange).toHaveBeenCalledWith(false); // 确认加载状态已经被设置为 false + expect(onLoadingChange).toHaveBeenCalledWith(false); // Confirm loading state is set to false }); }); @@ -910,6 +1397,18 @@ describe('ChatService', () => { // 需要在修改模拟后重新导入相关模块 const { chatService } = await import('../chat'); + // Mock processImageList to return expected image content + const processImageListSpy = vi.spyOn(chatService as any, 'processImageList'); + processImageListSpy.mockImplementation(async () => { + // Mock the expected return value for an image + return [ + { + image_url: { detail: 'auto', url: 'http://example.com/xxx0asd-dsd.png' }, + type: 'image_url', + }, + ]; + }); + const messages = [ { content: 'Hello', @@ -941,7 +1440,7 @@ describe('ChatService', () => { { content: 'Hey', role: 'assistant' }, // Regular user message ] as ChatMessage[]; - const output = chatService['processMessages']({ + const output = await chatService['processMessages']({ messages, model: 'gpt-4o', provider: 'openai', @@ -1062,7 +1561,7 @@ describe('ChatService', () => { }); }); - it('should handle empty tool calls messages correctly', () => { + it('should handle empty tool calls messages correctly', async () => { const messages = [ { content: '## Tools\n\nYou can use these tools', @@ -1075,7 +1574,7 @@ describe('ChatService', () => { }, ] as ChatMessage[]; - const result = chatService['processMessages']({ + const result = await chatService['processMessages']({ messages, model: 'gpt-4', provider: 'openai', @@ -1093,7 +1592,7 @@ describe('ChatService', () => { ]); }); - it('should handle assistant messages with reasoning correctly', () => { + it('should handle assistant messages with reasoning correctly', async () => { const messages = [ { role: 'assistant', @@ -1105,7 +1604,7 @@ describe('ChatService', () => { }, ] as ChatMessage[]; - const result = chatService['processMessages']({ + const result = await chatService['processMessages']({ messages, model: 'gpt-4', provider: 'openai', @@ -1128,6 +1627,70 @@ describe('ChatService', () => { }, ]); }); + + it('should inject INBOX_GUIDE_SYSTEMROLE for welcome questions in inbox session', async () => { + // Don't mock INBOX_GUIDE_SYSTEMROLE, use the real one + const messages: ChatMessage[] = [ + { + role: 'user', + content: 'Hello, this is my first question', + createdAt: Date.now(), + id: 'test-welcome', + meta: {}, + updatedAt: Date.now(), + }, + ]; + + const result = await chatService['processMessages']( + { + messages, + model: 'gpt-4', + provider: 'openai', + }, + { + isWelcomeQuestion: true, + trace: { sessionId: 'inbox' }, + }, + ); + + // Should have system message with inbox guide content + const systemMessage = result.find((msg) => msg.role === 'system'); + expect(systemMessage).toBeDefined(); + // Check for characteristic content of the actual INBOX_GUIDE_SYSTEMROLE + expect(systemMessage!.content).toContain('LobeChat Support Assistant'); + expect(systemMessage!.content).toContain('LobeHub'); + }); + + it('should inject historySummary into system message when provided', async () => { + const historySummary = 'Previous conversation summary: User discussed AI topics.'; + + const messages: ChatMessage[] = [ + { + role: 'user', + content: 'Continue our discussion', + createdAt: Date.now(), + id: 'test-history', + meta: {}, + updatedAt: Date.now(), + }, + ]; + + const result = await chatService['processMessages']( + { + messages, + model: 'gpt-4', + provider: 'openai', + }, + { + historySummary, + }, + ); + + // Should have system message with history summary + const systemMessage = result.find((msg) => msg.role === 'system'); + expect(systemMessage).toBeDefined(); + expect(systemMessage!.content).toContain(historySummary); + }); }); }); @@ -1139,6 +1702,379 @@ vi.mock('../_auth', async (importOriginal) => { return importOriginal(); }); +describe('ChatService private methods', () => { + describe('processImageList', () => { + beforeEach(() => { + vi.resetModules(); + }); + + it('should return empty array if model cannot use vision (non-deprecated)', async () => { + vi.doMock('@/const/version', () => ({ + isServerMode: false, + isDeprecatedEdition: false, + isDesktop: false, + })); + const { aiModelSelectors } = await import('@/store/aiInfra'); + vi.spyOn(aiModelSelectors, 'isModelSupportVision').mockReturnValue(() => false); + + const { chatService } = await import('../chat'); + const result = await chatService['processImageList']({ + imageList: [{ url: 'image_url', alt: '', id: 'test' } as ChatImageItem], + model: 'any-model', + provider: 'any-provider', + }); + expect(result).toEqual([]); + }); + + it('should process images if model can use vision (non-deprecated)', async () => { + vi.doMock('@/const/version', () => ({ + isServerMode: false, + isDeprecatedEdition: false, + isDesktop: false, + })); + const { aiModelSelectors } = await import('@/store/aiInfra'); + vi.spyOn(aiModelSelectors, 'isModelSupportVision').mockReturnValue(() => true); + + const { chatService } = await import('../chat'); + const result = await chatService['processImageList']({ + imageList: [{ url: 'image_url', alt: '', id: 'test' } as ChatImageItem], + model: 'any-model', + provider: 'any-provider', + }); + expect(result.length).toBe(1); + expect(result[0].type).toBe('image_url'); + }); + + it('should return empty array when vision disabled in deprecated edition', async () => { + vi.doMock('@/const/version', () => ({ + isServerMode: false, + isDeprecatedEdition: true, + isDesktop: false, + })); + + const { modelProviderSelectors } = await import('@/store/user/selectors'); + const spy = vi + .spyOn(modelProviderSelectors, 'isModelEnabledVision') + .mockReturnValue(() => false); + + const { chatService } = await import('../chat'); + const result = await chatService['processImageList']({ + imageList: [{ url: 'image_url', alt: '', id: 'test' } as ChatImageItem], + model: 'any-model', + provider: 'any-provider', + }); + + expect(spy).toHaveBeenCalled(); + expect(result).toEqual([]); + }); + + it('should process images when vision enabled in deprecated edition', async () => { + vi.doMock('@/const/version', () => ({ + isServerMode: false, + isDeprecatedEdition: true, + isDesktop: false, + })); + + const { modelProviderSelectors } = await import('@/store/user/selectors'); + const spy = vi + .spyOn(modelProviderSelectors, 'isModelEnabledVision') + .mockReturnValue(() => true); + + const { chatService } = await import('../chat'); + const result = await chatService['processImageList']({ + imageList: [{ url: 'image_url' } as ChatImageItem], + model: 'any-model', + provider: 'any-provider', + }); + + expect(spy).toHaveBeenCalled(); + expect(result.length).toBe(1); + expect(result[0].type).toBe('image_url'); + }); + }); + + describe('processMessages', () => { + describe('getAssistantContent', () => { + it('should handle assistant message with imageList and content', async () => { + const messages: ChatMessage[] = [ + { + role: 'assistant', + content: 'Here is an image.', + imageList: [{ id: 'img1', url: 'http://example.com/image.png', alt: 'test.png' }], + createdAt: Date.now(), + id: 'test-id', + meta: {}, + updatedAt: Date.now(), + }, + ]; + const result = await chatService['processMessages']({ + messages, + model: 'gpt-4-vision-preview', + provider: 'openai', + }); + + expect(result[0].content).toEqual([ + { text: 'Here is an image.', type: 'text' }, + { image_url: { detail: 'auto', url: 'http://example.com/image.png' }, type: 'image_url' }, + ]); + }); + + it('should handle assistant message with imageList but no content', async () => { + const messages: ChatMessage[] = [ + { + role: 'assistant', + content: '', + imageList: [{ id: 'img1', url: 'http://example.com/image.png', alt: 'test.png' }], + createdAt: Date.now(), + id: 'test-id-2', + meta: {}, + updatedAt: Date.now(), + }, + ]; + const result = await chatService['processMessages']({ + messages, + model: 'gpt-4-vision-preview', + provider: 'openai', + }); + + expect(result[0].content).toEqual([ + { image_url: { detail: 'auto', url: 'http://example.com/image.png' }, type: 'image_url' }, + ]); + }); + }); + + it('should not include tool_calls for assistant message if model does not support tools', async () => { + // Mock isCanUseFC to return false + vi.spyOn( + (await import('@/store/aiInfra')).aiModelSelectors, + 'isModelSupportToolUse', + ).mockReturnValue(() => false); + + const messages: ChatMessage[] = [ + { + role: 'assistant', + content: 'I have a tool call.', + tools: [ + { + id: 'tool_123', + type: 'default', + apiName: 'testApi', + arguments: '{}', + identifier: 'test-plugin', + }, + ], + createdAt: Date.now(), + id: 'test-id-3', + meta: {}, + updatedAt: Date.now(), + }, + ]; + + const result = await chatService['processMessages']({ + messages, + model: 'some-model-without-fc', + provider: 'openai', + }); + + expect(result[0].tool_calls).toBeUndefined(); + expect(result[0].content).toBe('I have a tool call.'); + }); + }); + + describe('reorderToolMessages', () => { + it('should correctly reorder when a tool message appears before the assistant message', () => { + const input: OpenAIChatMessage[] = [ + { + role: 'system', + content: 'System message', + }, + { + role: 'tool', + tool_call_id: 'tool_call_1', + name: 'test-plugin____testApi', + content: 'Tool result', + }, + { + role: 'assistant', + content: '', + tool_calls: [ + { id: 'tool_call_1', type: 'function', function: { name: 'testApi', arguments: '{}' } }, + ], + }, + ]; + + const output = chatService['reorderToolMessages'](input); + + // Verify reordering logic works and covers line 688 hasPushed check + // In this test, tool messages are duplicated but the second occurrence is skipped + expect(output.length).toBe(4); // Original has 3, assistant will add corresponding tool message again + expect(output[0].role).toBe('system'); + expect(output[1].role).toBe('tool'); + expect(output[2].role).toBe('assistant'); + expect(output[3].role).toBe('tool'); // Tool message added by assistant's tool_calls + }); + }); + + describe('getChatCompletion', () => { + it('should merge responseAnimation styles correctly', async () => { + const { fetchSSE } = await import('@/utils/fetch'); + vi.mock('@/utils/fetch', async (importOriginal) => { + const module = await importOriginal(); + return { + ...(module as any), + fetchSSE: vi.fn(), + }; + }); + + // Mock provider config + const { aiProviderSelectors } = await import('@/store/aiInfra'); + vi.spyOn(aiProviderSelectors, 'providerConfigById').mockReturnValue({ + id: 'openai', + settings: { + responseAnimation: 'slow', + }, + } as any); + + // Mock user preference + const { userGeneralSettingsSelectors } = await import('@/store/user/selectors'); + vi.spyOn(userGeneralSettingsSelectors, 'transitionMode').mockReturnValue('smooth'); + + await chatService.getChatCompletion( + { provider: 'openai', messages: [] }, + { responseAnimation: { speed: 20 } }, + ); + + expect(fetchSSE).toHaveBeenCalled(); + const fetchSSEOptions = (fetchSSE as any).mock.calls[0][1]; + + expect(fetchSSEOptions.responseAnimation).toEqual({ + speed: 20, + text: 'fadeIn', + toolsCalling: 'fadeIn', + }); + }); + }); + + describe('extendParams', () => { + it('should set enabledContextCaching to false when model supports disableContextCaching and user enables it', async () => { + const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); + const messages = [{ content: 'Test context caching', role: 'user' }] as ChatMessage[]; + + // Mock aiModelSelectors for extend params support + vi.spyOn(aiModelSelectors, 'isModelHasExtendParams').mockReturnValue(() => true); + vi.spyOn(aiModelSelectors, 'modelExtendParams').mockReturnValue(() => [ + 'disableContextCaching', + ]); + + // Mock agent chat config with context caching disabled + vi.spyOn(agentChatConfigSelectors, 'currentChatConfig').mockReturnValue({ + disableContextCaching: true, + searchMode: 'off', + } as any); + + await chatService.createAssistantMessage({ + messages, + model: 'test-model', + provider: 'test-provider', + plugins: [], + }); + + expect(getChatCompletionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + enabledContextCaching: false, + }), + undefined, + ); + }); + + it('should not set enabledContextCaching when disableContextCaching is false', async () => { + const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); + const messages = [{ content: 'Test context caching enabled', role: 'user' }] as ChatMessage[]; + + // Mock aiModelSelectors for extend params support + vi.spyOn(aiModelSelectors, 'isModelHasExtendParams').mockReturnValue(() => true); + vi.spyOn(aiModelSelectors, 'modelExtendParams').mockReturnValue(() => [ + 'disableContextCaching', + ]); + + // Mock agent chat config with context caching enabled (default) + vi.spyOn(agentChatConfigSelectors, 'currentChatConfig').mockReturnValue({ + disableContextCaching: false, + searchMode: 'off', + } as any); + + await chatService.createAssistantMessage({ + messages, + model: 'test-model', + provider: 'test-provider', + plugins: [], + }); + + // enabledContextCaching should not be present in the call + const callArgs = getChatCompletionSpy.mock.calls[0][0]; + expect(callArgs).not.toHaveProperty('enabledContextCaching'); + }); + + it('should set reasoning_effort when model supports reasoningEffort and user configures it', async () => { + const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); + const messages = [{ content: 'Test reasoning effort', role: 'user' }] as ChatMessage[]; + + // Mock aiModelSelectors for extend params support + vi.spyOn(aiModelSelectors, 'isModelHasExtendParams').mockReturnValue(() => true); + vi.spyOn(aiModelSelectors, 'modelExtendParams').mockReturnValue(() => ['reasoningEffort']); + + // Mock agent chat config with reasoning effort set + vi.spyOn(agentChatConfigSelectors, 'currentChatConfig').mockReturnValue({ + reasoningEffort: 'high', + searchMode: 'off', + } as any); + + await chatService.createAssistantMessage({ + messages, + model: 'test-model', + provider: 'test-provider', + plugins: [], + }); + + expect(getChatCompletionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + reasoning_effort: 'high', + }), + undefined, + ); + }); + + it('should set thinkingBudget when model supports thinkingBudget and user configures it', async () => { + const getChatCompletionSpy = vi.spyOn(chatService, 'getChatCompletion'); + const messages = [{ content: 'Test thinking budget', role: 'user' }] as ChatMessage[]; + + // Mock aiModelSelectors for extend params support + vi.spyOn(aiModelSelectors, 'isModelHasExtendParams').mockReturnValue(() => true); + vi.spyOn(aiModelSelectors, 'modelExtendParams').mockReturnValue(() => ['thinkingBudget']); + + // Mock agent chat config with thinking budget set + vi.spyOn(agentChatConfigSelectors, 'currentChatConfig').mockReturnValue({ + thinkingBudget: 5000, + searchMode: 'off', + } as any); + + await chatService.createAssistantMessage({ + messages, + model: 'test-model', + provider: 'test-provider', + plugins: [], + }); + + expect(getChatCompletionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + thinkingBudget: 5000, + }), + undefined, + ); + }); + }); +}); + describe('AgentRuntimeOnClient', () => { describe('initializeWithClientStore', () => { describe('should initialize with options correctly', () => { diff --git a/src/services/chat.ts b/src/services/chat.ts index 9988fbafd3..61d2f6952b 100644 --- a/src/services/chat.ts +++ b/src/services/chat.ts @@ -15,6 +15,7 @@ import { ChatCompletionErrorPayload, ModelProvider, } from '@/libs/model-runtime'; +import { parseDataUri } from '@/libs/model-runtime/utils/uriParser'; import { filesPrompts } from '@/prompts/files'; import { BuiltinSystemRolePrompts } from '@/prompts/systemRole'; import { getAgentStoreState } from '@/store/agent'; @@ -35,7 +36,7 @@ import { import { WebBrowsingManifest } from '@/tools/web-browsing'; import { WorkingModel } from '@/types/agent'; import { ChatErrorType } from '@/types/fetch'; -import { ChatMessage, MessageToolCall } from '@/types/message'; +import { ChatImageItem, ChatMessage, MessageToolCall } from '@/types/message'; import type { ChatStreamPayload, OpenAIChatMessage } from '@/types/openai/chat'; import { UserMessageContentPart } from '@/types/openai/chat'; import { parsePlaceholderVariablesMessages } from '@/utils/client/parserPlaceholder'; @@ -46,8 +47,10 @@ import { getMessageError, standardizeAnimationStyle, } from '@/utils/fetch'; +import { imageUrlToBase64 } from '@/utils/imageToBase64'; import { genToolCallingName } from '@/utils/toolCall'; import { createTraceHeader, getTraceId } from '@/utils/trace'; +import { isLocalUrl } from '@/utils/url'; import { createHeaderWithAuth, createPayloadWithKeyVaults } from './_auth'; import { API_ENDPOINTS } from './_url'; @@ -61,6 +64,14 @@ const isCanUseFC = (model: string, provider: string) => { return aiModelSelectors.isModelSupportToolUse(model, provider)(getAiInfraStoreState()); }; +const isCanUseVision = (model: string, provider: string) => { + // TODO: remove isDeprecatedEdition condition in V2.0 + if (isDeprecatedEdition) { + return modelProviderSelectors.isModelEnabledVision(model)(getUserStoreState()); + } + return aiModelSelectors.isModelSupportVision(model, provider)(getAiInfraStoreState()); +}; + /** * TODO: we need to update this function to auto find deploymentName with provider setting config */ @@ -205,7 +216,7 @@ class ChatService { // ============ 2. preprocess messages ============ // - const oaiMessages = this.processMessages( + const oaiMessages = await this.processMessages( { messages: parsedMessages, model: payload.model, @@ -475,7 +486,7 @@ class ChatService { onLoadingChange?.(true); try { - const oaiMessages = this.processMessages({ + const oaiMessages = await this.processMessages({ messages: params.messages as any, model: params.model!, provider: params.provider!, @@ -507,7 +518,7 @@ class ChatService { } }; - private processMessages = ( + private processMessages = async ( { messages = [], tools, @@ -520,29 +531,28 @@ class ChatService { tools?: string[]; }, options?: FetchOptions, - ): OpenAIChatMessage[] => { + ): Promise => { // handle content type for vision model // for the models with visual ability, add image url to content // refs: https://platform.openai.com/docs/guides/vision/quick-start - const getUserContent = (m: ChatMessage) => { + const getUserContent = async (m: ChatMessage) => { // only if message doesn't have images and files, then return the plain content if ((!m.imageList || m.imageList.length === 0) && (!m.fileList || m.fileList.length === 0)) return m.content; const imageList = m.imageList || []; + const imageContentParts = await this.processImageList({ imageList, model, provider }); const filesContext = isServerMode ? filesPrompts({ addUrl: !isDesktop, fileList: m.fileList, imageList }) : ''; return [ { text: (m.content + '\n\n' + filesContext).trim(), type: 'text' }, - ...imageList.map( - (i) => ({ image_url: { detail: 'auto', url: i.url }, type: 'image_url' }) as const, - ), + ...imageContentParts, ] as UserMessageContentPart[]; }; - const getAssistantContent = (m: ChatMessage) => { + const getAssistantContent = async (m: ChatMessage) => { // signature is a signal of anthropic thinking mode const shouldIncludeThinking = m.reasoning && !!m.reasoning?.signature; @@ -559,65 +569,70 @@ class ChatService { // only if message doesn't have images and files, then return the plain content if (m.imageList && m.imageList.length > 0) { + const imageContentParts = await this.processImageList({ + imageList: m.imageList, + model, + provider, + }); return [ !!m.content ? { text: m.content, type: 'text' } : undefined, - ...m.imageList.map( - (i) => ({ image_url: { detail: 'auto', url: i.url }, type: 'image_url' }) as const, - ), + ...imageContentParts, ].filter(Boolean) as UserMessageContentPart[]; } return m.content; }; - let postMessages = messages.map((m): OpenAIChatMessage => { - const supportTools = isCanUseFC(model, provider); - switch (m.role) { - case 'user': { - return { content: getUserContent(m), role: m.role }; - } - - case 'assistant': { - const content = getAssistantContent(m); - - if (!supportTools) { - return { content, role: m.role }; + let postMessages = await Promise.all( + messages.map(async (m): Promise => { + const supportTools = isCanUseFC(model, provider); + switch (m.role) { + case 'user': { + return { content: await getUserContent(m), role: m.role }; } - return { - content, - role: m.role, - tool_calls: m.tools?.map( - (tool): MessageToolCall => ({ - function: { - arguments: tool.arguments, - name: genToolCallingName(tool.identifier, tool.apiName, tool.type), - }, - id: tool.id, - type: 'function', - }), - ), - }; - } + case 'assistant': { + const content = await getAssistantContent(m); - case 'tool': { - if (!supportTools) { - return { content: m.content, role: 'user' }; + if (!supportTools) { + return { content, role: m.role }; + } + + return { + content, + role: m.role, + tool_calls: m.tools?.map( + (tool): MessageToolCall => ({ + function: { + arguments: tool.arguments, + name: genToolCallingName(tool.identifier, tool.apiName, tool.type), + }, + id: tool.id, + type: 'function', + }), + ), + }; } - return { - content: m.content, - name: genToolCallingName(m.plugin!.identifier, m.plugin!.apiName, m.plugin?.type), - role: m.role, - tool_call_id: m.tool_call_id, - }; - } + case 'tool': { + if (!supportTools) { + return { content: m.content, role: 'user' }; + } - default: { - return { content: m.content, role: m.role as any }; + return { + content: m.content, + name: genToolCallingName(m.plugin!.identifier, m.plugin!.apiName, m.plugin?.type), + role: m.role, + tool_call_id: m.tool_call_id, + }; + } + + default: { + return { content: m.content, role: m.role as any }; + } } - } - }); + }), + ); postMessages = produce(postMessages, (draft) => { // if it's a welcome question, inject InboxGuide SystemRole @@ -657,6 +672,37 @@ class ChatService { return this.reorderToolMessages(postMessages); }; + /** + * Process imageList: convert local URLs to base64 and format as UserMessageContentPart + */ + private processImageList = async ({ + model, + provider, + imageList, + }: { + imageList: ChatImageItem[]; + model: string; + provider: string; + }) => { + if (!isCanUseVision(model, provider)) { + return []; + } + + return Promise.all( + imageList.map(async (image) => { + const { type } = parseDataUri(image.url); + + let processedUrl = image.url; + if (type === 'url' && isLocalUrl(image.url)) { + const { base64, mimeType } = await imageUrlToBase64(image.url); + processedUrl = `data:${mimeType};base64,${base64}`; + } + + return { image_url: { detail: 'auto', url: processedUrl }, type: 'image_url' } as const; + }), + ); + }; + private mapTrace = (trace?: TracePayload, tag?: TraceTagMap): TracePayload => { const tags = sessionMetaSelectors.currentAgentMeta(getSessionStoreState()).tags || []; @@ -681,9 +727,6 @@ class ChatService { provider: string; signal?: AbortSignal; }) => { - const agentRuntime = await initializeWithClientStore(params.provider, params.payload); - const data = params.payload as ChatStreamPayload; - /** * if enable login and not signed in, return unauthorized error */ @@ -692,6 +735,9 @@ class ChatService { throw AgentRuntimeError.createError(ChatErrorType.InvalidAccessCode); } + const agentRuntime = await initializeWithClientStore(params.provider, params.payload); + const data = params.payload as ChatStreamPayload; + return agentRuntime.chat(data, { signal: params.signal }); }; diff --git a/src/utils/url.test.ts b/src/utils/url.test.ts index 26103fc62a..0ac056155a 100644 --- a/src/utils/url.test.ts +++ b/src/utils/url.test.ts @@ -1,7 +1,7 @@ import { vi } from 'vitest'; import { pathString } from './url'; -import { inferContentTypeFromImageUrl, inferFileExtensionFromImageUrl } from './url'; +import { inferContentTypeFromImageUrl, inferFileExtensionFromImageUrl, isLocalUrl } from './url'; describe('pathString', () => { it('should handle basic path', () => { @@ -398,3 +398,44 @@ describe('inferFileExtensionFromImageUrl', () => { expect(result).toBe('gif'); }); }); + +describe('isLocalUrl', () => { + it('should return true for URLs with 127.0.0.1 as hostname', () => { + expect(isLocalUrl('http://127.0.0.1')).toBe(true); + expect(isLocalUrl('https://127.0.0.1')).toBe(true); + expect(isLocalUrl('http://127.0.0.1:8080')).toBe(true); + expect(isLocalUrl('http://127.0.0.1/path/to/resource')).toBe(true); + expect(isLocalUrl('https://127.0.0.1/path?query=1#hash')).toBe(true); + }); + + it('should return false for URLs with "localhost" as hostname', () => { + expect(isLocalUrl('http://localhost')).toBe(false); + expect(isLocalUrl('http://localhost:3000')).toBe(false); + }); + + it('should return false for other IP addresses', () => { + expect(isLocalUrl('http://192.168.1.1')).toBe(false); + expect(isLocalUrl('http://0.0.0.0')).toBe(false); + expect(isLocalUrl('http://127.0.0.2')).toBe(false); + }); + + it('should return false for domain names', () => { + expect(isLocalUrl('https://example.com')).toBe(false); + expect(isLocalUrl('http://www.google.com')).toBe(false); + }); + + it('should return false for malformed URLs', () => { + expect(isLocalUrl('invalid-url')).toBe(false); + expect(isLocalUrl('http://')).toBe(false); + expect(isLocalUrl('a string but not a url')).toBe(false); + }); + + it('should return false for empty or nullish strings', () => { + expect(isLocalUrl('')).toBe(false); + }); + + it('should return false for relative URLs', () => { + expect(isLocalUrl('/path/to/file')).toBe(false); + expect(isLocalUrl('./relative/path')).toBe(false); + }); +}); diff --git a/src/utils/url.ts b/src/utils/url.ts index 8a287aca36..4f7f7ca465 100644 --- a/src/utils/url.ts +++ b/src/utils/url.ts @@ -123,3 +123,31 @@ export function inferContentTypeFromImageUrl(url: string) { return mimeType!; // Non-null assertion is safe due to whitelist validation } + +/** + * Check if a URL points to localhost (127.0.0.1) + * + * This function safely determines if the provided URL's hostname is '127.0.0.1'. + * It handles malformed URLs gracefully by returning false instead of throwing errors. + * + * @param url - The URL string to check + * @returns true if the URL's hostname is '127.0.0.1', false otherwise (including for malformed URLs) + * + * @example + * ```typescript + * isLocalUrl('http://127.0.0.1:8080/path') // true + * isLocalUrl('https://example.com') // false + * isLocalUrl('invalid-url') // false (instead of throwing) + * isLocalUrl('') // false (instead of throwing) + * ``` + * + * check: apps/desktop/src/main/core/StaticFileServerManager.ts + */ +export function isLocalUrl(url: string) { + try { + return new URL(url).hostname === '127.0.0.1'; + } catch { + // Return false for malformed URLs instead of throwing + return false; + } +}