重构: 文档汇总+进度看板+孤儿任务清理脚本+gitignore 噪音排除
- docs/02 架构设计: 新增 aichat审查/异步审批构想/流式渲染调研/generating状态机/密钥迁移健壮性/工作流脚本执行边界/条件表达式引擎/F-07 trait下沉/Agent架构说明/任务推进构想/功能创意池;更新功能决策记录+归档/对抗论证/文档记录规范/经验记录
- docs/03 模块文档: 新增 AI对话引擎/DAG引擎详解;更新 df-knowledge/df-nodes/df-storage/df-workflow/df-ai
- docs/05 代码审查: 新增 全栈审查/全局review/架构审查/近期改动审查/工作区多角度走查/自研memo流式渲染审查
- docs/09 问题排查: 新增 aichat-apikey-401
- docs/INDEX+README 索引同步;docs/todo 待办看板(2026-06-15 汇总)
- PROGRESS.md Sprint 22-25;URGENT.md 加急清单快照(5 项 P0 已全修)
- scripts/cleanup_orphan_tasks.{py,sh} 孤儿任务清理工具
- .gitignore 补 *.broken.bak + tmp/ 噪音排除
This commit is contained in:
201
docs/05-代码审查/近期改动代码审查-2026-06-15.md
Normal file
201
docs/05-代码审查/近期改动代码审查-2026-06-15.md
Normal file
@@ -0,0 +1,201 @@
|
||||
# 近期改动代码审查报告（2026-06-15）
|
||||
|
||||
> 范围：工作区未提交 Rust（FR-S1 密钥管理 / FR-S7 写入防护 / FR-S8 symlink 防护）+ 近 5 个提交（Rust 后端 7 文件 + 前端 12 文件），约 770 行。
|
||||
> 方法：主代理深读工作区安全改动（secret.rs / tool_registry.rs / commands.rs / lib.rs，含 resolve_workspace_path 双层校验、write_file 原子写、list_dir symlink 处理）+ 2 个 general-purpose 子代理并行审「近 5 提交 Rust 后端」「近 5 提交前端」，主代理对三路结果核实去重并补查 keyring 闭环。
|
||||
> 去重：与 [全栈代码审查报告-2026-06-14.md](全栈代码审查报告-2026-06-14.md) / [架构与缺陷复核报告-2026-06-14.md](架构与缺陷复核报告-2026-06-14.md) 范围互补——前两份是 06-14 全栈基线审查，本轮针对 **06-14 之后的提交（FR-S 安全修复、confirm 抽取、dag 拓扑 O(V+E)、流式健壮性）+ 工作区未提交**。aichat AR 系列、FR-C/S/R 已修项不重复，仅审本轮代码本身引入或暴露的新问题。
|
||||
> 互斥：本报告 §①（FR-S1 keyring 清理闭环）与 [aichat审查报告](../02-架构设计/aichat审查报告-2026-06-14.md) 的 FR-S1 条目同源（密钥迁移至 keyring），本轮发现其删除路径闭环缺失。
|
||||
|
||||
---
|
||||
|
||||
## 审查范围
|
||||
|
||||
**工作区未提交（Rust）**：
|
||||
- `src-tauri/src/commands/ai/secret.rs`（新文件，FR-S1 keyring 管理）
|
||||
- `src-tauri/src/commands/ai/tool_registry.rs`（write_file 原子写+.bak / read_file 二进制降级+limit 上限 / list_dir symlink 防护）
|
||||
- `src-tauri/src/commands/ai/commands.rs`（ai_list_providers mask / ai_save_provider 密钥转 keyring）
|
||||
- `src-tauri/src/commands/ai/{agentic,knowledge_inject,title,mod}.rs`、`commands/project.rs`、`lib.rs`（resolve_provider_secret 接入 + 启动迁移）
|
||||
|
||||
**近 5 提交（commit 4b5f096 → f58743e）**：
|
||||
- Rust：`df-ai/{anthropic_compat,openai_compat,context}.rs`、`df-workflow/{dag,executor}.rs`、`df-storage/crud.rs`、`audit.rs`
|
||||
- 前端：`ToolCard.vue`、`AiChat.vue`、`useAiEvents.ts`、`useConfirm.ts`、4 个 view、2 个 i18n
|
||||
|
||||
---
|
||||
|
||||
## 🔴 必须修复（1）
|
||||
|
||||
### ① [commands.rs:390-401] 删 provider 不清理 keyring — FR-S1 密钥残留泄漏
|
||||
|
||||
**现状**：FR-S1 把密钥迁到 OS keyring（`devflow-ai-provider/<id>`），DB `api_key` 列置空。`secret.rs:44` 已定义 `delete_provider_secret`，但 `ai_delete_provider` 只调 `state.ai_providers.delete`，**keyring entry 永久残留**。
|
||||
|
||||
**问题**：用户「删除 provider」本意含撤销密钥，残留密钥仍可被同用户下任意进程读取；迁移后若同 id 被复用（new_id 为 uuid 概率极低，但外部指定 id 场景存在），旧密钥复活。FR-S1 安全特性闭环缺一环。
|
||||
|
||||
**修法**：删 DB 后清理 keyring（失败仅日志不阻断——DB 已删，残留 keyring 无消费方）。
|
||||
|
||||
```diff
|
||||
pub async fn ai_delete_provider(
|
||||
state: State<'_, AppState>,
|
||||
provider_id: String,
|
||||
) -> Result<(), String> {
|
||||
state.ai_providers.delete(&provider_id).await.map_err(|e| e.to_string())?;
|
||||
+ // FR-S1:清理 keyring 残留密钥(失败仅日志,不阻断删除——DB 已删,残留 keyring 无消费方)
|
||||
+ if let Err(e) = super::secret::delete_provider_secret(&provider_id) {
|
||||
+ tracing::warn!("[FR-S1] 删除 provider 后清理 keyring 失败 {}: {}", provider_id, e);
|
||||
+ }
|
||||
let mut session = state.ai_session.lock().await;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🟡 建议改进（7）
|
||||
|
||||
### ② [audit.rs:50] `_` 把 DB Err 误报为「项目已不存在」
|
||||
|
||||
**现状**：`get_by_id` 的 `Err`（DB 故障/锁/连接断）与 `Ok(None)`（真不存在）被 `_` 合并，DB 出错时用户看到误导性「项目没了」而非错误。
|
||||
|
||||
**修法**：分三臂，`Err` 打日志回退裸 id。
|
||||
|
||||
```diff
|
||||
- match repo.get_by_id(id).await {
|
||||
- Ok(Some(p)) => format!("「{}」(id={})", p.name, id),
|
||||
- _ => format!("(项目已不存在, id={})", id),
|
||||
- }
|
||||
+ match repo.get_by_id(id).await {
|
||||
+ Ok(Some(p)) => format!("「{}」(id={})", p.name, id),
|
||||
+ Ok(None) => format!("(项目已不存在, id={})", id),
|
||||
+ Err(e) => { tracing::warn!(%id, error=%e, "查项目标签失败"); format!("(id={})", id) }
|
||||
+ }
|
||||
```
|
||||
|
||||
### ③ [openai_compat.rs:414] `anyhow::anyhow!(e)` 丢 error source 链
|
||||
|
||||
**现状**：非超时分支用 `anyhow!(e)` 把 `reqwest::Error` 整体塞进 message，丢失 `#[source]` 因果链（`?` 本会保留）。注释强调「不静默挂」，此处反而降低可追溯性。
|
||||
|
||||
**修法**：保留 source。
|
||||
|
||||
```diff
|
||||
- } else {
|
||||
- anyhow::anyhow!(e)
|
||||
- }
|
||||
+ } else {
|
||||
+ anyhow::Error::from(e)
|
||||
+ }
|
||||
```
|
||||
|
||||
### ④ [AiChat.vue:371-387] 第五份 confirm 逻辑未迁移 useConfirm
|
||||
|
||||
**现状**：本次「confirmDialog 抽 composable」收敛了 Projects/ProjectDetail/Ideas/Settings 四处 `{visible,msg,resolve}+Promise` 样板，但 `AiChat.vue` 是同模式第五处（reactive 版，删对话/清空消息），被遗漏。抽取做了一半，下次改 confirm 语义这里会脱节。
|
||||
|
||||
**修法**：直接复用 composable。
|
||||
|
||||
```diff
|
||||
- const confirmState = reactive({ visible:false, msg:'', resolve:null as null|((v:boolean)=>void) })
|
||||
- function confirmDialog(msg:string):Promise<boolean> { /* Promise 样板 */ }
|
||||
- function answerConfirm(ok:boolean) { /* ... */ }
|
||||
+ const { confirmState, confirmDialog, answerConfirm } = useConfirm()
|
||||
```
|
||||
|
||||
### ⑤ [ToolCard.vue:313] displayArgValue 对非 string id 降级为裸值
|
||||
|
||||
**现状**：项目名回显前提是 `typeof arg.val === 'string'`。若后端把 id 序列化成 number（JSON 无引号整数），`id` 变空串、分支不进、落到裸数字显示——恰是本次「可读化」要消灭的形态，且无告警。既然已为「查不到项目名」做 `projectIdNotFound` 兜底，类型不一致这一更基本的不可靠也应收口。
|
||||
|
||||
**修法**：归一为 string。
|
||||
|
||||
```diff
|
||||
- const id = typeof arg.val === 'string' ? arg.val : ''
|
||||
+ const id = typeof arg.val === 'string' || typeof arg.val === 'number' ? String(arg.val) : ''
|
||||
```
|
||||
|
||||
> 若确认后端 id 恒为 string uuid，此项可降为 ⚪。
|
||||
|
||||
### ⑥ [tool_registry.rs:469-484] `.bak`/`.tmp-write` 残留污染目录视图
|
||||
|
||||
**现状**：FR-S7 覆盖前备份 `path.bak` + 原子写用 `path.tmp-write`，两者留在 workspace 内且**永不清理**，多次写入堆积；`list_directory` 不过滤它们（`is_noise_dir` 只过滤目录），LLM/用户看到的目录混入噪音文件，LLM 可能误读 `.bak` 当真实文件。
|
||||
|
||||
**修法**：`.bak` 是给用户恢复用的安全网不能删；改为 `list_dir_recursive` 把 `.bak`/`.tmp-write` 后缀并入跳过，或写入前清理同 path 旧 `.bak`（只留最新一份）。
|
||||
|
||||
```diff
|
||||
+fn is_noise_file(name: &str) -> bool {
|
||||
+ name.ends_with(".bak") || name.ends_with(".tmp-write")
|
||||
+}
|
||||
// list_dir_recursive 推 entry 前过滤
|
||||
+if is_noise_file(&name) { continue; }
|
||||
```
|
||||
|
||||
### ⑦ [crud.rs:1187] COLS 手写列串易与表结构漂移
|
||||
|
||||
**现状**：`COLS` 硬编码列名，表加列/`KnowledgeRecord` 改字段时编译期不报错（query_map 按列名读，列少才运行时崩）。「列限定」目标未真正达成——隐式依赖从「表全列」挪到「手写列串」。注释偏长（10 行 TODO）。
|
||||
|
||||
**修法**：加一行测试断言列数 == 14 防漂移，或注释压缩并点明「改表结构需同步本常量」。
|
||||
|
||||
### ⑧ [ToolCard.vue:288-293] projectNameById computed 过度结构化
|
||||
|
||||
**现状**：computed 每次重建覆盖全部项目（含回收站）的 Map，而审批卡通常仅 1-2 行用项目 id。为单点查询做的全量索引，收益成本不匹配。
|
||||
|
||||
**修法**：直接 find，命中即返（审批场景项目数有限、find 提前退出，响应式仍由 store 数组保证）。
|
||||
|
||||
```diff
|
||||
- const projectNameById = computed<Record<string,string>>(() => { /* 遍历构建 map */ })
|
||||
- const name = projectNameById.value[id]
|
||||
+ const name = projectStore.projects.find(p => p.id === id)?.name
|
||||
+ ?? projectStore.deletedProjects.find(p => p.id === id)?.name
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ⚪ 可选优化（5）
|
||||
|
||||
### ⑨ [dag.rs:126 / executor.rs:91] 邻接表两处镜像重复
|
||||
|
||||
`topological_layers` 与 executor 各自内联重建邻接表，旧 `successors`/`predecessors`（O(E) 全扫）成死代码候选。建议抽 `Dag::adjacency_out()/adjacency_in()` 供两处复用，或确认无调用方后删。
|
||||
|
||||
### ⑩ [anthropic_compat.rs:167] 占位 id `tool_missing_{idx}` 理论可撞
|
||||
|
||||
流式缺 id 用 index 兜底，同 index 复用理论可撞。实际 index 唯一、低概率，注释已说明。可加计数器求稳。
|
||||
|
||||
### ⑪ [useConfirm.ts:1] `//!` 注释风格
|
||||
|
||||
Rust doc 风格，但本仓 composable 既有此约定，保持一致即可，不必动。
|
||||
|
||||
### ⑫ [ToolCard.vue:61+] `parsed?.` 冗余可选链
|
||||
|
||||
这些引用都在 `v-if/parsed` 链内，到达时必非空。非本次引入。
|
||||
|
||||
### ⑬ [useAiEvents.ts:79] 注释「沿用正向扫描」表述含糊
|
||||
|
||||
改为「消息内 toolCalls 通常 1-2 条，正序即可」更清晰。
|
||||
|
||||
---
|
||||
|
||||
## ✅ 亮点
|
||||
|
||||
- **FR-S7 原子写 + .bak 双保险**（tool_registry.rs:467-498）：write 崩溃不留半成品（tmp→rename），误覆盖有 .bak 兜底 + 缩减>90% warn。堵住「LLM 把 write_file 当 edit 致数据彻底丢失」的真实事故（会话 3473fcb7，PROGRESS.md 762 行/72KB 被覆盖成 248 字节）。
|
||||
- **FR-S8 symlink 防护**（tool_registry.rs:528-540）：`file_type()` 不跟随 symlink，symlink 标记但不递归——精准堵住「workspace 内 symlink 指向外部」的逃逸，且不取目标 metadata。防御点选在正确边界，非中间层冗余校验。
|
||||
- **findToolCall 反向扫描权衡留痕**（useAiEvents.ts:75-85）：注释讲透「为何不建 Map 索引」（messages 多处整体替换、独立索引易陈旧），行为等价、最坏复杂度明确，务实画像下优选。
|
||||
- **renderMd 流式纯文本短路**（AiChat.vue:347-360）：streaming 时跳过 marked+sanitize 避掉帧，刻意不缓存流式文本（防污染），false 翻转自动走 markdown 重渲染，决策清晰无副作用。
|
||||
- **resolve_workspace_path 双层校验**（tool_registry.rs:50-65）：词法层 `starts_with`（兜底不存在路径）+ canonicalize 层（解析存在路径的 symlink），返回词法 resolved 保证前端友好。设计扎实。
|
||||
|
||||
---
|
||||
|
||||
## 📊 摘要
|
||||
|
||||
| # | 等级 | 文件:行 | 修改内容 |
|
||||
|---|------|---------|----------|
|
||||
| ① | 🔴 | commands.rs:390 | 删 provider 补 keyring 清理（FR-S1 闭环） |
|
||||
| ② | 🟡 | audit.rs:50 | Err 与 None 分臂，防误报「项目不存在」 |
|
||||
| ③ | 🟡 | openai_compat.rs:414 | 保留 error source 链 |
|
||||
| ④ | 🟡 | AiChat.vue:371 | 第五份 confirm 迁移 useConfirm |
|
||||
| ⑤ | 🟡 | ToolCard.vue:313 | id 归一 string 覆盖 number |
|
||||
| ⑥ | 🟡 | tool_registry.rs:469 | .bak/.tmp-write 列表过滤防噪音 |
|
||||
| ⑦ | 🟡 | crud.rs:1187 | COLS 列数断言防漂移 |
|
||||
| ⑧ | 🟡 | ToolCard.vue:288 | projectNameById 改 find |
|
||||
| ⑨ | ⚪ | dag.rs/executor.rs | 邻接表抽公共方法消除重复 |
|
||||
| ⑩ | ⚪ | anthropic_compat.rs:167 | 占位 id 加计数器 |
|
||||
| ⑪ | ⚪ | useConfirm.ts:1 | 注释风格（保持现状） |
|
||||
| ⑫ | ⚪ | ToolCard.vue:61+ | 去冗余可选链 |
|
||||
| ⑬ | ⚪ | useAiEvents.ts:79 | 注释表述 |
|
||||
|
||||
**总计**：🔴1 🟡7 ⚪5
|
||||
|
||||
**总体评价**：改动方向扎实——FR-S7/S8 安全防护、拓扑 O(V+E)、流式健壮性、confirm 抽取都对路，注释质量高（权衡/根因都留痕）。唯一真实闭环漏洞是 ①（FR-S1 删 provider 漏清 keyring）。其余为 DRY 收口（④抽取只完成 4/5）与可维护性。
|
||||
|
||||
**质量评级**：良（修 ①④ 可达优）
|
||||
Reference in New Issue
Block a user