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