180 lines
7.4 KiB
Markdown
180 lines
7.4 KiB
Markdown
# ISS-003 MVP 设计与实现 Review 收敛
|
||
|
||
**状态**：已归档（Review 基线过时）
|
||
**严重程度**：高
|
||
**发现时间**：2026-07-03
|
||
**来源**：MVP 版本设计与实现 review
|
||
**归档时间**：2026-07-23
|
||
|
||
---
|
||
|
||
## 归档说明
|
||
|
||
本 Issue 基于早期 Planner/Executor/Verifier、Redis Session 和旧 Tool Trace 架构进行总体 Review。其主要问题后来分别由凭据清理、Session/Run 隔离、证据链加固、文档路径修复和 ISS-014 单体 Diagnosis Agent/Harness 重构处理或替代；原文件中的类名、入口、优先级和建议方案已经不能准确描述当前系统。
|
||
|
||
仍可能存在的测试隔离、CORS 或 Redis 序列化风险不继续挂在本旧 Review 下。后续需要处理时，应基于当前代码和 ISS-014 架构重新建立范围明确的 Issue。本 Issue 以“Review 基线过时”归档，不作为当前缺陷清单或实施依据。
|
||
|
||
---
|
||
|
||
## 背景
|
||
|
||
当前 MVP 已具备 Chat、Planner/Executor/Verifier、知识检索、诊断会话落库、反馈与 case library 等主线能力，但设计文档、运行时实现和可验证性之间仍存在明显偏差。
|
||
|
||
本 issue 用来收敛本次 review 的主要风险，方便后续拆 OpenSpec change 或工程任务。
|
||
|
||
---
|
||
|
||
## 核心问题
|
||
|
||
### P0：敏感配置直接提交到仓库
|
||
|
||
`src/main/resources/application.yml` 中包含真实基础设施地址、数据库密码、Redis 密码、Milvus token、LLM API key。
|
||
|
||
`src/test/java/com/superbiz/agent/service/SimpleMilvusTest.java` 中也硬编码了 Milvus/Zilliz token。
|
||
|
||
**影响**：
|
||
|
||
- 密钥泄漏后需要立即轮换。
|
||
- 合并 worktree 后会扩大泄漏面。
|
||
- `show-sql: true` 与 DEBUG 日志可能进一步暴露业务数据。
|
||
|
||
**建议**：
|
||
|
||
- 立即轮换已提交的 token/password/api-key。
|
||
- 将敏感配置改为环境变量或本地 profile 覆盖。
|
||
- 提交 `application-example.yml` 或 `.env.example`，不要提交真实值。
|
||
|
||
### P1：测试体系不能稳定离线运行
|
||
|
||
`mvn test` 编译阶段通过，但 surefire 阶段大量失败，主要原因是测试直接依赖外部 MySQL、Redis、Milvus、LLM/Embedding 服务。
|
||
|
||
典型失败：
|
||
|
||
- MySQL/Flyway 连接失败导致 repository、Redis、Spring context 测试失败。
|
||
- Milvus 连接测试出现 `DEADLINE_EXCEEDED`。
|
||
- 当前环境下 Mockito inline mock maker self-attach 失败。
|
||
|
||
**影响**：
|
||
|
||
- 无法在合并前获得可靠的回归信号。
|
||
- 实现变更与环境故障混在一起，问题定位成本高。
|
||
|
||
**建议**：
|
||
|
||
- 将纯单测、H2/JPA slice、外部集成测试分离。
|
||
- 用 Maven profile 或 JUnit tag 区分 `unit` / `integration`。
|
||
- 默认 `mvn test` 只跑不依赖外部服务的测试。
|
||
|
||
### P1：会话管理设计与实现不一致
|
||
|
||
`mvp/architecture/archive/2026-07-05-legacy/session-management.md` 设计 Redis 作为主会话存储，带 `session:{session_id}` 和 TTL。
|
||
|
||
实际 `/api/chat` 在 `ChatController` 中使用 JVM 内存 `ConcurrentHashMap` 管理历史消息，`RedisSessionManager` 虽然存在但没有接入 controller。
|
||
|
||
**影响**：
|
||
|
||
- 应用重启后会话历史丢失。
|
||
- 多实例部署时会话不一致。
|
||
- Redis TTL 与设计中的生命周期不生效。
|
||
- 前端 chat session id 与后端 diagnosis session id 存在分裂。
|
||
|
||
**建议**：
|
||
|
||
- 明确 MVP 阶段是否接受内存会话。
|
||
- 如果接受，需要同步更新文档并标注限制。
|
||
- 如果不接受，应将 `ChatController` 接入 `SessionManager`，统一 session id 与 diagnosis session id 的关系。
|
||
|
||
### P1：Verifier 证据链仍不完整
|
||
|
||
`ToolTraceSummaryService` 期望从 `tool_invocation` 汇总 `lookup_knowledge`、`query_logs`、`query_metrics`、`query_order` 等证据工具。
|
||
|
||
当前只有 `LookupKnowledgeTool` 主动写入 `tool_invocation`。`QueryMetricsTools` 和 `QueryLogsTools` 返回 JSON，但没有落库。
|
||
|
||
**影响**：
|
||
|
||
- verifier 无法稳定审计日志、指标、订单等非知识库工具事实。
|
||
- `thought` 或模型输出中看起来做了很多推理，但可追溯工具调用证据不足。
|
||
- 用户侧可观测性仍然偏低。
|
||
|
||
**建议**：
|
||
|
||
- 抽象统一的 `ToolInvocationRecorder`。
|
||
- 所有 evidence tool 都必须记录 input、output preview、success、duration、trace id。
|
||
- verifier 只消费结构化 trace summary，不依赖模型自由文本回忆工具调用。
|
||
|
||
### P1：上传文档路径存在重复拼接风险
|
||
|
||
`DocumentManagementService.saveToLocal()` 返回的是包含 `knowledge_base` 前缀的本地路径。
|
||
|
||
`KnowledgeIndexService.readDocument()` 又执行 `Paths.get(knowledgeBasePath, filePath)`。
|
||
|
||
**影响**：
|
||
|
||
- 上传文档进入 L0 索引后，命中时读取原文可能拼成 `knowledge_base/knowledge_base/...`。
|
||
- 这会降低 L0 命中后的答案质量，并造成“命中但读不到原文”的隐性故障。
|
||
|
||
**建议**：
|
||
|
||
- 统一 `filePath` 语义：要么存相对 `knowledge.base-path` 的路径，要么存绝对路径。
|
||
- `readDocument()` 对 absolute path、已带 base path 的 relative path 做兼容。
|
||
- 增加上传文档后 L0 命中并读取原文的回归测试。
|
||
|
||
### P2：SupervisorAgent 构建后未使用
|
||
|
||
`ChatService.executeChatComplex()` 中创建了 `SupervisorAgent`，但实际仍通过 `callAgent(planner/executor/verifier)` 手写顺序编排。
|
||
|
||
**影响**：
|
||
|
||
- 代码与设计文档中的 multi-agent 编排表述不一致。
|
||
- 后续维护者容易误判当前已由 Supervisor 执行调度。
|
||
|
||
**建议**：
|
||
|
||
- 删除未使用的 `SupervisorAgent` 构建，明确当前是手写编排。
|
||
- 或真正切到 Spring AI Alibaba SupervisorAgent flow，并补充行为验证。
|
||
|
||
### P2：生产安全边界偏弱
|
||
|
||
`SessionConfiguration` 使用 `activateDefaultTyping + LaissezFaireSubTypeValidator` 配置 Redis JSON 反序列化。
|
||
|
||
`WebMvcConfig` 对所有路径放开 CORS。
|
||
|
||
**影响**：
|
||
|
||
- Redis 若被非可信写入，存在多态反序列化风险。
|
||
- CORS 全放开适合本地 MVP，不适合公开环境。
|
||
|
||
**建议**：
|
||
|
||
- Redis value 使用明确 DTO 类型或受限 subtype validator。
|
||
- CORS 改为按 profile 配置允许域名。
|
||
|
||
---
|
||
|
||
## 优先级建议
|
||
|
||
1. 先处理敏感配置和密钥轮换，避免合并后扩大泄漏范围。
|
||
2. 建立可离线运行的单测基线，让默认 `mvn test` 可用于合并门禁。
|
||
3. 统一 session id 与 session storage，解决前后端、Redis、diagnosis session 的语义分裂。
|
||
4. 补齐所有 evidence tool 的 `tool_invocation` 落库，提升 verifier 可追溯性。
|
||
5. 修正上传文档路径语义，并补回归测试。
|
||
6. 清理或真正启用 `SupervisorAgent`，避免设计和实现长期漂移。
|
||
|
||
---
|
||
|
||
## 相关文件
|
||
|
||
- `src/main/resources/application.yml`
|
||
- `src/test/java/com/superbiz/agent/service/SimpleMilvusTest.java`
|
||
- `src/main/java/com/superbiz/agent/controller/ChatController.java`
|
||
- `src/main/java/com/superbiz/agent/service/session/impl/RedisSessionManager.java`
|
||
- `src/main/java/com/superbiz/agent/service/ToolTraceSummaryService.java`
|
||
- `src/main/java/com/superbiz/agent/tool/LookupKnowledgeTool.java`
|
||
- `src/main/java/com/superbiz/agent/agent/tool/QueryMetricsTools.java`
|
||
- `src/main/java/com/superbiz/agent/agent/tool/QueryLogsTools.java`
|
||
- `src/main/java/com/superbiz/agent/service/DocumentManagementService.java`
|
||
- `src/main/java/com/superbiz/agent/service/KnowledgeIndexService.java`
|
||
- `src/main/java/com/superbiz/agent/service/ChatService.java`
|
||
- `src/main/java/com/superbiz/agent/config/SessionConfiguration.java`
|
||
- `src/main/java/com/superbiz/agent/config/WebMvcConfig.java`
|