feat(channels): observe group names from inbound messages - #7155
Conversation
E2E test reportTested commit: ScenarioAn isolated test injected a DingTalk Stream callback using the official Result
Transport boundaryPR #7109's live E2E evidence remains the transport precondition: a real DingTalk Stream connection already verified direct and group inbound messages, complete CleanupTemporary test source, workspace, |
|
Thanks for the PR! Re-run confirms the previous assessment. Template looks good ✓ Problem: This is a real usability gap, not theoretical hardening. The observed-contact API returns opaque platform IDs as group labels even when the inbound callback already carries a readable name. Linked issue #7154 documents the need. Direction: Aligned. The change makes the observed-contact graph more useful for proactive-delivery target selection without adding API calls or permissions. Stays passive — only observes metadata already in the inbound envelope. Depends on #7109 (stacked PR). Size: No core modules touched ( Approach: Minimal and focused. One optional 中文说明感谢贡献！Re-run 确认此前评估。 模板完整 ✓ 问题：这是真实的可用性问题，不是理论性加固。即使入站回调已携带可读群名，观测联系人 API 仍返回不透明的平台 ID。关联 issue #7154 记录了该需求。 方向：对齐。在不增加 API 调用或权限的前提下，让观测联系人图谱在主动投递目标选择时更实用。纯被动观测——只使用入站 envelope 已有的元数据。依赖 #7109（stacked PR）。 规模：未触及核心模块（仅 方案：最小且聚焦。一个可选 — Qwen Code · qwen3.7-max Reviewed at |
|
Code review: No critical blockers or AGENTS.md violations found. The implementation cleanly reuses Independent proposal comparison: I would have done exactly what this PR does — add an optional Test ResultsAll 568 tests pass, including the 6 new test cases covering: usable group name propagation, unusable name fallback to complete ID, DM ignoring Real-scenario testing: N/A — this is a backend channel adapter metadata change with no CLI/TUI impact. Unit tests cover the contract. 中文说明代码审查：未发现关键缺陷或 AGENTS.md 违规。 实现干净地复用了 独立方案对比：如果我来实现，做法完全一致——添加可选 测试结果：568 项全部通过，包含 6 个新增测试用例。 真实场景测试：不适用——这是后端 channel adapter 元数据变更，无 CLI/TUI 影响。 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 — Clean across every stage; would merge without hesitation. This PR does exactly one thing and does it well: it preserves human-readable group names that are already sitting in the inbound callback, instead of discarding them. 26 production lines, no new abstractions, no API calls, no permissions. The sanitization reuses the proven 中文说明置信度：5/5 — 各阶段均通过，可直接合并。 这个 PR 只做一件事且做得很好：保留入站回调中已有的可读群名，而不是丢弃它。26 行生产代码，无新抽象，无 API 调用，无额外权限。清洗逻辑复用了已验证的 — Qwen Code · qwen3.7-max Reviewed at |
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "workspace_channel_observed_contacts" |
— Qwen Code · serve A/B
Review — LGTM (stacked delta over #7109)Reviewed at head What the delta doesAdds optional Verified
Minor notes (no action required)
Merge sequencingMerge #7109 first, then rebase/merge this so the diff collapses to the follow-up. Nothing here changes my ready-to-merge assessment of the base PR. 中文说明评审 — LGTM（基于 #7109 的 stacked 增量）评审 head 为 已验证
次要备注（无需修改）
合并顺序先合 #7109，再 rebase/合并本 PR 使 diff 收敛为跟进改动。本 PR 不改变我对基础 PR 可合并的评估。 |
|
@qwen-code /resolve |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| const sanitizedChatName = envelope.chatName | ||
| ? sanitizeSenderName(envelope.chatName) | ||
| : ''; |
There was a problem hiding this comment.
[Suggestion] Group display names from DingTalk conversationTitle and Telegram chat.title are sanitized through sanitizeSenderName, which hard-truncates to 64 code points — but the downstream store supports labels up to 256 characters and the design doc states the store's 256-character bound is the operative limit.
Failure scenario: A DingTalk group named "Engineering All-Hands — Cross-Team Sync for Platform, Infrastructure, Developer Experience, and Site Reliability Teams" (~95 chars) is truncated to its first 64 code points before reaching the store. The read API returns the truncated name to clients selecting a delivery target. Person names rarely exceed 64 characters, but group/conversation names routinely do on both DingTalk and Telegram.
Suggested fix: Use a dedicated sanitizer for group names that strips the same control characters and unsafe invisibles as sanitizeSenderName but truncates to MAX_LABEL_LENGTH (256) instead of 64.
— qwen3.7-max via Qwen Code /review
| private isIdentity(value: ObservedChannelIdentity): boolean { | ||
| return ( | ||
| this.isBoundedString(value.id, MAX_ID_LENGTH) && | ||
| this.isBoundedString(value.label, MAX_ID_LENGTH) | ||
| ); | ||
| } |
There was a problem hiding this comment.
[Suggestion] isIdentity (write path) validates labels against MAX_ID_LENGTH (4096), but parseIdentity (read path) validates against MAX_LABEL_LENGTH (256). The two validation gates disagree on the maximum label length.
Failure scenario: Today normalizeIdentity truncates labels to 256 before persistence, masking the mismatch. A maintainer reading isIdentity as the canonical validator could remove normalizeIdentity's truncation as redundant, or raise the parseIdentity bound to match — either change breaks the round-trip: list() throws on every request (malformed registry) or labels exceeding 256 characters silently reach the API response.
| private isIdentity(value: ObservedChannelIdentity): boolean { | |
| return ( | |
| this.isBoundedString(value.id, MAX_ID_LENGTH) && | |
| this.isBoundedString(value.label, MAX_ID_LENGTH) | |
| ); | |
| } | |
| private isIdentity(value: ObservedChannelIdentity): boolean { | |
| return ( | |
| this.isBoundedString(value.id, MAX_ID_LENGTH) && | |
| this.isBoundedString(value.label, MAX_LABEL_LENGTH) | |
| ); | |
| } |
— qwen3.7-max via Qwen Code /review
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution Summary — PR #7155Branch:
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| const sanitizedChatName = envelope.chatName | ||
| ? sanitizeSenderName(envelope.chatName) | ||
| : ''; | ||
| const groupLabel = | ||
| sanitizedChatName === 'unknown' | ||
| ? envelope.chatId | ||
| : sanitizedChatName || envelope.chatId; |
There was a problem hiding this comment.
[Suggestion] The label-derivation pattern (sanitize → sentinel check → fallback) is duplicated verbatim from the userLabel block four lines above. When topic labels eventually gain the same inbound-name treatment the design doc defers, a third copy will be added. Any future change to the fallback semantics must be replicated across all copies.
| const sanitizedChatName = envelope.chatName | |
| ? sanitizeSenderName(envelope.chatName) | |
| : ''; | |
| const groupLabel = | |
| sanitizedChatName === 'unknown' | |
| ? envelope.chatId | |
| : sanitizedChatName || envelope.chatId; | |
| const userLabel = this.resolveLabel(envelope.senderName, envelope.senderId); | |
| const groupLabel = this.resolveLabel(envelope.chatName, envelope.chatId); |
with a private helper:
private resolveLabel(raw: string | undefined, fallback: string): string {
const sanitized = raw ? sanitizeSenderName(raw) : '';
return sanitized === 'unknown' ? fallback : sanitized || fallback;
}— qwen3.7-max via Qwen Code /review
Local build & live-E2E verification at
|
Round-1 note (at d8ff3cba4) |
Severity | Status at e9c58ab5 |
|---|---|---|
Effective group-label cap is 64 code points via sanitizeSenderName (not the store's 256); [ ] folded to spaces |
docs nit | Unchanged, non-blocking — behavior confirmed live below |
A group literally named unknown falls back to the chat ID |
note (by design) | Unchanged — now exercised live (E2E check 5) |
2. Merge-resolution audit — byte-exact
The risk with a bot-resolved merge is keeping tests while dropping implementation (or vice versa). Compared the delta I reviewed at LGTM (026113b86..d8ff3cba4, on top of #7109's head) against the net delta the merge leaves on top of main (d51c21de8..e9c58ab5, with #7109 already squash-merged as 02cb3a63):
- Both stats identical: 12 files, +232/−6.
diff <(git diff 026113b86 d8ff3cba4) <(git diff d51c21de8 e9c58ab5)→ 24 lines, all blob hashes / hunk offsets; zero+/-content lines differ. The merge re-applies the reviewed feature byte-for-byte — nothing dropped, nothing smuggled in.- PR code confirmed inside the built artifacts:
grep sanitizedChatNamehitspackages/channels/base/dist/ChannelBase.jsanddist/chunks/chunk-RTP23QZB.js.
3. Real build + full suites of every touched package
Isolated worktree at e9c58ab5, real npm ci (runs the full bundle build).
| Suite | Result |
|---|---|
packages/channels/base (full) |
817/817 ✓ (15 files) |
packages/channels/dingtalk (full) |
119/119 ✓ |
packages/channels/telegram (full) |
14/14 ✓ |
packages/cli observed-contact-store.test.ts |
11/11 ✓ |
The five tests this PR adds/extends were additionally run by exact name (group/topic/user observation, unusable-name fallback, DM chatName isolation, DingTalk conversationTitle, Telegram group/supergroup chat.title) — all green.
4. Live daemon E2E (written for this round)
Real dist/cli.js serve daemon + real channel-worker process running a group-capable plugin fixture (a thin test driver whose Envelope forwards isGroup/chatName/threadId; everything downstream — preflight gates, recordObservedContact, persistence — is the merged tree's real @qwen-code/channel-base), with a fake OpenAI upstream answering each message. Six live inbound messages over WebSocket, then two oracles: the GET /workspace/channel/observed-contacts route and the raw on-disk registry.
| # | Live inbound | Observed result |
|---|---|---|
| 1 | group grp-1001, chatName: "Project Phoenix 项目群" |
label = sanitized name (CJK preserved) ✓ |
| 2 | group grp-2002, no chatName |
label falls back to chat ID ✓ |
| 3 | DM with stray chatName: "Sneaky DM Title" |
no group entry; title never persisted anywhere ✓ |
| 4 | grp-1001 renamed to "Phoenix Renamed 凤凰群" |
label refreshed in place, still exactly one entry ✓ |
| 5 | group grp-3003, chatName: "unknown" |
sanitizer sentinel → chat-ID fallback ✓ |
| 6 | grp-1001 with threadId: topic-7 |
topic label still ID-based (no regression) ✓ |
| 7 | SIGTERM daemon → respawn | labels persist from disk ✓ |
5. CI & merge state
At e9c58ab5: Ubuntu Test ✓ (32m14s), web-shell E2E Smoke ✓ (macOS/Windows legs skipped by workflow config, as usual). main has advanced by one commit since the merge (#7136, web-shell only — no file overlap; GitHub reports the PR MERGEABLE).
The E2E fixture and test file live only in my local verification worktree — nothing was pushed to this PR's branch.
中文版本（点击展开）
本地构建 + 真实 E2E 验证报告：e9c58ab5（/resolve 后复核）
继我在 d8ff3cba4 的 LGTM（前一条评论）之后，PR 唯一的新增是 @qwen-code /resolve 生成的合并提交 e9c58ab5（双亲 d8ff3cba4 × d51c21de8）。本轮核心是审计该冲突解决的正确性，并在合并后的 head 上重新做真实构建验证。
结论：可以合并。 合并解决与已评审的功能增量逐字节一致；961 个单元测试 + 7 项行为的真实 daemon E2E 全绿。
1. 上一轮备注 → 新 head 状态
上一轮备注（d8ff3cba4） |
级别 | e9c58ab5 状态 |
|---|---|---|
群标签实际上限是 sanitizeSenderName 的 64 码点（而非存储层的 256）；[ ] 会折叠为空格 |
文档小瑕疵 | 不变，不阻塞 —— 已在下方实测确认 |
群名恰好叫 unknown 时回退到会话 ID |
备注（符合设计） | 不变 —— 本轮 E2E 第 5 项已实测覆盖 |
2. 合并解决审计 —— 逐字节一致
机器人解决冲突的风险在于"保留测试却丢掉实现"（或相反）。将 LGTM 时评审的增量（026113b86..d8ff3cba4，基于 #7109 head）与合并后相对 main 的净增量（d51c21de8..e9c58ab5，此时 #7109 已以 02cb3a63 squash 合入）对比：
- 两侧 stat 完全一致：12 个文件，+232/−6。
diff <(git diff …) <(git diff …)→ 仅 24 行差异，全部是 blob 哈希 / hunk 行号偏移；没有任何一行+/-内容差异。合并逐字节重放了已评审的功能，无丢失、无夹带。- 构建产物中确认包含 PR 代码：
grep sanitizedChatName命中packages/channels/base/dist/ChannelBase.js与dist/chunks/chunk-RTP23QZB.js。
3. 真实构建 + 所有触及包的完整测试套件
在 e9c58ab5 的隔离 worktree 中执行真实 npm ci（含完整 bundle 构建）。
| 套件 | 结果 |
|---|---|
packages/channels/base（全量） |
817/817 ✓（15 个文件） |
packages/channels/dingtalk（全量） |
119/119 ✓ |
packages/channels/telegram（全量） |
14/14 ✓ |
packages/cli observed-contact-store.test.ts |
11/11 ✓ |
另按测试名精确运行了本 PR 新增/扩展的 5 个测试（群/话题/用户观察、不可用群名回退、私聊 chatName 隔离、钉钉 conversationTitle、Telegram 群/超级群 chat.title）—— 全部通过。
4. 真实 daemon E2E（本轮新写）
真实 dist/cli.js serve daemon + 真实 channel worker 进程，加载一个支持群消息的 plugin fixture（仅在 Envelope 上透传 isGroup/chatName/threadId 的瘦驱动；其下游 —— 预检门禁、recordObservedContact、持久化 —— 全部是合并树的真实 @qwen-code/channel-base），并由 fake OpenAI 上游应答。通过 WebSocket 注入 6 条真实 inbound 消息，随后用两个 oracle 校验：GET /workspace/channel/observed-contacts 路由与磁盘上的注册表原文。
| # | 实时 inbound | 观测结果 |
|---|---|---|
| 1 | 群 grp-1001，chatName: "Project Phoenix 项目群" |
标签 = 清洗后的群名（中文保留）✓ |
| 2 | 群 grp-2002，无 chatName |
标签回退到会话 ID ✓ |
| 3 | 私聊夹带 chatName: "Sneaky DM Title" |
不产生群条目；标题从未落盘 ✓ |
| 4 | grp-1001 改名为 "Phoenix Renamed 凤凰群" |
标签原地刷新，仍只有一条记录 ✓ |
| 5 | 群 grp-3003，chatName: "unknown" |
清洗哨兵值 → 回退会话 ID ✓ |
| 6 | grp-1001 携带 threadId: topic-7 |
话题标签仍为 ID（无回归）✓ |
| 7 | SIGTERM 重启 daemon | 标签从磁盘恢复 ✓ |
5. CI 与合并状态
e9c58ab5 上：Ubuntu Test ✓（32m14s）、web-shell E2E Smoke ✓（macOS/Windows 按工作流配置跳过，属常态）。合并后 main 又前进了一个提交（#7136，仅 web-shell —— 无文件重叠；GitHub 显示 MERGEABLE）。
E2E fixture 与测试文件仅存在于我本地的验证 worktree —— 未向本 PR 分支推送任何内容。
|
@qwen-code /triage |
| ...(envelope.isGroup | ||
| ? { | ||
| group: { id: envelope.chatId, label: envelope.chatId }, | ||
| group: { id: envelope.chatId, label: groupLabel }, |
There was a problem hiding this comment.
[Critical] Variable group labels make the store's equal-timestamp ordering observable. If user A is recorded with the old title and a later user B message carries the renamed title in the same millisecond, their relationship keys differ, both records survive, and the store's timestamp-only stable sort keeps the older record first; graph construction initializes the group from that old label and never updates it. Add an explicit newest-wins tie-breaker (for example, place the new observation before retained equal-timestamp entries) and a same-timestamp, two-user rename regression test.
— Codex GPT-5 via Qwen Code /review
| await ch.processAfterAdapterPreflight( | ||
| envelope({ | ||
| chatId: 'group-1', | ||
| chatName: 'Project Group', |
There was a problem hiding this comment.
[Suggestion] This changes the former unnamed-group fixture into the named success case, so the documented missing-chatName fallback for Feishu, WeCom, and other adapters is no longer covered. The new control-character case exercises the separate sanitizedChatName === 'unknown' branch, so a regression in the final empty-string fallback could still pass. Add a group observation with chatName omitted and assert the complete chatId is used as the label.
— Codex GPT-5 via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
4 participants
What this PR does
This follow-up teaches the workspace-scoped observed-contact graph to retain human-readable group names already present in accepted inbound messages. DingTalk forwards
conversationTitle, Telegram forwardschat.title, and the shared observation path sanitizes the value before using it asgroups[].label. Complete platform group IDs remain unchanged and remain the only delivery and identity keys.This is a stacked PR based on #7109. It should merge after #7109; once that dependency lands, this PR's diff will reduce to the group-name follow-up.
Why it's needed
The observed-contact API currently labels every group with its opaque platform ID even when the inbound callback already contains a readable name. This makes a proactive-delivery target selector unnecessarily difficult to use. The change remains passive: it does not call directory, group-detail, or chat-info APIs and does not add permissions.
Reviewer Test Plan
How to verify
Send an accepted group message to a daemon-managed DingTalk or Telegram channel, then read the workspace's observed contacts. The corresponding group should retain its complete platform ID while its label matches the inbound group name. Send a later accepted message after renaming the group and verify the existing graph node refreshes rather than duplicating. For Feishu, WeCom, missing, empty, or malformed names, verify the label continues to fall back to the complete group ID. Confirm no platform directory or group-detail request occurs.
Evidence (Before & After)
Before: every
groups[].labelfalls back to the opaquegroups[].id.After: DingTalk and Telegram return names observed directly on inbound messages; Feishu, WeCom, and topics retain the complete-ID fallback.
Tested on
Environment (optional)
macOS local workspace, Node.js 25.2.1. Focused base, DingTalk, Telegram, and observed-contact store tests passed (562 tests). Repository lint, typecheck, and build passed. Platform callback-to-read-API E2E evidence is posted separately.
Risk & Scope
Linked Issues
Closes #7154
Depends on #7109
中文说明
本 PR 做了什么
这个后续改动让 workspace 级动态观测联系人图谱保留已通过校验的入站消息中自带的可读群名。钉钉透传
conversationTitle，Telegram 透传chat.title，公共观测链路清洗后写入groups[].label。完整的平台群 ID 不变，并继续作为唯一的投递和身份标识。这是基于 #7109 的 stacked PR，应在 #7109 合并后再合并；依赖合入后，本 PR 的 diff 会自动收敛为群名观测的后续改动。
为什么需要
当前观测联系人接口即使入站回调已经携带可读群名，也始终用不透明的平台 ID 作为群标签，使主动投递目标选择器难以使用。本改动仍然是纯被动观测：不会调用通讯录、群详情或 chat-info API，也不增加权限。
Reviewer 测试计划
如何验证
向 daemon 管理的钉钉或 Telegram channel 发送一条通过校验的群消息，然后读取该 workspace 的 observed contacts。对应群应保留完整平台 ID，同时 label 等于入站消息携带的群名。群改名后再发送一条通过校验的消息，应刷新已有图节点而不是产生重复节点。飞书、企微以及名称缺失、为空或非法时，label 应继续回退为完整群 ID。整个过程不应出现平台通讯录或群详情请求。
前后对比证据
改动前：所有
groups[].label都回退为不透明的groups[].id。改动后：钉钉和 Telegram 返回直接从入站消息观测到的名称；飞书、企微和话题继续使用完整 ID 回退。
测试环境
macOS 本地 workspace，Node.js 25.2.1。公共层、钉钉、Telegram 和 observed-contact store 聚焦测试共 562 项通过，仓库 lint、typecheck 和 build 通过。平台 callback 到读取 API 的 E2E 证据会单独评论。
风险与范围
关联 Issue
Closes #7154
Depends on #7109