baa-conductor

2026-03-31_renewal-fixes-round2_review.md

  1# Review: Renewal 修复第二轮（cooldown / created_at / null link dedupe）
  2
  3- 审查者：Claude Opus 4.6
  4- 审查日期：2026-03-31
  5- 关联任务：BUG-032、BUG-033、BUG-035
  6- 任务文档：`bugs/BUG-032-dispatcher-missing-cooldown-after-success.md`、`bugs/BUG-033-upsert-local-conversation-overwrites-created-at.md`、`bugs/archive/FIX-BUG-035.md`
  7- 审查范围：`833be60..8affb3b`（4 commits）
  8- 执行者：Codex
  9- 关联文档：
 10  - `bugs/BUG-034-projector-route-unavailable-missing-detail.md`
 11  - `bugs/BUG-036-projector-cursor-updated-at-unit-mismatch.md`
 12  - `bugs/OPT-009-renewal-duplicate-utility-functions.md`
 13
 14## 涉及文件
 15
 16- `apps/conductor-daemon/src/renewal/dispatcher.ts`（修改，+17 行）
 17- `packages/artifact-db/src/store.ts`（修改，+251 行）
 18- `packages/artifact-db/src/schema.ts`（修改，+41/-7 行）
 19- `packages/d1-client/src/d1-setup.sql`（修改，+26/-1 行）
 20- `packages/d1-client/src/sync-worker.ts`（修改，+10/-1 行）
 21- `apps/conductor-daemon/src/index.test.js`（修改，+239 行）
 22- `packages/artifact-db/src/index.test.js`（修改，+386 行）
 23- `packages/d1-client/src/index.test.js`（修改，+182 行）
 24
 25## 实验结论
 26
 27三个 bug 均已修复，测试覆盖充分：
 28
 29| 修复 | commit | 验证 |
 30|---|---|---|
 31| BUG-032 cooldown | `5580526` | dispatcher 测试验证 cooldownUntil 写入 |
 32| BUG-033 created_at | `8affb3b` | artifact-db 测试验证 COALESCE 保留原值 |
 33| BUG-035 null dedupe | `cf7aa3a` | 启动去重迁移 + 5 个 partial unique index + 全套测试 |
 34
 35## 做对的事
 36
 371. BUG-035 的修复非常彻底：不仅加了 partial unique index 防止未来重复，还做了启动时的迁移逻辑去清理存量脏数据，merge 策略保留最新记录并吸收旧记录的非空字段
 382. COALESCE 保护同时应用到所有四条 UPSERT（sessions、local_conversations、conversation_links、renewal_jobs），不留死角
 393. D1 sync schema 同步更新了相同的 index 结构，保持 local/remote 一致
 404. 测试覆盖量大（807 行新测试），覆盖了 dedupe 迁移、null identity 查找、cooldown 回写等关键路径
 41
 42## 问题
 43
 44### 中等：deduplicateNullRemoteConversationLinks merge 可能静默丢失 localConversation 关联
 45
 46`store.ts:971-974`，当 canonical 和 candidate 指向不同的 `localConversationId` 时，merge 保留 canonical 的 `localConversationId`，然后 DELETE candidate。被删 candidate 关联的那个 `localConversation` 就失去了这条 link，且没有日志记录。
 47
 48```typescript
 49const merged = mergeConversationLinkRecords(canonical, candidate);
 50// merged.localConversationId = canonical.localConversationId
 51this.run(UPDATE_CONVERSATION_LINK_SQL, [...conversationLinkParams(merged).slice(1), merged.linkId]);
 52this.run("DELETE FROM conversation_links WHERE link_id = ?;", [candidate.linkId]);
 53// candidate.localConversationId 的关联静默丢失
 54```
 55
 56实际触发条件：同一 platform + routePath（或 pageUrl/targetId），remote_conversation_id 均为 NULL，但 localConversationId 不同。这在正常流程中不太可能，但如果历史数据有脏数据就会触发。
 57
 58建议：在 DELETE 前检查 `canonical.localConversationId !== candidate.localConversationId`，至少输出 warn 日志。
 59
 60### 低：resolveSuccessCooldownMs 与 intervalMs 强耦合
 61
 62`dispatcher.ts:801-807`：
 63
 64```typescript
 65return Math.max(DEFAULT_SUCCESS_COOLDOWN_MS, intervalMs + 1);
 66```
 67
 68fallback 逻辑把 cooldown 和 tick interval 耦合。如果 intervalMs 被调到 300000（5 分钟），cooldown 会跟着变成 5 分钟。cooldown 是业务参数（"多久后允许再续命"），不应受调度频率影响。
 69
 70建议：fallback 直接返回 `DEFAULT_SUCCESS_COOLDOWN_MS`，不与 intervalMs 取 max。`intervalMs + 1` 的意图（确保 cooldown 至少跨一个 tick）可以在 projector 的 eligibility 检查中由 `settleDelayMs` 保证。
 71
 72## 反向用例
 73
 74### 1. 高并发 observe 导致 null link 唯一索引冲突
 75
 76如果两个并发 `observeRenewalConversation` 同时为同一 platform + routePath 创建 link（remote_conversation_id 均为 NULL），第二个会命中 `idx_conversation_links_null_route` 的 UNIQUE 约束。当前 `upsertConversationLink` 的 `ON CONFLICT(link_id)` 不会匹配到这个 index 冲突（link_id 不同），SQLite 会抛 UNIQUE constraint 错误。
 77
 78当前单进程 + 同步 SQLite 下不会并发，但如果未来引入异步或多进程，需要处理此场景。
 79
 80### 2. 启动去重迁移在大数据量下的性能
 81
 82`deduplicateNullRemoteConversationLinks` 一次性加载所有 `remote_conversation_id IS NULL` 的行到内存。如果历史数据量大（数千条 null link），启动时间会被拉长。当前规模下不是问题。
 83
 84## 验收对照
 85
 86| 验收标准 | 是否满足 |
 87|---------|---------|
 88| BUG-032：dispatcher 成功后 cooldownUntil 被设置 | 通过 |
 89| BUG-032：projector 在 cooldown 内跳过该对话 | 通过（已有测试） |
 90| BUG-033：UPSERT 不会覆盖已有 created_at | 通过 |
 91| BUG-033：所有四条 UPSERT 都使用 COALESCE | 通过 |
 92| BUG-035：NULL remote_conversation_id 的 link 受唯一约束 | 通过 |
 93| BUG-035：启动时清理存量重复 link | 通过 |
 94| BUG-035：D1 schema 同步更新 | 通过 |
 95
 96## 建议
 97
 981. **deduplicateNullRemoteConversationLinks** 中跨 localConversation 的 merge 加 warn 日志（中等优先）
 992. **resolveSuccessCooldownMs** 去掉与 intervalMs 的耦合，直接用 `DEFAULT_SUCCESS_COOLDOWN_MS`（低优先）
1003. **TASK_OVERVIEW.md** 和 **STATUS_SUMMARY.md** 需更新：移除已修的 BUG-030，补充 BUG-032~036 和 OPT-007~009，更新基线到 `8affb3b`（文档卫生）