-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(channels): observe group names from inbound messages #7155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f840333
e5398ac
026113b
1dc3b01
acc9724
d8ff3cb
e9c58ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # Observed channel group names | ||
|
|
||
| ## Problem | ||
|
|
||
| The workspace-scoped observed-contact graph introduced by #7109 preserves complete platform group IDs, but every `groups[].label` currently falls back to that ID. Some inbound channel callbacks already carry a human-readable group name, and the adapters discard it before the shared observation boundary. | ||
|
|
||
| Users selecting a proactive-delivery target need the readable name alongside the complete, stable platform ID. The name is observational metadata, not a routing key. | ||
|
|
||
| ## Scope | ||
|
|
||
| Add an optional group name to the shared inbound envelope and populate it only from metadata already present in an accepted inbound message. | ||
|
|
||
| - DingTalk maps the Stream callback's `conversationTitle`. | ||
| - Telegram maps the inbound chat's `title` for groups and supergroups. | ||
| - Feishu keeps the complete `chat_id` fallback because `im.message.receive_v1` does not include a chat display name. | ||
| - Other adapters keep the ID fallback unless their existing inbound payload has a documented group-name field. | ||
|
|
||
| This change does not call a platform directory, group-detail, or chat-info API; add permissions; alter routing or session identity; discover authoritative membership; observe bot output; or add topic names. | ||
|
|
||
| ## Contract | ||
|
|
||
| `Envelope` gains one optional field: | ||
|
|
||
| ```ts | ||
| chatName?: string; | ||
| ``` | ||
|
|
||
| The field describes the display name of `chatId` as observed on that message. It is ignored for direct messages. `chatId` remains the complete platform delivery key and continues to determine sessions, deduplication, and graph identity. | ||
|
|
||
| The common observation path uses a sanitized, non-empty `chatName` as the group label. Missing or unusable values fall back to the complete `chatId`. The existing registry store bounds persisted labels to 256 UTF-16 code units without splitting surrogate pairs. | ||
|
|
||
| ## Refresh semantics | ||
|
|
||
| An accepted later message for the same channel, user, and group refreshes the observation. If it carries a different usable `chatName`, the existing store replacement semantics update the derived group label without creating another group node. Freshness remains `lastObservedAt`; names are not treated as permanent or authoritative. | ||
|
|
||
| A platform that omits a group name on a later message contributes the ID fallback for that observation. Graph derivation already selects the most recent observation, so the returned label represents the newest accepted evidence rather than a hidden long-lived name cache. | ||
|
|
||
| ## Platform evidence | ||
|
|
||
| - DingTalk's Stream robot-message example includes `conversationTitle` in the inbound callback: [DingTalk Stream protocol](https://opensource.dingtalk.com/developerpedia/docs/learn/stream/protocol/#%E5%9B%9E%E8%B0%83%E6%8E%A8%E9%80%81). | ||
| - Telegram defines `Message.chat` as a `Chat`, whose `title` is available for group chats and supergroups: [Telegram Bot API — Chat](https://core.telegram.org/bots/api/#chat). | ||
| - Feishu's receive-message event enumerates `chat_id`, `chat_type`, and `thread_id`, but no chat display name: [Feishu Open Platform — Receive message](https://open.feishu.cn/document/uAjLw4CM/ukTMukTMukTM/reference/im-v1/message/events/receive). | ||
|
|
||
| ## Test strategy | ||
|
|
||
| - Base-channel tests prove usable group names propagate, unusable names fall back to complete IDs, direct messages ignore `chatName`, and later observations can refresh labels. | ||
| - DingTalk adapter tests prove `conversationTitle` enters the envelope without changing callback handling. | ||
| - Telegram adapter tests prove group and supergroup titles enter the envelope while private chats remain unchanged. | ||
| - Existing Feishu tests continue to prove the ID fallback path without API traffic. | ||
| - Focused store tests cover replacement by newer labels; no schema migration is needed because persisted observations already contain `group.label`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| # Observed Channel Group Names Implementation Plan | ||
|
|
||
| > **For agentic workers:** Execute each task test-first and keep the change limited to inbound metadata already supplied by the platform. | ||
|
|
||
| **Goal:** Return human-readable observed group labels when an accepted inbound callback already contains a group name, while retaining complete platform IDs and the existing ID fallback. | ||
|
|
||
| **Architecture:** Channel adapters copy an optional inbound group name into `Envelope.chatName`. `ChannelBase` sanitizes that observation and writes it as `group.label`; the existing workspace store handles bounds, refresh, freshness, and graph derivation without a schema change. No adapter performs additional network requests. | ||
|
|
||
| ## Constraints | ||
|
|
||
| - Never call a platform directory, group-detail, or chat-info API. | ||
| - Keep `chatId` as the routing, session, deduplication, and graph identity key. | ||
| - Use `chatName` only for `groups[].label` on group messages. | ||
| - Preserve complete-ID fallback for missing or unusable names. | ||
| - Implement only platforms with verified inbound fields: DingTalk and Telegram. | ||
| - Keep Feishu, WeCom, and topic labels on their existing ID fallback paths. | ||
|
|
||
| ## Task 1: Shared envelope and observation behavior | ||
|
|
||
| **Files:** `packages/channels/base/src/types.ts`, `packages/channels/base/src/ChannelBase.test.ts`, `packages/channels/base/src/ChannelBase.ts` | ||
|
|
||
| 1. Add failing base-channel tests proving a group `chatName` becomes `group.label`, malformed or empty names fall back to the complete `chatId`, and direct messages ignore `chatName`. | ||
| 2. Run `cd packages/channels/base && npx vitest run src/ChannelBase.test.ts` and confirm the new assertions fail for the missing contract. | ||
| 3. Add optional `chatName?: string` to `Envelope`. | ||
| 4. Sanitize the observed group name at the existing post-preflight observation boundary and fall back to `chatId` when unusable. | ||
| 5. Re-run the focused base test and confirm it passes. | ||
|
|
||
| ## Task 2: DingTalk inbound group title | ||
|
|
||
| **Files:** `packages/channels/dingtalk/src/DingtalkAdapter.test.ts`, `packages/channels/dingtalk/src/DingtalkAdapter.ts` | ||
|
|
||
| 1. Add a failing adapter test whose Stream callback contains `conversationTitle` and assert the processed envelope contains `chatName` while preserving `chatId`. | ||
| 2. Run `cd packages/channels/dingtalk && npx vitest run src/DingtalkAdapter.test.ts` and confirm the new assertion fails. | ||
| 3. Add `conversationTitle` to the raw inbound type, validate it as a string, and place it on the group envelope. | ||
| 4. Do not change acknowledgements, webhook caching, routing, logging, or send behavior. | ||
| 5. Re-run the focused DingTalk test and confirm it passes. | ||
|
|
||
| ## Task 3: Telegram inbound chat title | ||
|
|
||
| **Files:** `packages/channels/telegram/src/TelegramAdapter.test.ts`, `packages/channels/telegram/src/TelegramAdapter.ts` | ||
|
|
||
| 1. Add failing tests proving group and supergroup `chat.title` values become `chatName`, while a private chat does not expose a group name. | ||
| 2. Run `cd packages/channels/telegram && npx vitest run src/TelegramAdapter.test.ts` and confirm the new assertions fail. | ||
| 3. Extend the adapter's local inbound chat shape with optional `title` and copy it only for group or supergroup envelopes. | ||
| 4. Re-run the focused Telegram test and confirm it passes. | ||
|
|
||
| ## Task 4: User-facing contract documentation | ||
|
|
||
| **Files:** `docs/design/2026-07-17-observed-channel-delivery-targets.md`, `docs/users/features/channels/overview.md` | ||
|
|
||
| 1. Replace the statement that all group labels fall back to IDs with the best-effort inbound-name behavior. | ||
| 2. Document that DingTalk and Telegram currently supply names and that Feishu/WeCom retain the complete-ID fallback. | ||
| 3. Keep topic-label and membership limitations explicit. | ||
|
|
||
| ## Task 5: Verification and publication | ||
|
|
||
| 1. Run Prettier on all changed files. | ||
| 2. Run the focused base, DingTalk, Telegram, and existing observed-contact store tests. | ||
| 3. Run `npm run lint && npm run typecheck && npm run build`. | ||
| 4. Inspect the complete diff in two clean self-audit passes; any fix resets the clean-pass count and relevant tests. | ||
| 5. Commit the implementation, push `feat/channel-observed-group-names`, and open a stacked ready-for-review PR that declares its dependency on #7109 and links #7154. | ||
| 6. Add the E2E plan/result as a separate PR comment. Exercise the complete DingTalk callback-to-read-API path with a `conversationTitle` payload, use #7109's live DingTalk transport result as the transport precondition, and record Feishu as a negative schema/fallback check rather than making an API request. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3859,11 +3859,18 @@ export abstract class ChannelBase { | |||||||||||||||||||
| sanitizedSenderName === 'unknown' | ||||||||||||||||||||
| ? envelope.senderId | ||||||||||||||||||||
| : sanitizedSenderName || envelope.senderId; | ||||||||||||||||||||
| const sanitizedChatName = envelope.chatName | ||||||||||||||||||||
| ? sanitizeSenderName(envelope.chatName) | ||||||||||||||||||||
| : ''; | ||||||||||||||||||||
|
Comment on lines
+3862
to
+3864
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Group display names from DingTalk 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 — qwen3.7-max via Qwen Code /review |
||||||||||||||||||||
| const groupLabel = | ||||||||||||||||||||
| sanitizedChatName === 'unknown' | ||||||||||||||||||||
| ? envelope.chatId | ||||||||||||||||||||
| : sanitizedChatName || envelope.chatId; | ||||||||||||||||||||
|
Comment on lines
+3862
to
+3868
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The label-derivation pattern (sanitize → sentinel check → fallback) is duplicated verbatim from the
Suggested change
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 |
||||||||||||||||||||
| const observation: ObservedChannelContactObservation = { | ||||||||||||||||||||
| user: { id: envelope.senderId, label: userLabel }, | ||||||||||||||||||||
| ...(envelope.isGroup | ||||||||||||||||||||
| ? { | ||||||||||||||||||||
| group: { id: envelope.chatId, label: envelope.chatId }, | ||||||||||||||||||||
| group: { id: envelope.chatId, label: groupLabel }, | ||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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 |
||||||||||||||||||||
| ...(envelope.threadId | ||||||||||||||||||||
| ? { | ||||||||||||||||||||
| topic: { | ||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] This changes the former unnamed-group fixture into the named success case, so the documented missing-
chatNamefallback for Feishu, WeCom, and other adapters is no longer covered. The new control-character case exercises the separatesanitizedChatName === 'unknown'branch, so a regression in the final empty-string fallback could still pass. Add a group observation withchatNameomitted and assert the completechatIdis used as the label.— Codex GPT-5 via Qwen Code /review