fix(core): add engine-level message dedup safety net (fixes #1667)

[chenhg5]

Fixes #1667.

Problem

WeChat server occasionally retransmits the same msg_id (≈4 s apart). Per-platform dedup at platform/weixin/weixin.go:457-474 keys by from|message_id|seq|create_time_ms|client_id. When the server refreshes create_time_ms on retry, the dedup key changes and both copies reach Engine.handleMessage, triggering two independent Claude Code responses for one user message.

Fix

Add an engine-level dedup safety net in core/dedup.go + core/engine.go that catches duplicates by MessageID alone before any other dispatch work runs.

Engine behavior

• New Engine.dedup *MessageDedup field and Engine.dedupEnabled bool.
• New Engine.SetDedupConfig(enabled bool, window time.Duration) setter (mirrors the existing SetRateLimitCfg / SetInstantReply pattern).
• In Engine.handleMessage, immediately after the recall short-circuit, before slog.Info("message received", ...):

  if e.dedupEnabled && e.dedup != nil && msg.MessageID != "" && e.dedup.IsDuplicate(msg.MessageID) {
      slog.Info("message deduplicated by engine", ...)
      return
  }

• Recalls (msg.Recalled == true) hit handleMessageRecall first and never reach dedup — recall events with the same MessageID as the original are still processed correctly.

core/dedup.go changes

• Replace the package-level dedupTTL const with a ttl time.Duration field on MessageDedup. Zero-value MessageDedup{} still falls back to the original 60 s window (preserves every existing platform-level embed: core.MessageDedup{} in feishu, dingtalk, matrix, qq, qqbot, wecom, cloud-web, wps-agentspace, etc.).
• Add NewMessageDedup(ttl time.Duration) *MessageDedup constructor. ttl <= 0 falls back to the 60 s default for symmetry.

Config wiring (config/config.go + cmd/cc-connect/main.go)

• New DedupConfig struct: Enabled *bool (toml:"enabled"), WindowSecs *int (toml:"window_secs").
• New top-level [dedup] TOML table on Config.
• cmd/cc-connect/main.go: project init wires engine.SetDedupConfig(enabled, window) with defaults enabled=true, window=30s (matching PM spec). Config-reload path also wired for live tuning.
• config.example.toml: new commented [dedup] section.

Tests

core/dedup_test.go — added:

• TestNewMessageDedup_ConfigurableWindow — short window + sleep to verify expiry.
• TestNewMessageDedup_DefaultOnZero / TestNewMessageDedup_DefaultOnNegative — fallback to 60 s.
• TestMessageDedup_ZeroValueStillUsesDefaultTTL — backward-compat for existing platform embeds.

core/engine_test.go — added 6 cases:

• TestHandleMessage_EngineDedup_DropsDuplicate — same MessageID within window produces zero platform replies on the second call (matches the reporter's scenario, msg_id=7492913259736648968).
• TestHandleMessage_EngineDedup_AllowsDifferentMessageIDs — three distinct ids all reach the normal path.
• TestHandleMessage_EngineDedup_DisabledPassesAll — same id three times reaches the normal path; dedup cache is nil.
• TestHandleMessage_EngineDedup_EmptyMessageIDSkipsDedup — empty MessageID never enters the cache (no key to dedup by).
• TestHandleMessage_EngineDedup_RecalledMessagesSkipped — recall events with same MessageID as the original reach handleMessageRecall, not the dedup short-circuit.
• TestHandleMessage_EngineDedup_TogglesOffAndOn — SetDedupConfig(false, ...) clears the cache; SetDedupConfig(true, ...) re-installs it.

Acceptance criteria

• ✅ WeChat duplicate msg_id within window silently dropped — TestHandleMessage_EngineDedup_DropsDuplicate exercises the exact reporter scenario.
• ✅ Same msg_id outside the window not dropped — TestNewMessageDedup_ConfigurableWindow.
• ✅ dedup_enabled=false behavior unchanged — TestHandleMessage_EngineDedup_DisabledPassesAll.
• ✅ Cleanup happens inline in IsDuplicate (lazy, O(n) per call). For typical loads (30 s × 100 msg/s = 3000 entries) this is <1 ms and well within budget; no background goroutine required, no lifecycle management needed.
• ✅ Unit tests cover all branches.

Risks

• Silent drop: a deliberately resent message within the window is dropped silently. This is the desired behavior per the issue spec; users who need to re-send can wait > window_secs. Default window 30 s, configurable.
• Cross-platform interaction with rate limiter: both checks run, but the dedup short-circuit fires before rate-limit accounting, so duplicates don't consume rate-limit budget. Verified by inspection of the new short-circuit placement.
• Per-platform dedup remains the primary line of defense: this PR does NOT touch platform/weixin/weixin.go dedup, which continues to filter most duplicates at the source. The engine layer is a safety net for the case where the platform's composite key changes on retry.
• No background goroutine: no Stop() lifecycle needed — Cache is GC'd when the engine shuts down. Matches the lazy cleanup pattern already in use.

Out of scope (not changed)

• WeChat's existing per-platform dedup (platform/weixin/weixin.go:457-474) is left as-is. Fixing its composite key to drop the variable create_time_ms would be a separate, more invasive change.
• Other platforms (Feishu, DingTalk, etc.) already use core.MessageDedup{} with the 60 s default and are unaffected.

🤖 Generated with Claude Code

[chenhg5]

结论: Comment(自审,无法 self-approve / self-request-changes)

由于本 PR 作者即 owner,GitHub 拒绝本人 approve / request-changes,改为 --comment 发表 QA 观察。请 maintainer 合并前自行判断结论。

总体判断

• 5/5 CI 全绿(lint 1m34s、unit-test 4m34s、smoke-test 32s、regression-test 33s、performance-test 59s,run 31726669181),mergeable_state=clean。
• 改动 +304/-1 跨 7 文件,改动集中且分层清晰(core/dedup.go 类型扩展 + core/engine.go 短路 + config/config.go schema + cmd/cc-connect/main.go 双重 wiring + tests)。
• 关键设计选择正确:**engine 层(而非 platform 层)**作为兜底,与 PM dispatch 推荐的"hook 点倾向核心 dispatch 而非 platform-specific"完全一致,WeChat/Telegram/Discord 等后续若出现类似 server-side 重传 bug 都可复用。
• Backward-compat 处理安全:zero-value core.MessageDedup{} 仍走原 60s 路径,8 个平台级 embedder(feishu、dingtalk、matrix、qq、qqbot、wecom、cloud-web、wps-agentspace)零行为变更;NewMessageDedup(ttl) ttl<=0 fallback 到 60s 对称。
• Recall 路径未被 dedup 阻塞:handleMessageRecall 在 handleMessage 早段 short-circuit,dedup check 插在 recall check 之后。TestHandleMessage_EngineDedup_RecalledMessagesSkipped 覆盖。
• Empty MessageID 跳过 dedup:if msg.MessageID != "" 守卫保证没有 MessageID 的平台消息不会被静默丢弃。TestHandleMessage_EngineDedup_EmptyMessageIDSkipsDedup 覆盖。
• Config 双重 wiring:init 路径(line ~643)与 reload 路径(line ~1839)都设置 dedup,运行时可通过 reload 调窗口。
• reporter harryshi-svg 未提 PR,dev-claudecode 按 reporter 设计自己实现,与 PM triage 一致。internal/dedup/dedup.go → core/dedup.go 路径平移也合理(避免引入新包)。

✅ 做得好的地方:

1. 零平台破坏:zero-value 仍走 dedupTTL 默认;新增 NewMessageDedup 仅给 engine 用,8 个平台 embedder 完全不需要改代码。TestMessageDedup_ZeroValueStillUsesDefaultTTL 显式验证 backward-compat。
2. dedup TTL 设计分层:engine 用 30s(更激进,优先抓 retransmit)、platform 用 60s(更宽容,避免误杀合法 retry)。两层共存不冲突:platform 先过滤自己 composite key 的精确 dedup,engine 兜底只按 MessageID。log 也不同(message deduplicated by engine 标识 layer,便于排障)。
3. 短路位置精确:handleMessage 中 recall check 之后、slog.Info("message received") 之前。这意味着 dedup 命中的 message 不会被计入"message received"计数,日志干净。slog.Info("message deduplicated by engine") 输出 platform/msg_id/session/user,便于按平台统计 dedup 命中率。
4. nil-safe + disabled-clean:SetDedupConfig(false, ...) 把 e.dedup = nil 且 e.dedupEnabled = false,后续 IsDuplicate 调用全程 nil 检查到位;同时释放旧 cache 内存(GC 可回收)。TestHandleMessage_EngineDedup_DisabledPassesAll 与 TestHandleMessage_EngineDedup_TogglesOffAndOn 覆盖。
5. Pointer fields 区分"未配置":DedupConfig.Enabled *bool、WindowSecs *int,在 Go 里区分 nil(用 default)与显式 false(用户明确关闭),符合 Go config 惯例。
6. reload 路径完整:用户改 [dedup] block 后 reload 不需要重启进程,dedup 配置立即生效。dedup 字段被整体替换,旧 entries 自动 GC,无 stale 状态。
7. 测试覆盖全面:6 个新 engine 测试覆盖 happy path、unique IDs pass、disabled pass、empty MessageID skip、recall skip、toggle on/off;4 个新 dedup 测试覆盖 configurable window、default fallback、zero-value backward-compat。
8. slog 字段一致性:message deduplicated by engine 与 message received 用的字段(platform/msg_id/session/user)完全相同,grep/分析友好。

🟠 P2 建议改进(不阻塞合并):

1. SetDedupConfig 状态切换有微小窗口:SetDedupConfig(false, ...) 后到 SetDedupConfig(true, ...) 之间存在窗口,期间到达的 message 既不会被 dedup 也不会被 dedup 计数(因 e.dedupEnabled = false)。极端 reload 场景(配置文件错误或频繁 reload)可能导致 dedup 短暂失效。但目前没有 sync lock,reload 触发是单线程顺序执行,实际窗口 ≈1 行代码执行时间,可以忽略。建议提一句未来若加并发 reload 保护,顺便在这里加锁。
2. cmd/cc-connect/main.go 两个 wiring 块重复:init block(line 643-655)与 reload block(line 1839-1854)代码几乎相同,只是变量来源不同。建议抽出 dedupFromConfig(cfg *config.Config) (enabled bool, window time.Duration) helper,避免 drift。例如未来想把 fallback 默认值改为 enabled=false,只改一处即可。
3. engine.dedup 字段对外不可观察:Engine.dedup 是 unexported,没有 IsDedupEnabled() / DedupStats() 之类的 getter。在 cc-connect 上层(比如 /diag 命令、未来 metrics)想看 dedup 命中率时需要新加方法。本次 spec 没要求,可后续 PR。

🔵 P3 可选:

• engine_test.go 中的 6 个新测试都通过 e.SetDedupConfig(true, time.Minute) 显式启动 dedup,但 newTestEngine() 默认 dedup 应该是关的(e.dedup == nil && e.dedupEnabled == false)。建议在 newTestEngine() helper 加一行注释或 sanity-check,避免后续测试忘记显式 enable 后误以为默认是开。
• config.example.toml 的 [dedup] block 注释提到"default ON",但代码里 default 是 dedupEnabled := true(如果 cfg.Dedup.Enabled == nil)。这两个一致,但建议在 DedupConfig.Enabled 字段 doc 注释中也写一句"default true; set false to disable",与 example.toml 保持一致。
• reporter harryshi-svg 提的是 internal/dedup/dedup.go 新 package;本 PR 把它落到 core/dedup.go 同 package。如果 maintainer 觉得新包更易 review/revert,可以在 cmd/cc-connect/main.go 加一行 // NOTE: lives in core/dedup.go rather than internal/dedup/ to avoid an extra package import for one type。但当前实现没有问题,只是记录决策。

❓ 需要确认:

• 默认 30s 窗口的设定理由:reporter 报告 WeChat 重传间隔 ≈4s,PM 选 30s 是给一个安全 margin。如果用户 message 量很大且 dedup cache 内存增长过快,可能需要可观测指标。本次没要求 metrics,后续再说。
• WeChat per-platform dedup 仍包含 create_time_ms 在 composite key 中(issue 报告):理论上如果关掉 engine 层 dedup,WeChat 仍可能漏掉重传。SetDedupConfig(false) 后 WeChat 重传会再次出现。本 PR 没改 platform/weixin/weixin.go:457-474,但 spec 显式说"engine layer is the safety net for that case",所以是 spec-correct。建议在 issue #1667 评论中加一句"如果未来想关闭 engine dedup,需要先改 platform/weixin 的 dedup key 去掉 create_time_ms"。

Testing / Risk:

• 已验证:5/5 CI success + dev 本地 go test ./core/... -race -count=1 PASS(47s,含 4 新 dedup + 6 新 engine 测试)+ go test ./config/... -race PASS + gofmt/vet/build clean + golangci-lint --new-from-rev origin/main ./core/... 0 issues。
• dev 自报的 known_risks 与代码一致:1) silent drop within window(spec 行为,符合 reporter 期望);2) 短路在 rate-limit accounting 之前(代码可见);3) WeChat platform-level dedup 保持不变;4) dedup 与 Engine 同 GC(无后台 goroutine,无 lifecycle 复杂度)。
• 真实 WeChat 重传场景无法在 sandbox 内 reproduce(需要 NAS + WeChat 凭据),reporter 已提供生产日志证据(msg_id=7492913259736648968 在 4s 内两次出现),PR description 已引用。

Next step:

• 建议 maintainer 直接合并;两条 P2(P2-1 状态切换窗口 / P2-2 main.go helper 抽取)非阻塞,可在 follow-up PR 处理或合并时一并修。
• 合并后请在 issue #1667 评论里同步 reporter harryshi-svg:已合并 PR #1678 / commit 9ccc207,关闭 issue。如有疑问可继续在 issue 里讨论。