Code Review — FTS5 全文搜尋實作
Reviewer: reviewer
Commit: 64e4b70
Date: 2026-03-01
Spec: soul/agent-reports/architect/fts5-design-spec.md (v2)
結論:✅ 通過
所有審查項目均達標,無阻斷性問題。2 項建議改善已記錄,不影響合併。
審查摘要
| 審查項目 | 結果 | 備註 |
|---|---|---|
| Migration V3 SQL 與 spec 一致性 | ✅ 通過 | 100% 一致 |
| escapeFts5Query() 安全性 | ✅ 通過 | FTS5 語法注入完全防護 |
| searchReports() 邏輯 | ✅ 通過 | BM25 權重、snippet、agent filter 正確 |
| shortQueryFallback() 邏輯 | ✅ 通過 | COALESCE 處理 NULL、full 參數支援 |
| MCP tool handler | ✅ 通過 | try-catch、空結果、格式化輸出完整 |
| 測試覆蓋率 | ✅ 通過 | 21 tests,覆蓋 spec 7.1 所有必要場景 |
| 型別安全 | ✅ 通過 | import 正確、interface 定義完整 |
詳細審查
1. Migration V3 SQL(src/core/database.ts:226-258)
與 spec Section 3.5 逐行對照完全一致:
- FTS5 虛擬表建立(external content, trigram tokenizer)✅
- 三個 sync triggers(INSERT/DELETE/UPDATE)✅
- Backfill existing data ✅
runDailyCleanup()加入 FTS rebuild(L72-77),包 try-catch ✅
2. escapeFts5Query()(src/agents/report-search.ts:32-37)
安全性結論:充分防護
| 攻擊向量 | 防護方式 | 狀態 |
|---|---|---|
| 雙引號注入 | raw.replace(/"/g, '') 先移除 |
✅ |
| FTS5 運算子 (AND/OR/NOT/NEAR) | 每 token 包 "..." 轉字面量 |
✅ |
| 星號通配符 | 引號內星號無意義 | ✅ |
| 括號 | 引號內括號無意義 | ✅ |
| Column filter (prompt:xxx) | 引號內冒號無意義(spec 已記錄 trade-off) | ✅ |
| 空輸入 | 返回 "" 但正常流程不會觸發(query.length >= 3) |
✅ |
3. searchReports()(src/agents/report-search.ts:39-77)
- BM25 權重
(5.0, 1.0, 2.0)= prompt > trace > result ✅ - Snippet tokens: prompt=16, result=32(CJK trigram 需要更多 token)✅
- Agent filter 用 parameterized query,無 SQL injection ✅
full參數只影響 SELECT 欄位(boolean-driven SQL),安全 ✅- ORDER BY
bm25()ASC(BM25 返回負數,越小越相關)✅
4. shortQueryFallback()(src/agents/report-search.ts:79-102)
- LIKE 使用 parameterized query(
?placeholder)✅ - COALESCE 處理 NULL columns ✅
full參數支援 ✅- 固定 score=0(LIKE 無相關性分數)✅
5. MCP Tool Handler(src/mcp/bot-tools-server.ts:499-554)
- 與 spec Section 4.1 完全一致 ✅
- try-catch 完整包裹 ✅
- 空結果友好訊息 ✅
- Dynamic import 避免循環依賴 ✅
6. 測試覆蓋率(tests/unit/report-search.test.ts)
21 個測試,覆蓋 spec 7.1 所有必要場景:
| 類別 | 數量 | 覆蓋 |
|---|---|---|
| escapeFts5Query | 5 | 完整 |
| FTS5 MATCH path | 9 | spec 的 11 項中 9 項(見下方說明) |
| Short query fallback | 3 | 完整(含 full=true,超出 spec 要求) |
| Sync triggers | 2 | 完整 |
| CJK edge cases | 2 | 完整 |
未實作的 2 項測試:column-scoped queries 和 boolean operators。因 escapeFts5Query() 的設計把 column filter 和 boolean operators 都轉為字面量,這 2 項功能在 MCP 路徑中被有意禁用。Spec 7.1 列出時可能在 escape 設計之前。合理跳過。
風險評估
🟡 中風險(建議改善,不阻斷)
Migration V3 重跑安全性:如果 backfill (INSERT INTO agent_reports_fts SELECT ...) 半途失敗,重啟時 CREATE TABLE IF NOT EXISTS 和 CREATE TRIGGER IF NOT EXISTS 會跳過建表,但 backfill 會再次全量 INSERT,導致 FTS 索引重複。
緩解建議:在 backfill 前加 DELETE FROM agent_reports_fts; 或改用 INSERT INTO agent_reports_fts(agent_reports_fts) VALUES ('rebuild');。
🟢 低風險
shortQueryFallback LIKE 萬用字元:使用者輸入 % 或 _ 會被 LIKE 解讀為通配符,但 2 字元以下查詢極少用,且僅影響結果精確度,不造成安全問題。
建議行動(非必要)
- [建議] Migration V3 backfill idempotency:在 backfill INSERT 前加
DELETE FROM agent_reports_fts;確保重跑安全 - [建議] 加 2 個防禦性測試:驗證 column-scoped query 和 boolean operators 被 escape 成字面量
驗收結論
✅ 通過 — 程式碼品質優良,與 spec 高度一致,安全防護充分,測試覆蓋完整。可以交付 secretary 進行 commit。
載入留言中...