Files
gochat/review-h24.md
T
Rogeeandrogee c922460649 H-97: fix Shangwutong conversation routing (#16)
* H-60: harden Captain migration rollback and concurrency

* chore(agent): baseline — uncommitted work from the local directory

* fix: route Shangwutong events by display id

---------

Co-authored-by: Rogee <rogee@ipao.vip>
2026-08-14 14:35:12 +08:00

14 lines
3.2 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.
审查结论:不通过,有阻断项;不能批准 H-23 当前实现。
1. **P0:kind=52 的 cid 实际不会持久化,反写闭环不可达。** `channels/shangwutong/internal/delivery/mapping.go:170-172` 只设置 `ContactCID`,但仍保持 `RawOnly=true` 且 `RequiresContact=false`;随后 `channels/shangwutong/internal/delivery/inbound.go:106-108` 直接完成事件,根本不会执行 `ensureResources` 的 `ChannelMetadata` 写入(`:165-167`)。即使只把 kind=52 标成需联系人,`:168` 的 `needsContactWrite` 也没有包含 `ContactCID`,已有联系人仍不会发 PATCH。结果是 `contact_inboxes.channel_metadata.cid` 为空,监听器永远不会创建 `contact_updated` job。最小修复:kind=52 走仅写渠道元数据的路径,并将 CID 变化纳入写入条件;保留不改变 Contact 可编辑字段的约束。
2. **P1:公共 Contact Create 的已有 source 分支丢弃 cid。** `backend/internal/service/widget_service.go:769-784` 调用 `setContactInboxChannelMetadata` 后只修改内存对象,没有 `contactInboxRepo.Update`;新建分支 `:798-807` 才会落库。只要 CID 到达已有 contact_inbox,就会静默丢失。应在 metadata 实际变化时持久化,并覆盖回归测试。
3. **P1:公共 API 接受客户端可编辑的 cid,存在错误访客反写风险。** `backend/internal/handler/widget/widget_handler.go:1394-1430` 从 public 请求绑定 `channel_metadata`,`backend/internal/service/widget_service.go:821-845` 在 PublicUpdateContact 中写入它。HMAC 可配置为非 mandatory;此时知道 source_id 的访客可提交任意 cid,客服后续改名会按该 cid 调用 changecname.aspx,可能修改错误商务通客户。这也违反“cid 不作为可编辑业务属性”。应将 cid 写入限制到受信任的 Connector 内部接口/强制 HMAC 路径,公共 Contact API 不接受该字段。
4. **P1:durable job 入队错误被吞掉,更新成功但反写任务可永久丢失。** `backend/internal/handler/api/v1/contact_handler.go:293-297` 忽略 `channelDispatcher.Dispatch` 错误。数据库/worker 暂时不可用时 Contact 已提交,HTTP 仍返回 200,且没有重试或补偿。应把更新与 durable enqueue 放进同一事务,或至少传播失败并提供可靠补偿。
5. **P2:Connector 回滚迁移会删除所有姓名反写 operation。** `channels/shangwutong/db/migrations/002_add_rename_operation.down.sql:1-21` 只复制 `operation='end_conversation'` 的行,随后 drop 原表;pending、delivered、uncertain 的 `change_contact_name` 记录都会被静默删除。回滚应拒绝存在这类数据,或先保留/迁移这些记录,不能无提示丢任务。
测试证据:`cd channels/shangwutong && go test ./...` 通过;相关后端 handler/widget、channel、service 定向测试通过。`cd backend && go test ./...` 仍失败于 `internal/database/TestRollbackMigrations_EmptyPath_Cov8` 和 `internal/service/TestMessageService_Create`,两项在父提交 `4bf6ba6b` 也可复现,非本改动引入。Connector 的 `ChangeContactName` 使用本地 httptest 验证了 form 与 `r: ok`,但没有覆盖 cid 从 kind=52 到 contact_inbox、跨 inbox、重复 contact_updated webhook 的端到端假服务测试。