MCP Email Service

# 关键修复 - 2025-10-22 ## 🐛 修复的问题 ### 第一轮修复 #### 1. ConnectionManager 方法调用错误 ✅ **问题**: 使用了不存在的 `get_connection()` 上下文管理器方法 **位置**: - `src/services/email_service.py:414` (get_email_headers) - `src/services/folder_service.py:250` (list_folders_with_unread_count) **原因**: 误假设 ConnectionManager 有上下文管理器接口 **修复**: ```python # ❌ 错误代码 with conn_mgr.get_connection() as mail: mail.select(folder, readonly=True) # ... # ✅ 正确代码 mail = conn_mgr.connect_imap() try: mail.select(folder, readonly=True) # ... finally: conn_mgr.close_imap(mail) ``` ### 2. 文件夹数据结构解析错误 ✅ **问题**: `folder_ops.list_folders()` 返回字典列表，但代码直接将字典传给 `mail.select()` **位置**: `src/services/folder_service.py:247` **原因**: `list_folders()` 返回 `{'name': 'INBOX', ...}` 格式，需要提取 `name` 字段 **修复**: ```python # ❌ 错误代码 for folder_name in folders_result.get('folders', []): mail.select(folder_name, readonly=True) # folder_name 是 dict # ✅ 正确代码 for folder_info in folders_result.get('folders', []): folder_name = folder_info['name'] # 提取 name mail.select(folder_name, readonly=True) ``` ### 3. 新参数未在 Service 层实现 ✅ **问题**: Schema 和 Handler 层定义了新参数，但 Service 层未接收或处理 **影响工具**: - `list_emails`: `offset`, `include_metadata` - `search_emails`: `offset` - `mark_emails`: `dry_run` - `delete_emails`: `dry_run` **修复**: #### 分页支持 (offset) ```python # list_emails 和 search_emails def list_emails( self, limit: int = 50, unread_only: bool = False, folder: str = 'INBOX', account_id: Optional[str] = None, offset: int = 0, # ✅ 新增 include_metadata: bool = True # ✅ 新增 ) -> Dict[str, Any]: # Fetch with offset fetch_limit = limit + offset if offset > 0 else limit result = fetch_emails(fetch_limit, unread_only, folder, account_id) # Apply pagination if offset > 0 and 'emails' in result: result['emails'] = result['emails'][offset:offset + limit] result['offset'] = offset result['limit'] = limit # Add metadata if include_metadata and 'emails' in result: for email in result['emails']: email['source'] = 'imap_fetch' return result ``` #### Dry Run 支持 ```python # mark_emails 和 delete_emails def mark_emails( self, email_ids: List[str], mark_as: str, folder: str = 'INBOX', account_id: Optional[str] = None, dry_run: bool = False # ✅ 新增 ) -> Dict[str, Any]: # Dry run mode if dry_run: return { 'success': True, 'dry_run': True, 'would_mark': len(email_ids), 'mark_as': mark_as, 'email_ids': email_ids, 'message': f'Dry run: would mark {len(email_ids)} emails as {mark_as}' } # ... 实际执行逻辑 ``` ### 第二轮修复 #### 4. get_email_headers 使用序号而非 UID ✅ **问题**: 使用 `mail.fetch(email_id, ...)` 但 `email_id` 实际是 UID **位置**: `src/services/email_service.py:482` **原因**: - IMAP 有两种标识：Sequence Number (1,2,3...) 和 UID (唯一不变) - 项目统一使用 UID，但 `fetch()` 默认使用 sequence number - 会导致获取错误邮件或失败 **修复**: ```python # ❌ 错误 _, msg_data = mail.fetch(email_id, '(BODY.PEEK[HEADER.FIELDS (...)])') # ✅ 正确：使用 uid('fetch', ...) _, msg_data = mail.uid('fetch', email_id, '(BODY.PEEK[HEADER.FIELDS (...)])') # 并正确解析 UID fetch 返回值 if isinstance(msg_data[0], tuple) and len(msg_data[0]) >= 2: header_data = msg_data[0][1] ``` **影响**: HIGH - 工具在大多数场景下都会失败 #### 5. search_emails fallback 未处理 offset ✅ **问题**: Schema 定义了 `offset`，但 fallback 分支未实现 **位置**: `src/services/email_service.py:429` **原因**: - 有 `optimized_search` 时正常工作 - fallback 到 `SearchOperations` 时忽略了 offset - 导致分页返回重复结果 **修复**: ```python # ❌ 错误：忽略 offset result = search_ops.search_emails(query=query, limit=limit) # ✅ 正确：fetch more and slice fetch_limit = limit + offset if offset > 0 else limit result = search_ops.search_emails(query=query, limit=fetch_limit) if offset > 0 and 'emails' in result: result['emails'] = result['emails'][offset:offset + limit] result['offset'] = offset result['limit'] = limit ``` **影响**: MEDIUM - 在大多数部署环境（无优化模块）分页会失败 ## ✅ 验证检查 ### 测试新工具 ```python # 1. list_unread_folders result = mcp.call("list_unread_folders", {"include_empty": False}) assert 'folders' in result assert all('unread_count' in f for f in result['folders']) # 2. get_email_headers result = mcp.call("get_email_headers", { "email_id": "123", "headers": ["From", "Subject"] }) assert 'headers' in result assert 'From' in result['headers'] # 3. get_recent_activity result = mcp.call("get_recent_activity", {"include_stats": True}) assert 'accounts' in result assert all('last_sync' in a for a in result['accounts']) ``` ### 测试新参数 ```python # 1. 分页 page1 = mcp.call("list_emails", {"limit": 10, "offset": 0}) page2 = mcp.call("list_emails", {"limit": 10, "offset": 10}) assert page1['emails'][0]['id'] != page2['emails'][0]['id'] # 2. 元数据 result = mcp.call("list_emails", {"include_metadata": True}) assert all('source' in email for email in result['emails']) # 3. Dry run dry = mcp.call("delete_emails", { "email_ids": ["1", "2"], "dry_run": True }) assert dry['dry_run'] == True assert dry['would_delete'] == 2 ``` ## 📊 修复统计 | 类别 | 第一轮 | 第二轮 | 总计 | |------|--------|--------|------| | 代码文件修改 | 3 个 | 1 个 | 3 个 | | 方法修复 | 3 个 | 2 个 | 5 个 | | 新参数实现 | 6 个 | 0 个 | 6 个 | | 关键问题 | 3 个 | 2 个 | 5 个 | | Linting 错误 | 0 个 | 0 个 | 0 个 | ## 🔍 代码审查要点 ### 连接管理模式 所有 IMAP 操作现在遵循统一模式： ```python mail = conn_mgr.connect_imap() try: # 操作 mail.select(folder, readonly=True) result = mail.fetch(...) return process(result) finally: conn_mgr.close_imap(mail) ``` ### 数据结构处理 处理 `list_folders()` 返回值： ```python folders_result = folder_ops.list_folders() # folders_result = { # 'success': True, # 'folders': [ # {'name': 'INBOX', 'attributes': '...', 'message_count': 10}, # {'name': 'Sent', 'attributes': '...', 'message_count': 5} # ] # } for folder_info in folders_result.get('folders', []): folder_name = folder_info['name'] # 正确提取 # 使用 folder_name ``` ### 参数传递链 ``` Schema (tool_schemas.py) ↓ Handler (tool_handlers.py) - 提取参数 ↓ Service (email_service.py) - 实现逻辑 ↓ Operations/Legacy (legacy_operations.py) - 底层调用 ``` 确保每一层都正确传递和处理参数。 ## 🎓 经验教训 ### 1. **假设验证** - ❌ 不要假设方法存在 - ✅ 检查实际接口定义 ### 2. **数据结构理解** - ❌ 不要假设返回格式 - ✅ 查看实际实现的返回结构 ### 3. **参数传递完整性** - ❌ 不要只在 schema 定义参数 - ✅ 确保整个调用链都处理参数 ### 4. **向后兼容性** - ✅ 所有新参数都有默认值 - ✅ 旧代码无需修改即可工作 ### 5. **理解底层协议** - ❌ 不要假设 IMAP 操作都一样 - ✅ 理解 UID vs Sequence Number 的区别 - ✅ 使用 `mail.uid()` 而非普通命令 ### 6. **覆盖所有代码路径** - ❌ 不要只测试优化路径 - ✅ 确保 fallback 分支也正确实现 ## 🚀 下一步 ### 短期 - [x] 修复连接管理错误 - [x] 修复数据结构解析 - [x] 实现新参数逻辑 - [ ] 添加集成测试 - [ ] 性能基准测试 ### 长期 - [ ] 统一连接管理接口（考虑添加上下文管理器） - [ ] 数据结构文档化（在代码注释中说明返回格式） - [ ] 参数验证层（统一检查参数有效性） ## 📝 相关文档 - [UID vs Sequence 详细说明](./CRITICAL_FIXES_SUPPLEMENT.md) - 第二轮修复详情 - [设计原则](./guides/MCP_DESIGN_PRINCIPLES.md) - [升级指南](./ATOMIC_OPERATIONS_UPGRADE.md) - [架构文档](./ARCHITECTURE.md) ## 🔍 下一步检查 ### 需要审查的代码模式 ```bash # 检查其他可能有问题的 IMAP 操作 grep -r "mail\.fetch(" src/ grep -r "mail\.store(" src/ grep -r "mail\.copy(" src/ # 应该全部改为 mail.uid('fetch'/'store'/'copy', ...) ``` ### 建议的集成测试 1. **UID 操作测试** - 测试真实 IMAP 环境的 UID fetch - 验证返回值解析正确 2. **分页一致性测试** - 测试 offset 在不同环境下的行为 - 验证优化路径和 fallback 路径结果一致 3. **边界条件测试** - offset 超过总数 - UID 不存在 - 空文件夹 --- **修复日期**: 2025-10-22 **修复人员**: AI Assistant **审核人员**: leo **修复轮次**: 2 轮 **测试状态**: ✅ Linting 通过，建议加集成测试