Merge: fix/architecture-smell-and-test-gaps — 架构分层修复 + 测试补充 + 简化 + 审查 + 沉淀
本次合并包含 5 个 commits 的完整工作流: 1. fix(arch): 迁移 LLM 异常到 llm/exceptions.py 消除分层异味 (5f5c098) - 新建 llm/exceptions.py,LLM 异常定义归位 LLM 层 - core/exceptions.py 用 PEP 562 __getattr__ 延迟 re-export 打破循环依赖 - 保持 27 个调用方的 from agentkit.core.exceptions import LLMProviderError 兼容 2. test: 补充 3 个高优先级测试缺口 (52 tests) (e0a22e0) - tests/unit/llm/test_migration.py (6 tests) - tests/unit/test_memory_models.py (31 tests) - tests/unit/test_chat_ws_routes.py (15 tests) 3. refactor: 简化 __getattr__ 和测试 helper (ce-simplify-code) (a4d3f22) - __getattr__ 用 for 循环+globals 替代逐个 import - 测试 helper 提取 3 个 _assert_* 函数 4. test: 补充 PEP 562 re-export 契约守护测试 (ce-code-review) (7449458) - tests/unit/test_core_exceptions_reexport.py (6 tests) - 9 reviewer 共识: re-export identity 契约需自动化守护 5. docs: 沉淀 PEP 562 re-export 架构模式 + 方案文档 (c429851) - docs/solutions/architecture-patterns/circular-import-pep562-lazy-reexport.md - docs/plans/ + docs/solutions/logic-errors/ 文档 验证: 58 个新测试全部通过,ruff check + format 通过
This commit is contained in:
commit
285354fc6f
|
|
@ -0,0 +1,246 @@
|
|||
---
|
||||
artifact_contract: ce-unified-plan/v1
|
||||
artifact_readiness: implementation-ready
|
||||
product_contract_source: ce-plan-bootstrap
|
||||
execution: code
|
||||
title: "fix: Resolve Core↔LLM architecture smell and fill high-risk test gaps"
|
||||
created: 2026-07-06
|
||||
plan_type: fix
|
||||
---
|
||||
|
||||
# fix: Resolve Core↔LLM architecture smell and fill high-risk test gaps
|
||||
|
||||
## Summary
|
||||
|
||||
基于知识图谱分析发现两类问题:(1) Core Agent 引擎与 LLM 网关之间存在架构分层异味 — LLM 异常定义在 `core/exceptions.py`,导致 LLM 层反向依赖 Core;(2) 2 个核心源文件完全无测试覆盖,1 个核心模块(WebSocket 聊天路由)缺少 WebSocket 路径测试。本方案通过 Re-export 兼容方式迁移异常定义,并为 3 个测试缺口补充单元测试。
|
||||
|
||||
## Problem Frame
|
||||
|
||||
### 背景
|
||||
|
||||
知识图谱分析(基于 understand-anything 生成的 1579 节点图谱)发现:
|
||||
|
||||
1. **架构分层异味**:`core/exceptions.py` 定义了 `LLMError`/`LLMProviderError`/`ModelNotFoundError` 三个 LLM 相关异常类(L135-154),导致 7 个 LLM 层文件反向依赖 Core 层获取自己的异常定义。虽然 `core/exceptions.py` 是叶子模块(不导入 agentkit.*),不构成 Python 循环导入,但这违反了分层架构原则。
|
||||
|
||||
2. **测试覆盖缺口**(经实际代码搜索验证,纠正了知识图谱 `tested_by` 边不足导致的误判):
|
||||
- `llm/migration.py` — 完全无测试,含高风险函数 `migrate_api_keys_to_secrets`
|
||||
- `memory/models.py` — 完全无测试,含 `EpisodeModel` 等数据模型
|
||||
- `server/routes/chat.py` 的 WebSocket 路径 — `chat_websocket`/`_handle_chat_message` 无直接 WebSocket 测试(现有 `test_chat_routes.py` 只覆盖 REST 端点)
|
||||
|
||||
### 不在范围内
|
||||
|
||||
- 知识图谱 imports/tested_by 边补全(图谱分析局限性,非代码问题)
|
||||
- experts/_*.py mixin 直接单元测试(通过 ExpertTeam 间接测试,用户确认跳过)
|
||||
- 大规模模块结构重组
|
||||
- 前端/桌面端测试补充
|
||||
|
||||
## Requirements
|
||||
|
||||
- **R1**: LLM 异常定义迁移到 `llm/exceptions.py`,消除 LLM → Core 的反向依赖
|
||||
- **R2**: `core/exceptions.py` 保留 Re-export,确保现有代码无需修改(向后兼容)
|
||||
- **R3**: `llm/migration.py` 有单元测试覆盖 `migrate_api_keys_to_secrets` 函数
|
||||
- **R4**: `memory/models.py` 有单元测试覆盖核心数据模型
|
||||
- **R5**: `server/routes/chat.py` WebSocket 路径有测试覆盖关键消息处理流程
|
||||
|
||||
## Key Technical Decisions
|
||||
|
||||
### KTD1: Re-export 兼容迁移策略
|
||||
|
||||
**决策**:将 `LLMError`/`LLMProviderError`/`ModelNotFoundError` 的定义迁移到 `llm/exceptions.py`,在 `core/exceptions.py` 末尾保留 `from llm.exceptions import LLMError, LLMProviderError, ModelNotFoundError`(Re-export)。
|
||||
|
||||
**理由**:
|
||||
- 消除架构异味 — LLM 异常定义在 LLM 层
|
||||
- 零破坏性变更 — 现有 `from core.exceptions import LLMProviderError` 的 7 个文件无需修改
|
||||
- 符合项目"未经明确请求不得修改"的约束 — 只移动定义,不改变 API 表面
|
||||
- 真正的异常定义位置(`llm/exceptions.py`)符合分层架构原则
|
||||
|
||||
**替代方案**:全量替换(更新所有 7 个文件的 import)— 破坏性更大,且 `core.exceptions` 中的 re-export 仍需保留以避免外部消费者破坏,反而增加维护负担。
|
||||
|
||||
### KTD2: 测试范围基于实际代码验证而非知识图谱
|
||||
|
||||
**决策**:测试补充范围基于实际 Grep 搜索验证,而非知识图谱的 `tested_by` 边。
|
||||
|
||||
**理由**:知识图谱中 `tested_by` 边只有 19 条(文件级),因为子代理分析时 `batchImportData` 为空。实际验证发现 `filter_pii`、`generate_cache_key`、`detect_verification_commands` 均已有充分测试,无需补充。只有 3 个真正的测试缺口需要处理。
|
||||
|
||||
### KTD3: WebSocket 测试使用 httpx.AsyncClient + WebSocketTestSession
|
||||
|
||||
**决策**:WebSocket 测试使用 FastAPI 的 `TestClient` WebSocket 支持(`client.websocket_connect`),遵循 `tests/unit/test_websocket.py` 的现有模式。
|
||||
|
||||
**理由**:项目已有 WebSocket 测试模式(`test_websocket.py`、`test_chat_plan_exec_ws.py`),无需引入新依赖。
|
||||
|
||||
## Implementation Units
|
||||
|
||||
### U1. 迁移 LLM 异常到 llm/exceptions.py(Re-export 兼容)
|
||||
|
||||
**Goal**: 将 `LLMError`/`LLMProviderError`/`ModelNotFoundError` 定义迁移到 `llm/exceptions.py`,`core/exceptions.py` 保留 re-export。
|
||||
|
||||
**Requirements**: R1, R2
|
||||
|
||||
**Dependencies**: 无
|
||||
|
||||
**Files**:
|
||||
- `src/agentkit/llm/exceptions.py`(新建)
|
||||
- `src/agentkit/core/exceptions.py`(修改 — 删除 L135-154 的 3 个类定义,添加 re-export)
|
||||
- `src/agentkit/llm/__init__.py`(修改 — 如有 `__all__` 则添加 exceptions 导出)
|
||||
|
||||
**Approach**:
|
||||
1. 创建 `src/agentkit/llm/exceptions.py`,包含从 `core/exceptions.py` 迁移的 3 个异常类定义(`LLMError`、`LLMProviderError`、`ModelNotFoundError`)
|
||||
2. `llm/exceptions.py` 中的 `LLMError` 继承 `Exception`(而非 `AgentFrameworkError`),因为 LLM 层不应依赖 Core 层。但为保持 `isinstance` 兼容性,`LLMError` 需同时继承 `AgentFrameworkError` — 通过在 `llm/exceptions.py` 中导入 `AgentFrameworkError` from `core.exceptions`(`AgentFrameworkError` 是框架基础异常,不是 LLM 异常,保留在 core 中是正确的)
|
||||
3. `core/exceptions.py` 删除 L135-154 的 3 个类定义,在文件末尾添加:
|
||||
```python
|
||||
# Re-export LLM exceptions from llm/exceptions.py for backward compatibility.
|
||||
# New code should import from llm.exceptions directly.
|
||||
from llm.exceptions import LLMError, LLMProviderError, ModelNotFoundError # noqa: F401
|
||||
```
|
||||
4. 验证所有 `from core.exceptions import LLMProviderError` 的导入仍能工作
|
||||
|
||||
**Patterns to follow**: 项目现有的异常定义模式(`class XError(ParentError): def __init__(self, ...): super().__init__(...)`)
|
||||
|
||||
**Test scenarios**:
|
||||
- Happy path: `from llm.exceptions import LLMProviderError` 能正常导入且 `isinstance(LLMProviderError("test", "reason"), AgentFrameworkError)` 为 True
|
||||
- Backward compat: `from core.exceptions import LLMProviderError` 仍能正常导入
|
||||
- isinstance 兼容: `isinstance(LLMProviderError(...), LLMError)` 为 True(继承链保持)
|
||||
- 构造函数: `LLMProviderError("openai", "timeout").provider == "openai"` 且消息包含 "openai" 和 "timeout"
|
||||
- `ModelNotFoundError("gpt-4").model == "gpt-4"` 且消息包含 "gpt-4"
|
||||
|
||||
**Verification**: `ruff check src/agentkit/llm/exceptions.py src/agentkit/core/exceptions.py && pytest tests/unit/ -x -q -k "exception or llm_provider or remote_provider"` 全部通过
|
||||
|
||||
---
|
||||
|
||||
### U2. 新建 llm/migration.py 单元测试
|
||||
|
||||
**Goal**: 为 `llm/migration.py` 的 `migrate_api_keys_to_secrets` 函数创建单元测试。
|
||||
|
||||
**Requirements**: R3
|
||||
|
||||
**Dependencies**: 无
|
||||
|
||||
**Files**:
|
||||
- `tests/unit/llm/test_migration.py`(新建)
|
||||
- `src/agentkit/llm/migration.py`(读取 — 确认函数签名和行为)
|
||||
|
||||
**Approach**:
|
||||
1. 阅读 `llm/migration.py` 了解 `migrate_api_keys_to_secrets` 的签名、参数、返回值和副作用
|
||||
2. 创建 `tests/unit/llm/test_migration.py`,遵循 `tests/unit/llm/test_cache.py` 的测试模式
|
||||
3. 使用 mock 隔离外部依赖(secrets store、配置文件、环境变量)
|
||||
4. 覆盖正常迁移、幂等性(重复迁移)、错误处理(源不存在/目标写入失败)场景
|
||||
|
||||
**Test scenarios**:
|
||||
- Happy path: 有 API Key 配置时,`migrate_api_keys_to_secrets` 将配置中的 key 迁移到 secrets store,返回迁移数量
|
||||
- Idempotent: 对已迁移的配置再次调用,不重复迁移,返回 0
|
||||
- Empty config: 无 API Key 配置时调用,返回 0,不修改 secrets store
|
||||
- Source missing: 源配置文件不存在时,函数优雅处理(返回 0 或抛出预期异常,视实现而定)
|
||||
- Target write failure: secrets store 写入失败时,函数抛出异常且不部分迁移(事务性)
|
||||
- Secret overwrite: 已存在同 key 的 secret 时,按实现行为验证(覆盖或跳过)
|
||||
|
||||
**Verification**: `pytest tests/unit/llm/test_migration.py -x -q` 全部通过
|
||||
|
||||
---
|
||||
|
||||
### U3. 新建 memory/models.py 单元测试
|
||||
|
||||
**Goal**: 为 `memory/models.py` 的核心数据模型创建单元测试。
|
||||
|
||||
**Requirements**: R4
|
||||
|
||||
**Dependencies**: 无
|
||||
|
||||
**Files**:
|
||||
- `tests/unit/test_memory_models.py`(新建)
|
||||
- `src/agentkit/memory/models.py`(读取 — 确认模型定义)
|
||||
|
||||
**Approach**:
|
||||
1. 阅读 `memory/models.py` 了解数据模型(`EpisodeModel` 等)的字段、验证器和序列化
|
||||
2. 创建 `tests/unit/test_memory_models.py`,遵循 `tests/unit/test_session_models.py` 的 Pydantic 模型测试模式
|
||||
3. 覆盖模型构造、字段验证、序列化/反序列化场景
|
||||
|
||||
**Test scenarios**:
|
||||
- Happy path: 用有效字段构造 `EpisodeModel`(或其他模型),字段值正确
|
||||
- Required fields: 缺少必填字段时抛出 `ValidationError`
|
||||
- Field validation: 字段类型/格式不正确时抛出 `ValidationError`(如时间戳格式、向量维度)
|
||||
- Serialization: `model_dump()` / `model_dump_json()` 输出格式正确
|
||||
- Deserialization: `model_validate_json()` / `model_validate()` 能从 dict/JSON 重建模型
|
||||
- Default values: 可选字段使用默认值时行为正确
|
||||
- Edge case: 空向量、超长文本、特殊字符等边界值处理
|
||||
|
||||
**Verification**: `pytest tests/unit/test_memory_models.py -x -q` 全部通过
|
||||
|
||||
---
|
||||
|
||||
### U4. 增强 server/routes/chat.py WebSocket 测试覆盖
|
||||
|
||||
**Goal**: 为 `server/routes/chat.py` 的 `chat_websocket` 和 `_handle_chat_message` 函数补充 WebSocket 路径测试。
|
||||
|
||||
**Requirements**: R5
|
||||
|
||||
**Dependencies**: 无
|
||||
|
||||
**Files**:
|
||||
- `tests/unit/test_chat_ws_routes.py`(新建 — 避免修改现有 `test_chat_routes.py` 的 REST 测试)
|
||||
- `src/agentkit/server/routes/chat.py`(读取 — 确认 WebSocket 处理逻辑)
|
||||
|
||||
**Approach**:
|
||||
1. 阅读 `server/routes/chat.py` 的 `chat_websocket` 函数,了解 WebSocket 消息处理流程(`message`/`reply`/`confirmation_reply`/`cancel`/`ping`)
|
||||
2. 参考 `tests/unit/test_websocket.py` 和 `tests/unit/test_chat_plan_exec_ws.py` 的 WebSocket 测试模式
|
||||
3. 使用 `TestClient.websocket_connect("/api/v1/chat/ws/...")` 发送消息并断言响应事件
|
||||
4. Mock LLM gateway 和 agent 执行,聚焦 WebSocket 协议层测试
|
||||
5. 覆盖连接、消息处理、取消、错误事件场景
|
||||
|
||||
**Test scenarios**:
|
||||
- Happy path: 客户端发送 `{"type": "message", "content": "hello"}`,服务端返回 `connected` → `token`* → `final_answer` 事件序列
|
||||
- Ping/Pong: 客户端发送 `{"type": "ping"}`,服务端返回 `{"type": "pong"}`
|
||||
- Cancel: 客户端发送 `{"type": "cancel"}`,服务端停止处理并返回确认(或静默关闭)
|
||||
- Confirmation flow: 服务端发送 `confirmation_request`,客户端回复 `confirmation_reply`,服务端继续执行
|
||||
- Error handling: 无效消息格式时,服务端返回 `{"type": "error", "message": ...}`
|
||||
- Session isolation: 不同 `session_id` 的 WebSocket 连接互不干扰
|
||||
- Connection cleanup: 客户端断开后,服务端清理资源(无孤立 future)
|
||||
|
||||
**Verification**: `pytest tests/unit/test_chat_ws_routes.py -x -q` 全部通过
|
||||
|
||||
## Verification Contract
|
||||
|
||||
### 自动化验证
|
||||
|
||||
```bash
|
||||
# Lint
|
||||
ruff check src/agentkit/llm/exceptions.py src/agentkit/core/exceptions.py
|
||||
|
||||
# 单元测试(本次新增 + 受影响的现有测试)
|
||||
python3 -m pytest tests/unit/llm/test_migration.py tests/unit/test_memory_models.py tests/unit/test_chat_ws_routes.py -x -q
|
||||
|
||||
# 回归测试(确保异常迁移不破坏现有行为)
|
||||
python3 -m pytest tests/unit/test_llm_provider.py tests/unit/test_remote_provider.py tests/unit/test_llm_retry.py tests/unit/test_llm_gateway.py -x -q
|
||||
|
||||
# 全量单元测试(最终验证)
|
||||
python3 -m pytest tests/unit/ -x -q
|
||||
```
|
||||
|
||||
### Definition of Done
|
||||
|
||||
- [ ] U1: `llm/exceptions.py` 创建,`core/exceptions.py` 保留 re-export,`ruff check` 通过
|
||||
- [ ] U2: `tests/unit/llm/test_migration.py` 创建,所有测试场景通过
|
||||
- [ ] U3: `tests/unit/test_memory_models.py` 创建,所有测试场景通过
|
||||
- [ ] U4: `tests/unit/test_chat_ws_routes.py` 创建,所有测试场景通过
|
||||
- [ ] 全量 `pytest tests/unit/ -x -q` 通过(无回归)
|
||||
- [ ] `ruff check src/ && ruff format src/` 通过
|
||||
|
||||
## Scope Boundaries
|
||||
|
||||
### In scope
|
||||
|
||||
- 迁移 3 个 LLM 异常类到 `llm/exceptions.py`(Re-export 兼容)
|
||||
- 为 3 个测试缺口补充单元测试(llm/migration.py、memory/models.py、chat WebSocket)
|
||||
|
||||
### Out of scope
|
||||
|
||||
- 知识图谱 imports/tested_by 边补全
|
||||
- experts/_*.py mixin 直接单元测试
|
||||
- 大规模模块结构重组
|
||||
- 前端/桌面端测试补充
|
||||
- 已有充分测试的函数(filter_pii、generate_cache_key、detect_verification_commands)的测试增强
|
||||
|
||||
### Deferred to Follow-Up Work
|
||||
|
||||
- 知识图谱增量更新时补充 tests/ 目录扫描,提高 `tested_by` 边覆盖率
|
||||
- experts/_*.py mixin 类的直接单元测试(如需更细粒度覆盖时)
|
||||
- `core/exceptions.py` 中其他异常类的分层审查(如 ToolNotFoundError 是否应迁移到 tools/)
|
||||
|
|
@ -0,0 +1,298 @@
|
|||
---
|
||||
title: "Breaking circular imports with PEP 562 module-level __getattr__ lazy re-export"
|
||||
date: 2026-07-06
|
||||
category: docs/solutions/architecture-patterns/
|
||||
module:
|
||||
- agentkit.core.exceptions
|
||||
- agentkit.llm.exceptions
|
||||
- agentkit.llm.retry
|
||||
problem_type: architecture_pattern
|
||||
component: tooling
|
||||
severity: medium
|
||||
applies_when:
|
||||
- Two modules form an import cycle (A imports B, B imports A) at module-init time that simple reordering cannot resolve
|
||||
- Migrating exception or symbol classes to a new home module while preserving many legacy 'from old.path import X' call sites
|
||||
- Eager re-export at module top would re-introduce the cycle; PEP 562 lazy per-name __getattr__ breaks the cycle while keeping the legacy import path working
|
||||
tags: [circular-import, pep562, lazy-reexport, module-getattr, backward-compat, exception-hierarchy]
|
||||
---
|
||||
|
||||
# 用 PEP 562 模块级 `__getattr__` 延迟 re-export 打破循环依赖
|
||||
|
||||
## Context
|
||||
|
||||
`agentkit` 项目里 `core.exceptions` 与 `llm.exceptions` 之间存在一条隐蔽的循环依赖链路:
|
||||
|
||||
```
|
||||
core.exceptions → llm.exceptions → llm.__init__ → llm.retry → core.exceptions.LLMProviderError
|
||||
```
|
||||
|
||||
这条链路之所以会出现,根因是一个**模块分层异味**:LLM 相关异常(`LLMError` / `LLMProviderError` / `ModelNotFoundError`)原本被定义在 `src/agentkit/core/exceptions.py`,但它们的真正消费者几乎全部位于 `llm/` 子系统内(`llm/retry.py:10` 通过 `from agentkit.core.exceptions import LLMProviderError` 拿到基类)。换句话说,core 层在为 llm 层"代持"异常定义,而 llm 层又反过来要被 core 引用——这就构成了**反向依赖**。
|
||||
|
||||
链路在常规顺序导入时不会暴露问题,但当 Python 解释器在 `core.exceptions` 部分初始化阶段(即 `core.exceptions` 还没把 `LLMProviderError` 绑定到模块命名空间)被拉起 `llm.__init__` 时,`llm.retry` 拿到的就是一个**未绑定符号**,触发 `ImportError`。这种 part-initialized failure 极难复现,往往只在某些导入顺序、测试收集顺序或打包工具的 import graph 重排后才显形,是典型的"代码味道"演化为"运行时炸弹"。
|
||||
|
||||
更棘手的是,`from agentkit.core.exceptions import LLMProviderError` 这一契约已经被 27 个调用方依赖。任何"把异常类直接搬走"的迁移方案,如果不提供 re-export 兼容层,都会触发大面积 `ImportError`,回归测试会瞬间失败。因此我们需要一种**既能搬走定义、又能在原路径透明访问**的手段。
|
||||
|
||||
## Guidance
|
||||
|
||||
推荐模式:**把定义迁移到正确的层级,再用 PEP 562 模块级 `__getattr__` 在原路径做延迟 re-export**。这是 Python 3.7+ 标准库原生能力,无需任何新依赖,也无需引入 lazy-import 框架。
|
||||
|
||||
### 步骤一:把异常定义搬到与消费者同层
|
||||
|
||||
新建 `src/agentkit/llm/exceptions.py`,作为 LLM 异常的新家。**顶层只 import 已经完全加载好的 `AgentFrameworkError`**,不在 `llm.exceptions` 内反向 import `core.exceptions` 的任何 LLM 符号,避免把循环又请回来:
|
||||
|
||||
```python
|
||||
# src/agentkit/llm/exceptions.py
|
||||
"""LLM 子系统的异常定义。
|
||||
|
||||
放在这里而不是 core.exceptions,因为这些异常的唯一消费者
|
||||
是 llm/ 子系统;core 层不应为 llm 层代持定义。
|
||||
"""
|
||||
from agentkit.core.exceptions import AgentFrameworkError
|
||||
|
||||
|
||||
class LLMError(AgentFrameworkError):
|
||||
"""LLM 调用相关的所有异常基类。"""
|
||||
|
||||
|
||||
class LLMProviderError(LLMError):
|
||||
"""Provider 配置或不可用导致的错误。"""
|
||||
|
||||
|
||||
class ModelNotFoundError(LLMError):
|
||||
"""请求的模型在当前 provider 中不存在。"""
|
||||
```
|
||||
|
||||
### 步骤二:在原路径用 PEP 562 `__getattr__` 延迟 re-export
|
||||
|
||||
修改 `src/agentkit/core/exceptions.py`,删除三个 LLM 异常类的原定义,改用模块级 `__getattr__`:
|
||||
|
||||
```python
|
||||
# src/agentkit/core/exceptions.py(节选)
|
||||
# 已删除 LLMError / LLMProviderError / ModelNotFoundError 的本地定义
|
||||
# 改为延迟 re-export,保持 27 个调用方的 import 契约不变
|
||||
|
||||
_LLM_EXCEPTION_NAMES = ("LLMError", "LLMProviderError", "ModelNotFoundError")
|
||||
|
||||
|
||||
def __getattr__(name: str): # PEP 562 模块级 __getattr__
|
||||
if name in _LLM_EXCEPTION_NAMES:
|
||||
# 首次访问时才 import llm.exceptions,避免模块加载期触发循环
|
||||
import agentkit.llm.exceptions as _llm_exc # noqa: PLC0415
|
||||
# 一次性把三个类都缓存进 globals(),后续访问直接命中
|
||||
for n in _LLM_EXCEPTION_NAMES:
|
||||
globals()[n] = getattr(_llm_exc, n)
|
||||
return globals()[name]
|
||||
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
|
||||
|
||||
|
||||
def __dir__():
|
||||
# 让 dir(core.exceptions) 和 IDE 自动补全仍然看到这三个名字
|
||||
return [*(globals().keys()), *_LLM_EXCEPTION_NAMES]
|
||||
```
|
||||
|
||||
### 步骤三:让 `llm/__init__.py` 直接从新家 re-export
|
||||
|
||||
```python
|
||||
# src/agentkit/llm/__init__.py(追加)
|
||||
from agentkit.llm.exceptions import LLMError, LLMProviderError, ModelNotFoundError
|
||||
|
||||
__all__ = [*__all__, "LLMError", "LLMProviderError", "ModelNotFoundError"]
|
||||
```
|
||||
|
||||
### 步骤四:写守护测试锁定契约
|
||||
|
||||
`tests/unit/test_core_exceptions_reexport.py`(6 个测试)——这一步**不可省略**。re-export 兼容性的失败模式非常安静(`is` identity 漂移、`__module__` 变化、`__dir__` 缺项),没有自动化守护就会在几个月后悄悄回退:
|
||||
|
||||
```python
|
||||
def test_reexport_is_identity():
|
||||
# 跨路径访问必须是同一个类对象,否则 isinstance 双重判断会裂开
|
||||
from agentkit.core.exceptions import LLMProviderError as A
|
||||
from agentkit.llm.exceptions import LLMProviderError as B
|
||||
assert A is B
|
||||
|
||||
|
||||
def test_reexport_isinstance_cross_path():
|
||||
# 实例在任一路径 isinstance 都成立
|
||||
from agentkit.core.exceptions import LLMProviderError
|
||||
from agentkit.llm.exceptions import LLMError
|
||||
assert isinstance(LLMProviderError("x"), LLMError)
|
||||
|
||||
|
||||
def test_unknown_name_raises_attribute_error():
|
||||
import agentkit.core.exceptions as ce
|
||||
with pytest.raises(AttributeError):
|
||||
ce.NotARealException
|
||||
|
||||
|
||||
def test_dir_contains_reexported_names():
|
||||
import agentkit.core.exceptions as ce
|
||||
assert "LLMProviderError" in dir(ce)
|
||||
|
||||
|
||||
def test_cached_after_first_access():
|
||||
# 首次访问后写入 globals(),第二次访问不再触发 __getattr__
|
||||
import agentkit.core.exceptions as ce
|
||||
_ = ce.LLMProviderError # 触发懒加载
|
||||
assert "LLMProviderError" in ce.__dict__ # 已缓存
|
||||
|
||||
|
||||
def test_inheritance_chain_preserved():
|
||||
from agentkit.core.exceptions import LLMProviderError, AgentFrameworkError
|
||||
assert issubclass(LLMProviderError, AgentFrameworkError)
|
||||
```
|
||||
|
||||
## Why This Matters
|
||||
|
||||
### 为什么 PEP 562 `__getattr__` 能打破循环
|
||||
|
||||
Python 模块级 `__getattr__`(PEP 562,3.7+)的关键性质是:**它只在属性访问时触发,不在模块加载时执行 import**。这与顶层 `from ... import ...` 形成对照——后者在模块字节码执行阶段就立即触发依赖解析,正是它把循环依赖变成 `ImportError` 的根源。
|
||||
|
||||
把循环链路拆解来看:
|
||||
|
||||
1. 解释器开始加载 `core.exceptions`,执行到文件底部 PEP 562 定义——此时 `__getattr__` **只是注册了一个函数对象**,不会调用,也不会触发任何 import。
|
||||
2. `core.exceptions` 完全加载完毕,`AgentFrameworkError`(core 顶层定义)已绑定。
|
||||
3. 后续某处 `from agentkit.core.exceptions import LLMProviderError` 触发 `__getattr__("LLMProviderError")`。
|
||||
4. 此时 `agentkit.llm` 通常已经在 `sys.modules` 中(即使部分加载,子模块 `llm.exceptions` 仍可独立 import);`import agentkit.llm.exceptions` 不会重新执行 `llm/__init__.py`,而是直接拿到子模块对象。
|
||||
5. `llm.exceptions` 顶层只需要 `AgentFrameworkError`——它在步骤 2 就已就绪,所以 import 成功,不会回到 `core.exceptions` 找未绑定符号。
|
||||
|
||||
整条链路从"加载期硬解析"退化为"访问期软解析",循环就断了。
|
||||
|
||||
### 一次性缓存机制
|
||||
|
||||
注意 `__getattr__` 内部用的是 `for n in _LLM_EXCEPTION_NAMES: globals()[n] = ...`,**一次性把三个类都写入模块 globals**,而不是只写当前请求的那个。这样做有两个好处:
|
||||
|
||||
- **后续访问零开销**:第二次 `core.exceptions.LLMProviderError` 直接命中 `globals()`,不再进入 `__getattr__`,等价于普通模块属性。
|
||||
- **避免半缓存状态**:如果只缓存当前请求的名字,访问 `LLMError` 后 `LLMProviderError` 仍需走 `__getattr__`,行为不对称,调试时容易误判。批量缓存让"已懒加载"成为一个布尔状态,而不是逐名字的状态机。
|
||||
|
||||
### 向后兼容的价值:27 调用方保护
|
||||
|
||||
`from agentkit.core.exceptions import LLMProviderError` 这个契约被 27 个调用方依赖,散落在 `llm/`、`tools/`、`server/`、`tests/` 等多个子系统。如果直接搬走定义而不做 re-export,这 27 处会同时报 `ImportError`,回归测试雪崩。PEP 562 方案让所有现有 import 语句**一字不改**地继续工作——这是该模式相对于"批量重写 import"的核心收益。
|
||||
|
||||
### `__module__` 变化的取舍
|
||||
|
||||
ce-code-review 的 9 个 reviewer 共识指出一个**已知副作用**:`LLMProviderError.__module__` 会从 `agentkit.core.exceptions` 变为 `agentkit.llm.exceptions`。这会影响 `pickle` 序列化路径和部分 observability 工具的异常归类。在本项目中,异常不跨进程 pickle、内部无 observability 消费者依赖 `__module__`,因此这是一个**可接受的取舍**。如果你的项目有跨进程 pickle 异常或依赖 `__module__` 的遥测管线,应在迁移前显式评估这一项。
|
||||
|
||||
### 传递依赖面扩大
|
||||
|
||||
另一个 reviewer 共识是:迁移后 `import agentkit.core` 的传递依赖面会扩大到整个 LLM 栈(因为 `__getattr__` 触发时会拉起 `llm.exceptions`,进而可能拉起 `llm.__init__`)。在 agentkit 中这是可接受的——core 与 llm 在同一进程、同一打包单元。如果你的项目把 core 做成"轻量可独立分发"的包,应评估这条传递依赖是否破坏 packaging 边界。
|
||||
|
||||
## When to Apply
|
||||
|
||||
满足以下任一条件时,优先考虑 PEP 562 延迟 re-export 模式:
|
||||
|
||||
- **模块分层异味**:某个层(如 core)在为另一个层(如 llm)"代持"定义,而定义的唯一消费者在后者,构成反向依赖。
|
||||
- **循环依赖链路**:`A → B → A` 的 import 链路在部分初始化时触发 `ImportError`,且无法通过简单的"把 import 移到函数内"消除(因为 import 出现在模块顶层类定义的基类位置)。
|
||||
- **需要保持 re-export 兼容性**:原 import 路径已被大量调用方依赖,无法或不希望批量重写。
|
||||
- **不想引入新依赖**:项目希望用标准库能力解决问题,避免 lazy-import 第三方库(如 `lazy-object-proxy`)。
|
||||
- **Python >= 3.7**:PEP 562 的硬性前置条件。agentkit 要求 3.11+,自然满足。
|
||||
|
||||
**不适用**的场景:
|
||||
|
||||
- 循环依赖可以通过**重新切分模块边界**(把共享定义下沉到更底层)干净消除时——优先重新切分,PEP 562 是兼容性约束下的退路。
|
||||
- 循环链路里没有"代持"问题,纯粹是双向业务依赖时——应重构为依赖注入或事件总线,而不是用 re-export 掩盖。
|
||||
- 需要跨进程 pickle 异常且 `__module__` 必须保持原值时——PEP 562 re-export 会改 `__module__`,需要额外用 `__reduce__` 或元类技巧补回,复杂度通常不值。
|
||||
|
||||
## Examples
|
||||
|
||||
### Before:原定义在 core,循环链路存在
|
||||
|
||||
```python
|
||||
# src/agentkit/core/exceptions.py(旧版本,问题代码)
|
||||
class AgentFrameworkError(Exception):
|
||||
...
|
||||
|
||||
|
||||
# 三个 LLM 异常定义在 core 层 —— 分层异味
|
||||
class LLMError(AgentFrameworkError):
|
||||
...
|
||||
|
||||
|
||||
class LLMProviderError(LLMError):
|
||||
...
|
||||
|
||||
|
||||
class ModelNotFoundError(LLMError):
|
||||
...
|
||||
```
|
||||
|
||||
```python
|
||||
# src/agentkit/llm/retry.py(旧版本,问题代码)
|
||||
from agentkit.core.exceptions import LLMProviderError # ← 顶层 import,触发循环
|
||||
|
||||
def with_retry(fn):
|
||||
...
|
||||
raise LLMProviderError(...)
|
||||
```
|
||||
|
||||
导入顺序不吉利时:`core.exceptions` 部分初始化 → 拉起 `llm.__init__` → `llm.retry` 顶层 import `LLMProviderError` → 该名字尚未绑定 → `ImportError`。
|
||||
|
||||
### After:定义迁移到 llm,core 用 PEP 562 re-export
|
||||
|
||||
```python
|
||||
# src/agentkit/llm/exceptions.py(新建)
|
||||
from agentkit.core.exceptions import AgentFrameworkError
|
||||
|
||||
|
||||
class LLMError(AgentFrameworkError):
|
||||
"""LLM 调用相关异常基类。"""
|
||||
|
||||
|
||||
class LLMProviderError(LLMError):
|
||||
"""Provider 配置或不可用错误。"""
|
||||
|
||||
|
||||
class ModelNotFoundError(LLMError):
|
||||
"""模型在 provider 中不存在。"""
|
||||
```
|
||||
|
||||
```python
|
||||
# src/agentkit/core/exceptions.py(修改后)
|
||||
class AgentFrameworkError(Exception):
|
||||
...
|
||||
|
||||
|
||||
# 已迁移走的 LLM 异常 —— 用 PEP 562 延迟 re-export 保持兼容
|
||||
_LLM_EXCEPTION_NAMES = ("LLMError", "LLMProviderError", "ModelNotFoundError")
|
||||
|
||||
|
||||
def __getattr__(name: str):
|
||||
if name in _LLM_EXCEPTION_NAMES:
|
||||
import agentkit.llm.exceptions as _llm_exc # noqa: PLC0415
|
||||
for n in _LLM_EXCEPTION_NAMES:
|
||||
globals()[n] = getattr(_llm_exc, n)
|
||||
return globals()[name]
|
||||
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
|
||||
|
||||
|
||||
def __dir__():
|
||||
return [*(globals().keys()), *_LLM_EXCEPTION_NAMES]
|
||||
```
|
||||
|
||||
```python
|
||||
# src/agentkit/llm/retry.py(不变!27 个调用方之一,零修改)
|
||||
from agentkit.core.exceptions import LLMProviderError # 仍然 work
|
||||
```
|
||||
|
||||
### 守护测试片段
|
||||
|
||||
```python
|
||||
# tests/unit/test_core_exceptions_reexport.py
|
||||
def test_reexport_is_identity():
|
||||
from agentkit.core.exceptions import LLMProviderError as A
|
||||
from agentkit.llm.exceptions import LLMProviderError as B
|
||||
assert A is B # 同一个类对象
|
||||
|
||||
|
||||
def test_dir_contains_reexported_names():
|
||||
import agentkit.core.exceptions as ce
|
||||
assert "LLMProviderError" in dir(ce) # IDE / 自动补全友好
|
||||
```
|
||||
|
||||
完整守护测试覆盖 6 个维度:`is` identity、跨路径 `isinstance`、未知名 `AttributeError`、`__dir__` 完整性、首次访问后缓存命中、继承链保持。这 6 个维度对应了 re-export 模式所有已知的安静失败模式——任何一个回退都会被测试抓住。
|
||||
|
||||
## Related
|
||||
|
||||
- ce-code-review 9 reviewer 共识:架构修复正确,关键风险集中在 `__module__` 变化与 `import agentkit.core` 传递依赖面扩大;7 reviewer 共识要求 re-export identity 契约必须有自动化守护测试。
|
||||
- ce-simplify-code 简化:初版用 3 个独立 `import ... as ...; globals()[...] = ...`,简化为 for 循环 + `getattr`;测试 helper 提取 3 个 `_assert_*` 函数。
|
||||
- PEP 562(Module level `__getattr__` and `__dir__`):https://peps.python.org/pep-0562/
|
||||
- 项目约定:所有 Pydantic 模型用 `model_config = ConfigDict(...)`、Python >= 3.11、`ruff check src/ && ruff format src/`(行宽 100)。
|
||||
|
|
@ -0,0 +1,98 @@
|
|||
---
|
||||
title: "Gate without caller silently disables feature"
|
||||
date: 2026-07-06
|
||||
module: experts/orchestrator, core/reflexion
|
||||
tags: [gate, trigger_reason, dead-code, design-pattern, code-review]
|
||||
problem_type: logic_error
|
||||
severity: P1
|
||||
resolution_type: code_fix
|
||||
category: logic-errors
|
||||
---
|
||||
|
||||
# Gate without caller silently disables feature
|
||||
|
||||
## Problem
|
||||
|
||||
`_decompose_task` 新增了 `trigger_reason: str | None = None` 参数和门控逻辑,用于控制 `retrieve_prompt_reflection` 的调用。但两个生产调用方都不传 `trigger_reason`,导致门控条件恒为 False,功能被静默禁用。
|
||||
|
||||
## Symptoms
|
||||
|
||||
- `retrieve_prompt_reflection` 在生产中永远不会被调用(死代码)
|
||||
- 无错误日志,无异常 — 功能只是"消失"了
|
||||
- 单元测试全部通过(因为测试直接传入 `trigger_reason="verify_retry"`,未覆盖生产调用路径)
|
||||
- 修复前是"每次分解都检索"(浪费 token),修复后变成"永不检索"(功能丢失)
|
||||
|
||||
## What Didn't Work
|
||||
|
||||
1. **单元测试无法发现**:测试直接调用 `_decompose_task(..., trigger_reason="verify_retry")`,验证门控逻辑本身正确,但未覆盖生产调用路径(两个调用方都不传 trigger_reason)
|
||||
2. **功能测试无法发现**:`retrieve_prompt_reflection` 返回空时不报错,只是降级到默认 prompt,行为差异不明显
|
||||
3. **类型检查无法发现**:`trigger_reason` 有默认值 `None`,类型检查通过
|
||||
|
||||
## Solution
|
||||
|
||||
在 `_rebalance` 路径传入 `trigger_reason="loop_detection"`,使重新规划时检索历史反思:
|
||||
|
||||
```python
|
||||
# 修复前 — 两个调用方都不传 trigger_reason
|
||||
phases = await self._decompose_task(lead, task) # 正常分解
|
||||
new_phases = await self._decompose_task(lead, augmented_task) # 重分解
|
||||
|
||||
# 修复后 — _rebalance 路径传入 trigger_reason
|
||||
new_phases = await self._decompose_task(
|
||||
lead, augmented_task, trigger_reason="loop_detection"
|
||||
)
|
||||
```
|
||||
|
||||
并在正常分解路径添加注释说明 verify/schema 重试路径待接入。
|
||||
|
||||
## Why This Works
|
||||
|
||||
门控设计的核心假设是"有调用方会传入触发条件"。当这个假设不成立时,门控会静默禁用功能:
|
||||
|
||||
- `trigger_reason: str | None = None` — 默认 None
|
||||
- `None not in frozenset({"verify_retry", ...})` — 门控条件恒为 False
|
||||
- 无错误、无日志、无异常 — 功能只是"消失"了
|
||||
|
||||
修复方式是确保至少有一个调用方传入触发条件。`_rebalance` 是重新规划路径,传入 `loop_detection` 语义合理(检测到规划问题后重新规划)。
|
||||
|
||||
## Prevention
|
||||
|
||||
### 1. 门控设计检查清单
|
||||
|
||||
设计门控/开关时,必须:
|
||||
- 列出所有调用方,确保至少有一个会传入触发条件
|
||||
- 添加断言或日志:如果门控条件从未为 True,发出警告
|
||||
- 在 docstring 中明确说明"调用方必须传入 X 才能触发 Y"
|
||||
|
||||
### 2. 测试覆盖生产调用路径
|
||||
|
||||
```python
|
||||
# 不要只测试门控逻辑本身
|
||||
async def test_gate_with_trigger():
|
||||
await decompose_task(lead, task, trigger_reason="verify_retry")
|
||||
assert retrieve_called # 这只验证门控逻辑
|
||||
|
||||
# 还要测试生产调用路径
|
||||
async def test_production_call_path_does_not_silently_disable():
|
||||
# 模拟生产调用(不传 trigger_reason)
|
||||
await decompose_task(lead, task)
|
||||
# 如果功能应该被禁用,assert 它被禁用
|
||||
# 如果功能不应该被禁用,assert 它仍然工作
|
||||
```
|
||||
|
||||
### 3. 代码审查重点
|
||||
|
||||
审查门控逻辑时,追踪所有调用方:
|
||||
- 门控条件是否可达?
|
||||
- 是否有调用方传入触发条件?
|
||||
- 默认值是否会导致功能被静默禁用?
|
||||
|
||||
### 4. 死代码检测
|
||||
|
||||
定期检查从未执行的代码路径。`retrieve_prompt_reflection` 如果从未被调用,是死代码的信号。
|
||||
|
||||
## Related
|
||||
|
||||
- `docs/solutions/runtime-errors/async-generator-yield-before-await-deadlock.md` — 同一 IQ-boost 工作中的另一个非显而易见 bug
|
||||
- PR #27 (feat/agent-iq-boost) — 原始 trigger_reason 门控引入
|
||||
- PR fix/agent-iq-boost-gaps — 本次修复
|
||||
|
|
@ -132,24 +132,27 @@ class EvolutionError(AgentFrameworkError):
|
|||
super().__init__(f"Evolution failed for agent '{agent_name}': {reason}")
|
||||
|
||||
|
||||
class LLMError(AgentFrameworkError):
|
||||
"""LLM 基础异常"""
|
||||
|
||||
def __init__(self, message: str = "LLM error"):
|
||||
super().__init__(message)
|
||||
# Re-export LLM exceptions from llm/exceptions.py for backward compatibility.
|
||||
# New code should import from agentkit.llm.exceptions directly.
|
||||
# ponytail: 架构分层修复 — LLM 异常定义归位 LLM 层;re-export 保留以避免破坏
|
||||
# 现有 `from agentkit.core.exceptions import LLMProviderError` 调用方。
|
||||
# 升级路径:调用方逐步迁移到 llm.exceptions 后可移除 __getattr__。
|
||||
#
|
||||
# 用 PEP 562 模块级 __getattr__ 延迟导入,打破循环依赖:
|
||||
# core.exceptions → llm.exceptions → llm.__init__ → llm.retry → core.exceptions.LLMProviderError
|
||||
# (若顶层 import,LLMProviderError 尚未绑定 → ImportError)
|
||||
_LLM_EXCEPTION_NAMES = ("LLMError", "LLMProviderError", "ModelNotFoundError")
|
||||
|
||||
|
||||
class LLMProviderError(LLMError):
|
||||
"""LLM Provider 特定异常"""
|
||||
def __getattr__(name: str): # PEP 562
|
||||
if name in _LLM_EXCEPTION_NAMES:
|
||||
import agentkit.llm.exceptions as _llm_exc # noqa: PLC0415
|
||||
|
||||
def __init__(self, provider: str, reason: str = ""):
|
||||
self.provider = provider
|
||||
super().__init__(f"LLM provider '{provider}' error: {reason}")
|
||||
for n in _LLM_EXCEPTION_NAMES:
|
||||
globals()[n] = getattr(_llm_exc, n)
|
||||
return globals()[name]
|
||||
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
|
||||
|
||||
|
||||
class ModelNotFoundError(LLMError):
|
||||
"""模型别名未找到异常"""
|
||||
|
||||
def __init__(self, model: str):
|
||||
self.model = model
|
||||
super().__init__(f"Model not found: {model}")
|
||||
def __dir__(): # 让 from agentkit.core.exceptions import * 仍能发现 re-export
|
||||
return [*(globals().keys()), *_LLM_EXCEPTION_NAMES]
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ from agentkit.llm.providers.anthropic import AnthropicProvider
|
|||
from agentkit.llm.providers.openai import OpenAICompatibleProvider
|
||||
from agentkit.llm.providers.tracker import UsageSummary, UsageTracker
|
||||
from agentkit.llm.providers.usage_store import UsageRecord
|
||||
from agentkit.llm.exceptions import LLMError, LLMProviderError, ModelNotFoundError
|
||||
from agentkit.llm.remote_provider import RemoteLLMProvider
|
||||
from agentkit.llm.retry import (
|
||||
CircuitBreaker,
|
||||
|
|
@ -38,4 +39,7 @@ __all__ = [
|
|||
"UsageTracker",
|
||||
"UsageRecord",
|
||||
"UsageSummary",
|
||||
"LLMError",
|
||||
"LLMProviderError",
|
||||
"ModelNotFoundError",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -0,0 +1,37 @@
|
|||
"""LLM 层异常定义。
|
||||
|
||||
架构分层原则:LLM 异常定义在 LLM 层,而非 core/exceptions.py。
|
||||
core/exceptions.py 保留 re-export 以保持向后兼容。
|
||||
"""
|
||||
|
||||
from agentkit.core.exceptions import AgentFrameworkError
|
||||
|
||||
|
||||
class LLMError(AgentFrameworkError):
|
||||
"""LLM 基础异常"""
|
||||
|
||||
def __init__(self, message: str = "LLM error"):
|
||||
super().__init__(message)
|
||||
|
||||
|
||||
class LLMProviderError(LLMError):
|
||||
"""LLM Provider 特定异常"""
|
||||
|
||||
def __init__(self, provider: str, reason: str = ""):
|
||||
self.provider = provider
|
||||
super().__init__(f"LLM provider '{provider}' error: {reason}")
|
||||
|
||||
|
||||
class ModelNotFoundError(LLMError):
|
||||
"""模型别名未找到异常"""
|
||||
|
||||
def __init__(self, model: str):
|
||||
self.model = model
|
||||
super().__init__(f"Model not found: {model}")
|
||||
|
||||
|
||||
__all__ = [
|
||||
"LLMError",
|
||||
"LLMProviderError",
|
||||
"ModelNotFoundError",
|
||||
]
|
||||
|
|
@ -0,0 +1,223 @@
|
|||
"""U15 — migrate_api_keys_to_secrets 顶层函数单元测试。
|
||||
|
||||
测试 ``src/agentkit/llm/migration.py`` 中的 ``migrate_api_keys_to_secrets(config_path)``:
|
||||
读取 ``agentkit.yaml``,把每个 LLM provider 的 plaintext ``api_key`` 迁移到
|
||||
``SecretsStore`` 加密存储,并把更新后的 providers 段写回 YAML。
|
||||
|
||||
覆盖场景:
|
||||
1. Happy path — plaintext → secrets_store,YAML 写回,report 标记 migrated
|
||||
2. Idempotent — 已迁移配置再次调用返回 skipped,不重复加密
|
||||
3. Empty config — 无 providers 时返回 {},YAML 仍可读
|
||||
4. Source missing — config_path 不存在时抛 FileNotFoundError(函数未捕获)
|
||||
5. Secrets store write failure — set_secret 抛异常时 status=failed,plaintext 保留(事务性)
|
||||
6. Dual source re-migrate — api_key_source=dual 时重新迁移覆盖旧 encrypted 值
|
||||
|
||||
隔离策略:
|
||||
- 用 ``tmp_path`` 隔离 YAML 文件 I/O
|
||||
- patch ``agentkit.channels.secrets.SecretsStore`` 注入可控实例(避免 env 依赖)
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
from agentkit.channels.secrets import SecretsStore
|
||||
from agentkit.llm.migration import migrate_api_keys_to_secrets
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# 测试辅助
|
||||
# ----------------------------------------------------------------------
|
||||
|
||||
|
||||
def _write_config(path: Path, providers: dict[str, dict[str, object]]) -> None:
|
||||
"""写一个最小的 agentkit.yaml(仅 llm.providers 段)。"""
|
||||
data: dict[str, dict[str, object]] = {"llm": {"providers": providers}}
|
||||
path.write_text(
|
||||
yaml.safe_dump(data, allow_unicode=True, sort_keys=False),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
def _make_real_store() -> SecretsStore:
|
||||
"""构造一个真实可用的 SecretsStore(注入固定 master_key,不依赖 env)。"""
|
||||
return SecretsStore(master_key=b"x" * 32)
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# 1. Happy path
|
||||
# ----------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_happy_path_migrates_plaintext_to_secrets_store(tmp_path: Path) -> None:
|
||||
"""plaintext api_key → secrets_store;YAML 写回,report 标记 migrated。"""
|
||||
config_path = tmp_path / "agentkit.yaml"
|
||||
_write_config(
|
||||
config_path,
|
||||
{
|
||||
"openai": {
|
||||
"api_key": "sk-secret",
|
||||
"base_url": "https://api.openai.com/v1",
|
||||
"type": "openai",
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
with patch("agentkit.channels.secrets.SecretsStore", return_value=_make_real_store()):
|
||||
report = migrate_api_keys_to_secrets(config_path)
|
||||
|
||||
assert report == {"openai": {"status": "migrated", "source": "secrets_store"}}
|
||||
|
||||
# YAML 应已更新:plaintext 清空,encrypted 写入,source 标记
|
||||
raw = yaml.safe_load(config_path.read_text(encoding="utf-8"))
|
||||
pconf = raw["llm"]["providers"]["openai"]
|
||||
assert pconf["api_key"] == ""
|
||||
assert pconf["api_key_encrypted"] is not None
|
||||
assert pconf["api_key_source"] == "secrets_store"
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# 2. Idempotent
|
||||
# ----------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_idempotent_second_call_returns_skipped(tmp_path: Path) -> None:
|
||||
"""已迁移的配置再次调用 → skipped,不重复加密。"""
|
||||
config_path = tmp_path / "agentkit.yaml"
|
||||
_write_config(
|
||||
config_path,
|
||||
{
|
||||
"openai": {
|
||||
"api_key": "sk-secret",
|
||||
"base_url": "https://api.openai.com/v1",
|
||||
"type": "openai",
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
with patch("agentkit.channels.secrets.SecretsStore", return_value=_make_real_store()):
|
||||
first_report = migrate_api_keys_to_secrets(config_path)
|
||||
second_report = migrate_api_keys_to_secrets(config_path)
|
||||
|
||||
assert first_report == {"openai": {"status": "migrated", "source": "secrets_store"}}
|
||||
assert second_report == {"openai": {"status": "skipped", "source": "secrets_store"}}
|
||||
|
||||
# 第二次调用后 YAML 仍一致(plaintext 仍为空)
|
||||
raw = yaml.safe_load(config_path.read_text(encoding="utf-8"))
|
||||
pconf = raw["llm"]["providers"]["openai"]
|
||||
assert pconf["api_key"] == ""
|
||||
assert pconf["api_key_source"] == "secrets_store"
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# 3. Empty config
|
||||
# ----------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_empty_config_returns_empty_report(tmp_path: Path) -> None:
|
||||
"""无 providers 时返回 {},YAML 仍可读且 providers 段为空。"""
|
||||
config_path = tmp_path / "agentkit.yaml"
|
||||
config_path.write_text("llm:\n providers: {}\n", encoding="utf-8")
|
||||
|
||||
with patch("agentkit.channels.secrets.SecretsStore", return_value=_make_real_store()):
|
||||
report = migrate_api_keys_to_secrets(config_path)
|
||||
|
||||
assert report == {}
|
||||
|
||||
raw = yaml.safe_load(config_path.read_text(encoding="utf-8"))
|
||||
assert raw["llm"]["providers"] == {}
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# 4. Source missing
|
||||
# ----------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_source_missing_raises_file_not_found(tmp_path: Path) -> None:
|
||||
"""config_path 不存在时抛 FileNotFoundError(函数未捕获,向上传播)。"""
|
||||
missing = tmp_path / "nonexistent.yaml"
|
||||
with pytest.raises(FileNotFoundError):
|
||||
migrate_api_keys_to_secrets(missing)
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# 5. Secrets store write failure
|
||||
# ----------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_secrets_store_write_failure_marks_failed_and_retains_plaintext(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""set_secret 抛异常 → status=failed,plaintext 保留(事务性,不部分迁移)。"""
|
||||
config_path = tmp_path / "agentkit.yaml"
|
||||
_write_config(
|
||||
config_path,
|
||||
{
|
||||
"openai": {
|
||||
"api_key": "sk-secret",
|
||||
"base_url": "",
|
||||
"type": "openai",
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
# mock store:set_secret 抛异常(get_secret 不会被调用到)
|
||||
fake_store = MagicMock(spec=SecretsStore)
|
||||
fake_store.set_secret = AsyncMock(side_effect=RuntimeError("redis down"))
|
||||
|
||||
with patch("agentkit.channels.secrets.SecretsStore", return_value=fake_store):
|
||||
report = migrate_api_keys_to_secrets(config_path)
|
||||
|
||||
assert report == {"openai": {"status": "failed", "source": "plaintext", "error": "redis down"}}
|
||||
|
||||
# plaintext 应保留(未清空),source 仍为 plaintext
|
||||
raw = yaml.safe_load(config_path.read_text(encoding="utf-8"))
|
||||
pconf = raw["llm"]["providers"]["openai"]
|
||||
assert pconf["api_key"] == "sk-secret"
|
||||
assert pconf["api_key_source"] == "plaintext"
|
||||
|
||||
# set_secret 被调用一次(验证读 get_secret 未执行)
|
||||
fake_store.set_secret.assert_awaited_once()
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# 6. Dual source re-migrate (secret overwrite path)
|
||||
# ----------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_dual_source_re_migrates_and_overwrites_encrypted(tmp_path: Path) -> None:
|
||||
"""api_key_source=dual(已有 encrypted 但 plaintext 保留)→ 重新迁移覆盖旧值。"""
|
||||
stale_encrypted = (
|
||||
'{"key":"llm:provider:openai:api_key","value":"old","nonce":"n","salt":"s",'
|
||||
'"key_id":"default","created_at":"","updated_at":""}'
|
||||
)
|
||||
config_path = tmp_path / "agentkit.yaml"
|
||||
_write_config(
|
||||
config_path,
|
||||
{
|
||||
"openai": {
|
||||
"api_key": "sk-secret",
|
||||
"base_url": "",
|
||||
"type": "openai",
|
||||
"api_key_encrypted": stale_encrypted,
|
||||
"api_key_source": "dual",
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
with patch("agentkit.channels.secrets.SecretsStore", return_value=_make_real_store()):
|
||||
report = migrate_api_keys_to_secrets(config_path)
|
||||
|
||||
assert report == {"openai": {"status": "migrated", "source": "secrets_store"}}
|
||||
|
||||
raw = yaml.safe_load(config_path.read_text(encoding="utf-8"))
|
||||
pconf = raw["llm"]["providers"]["openai"]
|
||||
assert pconf["api_key"] == ""
|
||||
assert pconf["api_key_source"] == "secrets_store"
|
||||
# encrypted 应被新值覆盖(不等于原来的 stale 占位)
|
||||
assert pconf["api_key_encrypted"] != stale_encrypted
|
||||
assert pconf["api_key_encrypted"] is not None
|
||||
|
|
@ -0,0 +1,348 @@
|
|||
"""Unit tests for chat.py WebSocket route — message handling flows.
|
||||
|
||||
Tests the WebSocket endpoint at /api/v1/chat/ws/{session_id}, focusing on
|
||||
the message handling protocol: connected event, ping/pong, cancel,
|
||||
API key auth, session validation, happy-path message → final_answer,
|
||||
confirmation request/reply flow, invalid JSON handling, session isolation,
|
||||
and connection cleanup.
|
||||
|
||||
The LLM gateway and agent execution are mocked — these tests focus on the
|
||||
WebSocket protocol layer, not real LLM calls.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI, WebSocket
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from agentkit.core.protocol import CancellationToken
|
||||
from agentkit.session.manager import SessionManager
|
||||
from agentkit.session.store import InMemorySessionStore
|
||||
|
||||
|
||||
# ── Stub handlers (match _handle_chat_message signature) ────────────────
|
||||
#
|
||||
# 10 params — all required by the call site in chat_websocket which passes
|
||||
# them positionally. Full signature avoids *args (no Any usage).
|
||||
|
||||
|
||||
async def _stub_final_answer(
|
||||
websocket: WebSocket,
|
||||
session_id: str,
|
||||
content: str,
|
||||
sm: SessionManager,
|
||||
cancellation_token: CancellationToken,
|
||||
pending_replies: dict[str, asyncio.Future],
|
||||
pending_confirmations: dict[str, asyncio.Future] | None = None,
|
||||
pending_spec_reviews: dict[str, asyncio.Future] | None = None,
|
||||
pending_autonomy_resumes: dict[str, asyncio.Future] | None = None,
|
||||
model_override: str | None = None,
|
||||
) -> None:
|
||||
"""Stub that immediately sends a final_answer event."""
|
||||
await websocket.send_json(
|
||||
{"type": "final_answer", "content": f"echo: {content}", "is_final": True}
|
||||
)
|
||||
|
||||
|
||||
async def _stub_with_confirmation(
|
||||
websocket: WebSocket,
|
||||
session_id: str,
|
||||
content: str,
|
||||
sm: SessionManager,
|
||||
cancellation_token: CancellationToken,
|
||||
pending_replies: dict[str, asyncio.Future],
|
||||
pending_confirmations: dict[str, asyncio.Future] | None = None,
|
||||
pending_spec_reviews: dict[str, asyncio.Future] | None = None,
|
||||
pending_autonomy_resumes: dict[str, asyncio.Future] | None = None,
|
||||
model_override: str | None = None,
|
||||
) -> None:
|
||||
"""Stub: send confirmation_request, wait for reply, send final_answer."""
|
||||
conf_id = "test-conf-1"
|
||||
# Register future BEFORE sending request so the WS loop can resolve it
|
||||
# even if the reply arrives before we await (race-safe).
|
||||
loop = asyncio.get_running_loop()
|
||||
future: asyncio.Future[bool] = loop.create_future()
|
||||
if pending_confirmations is not None:
|
||||
pending_confirmations[conf_id] = future
|
||||
await websocket.send_json(
|
||||
{
|
||||
"type": "confirmation_request",
|
||||
"data": {
|
||||
"confirmation_id": conf_id,
|
||||
"command": "rm -rf /tmp/test",
|
||||
"reason": "dangerous command",
|
||||
},
|
||||
}
|
||||
)
|
||||
try:
|
||||
approved = await asyncio.wait_for(future, timeout=10.0)
|
||||
except asyncio.TimeoutError:
|
||||
approved = False
|
||||
await websocket.send_json(
|
||||
{"type": "final_answer", "content": f"approved={approved}", "is_final": True}
|
||||
)
|
||||
|
||||
|
||||
# ── Fixtures ────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def app_with_chat_ws() -> FastAPI:
|
||||
"""Create a FastAPI app with Chat routes and mocked dependencies."""
|
||||
from agentkit.server.routes.chat import router
|
||||
|
||||
app = FastAPI()
|
||||
app.include_router(router, prefix="/api/v1")
|
||||
app.state.session_manager = SessionManager(store=InMemorySessionStore())
|
||||
app.state.llm_gateway = MagicMock()
|
||||
app.state.agent_pool = MagicMock()
|
||||
app.state.server_config = MagicMock()
|
||||
app.state.server_config.api_key = None
|
||||
return app
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client(app_with_chat_ws: FastAPI) -> TestClient:
|
||||
return TestClient(app_with_chat_ws)
|
||||
|
||||
|
||||
def _create_session(client: TestClient, agent_name: str = "test-agent") -> str:
|
||||
"""Create a chat session via REST and return its id."""
|
||||
resp = client.post("/api/v1/chat/sessions", json={"agent_name": agent_name})
|
||||
assert resp.status_code == 200
|
||||
return resp.json()["session_id"]
|
||||
|
||||
|
||||
# ── Connection & session validation ─────────────────────────────────────
|
||||
|
||||
|
||||
class TestWSConnection:
|
||||
"""Connection establishment and session validation."""
|
||||
|
||||
def test_connected_event_with_session_id(self, client: TestClient) -> None:
|
||||
session_id = _create_session(client)
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}") as ws:
|
||||
msg = ws.receive_json()
|
||||
assert msg["type"] == "connected"
|
||||
assert msg["session_id"] == session_id
|
||||
|
||||
def test_session_not_found_rejected(self, client: TestClient) -> None:
|
||||
with client.websocket_connect("/api/v1/chat/ws/nonexistent-session") as ws:
|
||||
msg = ws.receive_json()
|
||||
assert msg["type"] == "error"
|
||||
assert "not found" in msg["data"]["message"].lower()
|
||||
|
||||
def test_closed_session_rejected(self, client: TestClient) -> None:
|
||||
session_id = _create_session(client)
|
||||
client.delete(f"/api/v1/chat/sessions/{session_id}")
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}") as ws:
|
||||
msg = ws.receive_json()
|
||||
assert msg["type"] == "error"
|
||||
assert "closed" in msg["data"]["message"].lower()
|
||||
|
||||
|
||||
# ── Authentication ──────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestWSAuth:
|
||||
"""API key authentication via ?api_key= query param."""
|
||||
|
||||
def test_missing_api_key_rejected(self, app_with_chat_ws: FastAPI) -> None:
|
||||
app_with_chat_ws.state.server_config.api_key = "secret123"
|
||||
client = TestClient(app_with_chat_ws)
|
||||
session_id = _create_session(client)
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}") as ws:
|
||||
msg = ws.receive_json()
|
||||
assert msg["type"] == "error"
|
||||
assert "api_key" in msg["data"]["message"].lower()
|
||||
|
||||
def test_wrong_api_key_rejected(self, app_with_chat_ws: FastAPI) -> None:
|
||||
app_with_chat_ws.state.server_config.api_key = "secret123"
|
||||
client = TestClient(app_with_chat_ws)
|
||||
session_id = _create_session(client)
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}?api_key=wrong") as ws:
|
||||
msg = ws.receive_json()
|
||||
assert msg["type"] == "error"
|
||||
assert "api_key" in msg["data"]["message"].lower()
|
||||
|
||||
def test_valid_api_key_accepted(self, app_with_chat_ws: FastAPI) -> None:
|
||||
app_with_chat_ws.state.server_config.api_key = "secret123"
|
||||
client = TestClient(app_with_chat_ws)
|
||||
session_id = _create_session(client)
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}?api_key=secret123") as ws:
|
||||
msg = ws.receive_json()
|
||||
assert msg["type"] == "connected"
|
||||
assert msg["session_id"] == session_id
|
||||
|
||||
|
||||
# ── Message handling ────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestWSMessages:
|
||||
"""Message type handling: ping, cancel, message, confirmation, invalid JSON."""
|
||||
|
||||
def test_ping_returns_pong(self, client: TestClient) -> None:
|
||||
session_id = _create_session(client)
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}") as ws:
|
||||
ws.receive_json() # connected
|
||||
ws.send_json({"type": "ping"})
|
||||
msg = ws.receive_json()
|
||||
assert msg["type"] == "pong"
|
||||
|
||||
def test_cancel_sends_result_and_stays_open(self, client: TestClient) -> None:
|
||||
session_id = _create_session(client)
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}") as ws:
|
||||
ws.receive_json() # connected
|
||||
ws.send_json({"type": "cancel"})
|
||||
msg = ws.receive_json()
|
||||
assert msg["type"] == "result"
|
||||
assert msg["data"]["status"] == "cancelled"
|
||||
# Connection stays open — ping still works after cancel
|
||||
ws.send_json({"type": "ping"})
|
||||
assert ws.receive_json()["type"] == "pong"
|
||||
|
||||
def test_message_happy_path(
|
||||
self, app_with_chat_ws: FastAPI, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
from agentkit.server.routes import chat as chat_module
|
||||
|
||||
monkeypatch.setattr(chat_module, "_handle_chat_message", _stub_final_answer)
|
||||
client = TestClient(app_with_chat_ws)
|
||||
session_id = _create_session(client)
|
||||
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}") as ws:
|
||||
ws.receive_json() # connected
|
||||
ws.send_json({"type": "message", "content": "hello"})
|
||||
final = ws.receive_json()
|
||||
assert final["type"] == "final_answer"
|
||||
assert final["content"] == "echo: hello"
|
||||
assert final["is_final"] is True
|
||||
|
||||
def test_confirmation_flow(
|
||||
self, app_with_chat_ws: FastAPI, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Server sends confirmation_request → client replies → server continues."""
|
||||
from agentkit.server.routes import chat as chat_module
|
||||
|
||||
monkeypatch.setattr(chat_module, "_handle_chat_message", _stub_with_confirmation)
|
||||
client = TestClient(app_with_chat_ws)
|
||||
session_id = _create_session(client)
|
||||
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}") as ws:
|
||||
ws.receive_json() # connected
|
||||
ws.send_json({"type": "message", "content": "do something risky"})
|
||||
|
||||
conf_req = ws.receive_json()
|
||||
assert conf_req["type"] == "confirmation_request"
|
||||
assert conf_req["data"]["confirmation_id"] == "test-conf-1"
|
||||
|
||||
ws.send_json(
|
||||
{
|
||||
"type": "confirmation_reply",
|
||||
"confirmation_id": "test-conf-1",
|
||||
"approved": True,
|
||||
}
|
||||
)
|
||||
|
||||
final = ws.receive_json()
|
||||
assert final["type"] == "final_answer"
|
||||
assert final["content"] == "approved=True"
|
||||
|
||||
def test_confirmation_flow_rejected(
|
||||
self, app_with_chat_ws: FastAPI, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Client rejects confirmation → final_answer reflects approved=False."""
|
||||
from agentkit.server.routes import chat as chat_module
|
||||
|
||||
monkeypatch.setattr(chat_module, "_handle_chat_message", _stub_with_confirmation)
|
||||
client = TestClient(app_with_chat_ws)
|
||||
session_id = _create_session(client)
|
||||
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}") as ws:
|
||||
ws.receive_json() # connected
|
||||
ws.send_json({"type": "message", "content": "risky"})
|
||||
|
||||
conf_req = ws.receive_json()
|
||||
assert conf_req["type"] == "confirmation_request"
|
||||
|
||||
ws.send_json(
|
||||
{
|
||||
"type": "confirmation_reply",
|
||||
"confirmation_id": "test-conf-1",
|
||||
"approved": False,
|
||||
}
|
||||
)
|
||||
|
||||
final = ws.receive_json()
|
||||
assert final["type"] == "final_answer"
|
||||
assert final["content"] == "approved=False"
|
||||
|
||||
def test_invalid_json_ignored(self, client: TestClient) -> None:
|
||||
"""Invalid JSON text is silently ignored — connection stays open.
|
||||
|
||||
Note: the implementation catches JSONDecodeError with `continue`
|
||||
rather than sending an error event. This is the actual behavior.
|
||||
"""
|
||||
session_id = _create_session(client)
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}") as ws:
|
||||
ws.receive_json() # connected
|
||||
ws.send_text("not valid json {{{")
|
||||
# Connection stays open — ping still works
|
||||
ws.send_json({"type": "ping"})
|
||||
msg = ws.receive_json()
|
||||
assert msg["type"] == "pong"
|
||||
|
||||
def test_unknown_message_type_ignored(self, client: TestClient) -> None:
|
||||
"""Unknown message type is silently ignored — no error sent."""
|
||||
session_id = _create_session(client)
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}") as ws:
|
||||
ws.receive_json() # connected
|
||||
ws.send_json({"type": "unknown_type", "data": "whatever"})
|
||||
# Connection stays open — ping still works
|
||||
ws.send_json({"type": "ping"})
|
||||
msg = ws.receive_json()
|
||||
assert msg["type"] == "pong"
|
||||
|
||||
|
||||
# ── Isolation & cleanup ────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestWSIsolationAndCleanup:
|
||||
"""Session isolation and connection cleanup."""
|
||||
|
||||
def test_session_isolation(self, client: TestClient) -> None:
|
||||
"""Different session_ids get separate connected events."""
|
||||
sid1 = _create_session(client)
|
||||
sid2 = _create_session(client)
|
||||
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{sid1}") as ws1:
|
||||
msg1 = ws1.receive_json()
|
||||
assert msg1["session_id"] == sid1
|
||||
ws1.send_json({"type": "ping"})
|
||||
assert ws1.receive_json()["type"] == "pong"
|
||||
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{sid2}") as ws2:
|
||||
msg2 = ws2.receive_json()
|
||||
assert msg2["session_id"] == sid2
|
||||
ws2.send_json({"type": "ping"})
|
||||
assert ws2.receive_json()["type"] == "pong"
|
||||
|
||||
def test_connection_cleanup_on_disconnect(self, app_with_chat_ws: FastAPI) -> None:
|
||||
"""After disconnect, chat_manager no longer tracks the connection."""
|
||||
from agentkit.server.routes.chat import chat_manager
|
||||
|
||||
client = TestClient(app_with_chat_ws)
|
||||
session_id = _create_session(client)
|
||||
|
||||
assert len(chat_manager.get_connections(session_id)) == 0
|
||||
|
||||
with client.websocket_connect(f"/api/v1/chat/ws/{session_id}") as ws:
|
||||
ws.receive_json() # connected
|
||||
assert len(chat_manager.get_connections(session_id)) == 1
|
||||
|
||||
# After disconnect, the finally block removes the connection
|
||||
assert len(chat_manager.get_connections(session_id)) == 0
|
||||
|
|
@ -0,0 +1,81 @@
|
|||
"""PEP 562 re-export 契约守护测试。
|
||||
|
||||
core/exceptions.py 用模块级 __getattr__ 延迟 re-export LLM 异常类,
|
||||
打破 core.exceptions ↔ llm.exceptions 循环依赖。27 个调用方依赖
|
||||
`from agentkit.core.exceptions import LLMProviderError` 契约。
|
||||
|
||||
本测试锁定以下不变量,防止 __getattr__ 被误删或 re-export 断裂:
|
||||
1. 三条导入路径返回同一类对象(is identity)
|
||||
2. 跨路径 isinstance/except 捕获正常
|
||||
3. 未知属性抛 AttributeError
|
||||
4. __dir__() 使通配导入能发现 re-export
|
||||
5. __getattr__ 调用后缓存到模块 __dict__(后续访问不再触发 lazy import)
|
||||
"""
|
||||
|
||||
import agentkit.core.exceptions as core_exc
|
||||
import agentkit.llm.exceptions as llm_exc
|
||||
from agentkit.core.exceptions import (
|
||||
LLMError,
|
||||
LLMProviderError,
|
||||
ModelNotFoundError,
|
||||
)
|
||||
|
||||
_REEXPORT_NAMES = ("LLMError", "LLMProviderError", "ModelNotFoundError")
|
||||
|
||||
|
||||
def test_reexport_class_identity() -> None:
|
||||
"""core.exceptions 的 LLM 异常类与 llm.exceptions 中的是同一对象。"""
|
||||
assert core_exc.LLMError is llm_exc.LLMError
|
||||
assert core_exc.LLMProviderError is llm_exc.LLMProviderError
|
||||
assert core_exc.ModelNotFoundError is llm_exc.ModelNotFoundError
|
||||
|
||||
|
||||
def test_reexport_isinstance_cross_path() -> None:
|
||||
"""从 llm.exceptions raise 的异常能被 core.exceptions 的 except 捕获。"""
|
||||
for exc_cls in (llm_exc.LLMError, llm_exc.LLMProviderError, llm_exc.ModelNotFoundError):
|
||||
try:
|
||||
raise exc_cls("test")
|
||||
except LLMError:
|
||||
pass # 所有三个都是 LLMError 子类或自身
|
||||
else:
|
||||
raise AssertionError(f"{exc_cls.__name__} 未被 core.exceptions.LLMError 捕获")
|
||||
|
||||
|
||||
def test_getattr_raises_attributeerror_for_unknown() -> None:
|
||||
"""访问不存在的属性抛 AttributeError(PEP 562 契约)。"""
|
||||
try:
|
||||
core_exc.NonExistentException # noqa: B018
|
||||
except AttributeError as e:
|
||||
assert "NonExistentException" in str(e)
|
||||
else:
|
||||
raise AssertionError("预期 AttributeError 未抛出")
|
||||
|
||||
|
||||
def test_dir_includes_reexport_names() -> None:
|
||||
"""__dir__() 包含三个 re-export 名字,使 from ... import * 可发现。"""
|
||||
dir_names = set(dir(core_exc))
|
||||
for name in _REEXPORT_NAMES:
|
||||
assert name in dir_names, f"{name} 不在 dir(core.exceptions) 中"
|
||||
|
||||
|
||||
def test_getattr_caches_to_module_dict() -> None:
|
||||
"""__getattr__ 调用后,类被缓存到模块 __dict__,后续访问不再触发 lazy import。
|
||||
|
||||
白盒验证:直接调用 __getattr__ 后检查 __dict__。即使其他测试已触发缓存,
|
||||
本测试仍有效(幂等:再次调用 __getattr__ 会重新写入相同类对象)。
|
||||
"""
|
||||
result = core_exc.__getattr__("LLMError") # 显式触发 lazy import
|
||||
assert result is llm_exc.LLMError
|
||||
# 缓存后,三个名字都应存在于模块 __dict__(__getattr__ 一次性缓存全部)
|
||||
for name in _REEXPORT_NAMES:
|
||||
assert name in core_exc.__dict__, f"{name} 未被缓存到 core.exceptions.__dict__"
|
||||
assert core_exc.__dict__[name] is getattr(llm_exc, name)
|
||||
|
||||
|
||||
def test_llmprovider_error_inheritance_preserved() -> None:
|
||||
"""re-export 不影响继承链:LLMProviderError 仍是 LLMError 子类。"""
|
||||
assert issubclass(LLMProviderError, LLMError)
|
||||
assert issubclass(ModelNotFoundError, LLMError)
|
||||
err = LLMProviderError(provider="openai", reason="timeout")
|
||||
assert isinstance(err, LLMError)
|
||||
assert err.provider == "openai"
|
||||
|
|
@ -0,0 +1,282 @@
|
|||
"""Unit tests for SQLAlchemy ORM models in agentkit.memory.models.
|
||||
|
||||
Covers the two public ORM model classes — EpisodeModel and ExperienceModel:
|
||||
table mapping, column types/constraints, Python-side default callables,
|
||||
explicit-value construction, and edge cases (empty/long/unicode values).
|
||||
|
||||
Note: memory/models.py contains SQLAlchemy ORM models (not Pydantic), so
|
||||
Pydantic-specific scenarios (ValidationError / model_dump / model_validate)
|
||||
are replaced by ORM equivalents (column introspection + default callable
|
||||
invocation + attribute assignment).
|
||||
|
||||
DB-dependent helpers (create_episodic_session_factory,
|
||||
create_experience_session_factory, ensure_episodic_table) are NOT tested
|
||||
here — they require a live PostgreSQL + asyncpg connection.
|
||||
"""
|
||||
|
||||
import uuid
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from sqlalchemy import DateTime, Float, String, Text
|
||||
from sqlalchemy.dialects.postgresql import JSONB
|
||||
|
||||
from agentkit.memory.models import EpisodeModel, ExperienceModel
|
||||
|
||||
|
||||
def _assert_uuid_default(id_default) -> None:
|
||||
"""SQLAlchemy wraps Python-side default callables to receive an execution
|
||||
context; pass None for callables that don't inspect it."""
|
||||
val = id_default(None)
|
||||
assert isinstance(val, str)
|
||||
uuid.UUID(val) # raises ValueError if not a valid uuid4 string
|
||||
|
||||
|
||||
def _assert_utc_default(ts_default) -> None:
|
||||
ts = ts_default(None)
|
||||
assert isinstance(ts, datetime)
|
||||
assert ts.tzinfo is not None
|
||||
assert ts.utcoffset() == timezone.utc.utcoffset(None)
|
||||
|
||||
|
||||
def _assert_unique_ids(id_default, n: int = 100) -> None:
|
||||
ids = {id_default(None) for _ in range(n)}
|
||||
assert len(ids) == n
|
||||
|
||||
|
||||
class TestEpisodeModel:
|
||||
def test_tablename(self):
|
||||
assert EpisodeModel.__tablename__ == "episodic_memories"
|
||||
|
||||
def test_column_types(self):
|
||||
cols = EpisodeModel.__table__.c
|
||||
assert isinstance(cols.id.type, String)
|
||||
assert isinstance(cols.agent_name.type, String)
|
||||
assert isinstance(cols.task_type.type, String)
|
||||
assert isinstance(cols.input_summary.type, Text)
|
||||
assert isinstance(cols.output_summary.type, Text)
|
||||
assert isinstance(cols.outcome.type, String)
|
||||
assert isinstance(cols.quality_score.type, Float)
|
||||
assert isinstance(cols.reflection.type, Text)
|
||||
assert isinstance(cols.embedding.type, Text)
|
||||
assert isinstance(cols.metadata.type, JSONB)
|
||||
assert isinstance(cols.created_at.type, DateTime)
|
||||
|
||||
def test_primary_key(self):
|
||||
assert EpisodeModel.__table__.c.id.primary_key is True
|
||||
|
||||
def test_indexed_columns(self):
|
||||
cols = EpisodeModel.__table__.c
|
||||
assert cols.agent_name.index is True
|
||||
assert cols.task_type.index is True
|
||||
assert cols.created_at.index is True
|
||||
|
||||
def test_nullable_columns(self):
|
||||
cols = EpisodeModel.__table__.c
|
||||
assert cols.embedding.nullable is True
|
||||
assert cols.metadata.nullable is True
|
||||
|
||||
def test_metadata_attribute_maps_to_metadata_column(self):
|
||||
# Python attribute is `metadata_` (avoids clash with Base.metadata);
|
||||
# the underlying DB column name is "metadata".
|
||||
assert hasattr(EpisodeModel, "metadata_")
|
||||
assert EpisodeModel.__table__.c.metadata.name == "metadata"
|
||||
|
||||
def test_scalar_defaults(self):
|
||||
cols = EpisodeModel.__table__.c
|
||||
assert cols.input_summary.default.arg == ""
|
||||
assert cols.output_summary.default.arg == ""
|
||||
assert cols.reflection.default.arg == ""
|
||||
assert cols.outcome.default.arg == "success"
|
||||
assert cols.quality_score.default.arg == 0.5
|
||||
|
||||
def test_callable_default_id_is_uuid_string(self):
|
||||
_assert_uuid_default(EpisodeModel.__table__.c.id.default.arg)
|
||||
|
||||
def test_callable_default_created_at_is_utc(self):
|
||||
_assert_utc_default(EpisodeModel.__table__.c.created_at.default.arg)
|
||||
|
||||
def test_default_id_is_unique_across_calls(self):
|
||||
_assert_unique_ids(EpisodeModel.__table__.c.id.default.arg)
|
||||
|
||||
def test_construct_with_explicit_values(self):
|
||||
ts = datetime(2025, 1, 2, 3, 4, 5, tzinfo=timezone.utc)
|
||||
m = EpisodeModel(
|
||||
id="ep-1",
|
||||
agent_name="agent-a",
|
||||
task_type="analysis",
|
||||
input_summary="in",
|
||||
output_summary="out",
|
||||
outcome="failure",
|
||||
quality_score=0.9,
|
||||
reflection="ref",
|
||||
embedding="[0.1, 0.2]",
|
||||
metadata_={"k": "v"},
|
||||
created_at=ts,
|
||||
)
|
||||
assert m.id == "ep-1"
|
||||
assert m.agent_name == "agent-a"
|
||||
assert m.task_type == "analysis"
|
||||
assert m.input_summary == "in"
|
||||
assert m.output_summary == "out"
|
||||
assert m.outcome == "failure"
|
||||
assert m.quality_score == 0.9
|
||||
assert m.reflection == "ref"
|
||||
assert m.embedding == "[0.1, 0.2]"
|
||||
assert m.metadata_ == {"k": "v"}
|
||||
assert m.created_at == ts
|
||||
|
||||
def test_construct_minimal_yields_none_attrs(self):
|
||||
# ORM Python-side defaults fire at flush (INSERT) time, not instantiation,
|
||||
# so unset attributes read back as None.
|
||||
m = EpisodeModel()
|
||||
assert m.id is None
|
||||
assert m.agent_name is None
|
||||
assert m.created_at is None
|
||||
assert m.metadata_ is None
|
||||
|
||||
def test_edge_empty_embedding_and_text(self):
|
||||
m = EpisodeModel(embedding="", reflection="", input_summary="")
|
||||
assert m.embedding == ""
|
||||
assert m.reflection == ""
|
||||
assert m.input_summary == ""
|
||||
|
||||
def test_edge_empty_vector_json(self):
|
||||
m = EpisodeModel(embedding="[]")
|
||||
assert m.embedding == "[]"
|
||||
|
||||
def test_edge_long_text(self):
|
||||
long_text = "x" * 10_000
|
||||
m = EpisodeModel(input_summary=long_text, output_summary=long_text, reflection=long_text)
|
||||
assert len(m.input_summary) == 10_000
|
||||
assert len(m.output_summary) == 10_000
|
||||
assert len(m.reflection) == 10_000
|
||||
|
||||
def test_edge_unicode_and_special_characters(self):
|
||||
m = EpisodeModel(
|
||||
input_summary="中文 + emoji 🚀 + quote \" ' \n\t",
|
||||
metadata_={"unicode": "你好", "null": None, "nested": {"a": [1, 2]}},
|
||||
)
|
||||
assert "🚀" in m.input_summary
|
||||
assert "中文" in m.input_summary
|
||||
assert m.metadata_["unicode"] == "你好"
|
||||
assert m.metadata_["null"] is None
|
||||
assert m.metadata_["nested"] == {"a": [1, 2]}
|
||||
|
||||
def test_edge_empty_metadata_dict(self):
|
||||
m = EpisodeModel(metadata_={})
|
||||
assert m.metadata_ == {}
|
||||
|
||||
|
||||
class TestExperienceModel:
|
||||
def test_tablename(self):
|
||||
assert ExperienceModel.__tablename__ == "task_experiences"
|
||||
|
||||
def test_column_types(self):
|
||||
cols = ExperienceModel.__table__.c
|
||||
assert isinstance(cols.id.type, String)
|
||||
assert isinstance(cols.task_type.type, String)
|
||||
assert isinstance(cols.goal.type, Text)
|
||||
assert isinstance(cols.steps_summary.type, Text)
|
||||
assert isinstance(cols.outcome.type, String)
|
||||
assert isinstance(cols.duration_seconds.type, Float)
|
||||
assert isinstance(cols.success_rate.type, Float)
|
||||
assert isinstance(cols.failure_reasons.type, JSONB)
|
||||
assert isinstance(cols.optimization_tips.type, JSONB)
|
||||
assert isinstance(cols.embedding.type, Text)
|
||||
assert isinstance(cols.created_at.type, DateTime)
|
||||
|
||||
def test_primary_key_and_indexes(self):
|
||||
cols = ExperienceModel.__table__.c
|
||||
assert cols.id.primary_key is True
|
||||
assert cols.task_type.index is True
|
||||
assert cols.created_at.index is True
|
||||
|
||||
def test_nullable_embedding(self):
|
||||
assert ExperienceModel.__table__.c.embedding.nullable is True
|
||||
|
||||
def test_scalar_defaults(self):
|
||||
cols = ExperienceModel.__table__.c
|
||||
assert cols.goal.default.arg == ""
|
||||
assert cols.steps_summary.default.arg == ""
|
||||
assert cols.outcome.default.arg == "success"
|
||||
assert cols.duration_seconds.default.arg == 0.0
|
||||
assert cols.success_rate.default.arg == 1.0
|
||||
|
||||
def test_callable_list_defaults(self):
|
||||
cols = ExperienceModel.__table__.c
|
||||
# default=list (the type itself, callable → empty list when invoked)
|
||||
assert cols.failure_reasons.default.is_callable
|
||||
assert cols.optimization_tips.default.is_callable
|
||||
assert cols.failure_reasons.default.arg(None) == []
|
||||
assert cols.optimization_tips.default.arg(None) == []
|
||||
|
||||
def test_callable_default_id_is_uuid_string(self):
|
||||
_assert_uuid_default(ExperienceModel.__table__.c.id.default.arg)
|
||||
|
||||
def test_callable_default_created_at_is_utc(self):
|
||||
_assert_utc_default(ExperienceModel.__table__.c.created_at.default.arg)
|
||||
|
||||
def test_default_id_is_unique_across_calls(self):
|
||||
_assert_unique_ids(ExperienceModel.__table__.c.id.default.arg)
|
||||
|
||||
def test_construct_with_explicit_values(self):
|
||||
ts = datetime(2025, 6, 7, 8, 9, 10, tzinfo=timezone.utc)
|
||||
m = ExperienceModel(
|
||||
id="exp-1",
|
||||
task_type="build",
|
||||
goal="ship feature",
|
||||
steps_summary="step1; step2",
|
||||
outcome="partial",
|
||||
duration_seconds=42.5,
|
||||
success_rate=0.66,
|
||||
failure_reasons=["timeout", "oom"],
|
||||
optimization_tips=["retry", "cache"],
|
||||
embedding="[0.3, 0.4]",
|
||||
created_at=ts,
|
||||
)
|
||||
assert m.id == "exp-1"
|
||||
assert m.task_type == "build"
|
||||
assert m.goal == "ship feature"
|
||||
assert m.steps_summary == "step1; step2"
|
||||
assert m.outcome == "partial"
|
||||
assert m.duration_seconds == 42.5
|
||||
assert m.success_rate == 0.66
|
||||
assert m.failure_reasons == ["timeout", "oom"]
|
||||
assert m.optimization_tips == ["retry", "cache"]
|
||||
assert m.embedding == "[0.3, 0.4]"
|
||||
assert m.created_at == ts
|
||||
|
||||
def test_construct_minimal_yields_none_attrs(self):
|
||||
m = ExperienceModel()
|
||||
assert m.id is None
|
||||
assert m.task_type is None
|
||||
assert m.created_at is None
|
||||
assert m.failure_reasons is None
|
||||
assert m.optimization_tips is None
|
||||
|
||||
def test_edge_empty_lists_and_embedding(self):
|
||||
m = ExperienceModel(
|
||||
failure_reasons=[],
|
||||
optimization_tips=[],
|
||||
embedding="",
|
||||
)
|
||||
assert m.failure_reasons == []
|
||||
assert m.optimization_tips == []
|
||||
assert m.embedding == ""
|
||||
|
||||
def test_edge_long_text(self):
|
||||
long_text = "y" * 10_000
|
||||
m = ExperienceModel(goal=long_text, steps_summary=long_text)
|
||||
assert len(m.goal) == 10_000
|
||||
assert len(m.steps_summary) == 10_000
|
||||
|
||||
def test_edge_unicode_and_special_characters(self):
|
||||
m = ExperienceModel(
|
||||
goal="中文 🎯 ' \" \n\t",
|
||||
failure_reasons=["错误", "🚨"],
|
||||
optimization_tips=["优化"],
|
||||
)
|
||||
assert "🎯" in m.goal
|
||||
assert "中文" in m.goal
|
||||
assert m.failure_reasons == ["错误", "🚨"]
|
||||
assert m.optimization_tips == ["优化"]
|
||||
Loading…
Reference in New Issue