Files
DevFlow/docs/05-代码审查/全局代码review-2026-06-15.md
T
lxy 998a2f243d 文档: 架构方案文档(意图识别论证+多主题愿景/论证+文档物理分类+边界清晰化)
squash合并:
- 意图识别层论证(8维度+10业界佐证)
- 多主题上下文管理愿景+并存论证+补充论证(多轮agentic)
- 架构设计文档物理分类(四子目录+INDEX+命名规范+引用同步+边界清晰化)
- 前端架构技术债清单归档
2026-06-19 15:04:04 +08:00

151 lines
19 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 全局代码 Review2026-06-15
> 来源:7 维度 general-purpose agent 并行深入扫(workflow `wnrzlw8ub`DRY / 架构 / 潜在bug / 简洁性 / 安全 / AI可靠 / 工作流引擎)。汇总 agent stall(处理 7 维度大 JSON + 输出大 schema 卡死 6 次重试),主代理从 workflow journal 提取 7 维度 result 接手汇总。
> 原则:全局最优、不过度设计、务实修复(最小改动根治)、聚焦真问题(不报 style/吹毛求疵)。
> 去重源:本轮已修项排除(CR-01 keyring 清理 = security#1、CR-02 AiChat confirm = DRY#3),embed timeout 在 AI可靠 与 架构 两维度重复(合并)。
---
## 总览
- 原始 38 findings → 去重/排已修后 **~30**
- **P1 可执行 6**(明确小修,立即推进)
- **P1 需设计 3**(跨层/A-B/行为变更,进设计文档)
- **P2 可执行 ~13**(清债/死代码批)
- **P2 需设计 ~10**(进 todo
---
## 🔴 P1 — 可执行(立即推进)
| # | 维度 | 问题 | 位置 | 修复方向 |
|---|---|---|---|---|
| R-P1-1 | AI可靠 | **Anthropic 流式 error 事件误判正常完成**`apply_anthropic_event``type=="error"` 分支返 `finished:true`stream_llm 视为正常完成 → 残缺响应持久化、无 AiError、用户以为答完实际中途出错(OpenAI 路径走 Err,行为不一致) | `anthropic_compat.rs:201-205` + `stream_recv.rs:142-145` | error 事件走 Err 路径(StreamChunk 加 `error: Option<String>` 字段,stream_llm 识别后走 AiError + return None 丢弃残缺) |
| R-P1-2 | bug | **shell::execute 超时未 kill 子进程**`timeout` 包裹 `cmd.output()`,超时返 Err 但子进程未 killtokio Child 默认无 kill_on_drop)→ 僵尸/孤儿累积 + fd 泄漏 | `crates/df-execute/src/shell.rs:55-63` | 改 `spawn()` 拿 Child + `kill_on_drop(true)` 兜底,或超时分支显式 `child.kill().await` + wait 回收 |
| R-P1-3 | workflow | **审批 Ok 后取消 TOCTOU**HumanNode Ok 返回后 join_all 让出执行权窗口内前端 cancel → set_cancelled 改 Cancelled → 随后 set_completed 命中 is_legal 拒绝(Cancelled→Completed) → bail,已批准审批报失败(与 B-03b-R1 Err 分支防护对称的另一半) | `crates/df-workflow/src/executor.rs:126` | set_completed 前加 `if !is_cancelled(node_id)` 短路(与 139 行 Err 分支对称) |
| R-P1-4 | DRY | **build_provider+resolve+ensure_resolved_key 7 处复制,6 处漏空 key 早失败校验**:只有 agentic.rs 含 ensure_resolved_keytitle/knowledge_inject×2/project/ai_node×2 跳过 → keyring 读不到时空 key 照发请求致 401,用户看「key 已保存」反复重试无解 | 7 调用点:`agentic.rs:48-58`/`title.rs:60-65`/`knowledge_inject.rs:33,320`/`project.rs:378`/`ai_node.rs:118,309` | `secret.rs` 加工厂 `build_provider_for(record) -> Result<Box<dyn LlmProvider>, String>`resolve→ensure_resolved→build),7 处替换 |
| R-P1-5 | workflow | **update_task status 无值校验**:白名单只校验列名不校验值,任意字符串落库,TaskStatus 枚举形同虚设 → 拼写错误(in-progess/in progress)静默落库,按枚举查询全漏,任务「消失」(todo T-260614-03 旁路未根治) | `src-tauri/src/commands/task.rs:74-85` | field=="status" 时校验 value ∈ TaskStatus variantstypes.rs 补 is_valid 反查),非法返 Err |
| R-P1-6 | workflow | **HumanNode 无效 decision 直接 Err 无重试**decision ∉ options 或空 → return Err → 节点 Failed → 整层中止,用户一次手误(多尾空格/'y')杀死工作流,无法纠错 | `crates/df-nodes/src/human_node.rs:67-73` | return Err 改 continuewarn 日志),select! 续等下一条合法 Response,超时兜底 |
---
## 🔴 P1 — 需设计(进设计文档/todo)
### R-PD-1 编辑 provider 不改密钥默默清 DB 明文,密钥永久丢失(security P1
- 位置:`commands.rs:329-355`ai_save_provider 空密钥分支)+ `crud.rs:896`INSERT OR REPLACE 全字段覆盖)
- 问题:api_key 空=编辑不改约定下,无条件把 record.api_key 置空走 INSERT OR REPLACE。**未迁移态** provider(迁移失败 DB 仍有明文 keyring 空)用户仅改 name/base_url,空 api_key 把 DB 明文覆盖成空 → keyring 一直空 → resolve 返空 → provider 报废,密钥静默丢失。
- 修复方向:空 api_key 时先确认 keyring 有/DB 有再决定清 DB——若 keyring 无且原 DB 非空,先 set_provider_secret 补迁再清 DB(即时迁移),保住密钥不丢。
- 进:todo 新增条目 + 功能决策记录(密钥迁移健壮性取舍)
### R-PD-2 run_workflow IPC 经 ScriptNode 执行前端任意 shellsecurity P1
- 位置:`workflow.rs:36-44`run_workflow 接 DagDef+ `script_node.rs:34-42`config.command 直传)+ `shell.rs:38/42`cmd /C \| sh -c
- 问题:前端可构造任意 DagDef 提交,ScriptNode 从 config.command 取原始串交 shell 解释器,**无白名单/无工作目录锚定/无审批**(workflow 系统独立于 AI 工具 RiskLevel 审批链)。前端可 `del /S` 或 curl 外发。
- 修复方向(三选一):① state.rs build_registry 不注册 'script' 掐断(最安全最小);② 限定工作目录在已绑定项目 path 内 + 高危命令前缀走 HumanNode 审批;③ ScriptNode 命令白名单(npm/git/mvn 前缀 + 参数过滤)。
- 进:todo 新增 + 安全设计文档(工作流脚本执行边界)
### R-PD-3 DAG 条件边从不求值,条件分支整体失效(workflow P1
- 位置:`executor.rs:62-97`adjacency_in 构建 + inputs 收集不区分 condition+ `conditions.rs:16`ConditionEngine 从未被 executor 调用)
- 问题:边有 `condition: Option<String>`build_dag 写入 runtime Dag,但 executor 无条件把每条边前驱输出灌入 targettopological_layers 把条件边计入入度。`add_edge_with_condition("a","c","false")` 实际 c 永远执行。**条件分支这一 DAG 核心能力整体失效,且对用户静默**。
- 修复方向:executor inputs 收集处用 ConditionEngine.evaluate(edge.condition, &pred_output) 过滤;topological_layers 前过滤无效边或执行时按条件短路 target 为 Skipped。起步先打通数据流过滤,调度短路二期。
- 与 todo `T-260614-11 条件表达式引擎升级`同一根因(conditions 仅 true/false 字面量 + 从未接线)。进:任务推进构想 / 条件引擎设计文档
---
## 🟡 P2 — 可执行(清债/死代码批)
| # | 维度 | 问题 | 位置 | 修复 |
|---|---|---|---|---|
| R-P2-1 | AI可靠 | MAX_AGENT_ITERATIONS(=10) 达上限静默截断,末轮 tool_result 不回传 LLM 且无提示 | `agentic.rs:81-217` | 因达上限退出时 emit AiError/warn「达到最大轮次,可能未完成」 |
| R-P2-2 | bug | write_file 每次覆盖非空文件生成 .bak 不清理(污染 git/构建/list_directory AI 上下文) | `tool_registry.rs:471-476` | rename 成功后 remove .bak(保留 rename 失败分支不删兜底) |
| R-P2-3 | bug | build_dag 不校验边 source/target 存在,野边静默吞(节点空输入跑错无报错) | `registry.rs:52-61` | add_edge 前校验两端存在,bail 早返回 |
| R-P2-4 | bug | TokenAccumulator.add 用 u32 += 无饱和,恶意/异常 provider 返巨大值溢出回绕打乱 budget | `conversation.rs:26-29` | saturating_add |
| R-P2-5 | DRY | now_millis 时间工具两层重复(commands::now_millis + crud::now_millis_str | `commands/mod.rs:15` + `crud.rs:357` | df-core 提供 pub fn,两处 use |
| R-P2-6 | 架构 | extract_error_diag 字节窗口扫描 UTF-8 边界脆弱(依赖中文恰好 3 字节巧合) | `stream_recv.rs:28-40` | 改白名单码 raw.contains(code_str),去滑窗 |
| R-P2-7 | DRY | **client builder 三级降级逐字复制 + 中间级重建无意义**(第一级 connect_timeout(30s) 失败→第二级同配置重建必同因失败→只有第三级 Client::new() 不同)。⚠️ 本轮 B3 刚加的「中间级重建」审查质疑冗余 | `anthropic_compat.rs:235-247` + `openai_compat.rs:232-244` | 抽 `build_llm_client()` 到 df-ai/http.rs + 简化为两级(删中间同配置重建级) |
| R-P2-8 | 简洁 | df-ai::router 整模块死代码(route() 6 分支全返 default_model,零外部引用) | `router.rs:8-51` | 删文件 + lib.rs 删 mod |
| R-P2-9 | 简洁 | df-ai::stream::StreamCollector 整模块死代码(与 TokenAccumulator 重叠未用) | `stream.rs:6-45` | 删文件 + lib.rs 删 mod |
| R-P2-10 | 简洁 | Dag::predecessors/successors 死方法(executor 已自建 adjacency | `dag.rs:65-80` | 删两方法 |
| R-P2-11 | 简洁 | NodeRegistry::is_registered/registered_types 死方法 | `registry.rs:66-74` | 删两方法 |
| R-P2-12 | 简洁 | DagDef::from_dag_edges 死方法(注释自承无法还原节点配置) | `dag_def.rs:77-97` | 删方法 |
| R-P2-13 | 简洁 | set_waiting/set_skipped 死代码(全仓零调用,误导状态机认知) | `state.rs:87-111` | 删两方法(set_cancelled 保留+注释「唯一受控旁路」) |
---
## 🟡 P2 — 需设计(进 todo
- **R-PD-4** keyring 迁移失败明文密钥长期滞留 SQLite 文件(无加密)— `secret.rs:50-72`:补 N 次失败阈值警告(不改兼容时序)
- **R-PD-5** approve_human_approval IPC 不校验 decision ∈ options(放行非法值,依赖下游 HumanNode 兜底,IPC 成功+工作流失败割裂)— `workflow.rs:179-210`
- **R-PD-6** AiSession 单例:try_continue 读 active_conversation_id 竞态(靠 switch readonly 间接保护,脆弱耦合)— `agentic.rs:255-291`:从 pending_approvals 取 conversation_id 解耦
- **R-PD-7** LlmProvider trait 抽象缺口:name() 语义错位(OpenAI 返模型名/Anthropic 返固定串)+ supported_features/ProviderFeatures 死代码(两 provider 实现但零读取)— `provider.rs:155-160,217`:补 endpoint() 默认方法 + 删 supported_features
- **R-PD-8** AiProviderRecord 整条穿透 IPC 边界(DB schema 演进直接破坏前端契约,models 字段 provider 返串/conversation 返数组不一致)— `commands.rs:282-295`:定义 ProviderDto/ConversationSummary 映射层
- **R-PD-9** 命令层承担业务逻辑(agentic loop/tool_registry 717 行/audit reason 映射堆 commands/ai)— `agentic.rs`+`tool_registry.rs`+`audit.rs`:最小起步把 audit 工具名→文案映射作 display_hint 注册进 AiToolRegistry(消除双份);agentic loop 下沉 df-ai 较大进 todo
- **R-PD-10** .map_err(\|e\| e.to_string()) 10 文件 85 处复制(强类型 Error 拍平成自由文本,分类信息丢弃)— 全 commands/:加 err_str helper 统一日志点(是否带分类前缀需设计)
- **R-PD-11** 目录防重复绑定逻辑两处重复(find_binding_conflict vs bind_dir_to_project)— `project.rs:249-260` + `tool_registry.rs:92-102`:抽公共 find_path_conflict
- **R-PD-12** run_workflow AI 工具是 no-op 桩但 prompt/audit/ToolCard 当真实能力宣传(LLM 调用走审批拿空结果,体验断裂)— `tool_registry.rs:383-390`+`prompt.rs:57`+`audit.rs:120-123`+`ToolCard.vue:367`:删假能力 or 真接线(与 R-PD-2 协同)
- **R-PD-13** run_workflow 转发任务 Lagged 静默丢前端事件无补偿(256 容量击穿时关键终态事件永久丢)— `workflow.rs:70-97`:终态事件兜底重发 or Lagged 时从 DB 重读补发
- **R-PD-14** df-ideas::promotion IdeaPromoter/PromotionPolicy/try_promote 死代码且 do_promote 是空壳 TODO(误接入得虚假成功)— `promotion.rs:21-92`:删死码保留 PromotionResult(确认归属后)
---
## 架构洞察
1. **df-ai 多处重构残留死代码**routerroute 全返 default_model/stream(与 TokenAccumulator 重叠)/supported_features(零读取)——「为不存在需求预建的抽象」,零引用可删。
2. **LlmProvider trait 抽象缺口**name() 语义错位 + 无 base_url/endpoint 访问器 → 诊断(stream_recv fmt_diag)只能用 name() 近似 provider_type401 排查难定位是 key/url/model 哪个配置错(与本轮 B-260615-01 stream_recv 诊断约束呼应)。
3. **命令层臃肿**agentic loopReAct 编排)/tool_registry(项目CRUD+文件+安全校验 717 行闭包)/audit(工具名→文案映射)堆在 src-tauri/commands/ai,无法被 df-nodes/AiNode 复用(AiNode 自己重写 LLM 调用)。最小起步:audit 映射下沉 registrydisplay_hint)。
4. **df-workflow ConditionEngine 从未接线**:DAG 条件分支整体失效(R-PD-3),与 todo `T-260614-11 条件表达式引擎升级`同根因——conditions 仅 true/false 字面量 + executor 不调用。需统一在条件引擎设计文档收敛。
5. **工作流 ScriptNode 任意 shellR-PD-2**:独立于 AI 工具审批链的安全缺口,前端 IPC 直接触发,需工作流脚本执行边界设计。
---
## 推进编排
1. **P1 可执行 6 项**R-P1-1~6)→ workflow 并行推进(文件域隔离 + 实现/审查 pipeline
2. **P1 需设计 3 项**R-PD-1/2/3)→ 进设计文档(密钥迁移健壮性 / 工作流脚本执行边界 / 条件引擎)+ todo
3. **P2 可执行清债批**(R-P2-1~13)→ 顺带推进(死代码删除 + 小修,零风险减法)
4. **P2 需设计**R-PD-4~14)→ 补 todo
---
## 推进状态(2026-06-15
**批1 核查闭环**(主代理独立 cargo check 全量 + 三 crate test 全绿):
- ✅ R-P1-2 shell kill_on_drop`df-execute/shell.rs`kill_on_drop(true)+spawn+wait_with_outputtimeout drop child 触发 kill
- ✅ R-P1-3 审批 Ok TOCTOU 短路(`df-workflow/executor.rs:126`Ok 分支加 is_cancelled 短路,新测 `test_cancelled_node_skips_set_completed`15 passed
- ✅ R-P1-5 update_task status 校验(`df-core/types.rs` is_valid+valid_values+7 单测;`task.rs` status 值校验 Err 含合法值清单,df-core 7 passed
- ✅ R-P1-6 HumanNode 无效 decision continue`df-nodes/human_node.rs`Err→continue+warn2 测改 `_ignored_then_timeout`15 passed
**批2 核查闭环**(主代理独立核查 git diff + cargo check 全量 exit 0 + df-ai 10 passed:
- ✅ R-P1-1 Anthropic 流式 error 误判完成(`provider.rs` StreamChunk 加 `error: Option<String>` 字段 `#[serde(skip)]``anthropic_compat.rs:205` error 分支 finished→false+error=Some(全分支补 error:None 初始化);`stream_recv.rs:142` chunk.error 识别 emit AiError(MidStream)+return None,与 Err 分支对称;新单测 2 个,df-ai **10 passed**
- ✅ R-P1-4 build_provider DRY`secret.rs` build_provider_for 工厂已落(resolve→ensure_resolved→build),5 调用点全用;`ai_node.rs:50-53` parse_params 内联空 key 校验(df-nodes 架构边界不依赖 src-tauri,语义对齐 ensure_resolved_key);裸 build_provider 残留 = lib.rs 工厂定义 + ai_node 生产(前面 parse_params 已校验) + ai_node 测试,合理)
**P1 可执行 6 项全闭环**(批1 4 + 批2 2)。
---
## 推进状态(2026-06-15)— P2 清债批
**批3 核查闭环**(主代理独立核查 git diff + cargo workspace 全量 exit 0 + 5 crate test 共 126 passed:
- ✅ 批3-A df-ai 域:R-P2-7 client builder 三级→两级(删中间同配置重建级,两 provider 对称)+ R-P2-8 router.rs 删 + R-P2-9 stream.rs 删(lib.rs 删 2 mod);决策不抽 build_llm_client(过度设计),df-ai **31 passed**
- ✅ 批3-B df-workflow 域:R-P2-3 build_dag 边校验(node_ids HashSet O(1) + bail 野边)+ R-P2-10 dag 删 predecessors/successors + R-P2-11 registry 删 is_registered/registered_types + R-P2-12 dag_def 删 from_dag_edges(同步删 use Dag+ R-P2-13 state 删 set_waiting/set_skippedset_cancelled 保留扩注释),df-workflow **15 passed**
- ✅ 批3-C df-core+DRY 域:R-P2-4 TokenAccumulator saturating_add 3 处(add prompt/completion + total+ R-P2-5 now_millis 统一 df-coretypes.rs pub fn + lib.rs re-export + commands/mod.rs/df-storage crud.rs 转发保留函数名,37 调用点零破坏);review crate 归属错(df-core/conversation.rs 实为 src-tauri/commands/ai/、src-tauri/crud.rs 实为 df-storage/)已按真实位置修正,df-core **7 passed**
- ✅ 批3-D src-tauri 域:R-P2-1 agentic converged 互斥(达 MAX 且 has_tool_calls 仍 true 时 emit AiError+warn 前置警示「末轮工具结果未回传」,仍入库 Completed 保留内容,与 break 收敛分支互斥不重复 emit+ R-P2-2 write_file .bak 清理(rename 成功后 old_size>0 时 remove,忽略清理失败)+ R-P2-6 extract_error_diag 重写(`&bytes[i..i+3]` 字节切片→`chars().collect()`+char 迭代+HTTP_CODES 白名单,修多字节 UTF-8 panic 风险 + 长数字串内嵌误命中),devflow **58 passed**
**P2 可执行 13 项全闭环**
---
## 推进状态(2026-06-15)— P1 需设计 3 项设计文档
**3 设计文档已起草**general-purpose agent 读码 + 细化方案,待用户核对):
- 📄 **R-PD-1 密钥迁移健壮性** → [密钥迁移健壮性-2026-06-15.md](../02-架构设计/专项设计/密钥迁移健壮性-2026-06-15.md):推荐方案 A「编辑路径即时迁移」——DB 有明文+keyring 无时先 `set_provider_secret` 补密钥,成功后清 DB 明文,失败 Err 阻断 DB 不动(最小局部改动 commands.rs 一处,放弃 B 反方向维持未迁移态 / C 显式标志位 schema 演进成本)
- 📄 **R-PD-2 工作流脚本执行边界** → [工作流脚本执行边界-2026-06-15.md](../02-架构设计/专项设计/工作流脚本执行边界-2026-06-15.md):推荐方案① 删 `src-tauri/src/state.rs:227-229` script 注册掐断入口 + 下线 demoDagDevFlow workflow 纯演示,唯一构造点 demoDag 三步 echo,一行删换零攻击面,黑名单/参数过滤是不完备运行时博弈),未来真要脚本能力新建独立 BuildNode。**路径修正**build_registry 实在 `src-tauri/src/state.rs:225`review 原文 `state.rs` 无 crate 前缀易歧义——`crates/df-workflow/src/state.rs` 是 StateMachine 状态机与节点注册无关)。**R-PD-2 必须先于 R-PD-12**(否则 LLM 暗道绕过 RiskLevel 审批链)
- 📄 **R-PD-3 条件表达式引擎** → [条件表达式引擎-2026-06-15.md](../02-架构设计/专项设计/条件表达式引擎-2026-06-15.md):推荐分阶段 Phase1 数据流过滤(executor inputs 收集处调 ConditionEngine + adjacency_in 携 condition,零终态冲突独立可 ship)/ Phase2 调度短路(层调度前求值条件全 false 则 target 不执行,复活 `set_skipped` 旁路与 `set_cancelled` 同型,注释区分语义)。**待用户确认 3 决策点**:A 引擎实现(手写最小求值器 vs evalexpr 库)/ B 终态机制(set_skipped 复活 vs 其他)/ C 求值失败兜底(默认 true vs false
**与 R-P2-13 的张力**R-P2-13 删了 set_waiting/set_skipped(当时全仓零调用,扩写 set_cancelled 注释「唯一受控旁路」);R-PD-3 Phase2 接线条件引擎后 set_skipped 有了真实消费者需复活——set_cancelled「唯一旁路」注释届时需同步改。
**review 路径归属勘误汇总**(本轮 3 处,已分别在对应设计文档/实现修正):
- R-P2-4 `df-core/conversation.rs` → 实 `src-tauri/src/commands/ai/conversation.rs`TokenAccumulator
- R-P2-5 `src-tauri/src/crud.rs` → 实 `crates/df-storage/src/crud.rs`now_millis_str
- R-PD-2 `state.rs` build_registry → 实 `src-tauri/src/state.rs:225`(非 `crates/df-workflow/src/state.rs`